Clarify how to avoid favoring path extensions as well as deprecation warnings
Closed this issue · 9 comments
Affects: 5.2.4.RELEASE
According to 24179 favorPathExtension is deprecated, and we should all use the Accept header, which is perfect for me, since I always use favorPathExtension = false
The setter in ContentNegotiationManagerFactoryBean is Deprecated, but the field value is true (line 108) , so as far as I understand, the use of path seems in fact encouraged, since is the default value and the only way to set as false is Deprecated
Am I getting something wrong?
No, you're not.
This is very long standing behavior and changing the default can cause significant disruption but maintenance releases are intended to be trouble free so you can take advantage of fixes including security issues, if any.
The purpose of the deprecation and #24179 is:
- Provide an advance warning for a change to come in the next minor release 5.3, see #24179.
- Provide all options necessary to actually make the switch, which required a number of changes as you can see from the commits linked to #24179.
Note that even in 5.3 we will turn off automatic matching to all extensions, but continue to match explicitly registered extensions by default and so favorPathExtension will remain true. In the next minor or major release after that we'll consider completely removing path extension support. So it is a very phased approach.
I'm not quite sure I understand, please correct me if I'm wrong.
I used to have an url like
/something/{email}
And expect a json back. In order to make it work with
/something/data@gmail.com
I needed to set up
configurer.favorPathExtension(false);
Now in order to do that I need to call a deprecated method.
In the future, I will not need to call any method, since this is going to be the default behaibour
That's a very good point but switching the default would likewise defeat the goal of a gradual migration through advance warning and deprecation.
It means we need to explain how to be configured which is:
// 1)
@Override
public void configureContentNegotiation(ContentNegotiationConfigurer configurer) {
configurer.useRegisteredExtensionsOnly(true);
configurer.replaceMediaTypes(Collections.emptyMap());
}
// 2)
@Override
public void configurePathMatch(PathMatchConfigurer configurer) {
// the below will become the default values in 5.3
configurer.useSuffixPatternMatch(false);
configurer.useRegisteredExtensionsOnly(true);
}- Causes a path extension to never be resolved to a MediaType
- Suppresses the use suffix pattern matching and tries registered extensions only of which there should be none.
That's a very good point but switching the default would likewise defeat the goal of a gradual migration through advance warning and deprecation.
It means we need to explain how to be configured which is:
// 1) @Override public void configureContentNegotiation(ContentNegotiationConfigurer configurer) { configurer.useRegisteredExtensionsOnly(true); configurer.replaceMediaTypes(Collections.emptyMap()); } // 2) @Override public void configurePathMatch(PathMatchConfigurer configurer) { // the below will become the default values in 5.3 configurer.useSuffixPatternMatch(false); configurer.useRegisteredExtensionsOnly(true); }
- Causes a path extension to never be resolved to a MediaType
- Suppresses the use suffix pattern matching and tries registered extensions only of which there should be none.
I originally asked this same question on #24179, but I see you started to answer it here. Unfortunately with Spring 5.2.5 I cannot get 2) of your recommendation to work: there is no useSuffixPatternMatch or useRegisteredExtensionsOnly method in org.springframework.web.servlet.config.annotation.PathMatchConfigurer; I do see the old deprecated "set" versions of those methods, but I thought the suggestion here was how to get around the warnings before 5.3 is released.
I cannot get 2) of your recommendation to work: there is no useSuffixPatternMatch or useRegisteredExtensionsOnly method in org.springframework.web.servlet.config.annotation.PathMatchConfigurer
Indeed my bad.
Technically the proper thing to do would be to deprecate them in 5.3 when the defaults values will change and those methods can be avoided altogether.
That said I'd really like to keep the deprecations in 5.2 to maximize advance warning and provide extra time to adapt. Therefore in 5.2 applications will have to use these deprecated methods but as long as they are used to turn off suffix pattern matching, it's okay to suppress the deprecation warnings. Then in 5.3 we will change the default values (but not yet remove the config options), and at that applications can remove their use.
This is what I'll have to update the docs with.
Great, thanks. Here is what I have then.
@Override
public void configureContentNegotiation(
ContentNegotiationConfigurer configurer) {
configurer.useRegisteredExtensionsOnly(true);
configurer.replaceMediaTypes(Collections.emptyMap());
}
@Override
@SuppressWarnings("deprecation")
public void configurePathMatch(PathMatchConfigurer configurer) {
// The below will become the default values in 5.3
// Safe to use in 5.2 as long as disabling pattern match
configurer.setUseSuffixPatternMatch(Boolean.FALSE);
configurer.setUseRegisteredSuffixPatternMatch(Boolean.TRUE);
}
Also, I'm not sure if you saw my comment in #24179, but I had also suggested, to bridge the gap between 5.2.4+ and 5.3, adding a disableUseSuffixPatternMatch() method to be deprecated in 5.3 that disables the functionality (same as above). This would allow a non-deprecated method to set the 5.3 functionality while still allowing the to-be-removed methods to be deprecated in 5.2.4+.
I thought about that but I don't see how it helps since that method will also need to be removed together with the rest and therefore would also need to be deprecated.
Just for completeness, this solution worked for my production code but my unit testing with MockMvc it did not.
What worked for me (not sure if there is another way) is to use MockMvcBuilders.standaloneSetup(your controller).setContentNegotiationManager(new ContentNegotiationManager(List.of(new HeaderContentNegotiationStrategy()))).