Spring Framework
  1. Spring Framework
  2. SPR-8269

BeanFactoryPostProcessor breaks default post-processing of @Configuration classes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Complete
    • Affects Version/s: 3.1 M1
    • Fix Version/s: 3.1 M2
    • Component/s: Core
    • Labels:
      None
    • Last commented by a User:
      false

      Description

      When using AnnotationConfigApplicationContext, if I declare at least one @Bean of type BeanFactoryPostProcessor (even if it's a stub that doesn't do anything), this breaks default post-processing of the @Configuration bean, meaning that @Autowired fields are no longer injected, @PostConstruct methods are not called, etc.

      I'm attaching a test case to prove my point.

      A workaround is to manually add the relevant BeanPostProcessors (like AutowiredAnnotationBeanPostProcessor and CommonAnnotationBeanPostProcessor) to BeanFactory.

      Same thing happens in web app when I use ContextLoaderInitializer to load @Configuration classes or define them through XML config. My particular case is that I use MyBatis-Spring integration and I cannot declare a @Bean of type org.mybatis.spring.mapper.MapperScannerConfigurer using annotation config, because this bean is a BeanFactoryPostProcessor and thus breaks autowiring of @Configuration class.

        Issue Links

          Activity

          Hide
          Osvaldas Grigas added a comment -

          Of course, by "ContextLoaderInitializer" I meant ContextLoaderListener, but you got the point.

          Show
          Osvaldas Grigas added a comment - Of course, by "ContextLoaderInitializer" I meant ContextLoaderListener, but you got the point.
          Hide
          Chris Beams added a comment -

          Thanks for the report, Osvaldas. This has indeed been an issue known about, at least internally, for some time.

          There is a fundamental lifecycle conflict in handling BeanFactoryPostProcessor @Bean methods within @Configuration classes that use @Autowired, @PostConstruct, @Value, etc. Because BFPPs must be instantiated early in the lifecycle, they cause early instantiation of their declaring @Configuration class - too early to recieve the usual post-processing by AutowiredAnnotationBeanPostProcessor and friends.

          To resolve this issue, it is now possible to declare @Bean methods as static. This permits the container to detect and invoke these methods without instantiating the @Configuration class, thus avoiding the aforementioned lifecycle issues.

          @Bean Javadoc has been updated to reflect, and the commit comment follows:

          Author: Chris Beams <cbeams@vmware.com>
          Date:   Tue May 10 18:17:26 2011 +0800
          
              Allow static modifier on @Bean methods
              
              Declaring @Bean methods as 'static' is now permitted, whereas previously
              it raised an exception at @Configuration class validation time.
              
              A static @Bean method can be called by the container without requiring
              the instantiation of its declaring @Configuration class. This is
              particularly useful when dealing with BeanFactoryPostProcessor beans,
              as they can interfere with the standard post-processing lifecycle
              necessary to handle @Autowired, @Inject, @Value, @PostConstruct and
              other annotations.
              
              static @Bean methods cannot recieve CGLIB enhancement for scoping and
              AOP concerns. This is acceptable in BFPP cases as they rarely if ever
              need it, and should not in typical cases ever be called by another
              @Bean method.  Once invoked by the container, the resulting bean will
              be cached as usual, but multiple invocations of the static @Bean method
              will result in creation of multiple instances of the bean.
              
              static @Bean methods may not, for obvious reasons, refer to normal
              instance @Bean methods, but again this is not likely a concern for BFPP
              types. In the rare case that they do need a bean reference, parameter
              injection into the static @Bean method is technically an option, but
              should be avoided as it will potentially cause premature instantiation
              of more beans that the user may have intended.
              
              Note particularly that a WARN-level log message is now issued for any
              non-static @Bean method with a return type assignable to BFPP.  This
              serves as a strong recommendation to users that they always mark BFPP
              @Bean methods as static.
              
              See @Bean Javadoc for complete details.
              
              Issue: SPR-8257, SPR-8269
          
          Show
          Chris Beams added a comment - Thanks for the report, Osvaldas. This has indeed been an issue known about, at least internally, for some time. There is a fundamental lifecycle conflict in handling BeanFactoryPostProcessor @Bean methods within @Configuration classes that use @Autowired, @PostConstruct, @Value, etc. Because BFPPs must be instantiated early in the lifecycle, they cause early instantiation of their declaring @Configuration class - too early to recieve the usual post-processing by AutowiredAnnotationBeanPostProcessor and friends. To resolve this issue, it is now possible to declare @Bean methods as static . This permits the container to detect and invoke these methods without instantiating the @Configuration class, thus avoiding the aforementioned lifecycle issues. @Bean Javadoc has been updated to reflect, and the commit comment follows: Author: Chris Beams <cbeams@vmware.com> Date: Tue May 10 18:17:26 2011 +0800 Allow static modifier on @Bean methods Declaring @Bean methods as 'static' is now permitted, whereas previously it raised an exception at @Configuration class validation time. A static @Bean method can be called by the container without requiring the instantiation of its declaring @Configuration class. This is particularly useful when dealing with BeanFactoryPostProcessor beans, as they can interfere with the standard post-processing lifecycle necessary to handle @Autowired, @Inject, @Value, @PostConstruct and other annotations. static @Bean methods cannot recieve CGLIB enhancement for scoping and AOP concerns. This is acceptable in BFPP cases as they rarely if ever need it, and should not in typical cases ever be called by another @Bean method. Once invoked by the container, the resulting bean will be cached as usual, but multiple invocations of the static @Bean method will result in creation of multiple instances of the bean. static @Bean methods may not, for obvious reasons, refer to normal instance @Bean methods, but again this is not likely a concern for BFPP types. In the rare case that they do need a bean reference, parameter injection into the static @Bean method is technically an option, but should be avoided as it will potentially cause premature instantiation of more beans that the user may have intended. Note particularly that a WARN-level log message is now issued for any non-static @Bean method with a return type assignable to BFPP. This serves as a strong recommendation to users that they always mark BFPP @Bean methods as static. See @Bean Javadoc for complete details. Issue: SPR-8257, SPR-8269
          Hide
          Chris Beams added a comment -

          Osvaldas - as an additional note, it looks like MapperScannerConfigurer should really be a BeanDefinitionRegistryPostProcessor instead of a BeanFactoryPostProcessor. The reason for this is that it actually registers additional beans (through scanning) as opposed to simply post-processing additional bean definitions. I realize you're not responsible for this class, but you might want to relay it to the Mybatis team. At a glance, I'm not sure how nicely this class will play with the @Configuration world - it depends on what it scans, and who depends on those scanned types.

          I'd be happy to talk with the Mybatis team if they wish to discuss.

          Note that BeanDefinitionRegistryPostProcessor is a specialization of BeanFactoryPostProcessor, so they sholud be able to refactor in a backward-compatible way.

          Show
          Chris Beams added a comment - Osvaldas - as an additional note, it looks like MapperScannerConfigurer should really be a BeanDefinitionRegistryPostProcessor instead of a BeanFactoryPostProcessor. The reason for this is that it actually registers additional beans (through scanning) as opposed to simply post-processing additional bean definitions. I realize you're not responsible for this class, but you might want to relay it to the Mybatis team. At a glance, I'm not sure how nicely this class will play with the @Configuration world - it depends on what it scans, and who depends on those scanned types. I'd be happy to talk with the Mybatis team if they wish to discuss. Note that BeanDefinitionRegistryPostProcessor is a specialization of BeanFactoryPostProcessor, so they sholud be able to refactor in a backward-compatible way.
          Hide
          Osvaldas Grigas added a comment -

          Thanks for the fix!
          Does this also fix SPR-7868, on which you commented: "Because @Configuration classes are actually themselves processed by a BeanDefinitionRegistryPostProcessor, they cannot in turn register additional BeanDefinitionRegistryPostProcessors"?

          If you think that it's now possible to register BDRPP using @Configuration classes, I'll submit the MapperScannerConfigurer change proposal to MyBatis bug tracker.

          Show
          Osvaldas Grigas added a comment - Thanks for the fix! Does this also fix SPR-7868 , on which you commented: "Because @Configuration classes are actually themselves processed by a BeanDefinitionRegistryPostProcessor, they cannot in turn register additional BeanDefinitionRegistryPostProcessors"? If you think that it's now possible to register BDRPP using @Configuration classes, I'll submit the MapperScannerConfigurer change proposal to MyBatis bug tracker.
          Hide
          Putthibong Boonbong added a comment -

          MapperScannerConfigurer was changed to be BeanDefinitionRegistryPostProcessor in version 1.0.2 but it throw error when work with PropertyPlaceholderConfigurer. Please check my attachment.

          Show
          Putthibong Boonbong added a comment - MapperScannerConfigurer was changed to be BeanDefinitionRegistryPostProcessor in version 1.0.2 but it throw error when work with PropertyPlaceholderConfigurer. Please check my attachment.
          Hide
          Chris Beams added a comment -

          @Osvaldas,

          No, it is still not possible to register a BDRPP from within a @Configuration class, and for the same reasons originally stated.

          @Putthibong,

          Thanks for attaching the reproduction project. I ran the tests, and it fails as follows:

          Tests in error: 
            testPropertyPlaceholderConfigurer(org.mybatis.spring.mapper.PropertyPlaceholderTest): Error creating bean with name 'mapperScannerConfigurer' defined in class path resource [org/mybatis/spring/mapper/PropertyPlaceholderConfigurer-config.xml]: Cannot resolve reference to bean 'dataSource' while setting bean property 'dataSource'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'dataSource' defined in class path resource [org/mybatis/spring/mapper/PropertyPlaceholderConfigurer-config.xml]: Initialization of bean failed; nested exception is org.springframework.beans.TypeMismatchException: Failed to convert property value of type 'java.lang.String' to required type 'java.lang.Class' for property 'driverClass'; nested exception is java.lang.IllegalArgumentException: Cannot find class [${jdbc.driverClass}]
          Tests run: 2, Failures: 0, Errors: 1, Skipped: 0
          

          That last bit:

          nested exception is java.lang.IllegalArgumentException: Cannot find class [${jdbc.driverClass}]
          

          is happening because MapperScannerConfigurer depends on the DataSource bean, but the DataSource bean's properties have not yet been processed by the PropertyPlaceholderConfigurer registered by <context:property-placeholder/>.

          The reason for this is that PropertyPlaceholderConfigurer is a BeanFactoryPostProcessor and MapperScannerConfigurer is a BeanDefinitionRegistryPostProcessor.

          BDRPP implementations are always processed by the container before BFPP implementations. This means that you have an unresolvable chicken-and-egg problem here.

          As a rule, BeanDefinitionRegistryPostProcessor beans must not depend on {{$

          {...}}} property replacement, and they must not depend on other beans that need such replacement. It simply won't happen "in time".

          To work around this issue, if you are using Spring 3.1 you can consider injecting the Environment object and querying properties directly there, instead of using {{${...}

          }} replacement. See the Javadoc for Environment for more details.

          Show
          Chris Beams added a comment - @Osvaldas, No, it is still not possible to register a BDRPP from within a @Configuration class, and for the same reasons originally stated. @Putthibong, Thanks for attaching the reproduction project. I ran the tests, and it fails as follows: Tests in error: testPropertyPlaceholderConfigurer(org.mybatis.spring.mapper.PropertyPlaceholderTest): Error creating bean with name 'mapperScannerConfigurer' defined in class path resource [org/mybatis/spring/mapper/PropertyPlaceholderConfigurer-config.xml]: Cannot resolve reference to bean 'dataSource' while setting bean property 'dataSource'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'dataSource' defined in class path resource [org/mybatis/spring/mapper/PropertyPlaceholderConfigurer-config.xml]: Initialization of bean failed; nested exception is org.springframework.beans.TypeMismatchException: Failed to convert property value of type 'java.lang.String' to required type 'java.lang.Class' for property 'driverClass'; nested exception is java.lang.IllegalArgumentException: Cannot find class [${jdbc.driverClass}] Tests run: 2, Failures: 0, Errors: 1, Skipped: 0 That last bit: nested exception is java.lang.IllegalArgumentException: Cannot find class [${jdbc.driverClass}] is happening because MapperScannerConfigurer depends on the DataSource bean, but the DataSource bean's properties have not yet been processed by the PropertyPlaceholderConfigurer registered by <context:property-placeholder/> . The reason for this is that PropertyPlaceholderConfigurer is a BeanFactoryPostProcessor and MapperScannerConfigurer is a BeanDefinitionRegistryPostProcessor . BDRPP implementations are always processed by the container before BFPP implementations. This means that you have an unresolvable chicken-and-egg problem here. As a rule, BeanDefinitionRegistryPostProcessor beans must not depend on {{$ {...}}} property replacement, and they must not depend on other beans that need such replacement. It simply won't happen "in time". To work around this issue, if you are using Spring 3.1 you can consider injecting the Environment object and querying properties directly there, instead of using {{${...} }} replacement. See the Javadoc for Environment for more details.

            People

            • Assignee:
              Chris Beams
              Reporter:
              Osvaldas Grigas
              Last updater:
              Trevor Marshall
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                2 years, 21 weeks, 1 day ago