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

Handle Optional<?> @RequestParam() when generating URL using MvcUriComponentsBuilder #22656

Closed

Conversation

fprochazka
Copy link

@fprochazka fprochazka commented Mar 25, 2019

Example of an action:

@GetMapping("/{userId}")
public ModelAndView productGroups(
    @PathVariable @AssertUuid final String userId,
    @RequestParam("parentId") final Optional<@AssertUuid String> rawParentId,
    @PageableDefault(size = 100) final Pageable pageable
) throws ControllerException

this action validates, that the parentId is UUID, if it's present (which works great), but it also rejects the request completely, if it's not (with 400 BAD REQUEST). Which means any of the following ends with 400 - ?parentId, ?parentId=, ?parentId=garbage


  • Generating URL using MvcUriComponentsBuilder.fromMethodCall doesn't work without the OptionalUnwrappingConverter I've provided
  • Without the fix in RequestParamMethodArgumentResolver, it generates empty query args - resulting in url with ?parentId (without = and without value) - which is not valid and ends with 400

I'm not sure if this is the correct way to fix this, so if you'll please give me some feedback on this, I'd be happy to add some tests.


Related issue #16785 - it states that spring handles Optional<?> in handler arguments

@fprochazka fprochazka changed the title Handle optional @RequestParam() when generating URL using MvcUriComponentsBuilder Handle Optional<?> @RequestParam() when generating URL using MvcUriComponentsBuilder Mar 25, 2019
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 25, 2019
@rstoyanchev
Copy link
Contributor

@fprochazka, couldn't this all be handled in RequestParamMethodArgumentResolver, i.e. unpacking the value from the optional and then formatting it as usual? That would be a simpler solution, minus the converter it seems.

@fprochazka
Copy link
Author

fprochazka commented Mar 27, 2019

I'm not sure, you tell me :) I know Spring well enough to bend it, but not yet to solve this kind of problems in systemic way.

FYI, I've just copied already existing convertor and just reversed it with few tweaks. Imho the convertor should be available anyway.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Mar 27, 2019

Could you revise the PR to change only the argument resolver please? Also ideally a test to make the change complete.

@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 27, 2019
@rstoyanchev rstoyanchev added this to the 5.2 M1 milestone Mar 27, 2019
@rstoyanchev rstoyanchev modified the milestones: 5.2 M1, 5.2 M2 Apr 4, 2019
@rstoyanchev rstoyanchev self-assigned this Apr 17, 2019
@fprochazka fprochazka deleted the fp-handle-optional branch August 24, 2019 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants