-
Notifications
You must be signed in to change notification settings - Fork 4
Return 303 status code for unsupported application/vnd.crossref.unixref+xml content type
#151
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
Conversation
This reverts commit bc52b8a.
application/vnd.crossref.unixref+xml content type
| it "header" do | ||
| get "/#{doi}", nil, { "HTTP_ACCEPT" => "application/vnd.crossref.unixref+xml" } | ||
|
|
||
| expect(last_response.status).to eq(303) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would it return 303? Shouldn't it return 406 (looking at code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was basing this on the other tests here:
content-negotiation/spec/api/api_spec.rb
Lines 278 to 292 in ab7a875
| context "unknown accept header" do | |
| it "header" do | |
| get "/#{doi}", nil, { "HTTP_ACCEPT" => "application/xml" } | |
| expect(last_response.status).to eq(303) | |
| expect(last_response.headers["Location"]).to eq("https://blog.datacite.org/announcing-schema-4-2/") | |
| end | |
| it "link" do | |
| get "/application/xml/#{doi}" | |
| expect(last_response.status).to eq(404) | |
| expect(last_response.body).to eq("The resource you are looking for doesn't exist.") | |
| end | |
| end |
But good point. I can take a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's entirely possible that it does return a 303, I don't see where, but I'm not overly familiar with this codebase. I wouldn't worry too much as long as it's logically consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we return 303s when a content type is not recognized. Try this:
curl --location 'https://doi.org/10.48322/553p-pm90' \
--header 'Accept: application/not-real-content-type'
That aligns with my understanding of how content negotiation is intended to work. If the content type is supported, then the formatted content is returned; if the content type is not supported, then the landing page is returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the 303 is because it throws it off to the end DOI url to be processed. Alright fair enough
We don't actively support content negotiation requests for Crossref XML, i.e.
application/vnd.crossref.unixref+xml, but previously returned a blank response when using this header. This blank response was rendered by bolognese: https://github.com/datacite/bolognese/blob/64b18d1439801216c7ce59e5a76a7c383d171161/lib/bolognese/writers/crossref_writer.rb#L3-L11 This change removes internal support forapplication/vnd.crossref.unixref+xmland instead treats it as an unsupported content type.closes: datacite/datacite#2448