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
Re-add wildcard types and modify checks for acceptable types. #1977
Conversation
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.
Nice work @whikloj. Code looks good and it works as advertised.
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.
This looks good to me and is functioning as I would expect for GET and HEAD requests. The only thing I am wondering is if the integration test should verify that */*
accept header works as well (which it does in manual testing, but yeah).
I will add that in.
…On Fri., Mar. 11, 2022, 1:31 p.m. Ben Pennell, ***@***.***> wrote:
***@***.**** approved this pull request.
This looks good to me and is functioning as I would expect for GET and
HEAD requests. The only thing I am wondering is if the integration test
should verify that */* accept header works as well (which it does in
manual testing, but yeah).
—
Reply to this email directly, view it on GitHub
<#1977 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVZVYOYGYX6FABK5V4YBYLU7ONQFANCNFSM5P3UEYUQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraLdpIT.java
Show resolved
Hide resolved
For RDF sources, should we verify that |
Sorry about that, I read the need for testing |
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.
Looks good!
JIRA Ticket: https://fedora-repository.atlassian.net/browse/FCREPO-3810
What does this Pull Request do?
Re-adds wildcards to the Jersey endpoints otherwise Jersey rejects
Accept
headers that aren't in the list (which are all RDF types andtext/plain
andtext/html
).Also makes HEAD requests respond the same as a GET request.
How should this be tested?
Before this PR.
Put up a binary that is not
text/plain
ortext/html
(you can put up plain text and claim it isapplication/pdf
though)ie.
Do a HEAD request to see the Content-type is set correctly.
Now do a HEAD and/or GET request using that mimetype with the Accept header.
Pull in this PR, rebuild and try getting them again.
But it still fails if the type is not compatible.
Interested parties
Tag (@ mention) interested parties or, if unsure, @fcrepo/committers