spring-projects / spring-data-rest

Simplifies building hypermedia-driven REST web services on top of Spring Data repositories
https://spring.io/projects/spring-data-rest
Apache License 2.0
905 stars 558 forks source link

Document error codes for ItemResources #2384

Open sdavids opened 1 month ago

sdavids commented 1 month ago

https://docs.spring.io/spring-data/rest/reference/repository-resources.html#repository-resources.item-resource

Examples:

PATCH http://localhost:8080/orders/5 when no order with 5 exists PATCH http://localhost:8080/orders/A when orders have numeric IDs

https://docs.spring.io/spring-data/rest/reference/repository-resources.html#repository-resources.default-status-codes

odrotbohm commented 1 month ago

Wouldn't documenting that both cases return a 404 Not Found be documenting the obvious? If a resource does not exist, 404 is the natural status code by definition, isn't it?

sdavids commented 1 month ago
@Entity(name = "users")
public class User {

  @Id private UUID id;

  private String email;

// Getters/Setters
}
@RepositoryRestResource
public interface UserRepository extends CrudRepository<User, UUID> {}
$ curl -i -X "POST" "http://localhost:8080/users" \
     -H 'Content-Type: application/json' \
     -d $'{
  "id": "0bbe06f4-9751-4f97-af16-db10eb1dcac7",
  "email": "test@example.org"
}'
HTTP/1.1 201
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Location: http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7
Content-Type: application/hal+json
Transfer-Encoding: chunked
Date: Wed, 15 May 2024 14:16:05 GMT

{
  "email" : "test@example.org",
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7"
    },
    "user" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7"
    }
  }
}

Current behavior:

$ curl -i -X "PUT" "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7" \
     -H 'Content-Type: application/json' \
     -d $'{
  "id": "0bbe06f4-9751-4f97-af16-db10eb1dcac7",
  "email": "changed@example.org"
}'
HTTP/1.1 200
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Location: http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7
Content-Type: application/hal+json
Transfer-Encoding: chunked
Date: Wed, 15 May 2024 14:17:43 GMT

{
  "email" : "changed@example.org",
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7"
    },
    "user" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7"
    }
  }
}
$ curl -i -X "PUT" "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac8" \
     -H 'Content-Type: application/json' \
     -d $'{
  "id": "0bbe06f4-9751-4f97-af16-db10eb1dcac8",
  "email": "not-found@example.org"
}'
HTTP/1.1 201
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Location: http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac8
Content-Type: application/hal+json
Transfer-Encoding: chunked
Date: Wed, 15 May 2024 14:18:41 GMT

{
  "email" : "not-found@example.org",
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac8"
    },
    "user" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac8"
    }
  }
}
$ curl -i -X "PUT" "http://localhost:8080/users/1" \
     -H 'Content-Type: application/json' \
     -d $'{
  "id": "1",
  "email": "invalid-syntax@example.org"
}'
HTTP/1.1 500
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Content-Type: application/json
Transfer-Encoding: chunked
Date: Wed, 15 May 2024 14:19:12 GMT
Connection: close

{"cause":{"cause":null,"message":"Invalid UUID string: 1"},"message":"Failed to convert from type [java.lang.String] to type [java.util.UUID] for value [1]"}
$ curl -i -X "PUT" "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac9" \
     -H 'Content-Type: application/json' \
     -d $'{
  "id": "0bbe06f4-9751-4f97-af16-db10eb1dcac8",
  "email": "ids-do-not-match@example.org"
}'
HTTP/1.1 500
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Content-Type: application/json
Transfer-Encoding: chunked
Date: Wed, 15 May 2024 14:36:48 GMT
Connection: close

{"timestamp":"2024-05-15T14:36:48.464+00:00","status":500,"error":"Internal Server Error","path":"/users/0bbe06f4-9751-4f97-af16-db10eb1dcac9"}
$ curl -i -X "PATCH" "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7" \
     -H 'Content-Type: application/json' \
     -d $'{
  "email": "patched@example.org"
}'
HTTP/1.1 200
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Content-Type: application/hal+json
Transfer-Encoding: chunked
Date: Wed, 15 May 2024 14:41:50 GMT

{
  "email" : "patched@example.org",
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7"
    },
    "user" : {
      "href" : "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac7"
    }
  }
}
$ curl -i -X "PATCH" "http://localhost:8080/users/0bbe06f4-9751-4f97-af16-db10eb1dcac8" \
     -H 'Content-Type: application/json' \
     -d $'{
  "email": "not-found@example.org"
}'
HTTP/1.1 404
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Content-Length: 0
Date: Wed, 15 May 2024 14:42:30 GMT
$ curl -i -X "PATCH" "http://localhost:8080/users/1" \
     -H 'Content-Type: application/json' \
     -d $'{
  "email": "invalid-syntax@example.org"
}'
HTTP/1.1 500
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Content-Type: application/json
Transfer-Encoding: chunked
Date: Wed, 15 May 2024 14:44:19 GMT
Connection: close

{"cause":{"cause":null,"message":"Invalid UUID string: 1"},"message":"Failed to convert from type [java.lang.String] to type [java.util.UUID] for value [1]"}

Wouldn't documenting that both cases return a 404 Not Found be documenting the obvious? If a resource does not exist, 404 is the natural status code by definition, isn't it?

Current behavior:

a) PUT id exists ⇒ 200 OK b) PUT id does not exists ⇒ 201 Created c) PUT id syntactically incorrect ⇒ 500 Internal Server Error d) PUT ids do not match ⇒ 500 Internal Server Error e) PATCH id exists ⇒ 200 OK f) PATCH id does not exists ⇒ 404 Not Found g) PATCH id syntactically incorrect ⇒ 500 Internal Server Error

Right now c and g return the same error.

One might argue that f and g should return the same error.


The current documentation for PUT and PATCH states:

Custom Status Codes The PATCH method has only one custom status code: 405 Method Not Allowed

404 and 500 are not mentioned anywhere.

Looking at Default Status Codes:

200 OK: For plain GET requests. 201 Created: For POST and PUT requests that create new resources.

For 200 one could add: "; for PATCH and PUT requests that update resources"