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

Quarkus Rest empty @QueryParam is handled differently #44885

Closed
ia3andy opened this issue Dec 3, 2024 · 8 comments
Closed

Quarkus Rest empty @QueryParam is handled differently #44885

ia3andy opened this issue Dec 3, 2024 · 8 comments
Labels
area/rest kind/question Further information is requested

Comments

@ia3andy
Copy link
Contributor

ia3andy commented Dec 3, 2024

Describe the bug

I initially discovered this in code.quarkus unit tests. For example when using ?a= with a a List, the list size is not 0 while it was 1 before (with "") as content.

Same with String, before it was an empty string, not it's not defined.

Expected behavior

not sure but we need to clarify if it's an expected change

Actual behavior

it assumes ?a= is the same as ?

How to Reproduce?

reproducer-validation.zip

This reproducer works on 3.17

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

main (3.18)

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@ia3andy ia3andy added the kind/bug Something isn't working label Dec 3, 2024
@ia3andy
Copy link
Contributor Author

ia3andy commented Dec 3, 2024

It seems the change is not in validation but in how the empty query param is treated
Before (in 3.17-), ?a= meant a="", in main it's the same as not having a
This makes the validation constraint fail since there is a default value.

I have another failing test on a similar case where ?e= and e is a list before it was a List.of("") and now it's a List.of().

@ia3andy
Copy link
Contributor Author

ia3andy commented Dec 3, 2024

THis started to fail on Nov 18 and was working on Nov 15

@ia3andy
Copy link
Contributor Author

ia3andy commented Dec 3, 2024

here is a new reproducer clearly showing that empty query params are now handled differently:

reproducer-empty-queryparam.zip

@ia3andy ia3andy changed the title Quarkus Rest validation check on @NotEmpty @QueryParam is not working anymore Quarkus Rest empty @QueryParam is handled differently Dec 3, 2024
Copy link

quarkus-bot bot commented Dec 3, 2024

/cc @FroMage (rest), @stuartwdouglas (rest)

@FroMage
Copy link
Member

FroMage commented Dec 3, 2024

This change was intentional, and explained in #42468 (comment)

If you have use-cases that require the old behaviour, let us know, because we could not think of any.

@geoand
Copy link
Contributor

geoand commented Dec 3, 2024

Thanks for spotting that @FroMage!

I had completely forgot about that one...

@ia3andy
Copy link
Contributor Author

ia3andy commented Dec 3, 2024

@FroMage let's close this when this is in the migraition guide

@ia3andy ia3andy closed this as completed Dec 3, 2024
@ia3andy
Copy link
Contributor Author

ia3andy commented Dec 3, 2024

Thanks @FroMage @geoand @cescoffier

@geoand geoand added kind/question Further information is requested and removed kind/bug Something isn't working labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants