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

Allow bean definitions from @Configuration classes to override those defined in XML [SPR-7341] #12000

Closed
spring-projects-issues opened this issue Jul 1, 2010 · 5 comments
Assignees
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import 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

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 1, 2010

Eric Sirianni opened SPR-7341 and commented

The ConfigurationClassBeanDefinitionReader hardcodes the behavior that bean definitions from XML always override bean definitions from JavaConfig classes:

// has this already been overridden (e.g. via XML)?
if (this.registry.containsBeanDefinition(beanName)) {
     BeanDefinition existingBeanDef = registry.getBeanDefinition(beanName);
     // is the existing bean definition one that was created from a configuration class?
     if (!(existingBeanDef instanceof ConfigurationClassBeanDefinition)) {
          // no -> then it's an external override, probably XML
          // overriding is legal, return immediately
          if (logger.isDebugEnabled()) {
                   logger.debug(String.format("Skipping loading bean definition for %s: a definition for bean " +
                                     "'%s' already exists. This is likely due to an override in XML.", method, beanName));
          }
          return;
     }
}

I don't see why this decision was made. The normal semantics of using the order in which bean definitions were registered would seem to make sense to apply here as well.

For example:

@Configuration
@ImportResource("foo.xml")
class FooConfig {
  @Bean bean1() { ... }
}

@Configuration
@ImportResource("bar.xml")
class BarConfig {
  @Bean bean1() { ... }
  @Bean bean2() { ... }
}

...
new AnnotationConfigApplicationContext(FooConfig.class, BarConfig.class)

I would expect the following override order to apply:

  1. beans defined in foo.xml
  2. overridden by beans defined in FooConfig
  3. overridden by beans defined in bar.xml
  4. overridden by beans defined in BarConfig

Instead it appears the override order is:

  1. beans defined in FooConfig
  2. overridden by beans defined in BarConfig
  3. overridden by beans defined in foo.xml
  4. overridden by beans defined in bar.xml

This is not intuitive. Specifically, note the interleaving of beans between both Foo and Bar configurations. The clients who use FooConfig and BarConfig don't care whether the beans defined by those Configuration classes come from XML or java. Yet, the override ordering forces them to be aware of this implementation detail.

I realize changing the behavior outright might present a backwards compatibility issue, so is it possible to add a boolean option to the ApplicationContext to respect the registration ordering regardless of whether the bean definition comes from XML or JavaConfig?


Affects: 3.0.3

Issue Links:

24 votes, 21 watchers

@spring-projects-issues
Copy link
Collaborator Author

Eric Sirianni commented

Revising... I overlooked the already well-defined override order for imports (imported resources override beans defined in the importer) this would be my expected ordering:

  1. beans defined in FooConfig
  2. overridden by beans defined in foo.xml
  3. overridden by beans defined in BarConfig
  4. overridden by beans defined in bar.xml

@spring-projects-issues
Copy link
Collaborator Author

Christopher Ng commented

I wish this had a higher priority. Basically java config is second-class at the moment, it can make it difficult to migrate to java config bit by bit, you have to do it all at once.

btw, I'm not sure the ordering in Eric's comment is correct, afaik the ordering in the original post is the correct one.

@spring-projects-issues
Copy link
Collaborator Author

Abdoul Cisse commented

This feature is important for Integration testing. Since Beans could be overriden inside Test class to mock behaviour needed like throwing exeptions to assert retry aspects configuration for example. At this moment the only solution is to override beans using XML file but this generates addtional test XML files that we could get rid of if Java Config done inside the test class could override any of the beans defined in application xml imported by @ImportResource.

I'm not interested declaring the whole context with Java Config.

@spring-projects-issues
Copy link
Collaborator Author

Eric Sirianni commented

Agreed. My use case is similar to Abdoul's. We use JavaConfig extensively for JUnit testing (having to look in XML files for test fixtures is messy in my opinion), and this issue prevents us from injecting mocks in place of real collaborators.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 10, 2016

Juergen Hoeller commented

Marked as "Won't Fix" along with #11690. See the discussion there for further details.

@spring-projects-issues spring-projects-issues added status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import 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
Development

No branches or pull requests

2 participants