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

Globbing should return 200 OK even if empty #900

Open
JornWildt opened this issue Nov 2, 2018 · 7 comments
Open

Globbing should return 200 OK even if empty #900

JornWildt opened this issue Nov 2, 2018 · 7 comments

Comments

@JornWildt
Copy link

Assuming I have a container named /notes in my POD and this container contains resources matching /notes/note-* I get a nice list of those resources if I do a GET of /notes/note-*.

But if the /notes container is empty and I try GET /notes/note-* I get 404 - not found.

If think that is wrong, The container is there and any attempt to get a filtered list of its content should return 200 OK since the list is there (the container exists) but it is simply empty as there are no matching resources in the container.

Returning 404 tells the client "You made a mistake - there is nothing here". But the client did not make a mistake - it simply asked for an empty list, not a missing-non-existing list.

@scenaristeur
Copy link

scenaristeur commented Nov 15, 2018

if the /notes container is empty and You try GET /notes/note-* , there is nothing in /notes that match note-*, i'm agree with the 404.

@megoth megoth added the triage Issues that need team review label Nov 15, 2018
@Ryuno-Ki
Copy link

Ryuno-Ki commented Nov 15, 2018

What about using a 204 - No Content?

@RubenVerborgh
Copy link
Contributor

200 or 204 are the only right options. 4xx means client error; but there is no client error. Rather, the resource /notes/note-* corresponds to "the aggregation of…" and that resource always exists, regardless of whether it is empty.

200 is easiest for consistency reasons; 204 is equally fine.

@JornWildt
Copy link
Author

I like 204. It acknowledges that the query is valid - it just doesn't return anything.

Returning 404 would trigger error handling in the client - potentially yielding a warning message to the end user "Something went wrong - the query engine is not there!" confusing the end user since nothing did indeed go wrong.

If the /notes container does not exist at all, I would find 404 a proper answer.

It is kind of the same discussion for (non networked) code that return a list of items - should it return null if there are no elements or should it return an empty list? I would prefer the empty list.

But it is an old discussion - see for instance https://stackoverflow.com/questions/11746894/what-is-the-proper-rest-response-code-for-a-valid-request-but-an-empty-data

@RubenVerborgh
Copy link
Contributor

But it is an old discussion - see for instance stackoverflow.com/questions/11746894/what-is-the-proper-rest-response-code-for-a-valid-request-but-an-empty-data

That's a different case:

For example you run a GET request for users/9 but there is no user with id #9.

In our case, it's really simple. The resource "the aggregation of…" exists, so no client error. If the parent container does not exist, that's something else.

The reason we can choose between 200 or 204 is that the empty RDF document is also a syntactically valid Turtle document; a zero-byte document would be invalid HTML/XML/JSON-LD. But I like 204 because it is then the same across representations, and doesn't choke any parsers.

@michielbdejong michielbdejong removed the triage Issues that need team review label Mar 25, 2019
@michielbdejong
Copy link
Member

Won't fix while we're discussing the proposal to remove globbing from the spec

@RubenVerborgh
Copy link
Contributor

@michielbdejong Catch-22 then, as @melvincarvalho (understandably) is objecting to that change until there is an alternative.

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

No branches or pull requests

6 participants