katharsis-project / katharsis-framework

Katharsis adds powerful layer for RESTful endpoints providing implementenation of JSON:API standard
http://katharsis.io
Apache License 2.0
135 stars 65 forks source link

findOne is not returning 404 when entity does not exist #436

Open phuo1032 opened 7 years ago

phuo1032 commented 7 years ago

This issue seemed to be fixed at some point from a PR

https://github.com/katharsis-project/katharsis-framework/issues/272

But the latest version does not have this change any more. https://github.com/katharsis-project/katharsis-framework/blob/master/katharsis-core/src/main/java/io/katharsis/core/internal/dispatcher/controller/ResourceGet.java

The test case that verifies the NOT_FOUND status is also not there. https://github.com/katharsis-project/katharsis-framework/blob/master/katharsis-core/src/test/java/io/katharsis/core/internal/dispatcher/controller/resource/ResourceGetTest.java

Is this a design change for restoring the previous behavior or am I suppose to throw the NOT_FOUND exception manually?

phuo1032 commented 7 years ago

I just read the jsonapi spec. It states

404 Not Found

A server MUST respond with 404 Not Found when processing a request to fetch a single resource that does not exist, except when the request warrants a 200 OK response with null as the primary data (as described above).

Looks like this is not a bug. Is there a way we can alternate the behavior through the model or repository setup instead of going with the default behavior?

hibaymj commented 7 years ago

If I understand this right, you're saying despite the behavior matching the spec, you would like to have the ability to modify the response?

phuo1032 commented 7 years ago

Correct. I believe the spec accepts both response (404 or 200 with null data). We just want to go with the 404 response.

I have figured this out by throwing new ResourceNotFoundException. But I want to double check if this is the right (best) way.