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

Warn about non-static BeanDefinitionRegistryPostProcessor declarations on @Configuration classes [SPR-14234] #18808

Closed
spring-issuemaster opened this issue Apr 29, 2016 · 2 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 29, 2016

Joshua Hoke opened SPR-14234 and commented

When you implement the BeanDefinitionRegistryPostProcessor interface on your @Configuration class, it causes the class to be instantiated as a singleton bean before the Spring internal ConfigurationAnnotationProcessor has replaced it with the CGLIB-enhanced subclass in the bean definition.

This was found when using the AnnotationConfigWebApplicationContext contextClass in a Tomcat WAR. We initially saw it with Spring 3.2.8, but it still seems to be a problem in 4.2.5.

This breaks the following behavior described in the documentation:

@Bean Methods in @Configuration Classes

Typically, @Bean methods are declared within @Configuration classes. In this case, bean methods may reference other @Bean methods in the same class by calling them directly. This ensures that references between beans are strongly typed and navigable. Such so-called 'inter-bean references' are guaranteed to respect scoping and AOP semantics, just like getBean() lookups would. These are the semantics known from the original 'Spring JavaConfig' project which require CGLIB subclassing of each such configuration class at runtime. As a consequence, @Configuration classes and their factory methods must not be marked as final or private in this mode. For example:

@Configuration
public class AppConfig {
@Bean
public FooService fooService() {
return new FooService(fooRepository());
}
@Bean
public FooRepository fooRepository() {
return new JdbcFooRepository(dataSource());
}
// ...
}

Instead, fooRepository() in this example will end up running multiple times because the bean instance is the actual @Configuration class instead of the enhanced subclass that caches singleton beans.

Desired behavior:

Either this should work as documented (which could be a breaking change if anyone was depending on this particular bug invoking methods in @Configuration more than once), or at least Spring should log an error or warning if it finds this kind of configuration.


Affects: 4.2.5

Issue Links:

  • #12917 BeanFactoryPostProcessor breaks default post-processing of @Configuration classes

Referenced from: commits 0589c1b, 7737c3c

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 29, 2016

Juergen Hoeller commented

As far as I can see, this is an unpleasant but known and accepted side effect: BeanDefinitionRegistryPostProcessor is a special early callback with significant lifecycle implications. We generally recommend static @Bean methods for post-processor declarations within configuration classes, formally not requiring instantiation of the containing class; this in particular applies to this case.

We need to document this more clearly and should consider warn log entries for any such post-processor definitions.against instance-based factory methods. I'll revisit this for 4.3 RC2.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 2, 2016

Juergen Hoeller commented

ConfigurationClassPostProcessor logs a warning now if a configuration class has been instantiated too early, that is, before we can run our CGLIB enhancement phase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.