Skip to content

When using Configuration annotation BeanDefinitionRegistryPostProcessor, the instance type of dependency injection is inconsistent with getBeanClassName in BeanDefinition #22990

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
brucelwl opened this issue May 18, 2019 · 12 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: feedback-provided Feedback has been provided

Comments

@brucelwl
Copy link

brucelwl commented May 18, 2019

When using Configuration annotation BeanDefinitionRegistryPostProcessor, the instance type of dependency injection is inconsistent with getBeanClassName in BeanDefinition. The code is as follows

@Configuration
public class MyBeanDefinitionRegistryPostProcessor implements BeanDefinitionRegistryPostProcessor {

    @Override
    public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry beanDefinitionRegistry) throws BeansException {
        System.out.println();
    }

    @Override
    public void postProcessBeanFactory(ConfigurableListableBeanFactory configurableListableBeanFactory) throws BeansException {
        System.out.println();
    } 
}
@RunWith(SpringRunner.class)
@SpringBootTest(classes = {DemoApplication.class})
public class DemoApplicationTests {

    @Autowired
    private ConfigurableApplicationContext context;

    @Autowired
    private MyBeanDefinitionRegistryPostProcessor myBeanDefinitionRegistryPostProcessor;

    @Test
    public void beanClassTest() {

        String beanClass = myBeanDefinitionRegistryPostProcessor.getClass().getName();

        String beanDefinitionBeanClass = context.getBeanFactory()
                .getBeanDefinition("myBeanDefinitionRegistryPostProcessor")
                .getBeanClassName();

        System.out.println(beanClass);
        System.out.println(beanDefinitionBeanClass);

        Assert.assertEquals("beanClass should equals beanDefinitionBeanClass", beanClass, beanDefinitionBeanClass);
    }
}

image

The reasons are as follows:

The method enhanceConfigurationClasses in org.springframework.context.annotation.ConfigurationClassPostProcessor ,as long as beanDef is a full configuration, it will be added to configBeanDefs and then generated by cglib into the new class, resulting in updating the beanClassName in BeanDefinition. The code should be changed to the following

image

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 18, 2019
@jhoeller jhoeller self-assigned this May 18, 2019
lgxbslgx added a commit to lgxbslgx/spring-framework that referenced this issue May 18, 2019
Cannot enhance @configuration bean definition if its
singleton instance has been created too early.
Add the corresponding testcase.
See spring-projects#22990.
@lgxbslgx
Copy link
Contributor

I submit a PR to solve this issue. The PR follows the log that says Cannot enhance @Configuration bean definition since its singleton instance has been created too early.

In my opinion, I think we could enhance it even if it has been created. Maybe we should use the enhanced bean definition to create the new instance and override the old one.

@jhoeller What is your opinion. If you agree with me, I would modify my PR which would create the new instance instead of following the log.

@brucelwl
Copy link
Author

I submit a PR to solve this issue. The PR follows the log that says Cannot enhance @Configuration bean definition since its singleton instance has been created too early.

In my opinion, I think we could enhance it even if it has been created. Maybe we should use the enhanced bean definition to create the new instance and override the old one.

@jhoeller What is your opinion. If you agree with me, I would modify my PR which would create the new instance instead of following the log.

@lgxbslgx @jhoeller This is not a good solution because it breaks the Spring bean singleton,because the beans created in advance have been invoked, if replacing the beans causes inconsistencies, I think it's only necessary to enhance the configuration beans that are not instantiated

@brucelwl
Copy link
Author

brucelwl commented May 19, 2019

@lgxbslgx
image
It is suggested that we should change it to this way of writing.
......
else if (beanFactory.containsSingleton(beanName) && logger.isInfoEnabled()) {
logger.info("Cannot enhance @configuration bean definition '" + beanName +
"' since its singleton instance has been created too early. The typical cause " +
"is a non-static @bean method with a BeanDefinitionRegistryPostProcessor " +
"return type: Consider declaring such methods as 'static'.");
}
else {
configBeanDefs.put(beanName, (AbstractBeanDefinition) beanDef);
}
......

@lgxbslgx
Copy link
Contributor

else if (beanFactory.containsSingleton(beanName) && logger.isInfoEnabled()) {
	logger.info("Cannot enhance @Configuration bean definition '" + beanName .......);
}
else {
	configBeanDefs.put(beanName, (AbstractBeanDefinition) beanDef);
}

The code above maybe causes some problem. If beanFactory.containsSingleton(beanName) is true and logger.isInfoEnabled() is false, the configBeanDefs.put(beanName, (AbstractBeanDefinition) beanDef) would be executed, which is not our expectation.

@brucelwl
Copy link
Author

@lgxbslgx Sorry, you're right.

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label May 23, 2019
@brucelwl
Copy link
Author

@lgxbslgx @sbrannen @jhoeller This problem has been reported for 9 days. Why not deal with it?

@lgxbslgx
Copy link
Contributor

lgxbslgx commented Sep 4, 2019

@sbrannen @jhoeller This problem has been backlogged for a long time. Do you have time to review the PR? Thanks.

@sbrannen
Copy link
Member

sbrannen commented Sep 6, 2019

MyBeanDefinitionRegistryPostProcessor should not be annotated with @Configuration since it is not a Java Config class.

The fact that you have annotated it with @Configuration is why you see $$EnhancerBySpringCGLIB appended to the class name.

How does your application behave if you annotate MyBeanDefinitionRegistryPostProcessor with @Component?

@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Sep 6, 2019
@brucelwl
Copy link
Author

brucelwl commented Sep 9, 2019

@sbrannen Yes, using @component would be the right performance.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 9, 2019
@sbrannen
Copy link
Member

sbrannen commented Sep 9, 2019

@sbrannen Yes, using @component would be the right performance.

Thanks for the feedback, @brucelwl.

Does that mean that we can close this issue as resolved from your end?

@brucelwl
Copy link
Author

brucelwl commented Sep 9, 2019

@sbrannen Yes, using @component would be the right performance.

Thanks for the feedback, @brucelwl.

Does that mean that we can close this issue as resolved from your end?

I just think that if a developer accidentally uses @Configuration, there will be an imperfect warning message or even a bug in this place, because the log says that there is no proxy, but it changes that class is a proxy class. If you think this is an unimportant problem, you can close this issue.

@sbrannen
Copy link
Member

Thanks for the additional input, @brucelwl.

We'll consider whether it makes sense to provide additional diagnostic information for such use cases.

@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 8, 2021
@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Aug 23, 2023
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) status: feedback-provided Feedback has been provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants