-
Notifications
You must be signed in to change notification settings - Fork 38.5k
Provide better lifecyle for WebMvcConfigurer.configurePathMatch #26427
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
Comments
We could expose an The one catch is that by doing this Spring Data REST opts into always using parsed patterns, which may be a reasonable choice I think. It performs better and supports largely the same syntax (except for notably |
This commit ensures the PathPatternParser cannot be set after request mappings have been initialized when it is too late. See gh-26427
I've added assertions in HandlerMapping implementations to ensure the PathPatternParser is rejected if it is too late and I've also added an |
…ing. Instead of registering the PathPatternParser on DelegatingHandlerMapping via WebMvcConfigurer.configurePathMatch(…) we now consume the bean exposed in context of the fix for spring-projects/spring-framework#26427.
…ing. Instead of registering the PathPatternParser on DelegatingHandlerMapping via WebMvcConfigurer.configurePathMatch(…) we now consume the bean exposed in context of the fix for spring-projects/spring-framework#26427. We also use the newly introduced RequestMappingInfo.mutate() to add our customizations of the produces clause for Spring Data REST's mappings. Fixes GH-1965.
…ing. Instead of registering the PathPatternParser on DelegatingHandlerMapping via WebMvcConfigurer.configurePathMatch(…) we now consume the bean exposed in context of the fix for spring-projects/spring-framework#26427. We also use the newly introduced RequestMappingInfo.mutate() to add our customizations of the produces clause for Spring Data REST's mappings. Fixes GH-1965.
@odrotbohm are the current changes sufficient in which case we can close this? |
I think we're set. Fixes already in Spring Data REST master and supported branches for quite a while. |
Okay thanks. |
Currently
WebMvcConfigurer.configurePathMatch(...)
is called as a side-effect ofWebMvcConfigurationSupport.getPathMatchConfigurer()
. This makes it quite hard to use in@Bean
method since it's hard to tell if the@Bean
method or theconfigurePathMatch(...)
method will be called first.You can see an example of this in Spring Data Rest where the restHandlerMapping bean would ideally have a
PatternParser
on the delegates before theafterPropertiesSet()
method is called. The current solution of overridingconfigurePathMatch
and callingsetPatternParser
directly is brittle because the parser is needed beforeafterPropertiesSet()
is called. This leads to issues such as #26415It's possible to work-around the issue by adding
@DependsOn("mvcPathMatcher")
which will indirectly trigger theWebMvcConfigurationSupport.getPathMatchConfigurer()
method, but this isn't ideal.I wonder if we could make
PathMatchConfigurer
or thePatternParser
a@Bean
so that it can be injected directly if it's needed?The text was updated successfully, but these errors were encountered: