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

Fix null query parameter handling in generated RestEasy Reactive clients #25870

Merged

Conversation

kdubb
Copy link
Contributor

@kdubb kdubb commented May 30, 2022

Also adds missing query handling tests.

The tests for using a Map<> as a query param via the RestQuery annotation are commented out. This is because I cannot figure out how to get these to work. If I uncomment them then the Quarkus refuses to build the client with an error of Failed to find converter for java.util.Map; I am not sure if this is itself a bug or user error.

@kdubb
Copy link
Contributor Author

kdubb commented May 30, 2022

@geoand This is a fix for #23876 that I commented on today. Note that my comments were wrong lol.

@gastaldi gastaldi requested a review from geoand May 31, 2022 00:09
@geoand
Copy link
Contributor

geoand commented May 31, 2022

Map is currently not supported for @RestQuery (or any of the other types actually). I do think it would be useful to have (and not that difficult to add), but let's please remove the commented part of the code for now.

@kdubb
Copy link
Contributor Author

kdubb commented May 31, 2022

Ok. I'm confused by #24783 and its associated issue #24764 that seem to say & implement exactly that.

@geoand
Copy link
Contributor

geoand commented May 31, 2022

That is for the client, not the server

@kdubb
Copy link
Contributor Author

kdubb commented May 31, 2022

Yes. This PR is for the client and the PR/Issue I referenced are about using a Map in the client.

I must be completely missing something here lol.

@geoand
Copy link
Contributor

geoand commented May 31, 2022

I must be completely missing something here lol.

The failure you are getting is from the Server where you are trying to use Map to read all query params - that's what's not supported.

@kdubb kdubb force-pushed the fix/resteasy_reactive_null_query_params branch from ab857bc to 46b91cb Compare May 31, 2022 06:07
@kdubb
Copy link
Contributor Author

kdubb commented May 31, 2022

I knew I was missing something obvious...

I reinstated the tests for map but I'm using UriInfo on the server to extract all the query parameters. This way we're still testing the client's map (or null map) support.

@geoand
Copy link
Contributor

geoand commented May 31, 2022

Great, thanks!

@geoand geoand merged commit 3f4562f into quarkusio:main May 31, 2022
@quarkus-bot quarkus-bot bot added this to the 2.10 - main milestone May 31, 2022
@kdubb kdubb deleted the fix/resteasy_reactive_null_query_params branch June 1, 2022 08:47
@gsmet gsmet modified the milestones: 2.10.0.CR1, 2.7.7.Final Jan 13, 2023
@Sgitario Sgitario mentioned this pull request Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants