Properly document the use of MVC Java config with multiple (independent) components contributing configuration [SPR-10565]

[spring-projects-issues]

Oliver Drotbohm opened SPR-10565 and commented

Assume you have to JavaConfig classes extending WebMvcConfigurationSupport, both overriding one of the callbacks of the parent class. Only the first one registered actually gets the callback invoked. See the following test case, the assertion fails with only one resolver registered.

@RunWith(MockitoJUnitRunner.class)
public class ConfigTest {

	@Mock
	static HandlerMethodArgumentResolver resolver;

	@Configuration
	static class FirstConfig extends WebMvcConfigurationSupport {

		@Override
		protected void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentResolvers) {
			argumentResolvers.add(resolver);
		}
	}

	@Configuration
	static class SecondConfig extends WebMvcConfigurationSupport {

		@Override
		protected void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentResolvers) {
			argumentResolvers.add(resolver);
		}
	}

	@Test
	public void foo() {

		AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
		context.setServletContext(new MockServletContext());
		context.register(FirstConfig.class);
		context.register(SecondConfig.class);
		context.refresh();

		RequestMappingHandlerAdapter adapter = context.getBean(RequestMappingHandlerAdapter.class);
		assertThat(adapter.getCustomArgumentResolvers().size(), is(2));
	}
}

---

Affects: 3.2.2

0 votes, 5 watchers

[spring-projects-issues]

Oliver Drotbohm commented

I raised the priority to major as this has quite severe implications. If you use an @Enable annotation that is backed by a class extending WebMvcConfigurationSupport on a configuration class itself extending WMCS the callbacks of the configuration class don't get invoked:

@Configuration
@MyEnableAnnotation // backed by config class extending WebMvcConfigurationSupport
class MyWebConfiguration extends WebMvcConfigurationSupport {

  @Override
  public void configureContentNegotiation(ContentNegotiationConfigurer configurer) {
    // this method is not getting called
  }
}

[spring-projects-issues]

Rossen Stoyanchev commented

I don't think it was intended to work this way. Each instance of WebMvcConfigurationSupport provides a set of Spring MVC infrastructure beans. By declaring two, you get two copies of it and in effect two RequestMappingHandlerAdapter's.

The @EnableWebMvc annotation OTOH is backed by DelegatingWebMvcConfiguration, which detects and invokes any number of WebMvcConfigurer beans. Since DelegatingWebMvcConfiguration is a sub-class of WebMvcConfiguration essentially there is a single copy of the Spring MVC infrastructure with any number of configurers that can customize it. So DelegatingWebMvcConfiguration seems closer to what you're trying to do.

[spring-projects-issues]

Oliver Drotbohm commented

The annotation side-effect aside, isn't that something that should at least be documented? I can imagine a modular application shipping with a configuration class per module where one wants to configure one part of MVC and the other trying to customize another. Wouldn't it make sense to collect the beans of type WebMvcConfigurationSupport (maybe considering @Ordered) and invoking the callbacks for each of them then?

[spring-projects-issues]

Rossen Stoyanchev commented

We can document the current behavior better with regards to multiple components customizing the configuration. The behavior is that @EnableWebMvc (backed by DelegatingWMCS) detects and calls any number of WebMvcConfigurer beans. When you extend directly from WebMvcConfigurationSupport you lose that ability, but you could extend DelegatingWebMvcConfiguration instead if you still wanted to allow WebMvcConfigurer beans to customize it.

I can imagine a modular application shipping with a configuration class per module where one wants to configure one part of MVC and the other trying to customize another. Wouldn't it make sense to collect the beans of type WebMvcConfigurationSupport (maybe considering @Ordered) and invoking the callbacks for each of them then?

No, it doesn't really because WebMvcConfigurationSupport is the configuration itself. Having multiple copies with @Ordered isn't going to help since you need one RequestMappingHandlerAdapter with both argument resolvers, not two RequestMappingHandlerAdapter's with one argument resolver each.

Couldn't your @MyEnableAnnotation be backed by a WebMvcConfigurer? That would allow multiple modules to register their own WebMvcConfigurer's, with the main application module declaring @EnableWebMvc or alternatively extending DelegatingWebMvcConfiguration. The only limitation here is that WebMvcConfigurer doesn't expose everything, and only the main module can do advanced customizations. If this is an issue, we could consider a sub-interface of WebMvcConfigurer that allows more advanced customizations but it shouldn't be needed for your example above.

[spring-projects-issues]

Oliver Drotbohm commented

I've changed the implementation of my @Enable annotation to be backed by an extension of WebMvcConfigurerAdapter. This improves things quite a bit although I now have to access components registered by @EnableWebMvc through qualified autowiring. Trying to access these components more easily was the reason I went with WebMvcConfigurationSupport in the first place. In my sample application I now moved to extending DelegatingWebMvcConfiguration (as per your suggestion) instead of WebMvcConfigurationSupport.

So summarizing the discussion I think it makes sense to alter the ticket more into the direction of improving the documentation to focus more on DelegatingWebMvcConfiguration instead of WebMvcConfigurationSupport as the former disables the WebMvcConfigurerAdapter instances registered.