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

ConflictingBeanDefinitionException on repeated index evaluation #24978

Closed
ptahchiev opened this issue Apr 26, 2020 · 10 comments
Closed

ConflictingBeanDefinitionException on repeated index evaluation #24978

ptahchiev opened this issue Apr 26, 2020 · 10 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@ptahchiev
Copy link

This is a follow-up from the spring-boot gitter channel.
When upgrading the spring-boot bom from 2.2.5.RELEASE to 2.2.6.RELEASE I get this exception:

Caused by: org.springframework.context.annotation.ConflictingBeanDefinitionException: Annotation-specified bean name 'aBean' for bean class [com.example.demo.ABean] conflicts with existing, non-compatible bean definition of same name and class [com.example.demo.ABean]
	at org.springframework.context.annotation.ClassPathBeanDefinitionScanner.checkCandidate(ClassPathBeanDefinitionScanner.java:349) ~[spring-context-5.2.5.RELEASE.jar:5.2.5.RELEASE]
	at org.springframework.context.annotation.ClassPathBeanDefinitionScanner.doScan(ClassPathBeanDefinitionScanner.java:287) ~[spring-context-5.2.5.RELEASE.jar:5.2.5.RELEASE]

The problem remains even when using 2.2.7.BUILD-SNAPSHOT.
Here's what I've discovered while debugging it:

This method returns false so an exception is raised.

  • It is important to have the spring-context-indexer dependency in your classpath. If you have it then the beans passed to isCompatible method will get getSource() return null.

  • it is also important to name the bean ABean. If I name it DemoBean then the exception does not happen - I think the component scan finds candidates sorted by beanName. So if ABean comes before DemoApplication then it will fail if I name it DemoBean then it will come after DemoApplication and it will not fail.

Here's a repository that reproduces the problem:
https://github.com/ptahchiev/conflicting-bean

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 26, 2020
@jhoeller jhoeller self-assigned this Apr 26, 2020
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 26, 2020
@jhoeller jhoeller added this to the 5.2.6 milestone Apr 26, 2020
@jhoeller
Copy link
Contributor

jhoeller commented Apr 26, 2020

This looks like a side effect of #24638, it seems we need to adapt the isCompatible check accordingly. We also need a backport here since the original change got backported as well (even if not released yet in the backport branches).

@jhoeller
Copy link
Contributor

@ptahchiev I see why the source would not match here... which now kicks in after the previous fix since both the existing and the new bean definition are of type ScannedGenericBeanDefinition now. However, shouldn't the definitions be equal according to the subsequent equals check in isCompatible? Any idea why they might differ?

@jhoeller
Copy link
Contributor

Also, why isn't the index kicking in for the second scan attempt? @snicoll I wonder whether this has to do with some Boot-specific behavior? This wouldn't have materialized before since the index effectively registered its bean definitions like explicit user definitions which always override... Now a mismatch between index and manual scan would show up here.

@ptahchiev
Copy link
Author

@jhoeller They differ because ScannedGenricBeanDefinition newDefinition has no attributes, while ScannedGenericBeanDefinition existingDefinition has 1 attribute in the attributes map:

org.springframework.context.annotation.ConfigurationClassPostProcessor.configurationClass -> {BeanMetadataAttribute@5436} "metadata attribute 'org.springframework.context.annotation.ConfigurationClassPostProcessor.configurationClass'"

@snicoll
Copy link
Member

snicoll commented Apr 27, 2020

@jhoeller there is nothing in Spring Boot that touches the index, it is 100% baked in a single place in the core framework.

@jhoeller
Copy link
Contributor

@ptahchiev Thanks for checking! Our equality check is clearly too rigid then, and we'll have to relax it for 5.2.6. We should also store the source for the index-originated bean definitions there, even if that will not necessarily be identical to the source resouce of an equivalent scan result.

@snicoll I just wonder why a second scan attempt would bypass the index in such a scenario. Is there anything in Boot that could influence this, e.g. custom include filters? The attached repro project looks straightforward enough to me, does it reveal anything out of the ordinary to you?

To be clear, even with a second scan bypassing the index, the outcome should not fail with a conflicting bean definition exception. So this is not immediately relevant for this issue, we'll have to fix in the bean definition compatibility check. It'd just be good to understand how we get there.

@snicoll
Copy link
Member

snicoll commented Apr 27, 2020

If you configure the scan with an include or exclude filter that the index cannot honour, we will fallback to regular component scan at runtime. But that's not what's happening here, the index is used in both cases.

@jhoeller
Copy link
Contributor

After checking with Stephane here, it turns out that the index is indeed used in both cases... but due to the lack of a source being stored and the difference of the bean definition attributes, it fails with a conflict. For aligned behavior between index and scan, all we need is consistent source comparisons. I'll fix this right away, to be released in 5.2.6 tomorrow.

Thanks for the immediate turnaround to both of you!

@jhoeller jhoeller changed the title ConflictingBeanDefinitionException when upgrading to 5.2.5.RELEASE ConflictingBeanDefinitionException on repeated index evaluation Apr 27, 2020
@jhoeller
Copy link
Contributor

@snicoll @ptahchiev This should be available in the latest 5.2.6 snapshot now. Please give it a try!

@snicoll
Copy link
Member

snicoll commented Apr 27, 2020

I did and the test in the sample Petar shared now works. Thank for the quick turnaround Juergen!

jhoeller added a commit that referenced this issue Apr 27, 2020
Includes consistent constructor-level storage of derived resource in ScannedGenericBeanDefinition and ConfigurationClassBeanDefinition.

See gh-24978
jhoeller added a commit that referenced this issue Apr 27, 2020
Includes consistent constructor-level storage of derived resource in ScannedGenericBeanDefinition and ConfigurationClassBeanDefinition.

See gh-24978
zx20110729 pushed a commit to zx20110729/spring-framework that referenced this issue Feb 18, 2022
Includes consistent constructor-level storage of derived resource in ScannedGenericBeanDefinition and ConfigurationClassBeanDefinition.

See spring-projectsgh-24978
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) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

4 participants