Skip to content

Conversation

aureamunoz
Copy link
Member

Fixes #50321

Validate presence of parameters during the RESOLVE_METHOD_PARAMETERS and, if absent, throw exception to stop execution

@quarkus-bot quarkus-bot bot added the area/spring Issues relating to the Spring integration label Oct 3, 2025

This comment has been minimized.

@aureamunoz aureamunoz force-pushed the request-param-required-fix-50321 branch from 3ba49c8 to 5c2cd40 Compare October 7, 2025 16:20
@aureamunoz
Copy link
Member Author

Could you please take a look @geoand when you have some time?

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Oct 8, 2025

It would be nice to have a test for this

@aureamunoz aureamunoz force-pushed the request-param-required-fix-50321 branch from 5c2cd40 to f2fbe42 Compare October 9, 2025 10:21
@aureamunoz
Copy link
Member Author

It would be nice to have a test for this
Done, I've improved the one we already had.

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Oct 9, 2025

Thanks for the test!

Unfortunately, the test is not specific enough, meaning that when someone comes back to this commit in the future, it will be very hard to understand why the test actually tests the requested feature.

@aureamunoz aureamunoz force-pushed the request-param-required-fix-50321 branch from f2fbe42 to ef996a0 Compare October 9, 2025 12:51
@aureamunoz
Copy link
Member Author

How about now?

@geoand
Copy link
Contributor

geoand commented Oct 9, 2025

Much better! Can we also have a test that shows the use of required=false also?

Copy link

quarkus-bot bot commented Oct 9, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit ef996a0.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@aureamunoz
Copy link
Member Author

Much better! Can we also have a test that shows the use of required=false also?

This one was already in here

@geoand
Copy link
Contributor

geoand commented Oct 9, 2025

👌

@geoand geoand merged commit a34d022 into quarkusio:main Oct 9, 2025
27 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.29 - main milestone Oct 9, 2025
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Oct 9, 2025
@aureamunoz
Copy link
Member Author

thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/spring Issues relating to the Spring integration kind/enhancement New feature or request triage/backport triage/backport-3.27
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@RequestParam required true
3 participants