Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload allows new AOI with same name as existing AOI #51

Open
jkeifer opened this issue Mar 21, 2016 · 6 comments
Open

Upload allows new AOI with same name as existing AOI #51

jkeifer opened this issue Mar 21, 2016 · 6 comments

Comments

@jkeifer
Copy link
Member

jkeifer commented Mar 21, 2016

The upload should be rejected rather than allowing the transfer to complete and a task to start, just to find the name is already used.

@lbross
Copy link

lbross commented Nov 22, 2017

Reminder for @lbross to check how this works when re-uploading an aoi that was recently deleted after this issue is implemented. Does it throw an error code that PC-BAGIS can catch?

@jkeifer
Copy link
Member Author

jkeifer commented Nov 23, 2017

@lbross this check is implemented in test. Give it a go and let me know if that works for you. Note that with a non-chunked upload you will still have to upload the whole file for validation to run.

Note that current solution does not solve the race condition between checking the name and the AOI save. In other words, it is possible for two uploads with the same name to be uploaded at the same time. Whichever one gets to the AOI save first will win, and the second will error with a traceback, just like before this extra validation check was in place. Solving that race is much more complicated than the current solution, so I have chosen to leave it. If it becomes an issue in the future we can address it then.

@lbross
Copy link

lbross commented Nov 27, 2017

@jkeifer I'm running into a bit of a snag processing the JSON payload that this exception returns:
{"detail":{"all":["[u'Aoi with this Name and active already exists.']"]}}

On my end, this comes across as the same type of exception as a permissions error. However, the permissions payload is simpler and returns something like this:
{"detail":"You do not have permission to perform this action"]

If you think it is best to have the two types of errors return the JSON in these different structures, I can handle this on my end but I wanted to check before doing the extra work.

@jkeifer
Copy link
Member Author

jkeifer commented Nov 27, 2017

A bad name should have a 400 error, right? And the latter permissions problem returns a 403 error, no? Or am I mistaken?

@lbross
Copy link

lbross commented Nov 28, 2017

I found a way to extract the HttpStatusCode from the WebException in .NET. I will write some code to process the JSON payload differently depending on the HttpStatusCode.

There are some 20-odd HttpStatusCodes in the API and I don't want to write special handling for each of them, especially since we'll likely never encounter most of them. Do you have a feel for which of the payload structures would be most common for a server exception? I will use this for the default. Also, if there are other HttpStatusCodes with unique payloads that you think we'll commonly encounter, could you send me the details?

Finally, what is the meaning of the leading 'u' in the error message for duplicate AOI?

@lbross
Copy link

lbross commented Dec 15, 2017

@jkeifer Missed my chance to bring this up yesterday. Can you take a look when you have a moment? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants