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

Spring MVC's locale resolver can no longer be customized in parent context #25290

Closed
wilkinsona opened this issue Jun 19, 2020 · 43 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: regression A bug that is also a regression
Milestone

Comments

@wilkinsona
Copy link
Member

Affects: 5.3 snapshots

The fix for #25209 appears to have removed or at least limited the ability to customize the locale resolver that is used by the DispatcherServlet.

The introduction of a default LocaleResolver bean means that Spring Boot's auto-configuration of a LocaleResolver backs off as it is presumed that the bean that's now present is one provided by the user. This means that the spring.mvc.locale and spring.mvc.locale-resolver properties are no longer honoured.

This change will also break any Spring Boot user who has defined their own localeResolver bean as it would attempt to override the bean defined by WebMvcConfigurationSupport. This will fail as bean definition overriding is disabled by default in Boot.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 19, 2020
@bclozel bclozel added type: bug A general bug type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on type: bug A general bug labels Jun 19, 2020
@bclozel bclozel added this to the 5.3 M1 milestone Jun 19, 2020
@bclozel bclozel added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 19, 2020
@sdeleuze sdeleuze self-assigned this Jun 20, 2020
@sdeleuze
Copy link
Contributor

@wilkinsona @bclozel I would like to discuss with you in order to see if there is way to keep achieving the goal of #25209 which is important for our GraalVM story and an interesting improvement on the JVM as well since it avoids to load DispatcherServlet.properties by default and improve consistency of how Spring MVC beans are managed.

On Spring Framework side, I should indeed probably remove this newly added LocaleResolver bean in order to let Boot or users customize it more easily.

The problem is that with current Boot autoconfiguration, DispatcherServlet#getDefaultStrategies is invoked, DispatcherServlet.properties is parsed and a reflection call invisible to GraalVM native compiler happens for this single bean since no LocaleResolver bean is defined by default.

Current configuration in WebMvcAutoConfiguration.WebMvcAutoConfigurationAdapter#localeResolver only configures a LocaleResolver bean when the user configure it because of the double @ConditionalOnMissingBean + @ConditionalOnProperty(prefix = "spring.mvc", name = "locale") conditions.

I would need Boot to configure a default AcceptHeaderLocaleResolver bean by default as well. Is it possible?

@wilkinsona
Copy link
Member Author

Yeah, I think we could do that. We document that, by default, we'll use the Accept-Language header to resolve the locale. This relies on DispatcherServlet using AcceptHeaderLocaleResolver by default. It would be safer if Boot configured that default instead as we then wouldn't have to worry about an (unlikely) change to the DispatcherServlet default.

@sdeleuze
Copy link
Contributor

As pointed out by @jhoeller, in WebFluxConfigurationSupport we create a LocaleContextResolver and provide a protected method to customize it.

protected LocaleContextResolver createLocaleContextResolver() {
	return new AcceptHeaderLocaleContextResolver();
}

@Bean
public LocaleContextResolver localeContextResolver() {
	return createLocaleContextResolver();
}

Why this out-of-the-box definition of a locale resolver is a problem with MVC but not with WebFlux?
Does Boot auto-configure locale resolution with WebFlux? If yes, then how?
Could we use similar arrangement in order to allow Boot to configure the LocaleReolver bean used?

I am asking that because it would be more consistent and would avoid Spring Framework to introduce a dedicated config subclass like shown below for Spring MVC without Spring Boot purpose:

@Configuration(proxyBeanMethods = false)
public class DefaultWebMvcConfiguration extends DelegatingWebMvcConfiguration {
	@Bean
	public LocaleResolver localeResolver() {
		return new AcceptHeaderLocaleResolver();
	}
}

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Documented
@Import(DefaultWebMvcConfiguration.class)
public @interface EnableWebMvc {
}

@wilkinsona
Copy link
Member Author

We don't have any support for customising WebFlux's LocaleContextResolver at the moment. I don't think there's any specific reason for that and we could add some, but it wouldn't be as flexible as what is possible with Spring MVC in Framework 5.2.

If MVC switched to the WebFlux approach, users would no longer be able to provide their own LocaleResolver bean. It would require overriding the MVC-configured bean and bean overriding is disabled by default in Boot. This change in behaviour would be a regression from what was possible with Boot 2.3.

@sdeleuze
Copy link
Contributor

sdeleuze commented Jun 22, 2020

As discussed with @wilkinsona today, it seems possible to turn those newly added beans into Spring Boot compliant ones.

In a nutshell, in Boot it is possible override WebMvcConfigurationSupport beans with @ConditionalOnMissingBean and similar Boot annotations, something like:

public static class EnableWebMvcConfiguration extends DelegatingWebMvcConfiguration implements ResourceLoaderAware {

		@Bean
		@ConditionalOnMissingBean
		@ConditionalOnProperty(prefix = "spring.mvc", name = "locale", matchIfMissing = true)
		@Override
		public LocaleResolver localeResolver() {
			if (this.mvcProperties.getLocaleResolver() == WebMvcProperties.LocaleResolver.FIXED) {
				return new FixedLocaleResolver(this.mvcProperties.getLocale());
			}
			AcceptHeaderLocaleResolver localeResolver = new AcceptHeaderLocaleResolver();
			localeResolver.setDefaultLocale(this.mvcProperties.getLocale());
			return localeResolver;
		}

		@Bean
		@ConditionalOnMissingBean
		@Override
		public ThemeResolver themeResolver() {
			return super.themeResolver();
		}

		@Bean
		@ConditionalOnMissingBean
		@Override
		public FlashMapManager flashMapManager() {
			return super.flashMapManager();
		}

		@Bean
		@ConditionalOnMissingBean
		@Override
		public RequestToViewNameTranslator viewNameTranslator() {
			return super.viewNameTranslator();
		}
}

The original LocaleResolver bean declaration in Spring Boot should be removed since replaced by the one above.

3 possibilities or LocaleResolver:

  • User bean : Boot does not create LocaleResolver bean
  • No user bean and spring.mvc.locale set: Boot creates the right LocaleResolver bean based on the property.
  • No user bean and spring.mvc.locale not set : the localResolver() method is still invoked due to matchIfMissing = true but here to mimic the Spring MVC original return new AcceptHeaderLocaleResolver();. this.mvcProperties.getLocale() returns null by default so this is equivalent.

This also allows ThemeResolver, FlashMapManager and RequestToViewNameTranslator easy override by the user via declaring a bean with the name name.

As a consequence I close this issue, please reopen in case of blocking point.

@sdeleuze sdeleuze removed this from the 5.3 M1 milestone Jun 22, 2020
@schuch
Copy link

schuch commented Oct 28, 2020

I just tried to migrate a "classic" Spring MVC Webapp to 5.3.0

It has a custom LocaleResolver defined in applicationContext.xml

<bean id="localeResolver" class="...CustomLocaleResolver" />

This config is no longer honored. Feels like a breaking change to me.

@sdeleuze
Copy link
Contributor

I agree this is an issue, we are discussing the best solution to fix it. I will keep you informed.

@sbrannen
Copy link
Member

@schuch, are you using <mvc:annotation-driven /> in your XML config file?

@sbrannen
Copy link
Member

@schuch, would you mind creating a new issue to report this possible regression, since this issue has already been closed?

@schuch
Copy link

schuch commented Oct 28, 2020

@schuch, would you mind creating a new issue to report this possible regression, since this issue has already been closed?

I assumed you would be open to reopen, due to:

As a consequence I close this issue, please reopen in case of blocking point.
(#25290 (comment))

But yes, i can raise a new issue.

@schuch
Copy link

schuch commented Oct 28, 2020

@schuch, are you using <mvc:annotation-driven /> in your XML config file?

yes, this is part of the XML configuration

@sdeleuze
Copy link
Contributor

Indeed we can probably reopen it since it is not targeting a specific release yet.

@sdeleuze sdeleuze reopened this Oct 28, 2020
@sdeleuze sdeleuze added this to the 5.3.1 milestone Oct 28, 2020
@nikomiranda
Copy link

Same problem with Java Config.


    @Bean
    public LocaleResolver localeResolver() {
    	Locale fixedLocale = new Locale("ja", "JP");
        FixedLocaleResolver localeResolver = new FixedLocaleResolver();
        localeResolver.setDefaultLocale(fixedLocale);
        
        return localeResolver;
    }         

Is ignored and accept-header is used.

@sdeleuze
Copy link
Contributor

sdeleuze commented Nov 9, 2020

My mistake, after testing again it works as expect.

@schuch Feel free to test on your side with the latest 5.3.1-SNAPSHOT.

@sdeleuze sdeleuze closed this as completed Nov 9, 2020
@schuch
Copy link

schuch commented Nov 9, 2020

@schuch Feel free to test on your side with the latest 5.3.1-SNAPSHOT.

Tested successfully with 5.3.1-SNAPSHOT 🥳

@bjornharvold
Copy link

Hi Spring Team. Just upgraded to Spring Boot 2.4.0. This is still an issue. Attaching sample project.
demo.zip

@sdeleuze
Copy link
Contributor

@bjornharvold I think what happens is that when using @EnableWebMvc Spring Boot web config backs off, so @ConditionalOnMissingBean on LocalResolver bean which is Boot specific is not taken anymore in account and you are just using Spring MVC LocaleResolver bean which requires spring.main.allow-bean-definition-overriding=true as rightfully explained in the error message. I think this is the expected behavior.

For typical use case I would advise to not use @EnableWebMvc with Boot, and if you really need it,setting spring.main.allow-bean-definition-overriding=true is the way to go.

@bjornharvold
Copy link

Thank you for clarifying @sdeleuze.

@Y00sh00
Copy link

Y00sh00 commented Nov 13, 2020

Ran into the same issue as bjornharvold however not with @EnableWebMvc but I think from WebMvcConfigurer in order to use i18n / internalization would this indeed be such a use-case @bjornharvold where overriding would be the best alternative or am I missing something. Can't seem to find an alternative that doesn't involve using the addInterceptor method from that class.

    @Override
    public void addInterceptors(InterceptorRegistry registry) {
        registry.addInterceptor(localeChangeInterceptor());
        registry.addInterceptor(new SquareInterceptor(loggingUtils()));
    }

@PiotrTraczynski
Copy link

PiotrTraczynski commented Nov 14, 2020

spring53issueLocalRevoler.zip

I updated spring boot to 2.4.0. And I have the same issue with localResolver.

@PiotrTraczynski
Copy link

@bjornharvold I think what happens is that when using @EnableWebMvc Spring Boot web config backs off, so @ConditionalOnMissingBean on LocalResolver bean which is Boot specific is not taken anymore in account and you are just using Spring MVC LocaleResolver bean which requires spring.main.allow-bean-definition-overriding=true as rightfully explained in the error message. I think this is the expected behavior.

For typical use case I would advise to not use @EnableWebMvc with Boot, and if you really need it,setting spring.main.allow-bean-definition-overriding=true is the way to go.

I know, but if I on this option. I will disabled mechanizm which control duplicates of bean. So It open potencial debugging issue.

@sdeleuze
Copy link
Contributor

@PiotrTraczynski Your repro has @EnableWebMvc so it seems to be exactly the use case I already answered to. You can avoid to set spring.main.allow-bean-definition-overriding=true by not using @EnableWebMvc which is IMO a good practice in modern Boot apps.

@sdeleuze
Copy link
Contributor

sdeleuze commented Nov 16, 2020

@PiotrTraczynski @schuch Complementary note: if you really need for your use case to customize Spring MVC web configuration instead of Spring Boot one, there is a third solution suggested by @rstoyanchev which is probably better than @EnableWebMvc + spring.main.allow-bean-definition-overriding=true : extend WebMvcConfigurationSupport and override localeResolver():

@Configuration
public class MyWebMvcConfig extends WebMvcConfigurationSupport {

	@Bean
	@Override
	public SessionLocaleResolver localeResolver() {
		SessionLocaleResolver slr = new SessionLocaleResolver();
		slr.setDefaultLocale(StringUtils.parseLocale("pl"));
		return slr;
	}
}

That will give you all the flexibility to override the default beans in a Spring MVC fashion without having to set spring.main.allow-bean-definition-overriding=true on Spring Boot side. Ccing @snicoll in case Boot time want to provide some guidance about that in Spring Boot 2.4 release notes.

@PiotrTraczynski
Copy link

PiotrTraczynski commented Nov 16, 2020 via email

@fdlk
Copy link

fdlk commented Mar 18, 2021

I am upgrading to 5.3.

  • it's a WebMVC application, no Spring Boot
  • with a custom LocaleResolver bean
  • and multiple WebMvcConfigurer implementations

The suggestions given here and in the upgrade docs are useful but incomplete.

  • If you move the localeResolver bean factory method to a WebMvcConfigurer, you get BeanDefinitionOverrideException, because it clashes with the bean created in the DelegatingWebMvcConfiguration.
  • If you replace @EnableWebMvc with a WebMvcConfig that extends WebMvcConfigurationSupport, the application boots but the WebMvcConfigurers get skipped(!)

What worked for me was to replace @EnableWebMvc with a WebMvcConfig that extends DelegatingWebMvcConfiguration instead of WebMvcConfigurationSupport:

@Configuration
public class WebMvcConfig extends DelegatingWebMvcConfiguration {
  @Bean
  @Override
  public LocaleResolver localeResolver() { ... }
}

@prdebski
Copy link

prdebski commented Mar 30, 2021

Another solution is to name your bean differently than "localeResolver" and annotate it with @Primary.

@Bean
@Primary
public LocaleResolver customLocaleResolver() { ... }

@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 9, 2021

Thanks for your feedback, I have updated the upgrade notes with @fdlk proposal which seems the most relevant one.

tommydeboer pushed a commit to molgenis/molgenis that referenced this issue Sep 23, 2021
* chore: upgrade to spring 2.4.3

* fix: move localeResolver bean definition to WebMvcConfig class

See spring-projects/spring-framework#25290 (comment)

* fix(web): drop special cases

MethodArgumentNotValidException now extends Errors

* fix: manually specify scopes

Spring security behavior changed, no longer requests all scopes but only openid

* fix: do not use the deprecated method

* chore: upgrade to spring 2.4.10

* style: format sources
@catfishlty
Copy link

catfishlty commented Mar 16, 2022

  • Spring Boot Application with starter
  • Spring Boot 2.6.4

Here're some tips when using custom starter. In my projects, if a LocaleResolver is setup in a custom starter, it won't work in the application.

So we need to make sure LocaleResolver is setup in application configuration but not starter configuration . For example, there're 2 repos named test-demo & test-demo-starter, which means test-demo-starter is used by test-demo. So make sure that LocaleResolver is setup in application config not starter config. For example, there're 2 repos named test-demo & test-demo-starter, which means test-demo-starter is used by test-demo.

WebConfig in test-demo-starter

@Configuration
@ConditionalOnWebApplication
@ConditionalOnMissingBean(WebMvcConfig.class)
public class WebMvcConfig extends DelegatingWebMvcConfiguration {
   ...
}

public class CommonLocaleResolver implements LocaleResolver {
   ...
}

CustomWebMvcConfig in test-demo

@Configuration
@ConditionalOnWebApplication
public class CustomWebMvcConfig extends WebMvcConfig {
    @Bean
    public LocaleResolver localeResolver() {
        return new CommonLocaleResolver();
    }

   ...
}

I hope it can help.

@max91
Copy link

max91 commented Apr 4, 2022

Hello. Faced the following problem. Overridden LocaleResolver by deriving a new class. When starting the application from the idea, everything works fine, but if you pack the application in a jar package, then when you start app the bin conflicts with what is in the class DelegatingWebMvcConfiguration. Please tell me how to solve the problem. Spring version 5.3.18

@Configuration
public class CustomWebMvcConfiguration extends DelegatingWebMvcConfiguration {

    public static final String COOKIE_NAME = "lang";
    public static final Locale DEFAULT_LOCALE = new Locale("ru");
    public static final TimeZone DEFAULT_TIMEZONE = TimeZone.getTimeZone(ZoneOffset.UTC);

    @Bean
    @NotNull
    @Override
    public LocaleResolver localeResolver() {
        CookieLocaleResolver resolver = new CookieLocaleResolver();
        resolver.setCookieName(COOKIE_NAME);
        resolver.setDefaultLocale(DEFAULT_LOCALE);
        resolver.setDefaultTimeZone(DEFAULT_TIMEZONE);
        return resolver;
    }

    @Bean
    public LocaleChangeInterceptor localeChangeInterceptor() {
        LocaleChangeInterceptor interceptor = new LocaleChangeInterceptor();
        interceptor.setParamName(COOKIE_NAME);
        return interceptor;
    }

}

@bonkerman
Copy link

Faced this issue too while trying to create a bean inside a configuration imported from library using @AutoConfiguration. As the LocaleResolver bean was being created in WebMvcAutoConfiguration which has @AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE + 10), the configuration I was providing should have had higher precedence, so adding @AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE + 9) to my configuration class solved the issue.

