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

Provide a way to modify RequestMappingInfo conditions #26428

Closed
philwebb opened this issue Jan 21, 2021 · 6 comments
Closed

Provide a way to modify RequestMappingInfo conditions #26428

philwebb opened this issue Jan 21, 2021 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

Spring Data Rest needs to take a RequestMappingInfo and modify the produces condition. It's currently not possible to do this because the required constructor is private.

I think that constructor either needs to be public, or we need some with... methods that return new instances with updated values (similar to addCustomCondition.

@philwebb
Copy link
Member Author

Relates to #26415

@rstoyanchev
Copy link
Contributor

RequestMappingInfo has a builder which is why the public constructors are deprecated. We can add a .mutate() instance method on RequestMappingInfo that gives a Builder pre-populated with the content of the instance. Does that sound okay? What exactly needs to be changed on the produces condition, perhaps we can tailor the Builder even better to that somehow?

@rstoyanchev rstoyanchev self-assigned this Jan 22, 2021
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Jan 22, 2021
@rstoyanchev rstoyanchev added this to the 5.3.4 milestone Jan 22, 2021
@philwebb
Copy link
Member Author

We probably need some input from @odrotbohm. Currently the system looks quite flexible, it takes the ProducesRequestCondition and calls a protected method that can adapt it. You can see the code in BasePathAwareHandlerMapping.

We can add a .mutate() instance method on RequestMappingInfo that gives a Builder pre-populated with the content of the instance

That might be an option, but there's currently no builder methods that take the ProducesRequestCondition objects. There's only produces(String... produces).

@rstoyanchev
Copy link
Contributor

There is an issue in the referenced code since the RequestMapingInfo may have a PathPatternsRequestCondition instead of a PatternsRequestCondition. I'll add a .mutate() method on RequestMappingInfo to support the use case of changing it.

@philwebb
Copy link
Member Author

Unfortunately the mutate() build won't allow us to change the ProducesRequestCondition. It only has a produces(String...) version. I think we need a version that will allow us to set the ProducesRequestCondition directly.

Either that, or we need to change the BasePathAwareHandlerMapping contract.

@odrotbohm
Copy link
Member

I have a fix ready that customizes the ProducesCondition and then extracts the media types to pass them into the produces(…) method. This looks like a bit of extra work but does the trick for now. Just waiting for @mp911de decision on when we can actually merge this as it requires Spring Data to upgrade to 5.3.4 snapshots.

This was referenced Mar 13, 2021
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

No branches or pull requests

4 participants