buda-base / lds-pdi

http://purl.bdrc.io BDRC Linked Data Server
Apache License 2.0
2 stars 0 forks source link

http code 406 when unavailable encoding #54

Closed eroux closed 6 years ago

eroux commented 6 years ago

When a resource is asked with an Accept header that we don't serve, I think we should return an http 406

MarcAgate commented 6 years ago

This was implemented for POST/GET methods for the following endpoints:

-@Path("/resource/{res}") -@Path("/resource/{res}.{ext}") in the second case, the extension is converted to mimeType in order to do the checking.

eroux commented 6 years ago

thanks, it's interesting to have done that for the second case... I think 404 was fine, but why not!

eroux commented 6 years ago

I can't trigger it:

curl -I http://purl.bdrc.io/resource/P1583 -H "Accept: text/invalid"

still gives

HTTP/1.1 303 See Other
Connection: keep-alive
Location: http://library.bdrc.io/show/bdr:P1583

while it should give a 406. Interestingly, the spec specifies that in this case there should be a list of alternatives in the payload, so we could send the html page in this case (the one we generate for http 300).

Note that it's pretty clear in the spec that this code shouldn't be used for the path /resource/{res}.{ext} (only for the one with no extension). For this path, we should send a normal 404, but we can certainly send the html page (with the choices) in the payload.

MarcAgate commented 6 years ago

I think we can close this one, don't you ?

eroux commented 6 years ago

Nope:

$ curl -I http://purl.bdrc.io/resource/P1583 -H "Accept: text/invalid"

HTTP/1.1 300 Multiple Choices

please make a unit test for this case

MarcAgate commented 6 years ago

Well, this is how we implemented it in GET method (returning a the content location of a html page containing possible urls for the requested resouce and the header alternates) . 406 is only returned for POST method This case is variant==null

eroux commented 6 years ago

The behavior is correct for post, but for get it should be 406 except when the accept header is empty, in which case it should be 300. (It used to be this way, but there were some problems during the manual merge of the code I wrote I think)

eroux commented 6 years ago

Thanks, I think the code I wrote took less lines:

https://github.com/BuddhistDigitalResourceCenter/lds-pdi/blob/testVariants/src/main/java/io/bdrc/ldspdi/rest/resources/PublicDataResource.java#L129

but I'm probably being picky...

MarcAgate commented 6 years ago

Done as of commit ce9f7e6 (Including corresponding test units)

eroux commented 6 years ago

great! cool to see some unit tests!