@rsebestik
Copy link

Without overriding beans globally (without spring.main.allow-bean-definition-overriding=true),
it is possible to override just the "localeResolver" default from Spring via interceptor.
Define interceptor:

    @Bean
    HandlerInterceptor localeResolverInterceptor(...) {
        return new LocaleResolverHandlerInterceptor(new MyCustomAcceptHeaderLocaleResolver(...));
    }
class LocaleResolverHandlerInterceptor implements HandlerInterceptor {

    private final MyCustomAcceptHeaderLocaleResolver customLocaleResolver;

    @Override
    public boolean preHandle(@Nonnull final HttpServletRequest request,
                             @Nonnull final HttpServletResponse response,
                             @Nonnull final Object handler) {

        if (handler instanceof HandlerMethod) {
            request.setAttribute(DispatcherServlet.LOCALE_RESOLVER_ATTRIBUTE, customLocaleResolver);
        }

        return true;
    }
}

Use interceptor:

class MyWebMvcConfigurer  implements WebMvcConfigurer {

    @Autowired
    private LocaleResolverHandlerInterceptor localeResolverHandlerInterceptor;

    @Override
    public void addInterceptors(final InterceptorRegistry registry) {
        registry.addInterceptor(localeResolverHandlerInterceptor);
    }

that will override code from DispatcherServlet, where "localeResolver" bean is set to request via request.setAttribute(LOCALE_RESOLVER_ATTRIBUTE, this.localeResolver);

@clfsoft
Copy link

clfsoft commented Oct 7, 2024

Faced this issue too while trying to create a bean inside a configuration imported from library using @AutoConfiguration. As the LocaleResolver bean was being created in WebMvcAutoConfiguration which has @AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE + 10), the configuration I was providing should have had higher precedence, so adding @AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE + 9) to my configuration class solved the issue.

use following is better
@AutoConfigureBefore(WebMvcAutoConfiguration.class)

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) status: feedback-provided Feedback has been provided type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests