RESTEasy Reactive - Test selecting from multiple media types #960
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michalvavrik
added
the
triage/backport-2.13?
|
Backport comment - only works with 2.13.6. |
|
Hello @rsvoboda , @mjurc , @afalhambra I've added you all as reviewers for I don't know who is still available. First one to review win! |
|
Hmm, I just thought of the way how to make the PR more interesting, give me 30 minutes. |
michalvavrik
force-pushed
the
feature/multiple-media-types-selection
branch
from
f3c646c to
6224ddf
Compare
rsvoboda
requested changes
Dec 22, 2022
http/jaxrs-reactive/src/main/java/io/quarkus/ts/http/jaxrs/reactive/MediaTypeResource.java
Show resolved
Hide resolved
http/jaxrs-reactive/src/test/java/io/quarkus/ts/http/jaxrs/reactive/MediaTypeSelectionIT.java
Outdated
Show resolved
Hide resolved
http/jaxrs-reactive/src/main/java/io/quarkus/ts/http/jaxrs/reactive/MediaTypeResource.java
Show resolved
Hide resolved
http/jaxrs-reactive/src/test/java/io/quarkus/ts/http/jaxrs/reactive/MediaTypeSelectionIT.java
Show resolved
Hide resolved
afalhambra
approved these changes
Dec 22, 2022
There was a problem hiding this comment.
minor comments, otherwise looks good to me!
Nice work @michalvavrik
http/jaxrs-reactive/src/test/java/io/quarkus/ts/http/jaxrs/reactive/MediaTypeSelectionIT.java
Outdated
Show resolved
Hide resolved
michalvavrik
force-pushed
the
feature/multiple-media-types-selection
branch
from
6224ddf to
15a87c3
Compare
|
Added suggest "bad user" cases to |
michalvavrik
force-pushed
the
feature/multiple-media-types-selection
branch
from
15a87c3 to
f0c4846
Compare
Tests [matching requests to resource methods](https://jakarta.ee/specifications/restful-ws/3.1/jakarta-restful-ws-spec-3.1.html#mapping_requests_to_java_methods) with multiple [media types](https://www.iana.org/assignments/media-types/media-types.xhtml). I combined it with [content negotiation and wildcards](https://www.rfc-editor.org/rfc/rfc9110.html#section-12.4.2) as they are handled by the same [MediaTypeMapper](https://github.com/quarkusio/quarkus/blob/main/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/MediaTypeMapper.java#L38), however I didn't dig every hole (e.g. left out quality values and so on). Main purpose of this PR is to verify quarkusio/quarkus#29732, intention is not to cover all scenarios (f.e. I didn't cover HTTP status 416) as these situations are very rare and users are unlikely to expose multiple endpoints that needs this mapper, it is also not efficient as the mapper is not optimized. succees: `mvn clean verify -Dit.test=MediaTypeSelectionIT -Dquarkus.platform.group-id=io.quarkus.platform -Dquarkus.platform.version=2.13.6.Final` failure: `mvn clean verify -Dit.test=MediaTypeSelectionIT -Dquarkus.platform.group-id=io.quarkus.platform -Dquarkus.platform.version=2.13.5.Final`
michalvavrik
force-pushed
the
feature/multiple-media-types-selection
branch
from
f0c4846 to
3889f75
Compare
|
Fixed 2 comment typos. |
rsvoboda
approved these changes
Dec 22, 2022
|
One test failure is in |
9 tasks
pjgg
removed
the
triage/backport-2.13?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Tests matching requests to resource methods with multiple media types. I combined it with content negotiation and wildcards as they are handled by the same MediaTypeMapper. Main purpose of this PR is to verify quarkusio/quarkus#29732, intention is not to cover all scenarios (f.e. I didn't cover HTTP status 416) as these situations are very rare and users are unlikely to expose multiple endpoints that needs this mapper, it is also not efficient as the mapper is not optimized. There is no point to run added tests on OpenShift (no relation). I also didn't find any related JIRA ticket.
TCKS RESTEasy Reactive tests that are part of Quarkus covers JAX-RS specs media type selection very well, therefore this PR only concentrates on the parts that could have been affected by quarkusio/quarkus#29734.
succees:
mvn clean verify -Dit.test=MediaTypeSelectionIT -Dquarkus.platform.group-id=io.quarkus.platform -Dquarkus.platform.version=2.13.6.Finalfailure:
mvn clean verify -Dit.test=MediaTypeSelectionIT -Dquarkus.platform.group-id=io.quarkus.platform -Dquarkus.platform.version=2.13.5.FinalPlease select the relevant options.
run testsphrase in comment)Checklist: