Return empty lists with status 200 instead of 400 - Fix for issue #70

xiaohutai / jsonapi

[Bolt Extension] JSON API for Bolt CMS

https://market.bolt.cm/view/bolt/jsonapi

MIT License

55 stars 18 forks source link

Return empty lists with status 200 instead of 400 - Fix for issue #70 #80

Closed d-m- closed 6 years ago

d-m- commented 6 years ago

If the response to a request is an empty list, return that empty list with status 200, instead of 400. Fixes issue #70.

Removed method throwErrorOnNoResults, as it would be used only in SingleAction.php (for retrieving a single record). Changed the status code from 400 to 404 for the response if this record does not exist, according to http://jsonapi.org/format/#fetching-resources-responses.

Raistlfiren commented 6 years ago

Although, I am in agreement with this PR, I feel like this will need to be pushed in such a way to inform users that it breaks the current flow. This push could break several websites.

jadwigo commented 6 years ago

Yes it might break existing sites - but it certainly does break sites that are expecting a standards conforming reply according to http://jsonapi.org/format/#fetching-resources-responses

You're basically "punishing" clients that conform while being lenient to clients that do not conform.

As for a my opinion what it should do:

If you request a resource listing with a content type that does not exist a response with "404: Not found" is expected.
If you request a resource listing that returns no results the response with a "200: OK" and an empty result set should be returned.
A response with "400: Bad request" may be returned when the the extra params in the request contain filters that are wrong somehow - so this would include querying on a non existing fieldname because you made a typo.

xiaohutai commented 6 years ago

I'm not sure what is the best one:

V4: Because functionality has changed (and basically informing: please beware things have changed)
V3: Because returning 400 is actually a bug/fault from our side

I feel we can keep it V3, but I'm open for ideas/suggestions.

xiaohutai commented 6 years ago

I'm leaning towards a minor version upgrade.

As @GawainLynch said on Slack:

Given that you're fixing a bug, a BC break like that in a minor is permitted

Raistlfiren commented 6 years ago

@jadwigo and @xiaohutai - That works for me then. Merge away!

xiaohutai commented 6 years ago

Thanks @d-m- for the PR!