Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-8984

Make the ConfigurableWebBindingInitializer easy to customize in WebMvcConfigurationSupport

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Complete
    • Affects Version/s: 3.1 GA
    • Fix Version/s: 3.2 RC1
    • Component/s: Web
    • Last commented by a User:
      false

      Description

      When we want to customize binding and want to do it on the global scale by using a WebBindingInitializer we are overriding the one already set on the RequestMappingHandlerAdapter when extending WebMvcConfigurationSupport. It would be nice if the internally used ConfigurableWebBindingInitializer would be a top-level bean so that we could append some configuration options (like direct field access, message code resolver etc. now we need to do a full reconfiguration).

        Activity

        Hide
        rstoya05-aop Rossen Stoyanchev added a comment - - edited

        It should be possible without full reconfiguration:

        public class WebConfig extends WebMvcConfigurationSupport {
         
          @Bean
          public RequestMappingHandlerAdapter requestMappingHandlerAdapter() {
            RequestMappingHandlerAdapter ha = super.requestMappingHandlerAdapter();
            ConfigurableWebBindingInitializer initializer = (ConfigurableWebBindingInitializer) ha.getWebBindingInitializer();
            // use setters on initializer ..
          }
        }

        Not the same as overriding a method for initializing a WebBindingInitializer but still relatively simple.

        Show
        rstoya05-aop Rossen Stoyanchev added a comment - - edited It should be possible without full reconfiguration: public class WebConfig extends WebMvcConfigurationSupport {   @Bean public RequestMappingHandlerAdapter requestMappingHandlerAdapter() { RequestMappingHandlerAdapter ha = super .requestMappingHandlerAdapter(); ConfigurableWebBindingInitializer initializer = (ConfigurableWebBindingInitializer) ha.getWebBindingInitializer(); // use setters on initializer .. } } Not the same as overriding a method for initializing a WebBindingInitializer but still relatively simple.
        Hide
        mdeinum Marten Deinum added a comment -

        That is the code which I wrote also. However it requires more knowledge of what is in the configuration, also if you want to use it a MultiActionController and/or BaseCommandController (due to moving over to @Controller style but still having old controllers) it would become quite ugly.

        Show
        mdeinum Marten Deinum added a comment - That is the code which I wrote also. However it requires more knowledge of what is in the configuration, also if you want to use it a MultiActionController and/or BaseCommandController (due to moving over to @Controller style but still having old controllers) it would become quite ugly.
        Hide
        rstoya05-aop Rossen Stoyanchev added a comment -

        If you extend from WebMvcConfigurationSupport as opposed to relying on the methods in WebMvcConfigurer, you probably have more knowledge of the configuration and you're doing it presumably because you want to modify the underlying configuration (beans) directly. The point about MultiActionController is interesting, could you elaborate a little more?

        Show
        rstoya05-aop Rossen Stoyanchev added a comment - If you extend from WebMvcConfigurationSupport as opposed to relying on the methods in WebMvcConfigurer, you probably have more knowledge of the configuration and you're doing it presumably because you want to modify the underlying configuration (beans) directly. The point about MultiActionController is interesting, could you elaborate a little more?
        Hide
        mdeinum Marten Deinum added a comment -

        You got a point there.

        Regarding the MultiActionController (and also the deprecated BaseCommandController) in the past I have used the WebBindingInitializer to have a single location to do general configuration (for all controllers). We used the ConfigurableWebBindingInitializer for this. If these clients (and there are probably a lot more then just the few I have seen) migrate there app to spring 3.1 with java configuration and want to use their configuration what they probably would do is 1-on-1 migrate their xml config to java config including the ConfigurableWebBindingInitializer and register it with the HandlerAdapters as well as their old controllers, this would override and destroy the mvcValidator and conversionService. It would be nice to have a already (partially) configured ConfigurableWebBindingInitializer (with some javadoc stating what it already does) and allow for further customization.

        Might be a corner case but I think it would be nice to have this option as (part of) a migration path.

        Show
        mdeinum Marten Deinum added a comment - You got a point there. Regarding the MultiActionController (and also the deprecated BaseCommandController) in the past I have used the WebBindingInitializer to have a single location to do general configuration (for all controllers). We used the ConfigurableWebBindingInitializer for this. If these clients (and there are probably a lot more then just the few I have seen) migrate there app to spring 3.1 with java configuration and want to use their configuration what they probably would do is 1-on-1 migrate their xml config to java config including the ConfigurableWebBindingInitializer and register it with the HandlerAdapters as well as their old controllers, this would override and destroy the mvcValidator and conversionService. It would be nice to have a already (partially) configured ConfigurableWebBindingInitializer (with some javadoc stating what it already does) and allow for further customization. Might be a corner case but I think it would be nice to have this option as (part of) a migration path.
        Hide
        rstoya05-aop Rossen Stoyanchev added a comment -

        I see, I guess it does makes sense to expose WebBindingInitializer as a bean in those cases. On the other hand the existing controllers probably already have a WBI injected. If that's done via @Autowired, which isn't unlikely since it's been available from 2.5.6, introducing an additional WBI via WebMvcConfigurationSupport would result in an error and require resolving the conflict through a qualifier or by removing the original bean. Not too difficult but not a better experience either.

        There is actually nothing preventing the following as far as I can see:

        public class WebConfig extends WebMvcConfigurationSupport {
         
          @Bean
          public WebBindingInitializer bindingInitializer() {
            ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
            initializer.setConversionService(mvcConversionService());
            initializer.setValidator(mvcValidator());
            // ...
            return initializer;
          }
         
          @Bean
          public RequestMappingHandlerAdapter requestMappingHandlerAdapter() {
            RequestMappingHandlerAdapter adapter = super.requestMappingHandlerAdapter();
            adapter.setWebBindingInitializer(bindingInitializer());
            return adapter;
          }
        }

        Existing controllers will not be impacted by the introduction of MVC Java config and @Controllers. When all controllers need to use the same initializer, the WBI in MVC Java config can be exposed as a bean to have it injected. Not obvious but at least there is an option.

        Show
        rstoya05-aop Rossen Stoyanchev added a comment - I see, I guess it does makes sense to expose WebBindingInitializer as a bean in those cases. On the other hand the existing controllers probably already have a WBI injected. If that's done via @Autowired, which isn't unlikely since it's been available from 2.5.6, introducing an additional WBI via WebMvcConfigurationSupport would result in an error and require resolving the conflict through a qualifier or by removing the original bean. Not too difficult but not a better experience either. There is actually nothing preventing the following as far as I can see: public class WebConfig extends WebMvcConfigurationSupport {   @Bean public WebBindingInitializer bindingInitializer() { ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); initializer.setConversionService(mvcConversionService()); initializer.setValidator(mvcValidator()); // ... return initializer; }   @Bean public RequestMappingHandlerAdapter requestMappingHandlerAdapter() { RequestMappingHandlerAdapter adapter = super .requestMappingHandlerAdapter(); adapter.setWebBindingInitializer(bindingInitializer()); return adapter; } } Existing controllers will not be impacted by the introduction of MVC Java config and @Controllers. When all controllers need to use the same initializer, the WBI in MVC Java config can be exposed as a bean to have it injected. Not obvious but at least there is an option.
        Hide
        rstoya05-aop Rossen Stoyanchev added a comment -

        With SPR-9223 it is now possible to register a MessageCodesResolver through the MVC Java Config. This mirrors the MVC namespace and for more advanced cases extending WebMvcConfigurationSupport should be possible as well.

        Show
        rstoya05-aop Rossen Stoyanchev added a comment - With SPR-9223 it is now possible to register a MessageCodesResolver through the MVC Java Config. This mirrors the MVC namespace and for more advanced cases extending WebMvcConfigurationSupport should be possible as well.
        Hide
        marceloverdijk Marcel Overdijk added a comment -

        @Rossen I still believe there is room for improvement here.
        For your both code examples - how simple they might be - there is more knowledge needed as Martin is mentioning.

        Why not add something like

        configureWebBindingInitializer(ConfigurableWebBindingInitializer initializer) { .. }

        to WebMvcConfigurationSupport?

        Show
        marceloverdijk Marcel Overdijk added a comment - @Rossen I still believe there is room for improvement here. For your both code examples - how simple they might be - there is more knowledge needed as Martin is mentioning. Why not add something like configureWebBindingInitializer(ConfigurableWebBindingInitializer initializer) { .. } to WebMvcConfigurationSupport?
        Hide
        marceloverdijk Marcel Overdijk added a comment -

        What do you think about my previous comment. The suggested

        public void configureWebBindingInitializer(ConfigurableWebBindingInitializer initializer) { .. }

        in WebMvcConfigurationSupport would be in line with other configureX methods in WebMvcConfigurationSupport.

        Show
        marceloverdijk Marcel Overdijk added a comment - What do you think about my previous comment. The suggested public void configureWebBindingInitializer(ConfigurableWebBindingInitializer initializer) { .. } in WebMvcConfigurationSupport would be in line with other configureX methods in WebMvcConfigurationSupport.
        Hide
        rstoya05-aop Rossen Stoyanchev added a comment -

        I've added a method to WebMvcConfigurationSupport that can be used to override or customize directly the ConfigurableWebBindingInitializer. I've not added a similar method to the WebMvcConfigurer interface, which is intended to be simpler. The goal here is to draw a line between simpler and more advanced configuration.

        Show
        rstoya05-aop Rossen Stoyanchev added a comment - I've added a method to WebMvcConfigurationSupport that can be used to override or customize directly the ConfigurableWebBindingInitializer. I've not added a similar method to the WebMvcConfigurer interface, which is intended to be simpler. The goal here is to draw a line between simpler and more advanced configuration.

          People

          • Assignee:
            rstoya05-aop Rossen Stoyanchev
            Reporter:
            mdeinum Marten Deinum
            Last updater:
            Chris Beams
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              5 years, 4 weeks, 4 days ago