BeanFactoryPostProcessor breaks default post-processing of @Configuration classes [SPR-8269]

[spring-projects-issues]

Osvaldas Grigas opened SPR-8269 and commented

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.

---

Affects: 3.1 M1

Attachments:

• org.mybatis.spring.sample.zip (8.37 kB)
• testcase.zip (1.98 kB)

Issue Links:

• @PostConstruct and PropertyPlaceholderConfigurer do not work together in AnnotationConfigApplicationContext [SPR-8257] #12905 @PostConstruct and PropertyPlaceholderConfigurer do not work together in AnnotationConfigApplicationContext ("is duplicated by")
• BeanDefinitionRegistryPostProcessor registered via @Bean will not be invoked [SPR-7868] #12525 BeanDefinitionRegistryPostProcessor registered via @Bean will not be invoked
• Method postProcessBeanDefinitionRegistry is not called if the bean implements BeanDefinitionRegistryPostProcessor [SPR-9464] #14099 Method postProcessBeanDefinitionRegistry is not called if the bean implements BeanDefinitionRegistryPostProcessor
• Warn about non-static BeanDefinitionRegistryPostProcessor declarations on @Configuration classes [SPR-14234] #18808 Warn about non-static BeanDefinitionRegistryPostProcessor declarations on @Configuration classes

Referenced from: commits 52bef0b

[spring-projects-issues]

Osvaldas Grigas commented

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

[spring-projects-issues]

Chris Beams commented

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 <[email protected]>
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

[spring-projects-issues]

Chris Beams commented

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.

[spring-projects-issues]

Osvaldas Grigas commented

Thanks for the fix!
Does this also fix #12525, 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.

[spring-projects-issues]

Putthibong Boonbong commented

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

[spring-projects-issues]

Chris Beams commented

@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.