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

HTTP POST response of an existing resource #422

Closed
jorgejesus opened this issue Jul 31, 2020 · 7 comments · Fixed by #442
Closed

HTTP POST response of an existing resource #422

jorgejesus opened this issue Jul 31, 2020 · 7 comments · Fixed by #442

Comments

@jorgejesus
Copy link

On the pygeoapi data transactions implementation (@alex-mathew ), there was some questions concerning the following scenario:

Scenario:

  • Resource was initially created with a POST request, and had a HTTP/1.1 201 Created
  • Client wants to update/change BUT uses POST instead of PUT, payload it correct. But HTTP verb option is not the best

Current docs indicate:

  • 403 Forbidden - Indicates that authentication is not the issue, but the client is not authorized to perform the requested operation on the resource.

Comments:

  • This maybe misleading since this is not related to authorization but a bad request, the payload is correct it could be implemented but using PUT
  • There is alot of discussion on stackoverflow here
  • Exceptions should allow: 422 Unprocessable Entity for this case (The 422 Unprocessable Entity status code means the server understands the content type of the request entity (hence a 415 Unsupported Media Type status code is inappropriate), and the syntax of the request entity is correct (thus a 400 Bad Request status code is inappropriate) but was unable to process the contained instructions.).
  • Other options: 400, 403, 409 (Conflict) don't seem to be appropriate

Sugguestion:

  • Add 422 Unprocessable Entity to allowed exceptions
@m-mohr
Copy link
Contributor

m-mohr commented Jul 31, 2020

Is the second post going towards /collections/id/items/id or /collections/id/items?

For /collections/id/items/id: Shouldn't that just be a 405 - Method not allowed?
For /collections/id/items: Shouldn't that just create a new resource?

@cportele
Copy link
Member

I assume this is about a POST request to /collections/mycollection/items and the API always uses information in the payload to determine the new feature id and that id already exists (otherwise it would simply create another feature).

I agree that 403 is not appropriate. The issue is not about authorization. But I also do not see where the current draft says that 403 should be used?

I would return a 400 as this is a client error and point out the id conflict in the response. 400 in HTTP is not limited to syntactical errors.

Also, 422 is not specified in RFC 7231 (HTTP). It is specified by WebDAV and I would avoid 422 in an API that does not implement WebDAV.

@jorgejesus
Copy link
Author

@cportele

Your assumption is correct, POST is intended for /collections/{collectionId}/items as indicated on the docs (here). (@m-mohr), therefore the the method is allowed

The draft doesn't say that 403 should be used but on the exception section 6.2.5 - Exception it points to: See HTTP status codes. So, I assumed that the topic was not addressed and we could simple use one of the 4xx

Also, I was not aware that 422 was not part of RFC 7231 (HTTP). Thank you for the info now is clear that the response should be 400

@pvretano
Copy link
Contributor

If the target of the POST is /collections/{collectionsId}/items/{id} then I think 405 is appropriate since, in fact, we have no POST behaviour defined for /collections/{collectionId}/items/{id}. If the target of the POST is /collections/{collectionId}/items then the server should just create a new feature. You cannot change a feature using a POST at the /collections/{collectionId}/items endpoint, you can only do that at the /collection/{collectionId}/items/{id} endpoint with a PUT or PATCH method.

@pvretano
Copy link
Contributor

Oops ... didn't completely read the post! Yes, in this case, 400 seems reasonable ... so stop making bulging eyes at me! ;) LOL!

@cportele
Copy link
Member

2020-08-16: @pvretano will verify that the spec is consistent with the conclusion here and update the document, if necessary.

@pvretano
Copy link
Contributor

I have reviewed clause 4.3.3 of RFC7231 and as far as I can tell, the behaviour of POST is correctly specified in Part 4. The two key paragraphs from clause 4.3.3 are:

  • ... POST is used for the following functions (among others): ... o Creating a new resource that has yet to be identified by the origin server; ...
  • If one or more resources has been created on the origin server as a result of successfully processing a POST request, the origin server SHOULD send a 201 (Created) response containing a Location header field that provides an identifier for the primary resource created (Section 7.1.2) and a representation that describes the status of the request while referring to the new resource(s).

I am not sure that I agree that we should return a 400 when a POST is used at the /collection/{collectionId}/items/{featureId} endpoint. I think that, in fact, a 405 is more appropriate. Clause 6.5.5 of RFC7231 says:

"The 405 (Method Not Allowed) status code indicates that the method received in the request-line is known by the origin server but not supported by the target resource. The origin server MUST generate an Allow header field in a 405 response containing a list of the target resource's currently supported methods."

To be clear about which combinations of resource endpoints and HTTP methods are defined in the specification I added a table to scope that lists the resource endpoint/HTTP method combination and links to the relevant clause in the specification. See this pull request.

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

Successfully merging a pull request may close this issue.

4 participants