Skip to content

Ignore identical BeanDefinition registration when allowBeanDefinitionOverriding is false #27978

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
bironran opened this issue Jan 26, 2022 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@bironran
Copy link

bironran commented Jan 26, 2022

Affects: 2.5.2 - 5.3, Spring Boot 2.1+

Calling DefaultListableBeanFactory.registerBeanDefinition() twice with the same bean name and same or equal BeanDefinition object results in an exception if allowBeanDefinitionOverriding was set to false (as is the default in Spring Boot 2.1+).

The javadoc for DefaultListableBeanFactory.setAllowBeanDefinitionOverriding specifically defines (emphasis is mine):

Set whether it should be allowed to override bean definitions by registering a different definition with the same name, automatically replacing the former...

Practically, this makes splitting a monolith into modules and libraries a lot harder as it bans common bean definitions due to recursive imports, e.g.:

  1. Module 1 uses db hence @Import(DbConfig.class).
  2. Module 2 uses db and also @Import(DbConfig.class).
  3. Service 3 uses both module 1 and 2 and fails to load because of the double import if allowBeanDefinitionOverriding=false.

Allowing for silent-ignore of identical definitions will ease transitions from "one large context" to smaller modules that can be self-descriptive in their dependencies.

This behavior was introduced in #9052.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 26, 2022
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Jan 26, 2022
@sbrannen sbrannen modified the milestones: Triage Queue, 5.3.16 Jan 26, 2022
@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 26, 2022
@sbrannen sbrannen changed the title Bean redefinition protection triggers on identical BeanDefinition objects Ignore identical BeanDefinition registration when allowBeanDefinitionOverriding is false Jan 26, 2022
@sbrannen
Copy link
Member

Good point, @bironran.

We aim to loosen the restrictions (as indicated by the updated title for this issue) in 5.3.x.

Thanks for bringing this up.

@bclozel bclozel modified the milestones: 5.3.16, 5.3.17 Feb 16, 2022
@jhoeller jhoeller modified the milestones: 5.3.17, 5.3.x Mar 15, 2022
@sbrannen sbrannen modified the milestones: 5.3.x, 6.1.x Feb 1, 2023
@sbrannen sbrannen changed the title Ignore identical BeanDefinition registration when allowBeanDefinitionOverriding is false Ignore identical BeanDefinition registration when allowBeanDefinitionOverriding is false Feb 1, 2023
@jhoeller jhoeller self-assigned this Feb 14, 2023
@jhoeller jhoeller modified the milestones: 6.1.x, 6.1.0-M1 Feb 14, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Jun 2, 2023

As far as I was able to investigate this, this is only a problem with programmatic registrations and external configuration formats such as XML, whereas double @Import usage works fine since the configuration class parser explicitly merges multiple imports for the same target class already. This should actually cover the scenario that you describe?

In general, we aim to explicitly skip re-registration (such as in the configuration class parser) rather than silently swallow it in DefaultListableBeanFactory itself. From that perspective, I'm inclined to close this issue since we do not intend to revisit the factory-level registry behavior there. If there is a concrete @Configuration/@Import scenario where this does not work as intended, feel free to reopen this issue with a corresponding test or to raise a separate issue for the specific target scenario.

@jhoeller jhoeller closed this as completed Jun 2, 2023
@jhoeller jhoeller removed this from the 6.1.0-M1 milestone Jun 2, 2023
@bclozel bclozel added the status: declined A suggestion or change that we don't feel we should currently apply label Jun 2, 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: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
5 participants