Return an HTTP 303 for PUT/POST request on a ReST API is a bad practice

Description

Based on what I read about design patterns for ReST APIs on several web pages (here is one example: http://www.restapitutorial.com/lessons/httpmethods.html), it seems like a POST request should return 201 with a Location header on a successful response, and a PUT request should return either 200 or 204. They should NEVER return a 303 unless you want to FORCE the client to perform a GET request, which I think it is not required on any of the OpenNMS ReST end points, especially on Requisitions because of https://opennms.atlassian.net/browse/NMS-7872#icft=NMS-7872.

A little bit of history. When the Requisitions ReST API was introduced, the whole requisition was returned as part of a response of a PUT/POST request. Then, that was changed by returning a 303 with a Location header. The problem is that this is EXACLY the same thing in terms of a Web Browser, or any well implemented web client, as the Location will always going to be followed no matter if you need the data or not. So, that is basically doing the same wrong thing differently. It has to be OPTIONAL, the retrieval of the updated data for the client (NOT MANDATORY).

Also the rest of the return codes are not consistent, and different rules are used which is not theoretically correct.

Acceptance / Success Criteria

None

Lucidchart Diagrams

Activity

Alejandro Galue March 14, 2016 at 12:31 PM

Alejandro Galue March 3, 2016 at 4:25 PM

On those cases when an explanation is returned with NOT_FOUND, BAD_REQUEST, etc. The format of the plain text returned has been normalized on all the end points that provide this information.

Alejandro Galue March 3, 2016 at 4:24 PM
Edited

Here are the rules already implemented on the branch:

1. All the DELETE requests are going to return a 204 (NO_CONTENT) on success.
2. All the PUT requests are going to return a 204 (NO_CONTENT) on success.
3. All the POST requests that can either add or update an entity are going to return a 204 (NO_CONTENT) on success.
4. All the POST associated to resource addition are going to return a 201 (CREATED) on success.
5. All the POST requests where it is required to return an object will return 200 (OK).
6. All the requests excepts GET for the Requisitions end-point and the Foreign Sources Definitions end-point will return 202 (ACCEPTED). This is because all the requests are actually executed asynchronously and there is no way to know the status of the execution, or wait until the processing is done.
7. If a resource is not modified during a PUT request, a NOT_MODIFIED will be returned. A NO_CONTENT will be returned only on a success operation.
8. All GET requests are going to return 200 (OK) on success.
9. All GET requests are going to return 404 (NOT_FOUND) when a single resource doesn't exist; but will return 400 (BAD_REQUEST), if an intermediate resource doesn't exist. For example, if a specific IP doesn't exist on a valid node, return 404. But, if the IP is valid and the node is not valid, because the node is an intermediate resource, a 400 will be returned.
10. If something not expected is received from the Service/DAO Layer when processing any HTTP request, like an exception, a 500 (INTERNAL_SERVER_ERROR) will be returned.
11. Any problem related with the incoming parameters, like validations, will generate a BAD_REQUEST.

Fixed

Details

Assignee

Reporter

Affects versions

Priority

PagerDuty

Created March 2, 2016 at 10:42 AM
Updated March 18, 2016 at 12:54 PM
Resolved March 18, 2016 at 8:54 AM

Flag notifications