Skip to content

BeanPostProcessor can eagerly initialize factory beans that use constructor injection #24553

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

Closed
AlesD opened this issue Feb 18, 2020 · 8 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression)

Comments

@AlesD
Copy link

AlesD commented Feb 18, 2020

Auto configuration of MethodValidationPostProcessor in class org.springframework.boot.autoconfigure.validation.ValidationAutoConfiguration requires instance of Environment. Since the envirnonemt does not have @Lazy annotation it triggers autowire candidate resolution with lazyInit=true which in turn results in initialization of (at least) FactoryBeans.

Bean post processors should NOT use autowiring since the beans created just to check that they do not implement Environment are later not eligible for all other bean post processors - which is actually confirmed in log by lots of:

Bean 'xxx' of type [XXX] is not eligible for getting processed by all BeanPostProcessors

I have implemented custom ImportBeanDefinitionRegistrar. When I use it like this:

@SpringBootApplication
@Import(GenesysRegistrar.class)
public class IntegrationServer {
...
}

everything works fine. But if I create my own auto-configuration class:

@Configuration
@Import(GenesysRegistrar.class)
public class GenesysAutoConfiguration {
}

then bean definitions registered by GenesysRegistrar are created BEFORE MethodValidationPostProcessor during search for bean implementing Environment interface.

@wilkinsona
Copy link
Member

We have a number of auto-configuration classes of our own that @Import an ImportBeanDefinitionRegistrar and we haven't seen any problems with beans not being eligible for post-processing. I'd like to make sure that we fully understand the problem before making a change. @AlesD, to help us to do that, can you please provide a small sample that reproduces the behaviour you have described?

@AlesD
Copy link
Author

AlesD commented Feb 19, 2020

I have created small example to demosnstrate the issue. There is no way to upload files here so below is base64 encoded content od zip file. Save the text to file - example.txt and use commad line:

Anyway duritn creation of the example I have found the root cause of the problem. It is bacause I'm using factory bean that takes other bean as dependecy via constructor. The reason why I have chosen this aproach is beacause sometimes the bean definiton uses factory bean to create the bean instance and sometimes creates the bean instance directly. The class takes one parameter as constructor argument and some others as peorperties. Therefore I have created my factory bean in same way.

However during autoconfiguration the bean that is injected as constructor argument of the factory bean is eagerly initialized. Interesting thing is that this happens only if yusing auto configuration. If you remove META-INF/spring-factories from the example, the MyAutoConfiguration class will still be found and the beans "bean1" and "bean2" will still be instantiated - but this time correctly.

I have not found any information that using constructor arguments on factory beans is prohibited or bad practise. For "plain classes" I definitelly prefer constructor injection for required colaborators - especially if they can't be changed nad can be declared as final inside the class.

I can get around the issue by replacing constror argument on my factory with property. The registrar will have to handle "factory case" differently than "plain class" case, but that can be easily done.

I still belive, however, that the search for autowire candidates should be more careful about eagerly initializing every bean in inspects.

snicoll referenced this issue in snicoll-scratches/gh-20219 Feb 19, 2020
@snicoll
Copy link
Member

snicoll commented Feb 19, 2020

Thanks for the sample.

There is no way to upload files here

The comment area has a section that states you can attach files by dragging & dropping. I've moved that sample to a proper github repo, see https://github.com/snicoll-scratches/gh-20219. I've also edited your comment to remove the base64 content.

@snicoll snicoll changed the title Auto configuration of MethodValidationPostProcessor uses autowiring BeanPostProcessor can eagerly initialize factory beans that use constructor injection Feb 19, 2020
@snicoll snicoll transferred this issue from spring-projects/spring-boot Feb 19, 2020
@snicoll
Copy link
Member

snicoll commented Feb 19, 2020

@jhoeller Andy and I looked at it and we don't think there is anything we can do in Spring Boot. Andy also feels this can be related to #13893. Thoughts?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 19, 2020
@AlesD
Copy link
Author

AlesD commented Feb 19, 2020

OK - I changed the factory beans to use property injection instead of constructor injection.

What is strange is that the issue does not occur if you put @Import(MyRegistrar.class) on the main class or any other @Configuration class in application. It only happens during auto-configuration. I would expect that the bean definitions created by Registrar shall be created before MethodValidationPostProcessor is instantiated. But maybe tat is not true with JavaConfig.

Maybe it would be good to mention somewhere that constructor injection should be avoided for FactoryBean implementations. Maybe javadoc of PostProcessorRegistrationDelegate or its inner class BeanPostProcessorChecker that emits the "not eligible" info message?

Anyway thanks for feedback.

@wyhasany
Copy link
Contributor

wyhasany commented Apr 8, 2021

I've also stumbled upon on this issue in my starter. I've used BeanFactory to help instantiate bean definition with help of autowiring constructor arguments. Because of logic in MethodValidationPostProcessor all arguments has been initiated too early causing:

Bean 'x' of type [y] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)

Workaround with property autowiring works smoothly. Thanks @AlesD !

@snicoll / @wilkinsona could you consider add some notes in documentation encouraging autowiring custom BeanFactories by property?

@snicoll
Copy link
Member

snicoll commented Nov 13, 2023

I have updated the sample to a supported version of the framework (5.3.x) and I can't reproduce the problem. Sorry we didn't get to it sooner but I can't reproduce. If you can, then please update the sample and we can reopen.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Nov 13, 2023
@snicoll snicoll removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 13, 2023
@AndreTeigler
Copy link

Maybe I can provide a sample. Basic Spring Boot App including RabbitMQ support. Extending the RabbitConnectionFactoryBean and importing the SslProperties from Spring:

@component
public class SSLRabbitConnectionFactoryBean extends RabbitConnectionFactoryBean {

private final SslProperties SslProperties;

public SSLRabbitConnectionFactoryBean(SslProperties SslProperties){
    this.SslProperties = SslProperties;
}

}

Starting the application results in the error:

Bean 'spring.ssl-org.springframework.boot.autoconfigure.ssl.SslProperties' of type [org.springframework.boot.autoconfigure.ssl.SslProperties] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). Is this bean getting eagerly injected into a currently created BeanPostProcessor [persistenceExceptionTranslationPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.

If injection is done by Autowire the warning disappears.

demo.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression)
Projects
None yet
Development

No branches or pull requests

7 participants