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

ConfigurationClassBeanDefinitionReader.loadBeanDefinitionsForModelMethod should take into account the allowBeanDefinitionOverride settings [SPR-9682] #14316

Closed
spring-issuemaster opened this issue Aug 13, 2012 · 9 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Aug 13, 2012

László Váradi opened SPR-9682 and commented

The ConfigurationClassBeanDefinitionReader.loadBeanDefinitionsForModelMethod contains the following:

// 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.", beanMethod, beanName));
          }
          return;
     }
}

But it is not true ("overriding is legal") if the application context (AbstractRefreshableApplicationContext) is set to disallow it by setAllowBeanDefinitionOverriding(true).

I think the bean definition should be registered, and let the application context decide whether the override is legal or not. And if it is legal, which bean should be active?

In our case, classpath scanning have found the bean, and the bean definition in a @Configuration class with the same name (but with more specific configuration) was not created, causing some problem.


Affects: 3.1.2

Issue Links:

  • #15421 Cannot use @Primary to override @ComponentScan-ed beans
  • #14201 Beans defined by @ComponentScan are skipped in @Configuration override

6 votes, 12 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 12, 2013

Michael Moossen commented

I have the same problem.
I have a main config class, and i thought that i can just have a test config class for test cases that imports the main config and overwrites the needed stuff with mocks for the test cases. this does not work. (btw. i do not have any XML config at all in the project).
any idea how to reuse the main config replacing some beans with mocks?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 12, 2013

Michael Moossen commented

i got further, wenn the bean i want to override is defined in the config. then it works! if the been is discovered by ComponentScan then it does not.
so again the question:
is there any way to override beans discovered by ComponentScan?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 25, 2014

Steve Ash commented

Am I correct that this means I can't use spring testing to override beans and component scan at the same time? Is this not part of the entire point of spring? Seems like this should be upgraded to Critical...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 25, 2014

Steve Ash commented

This is related to #15421

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 14, 2014

Steve Ash commented

We looked a little further in to this, and in 3.2.7 its the else clause on lines 200-208 that is the problem. We always use the annotation application context and want the import order to always determine bean overrides, so I think just removing the else clause entirely solves this problem. We tried this locally and it works as we expect/want. I'm not sure I understand the use case for why you want to allow certain been defs to "always win". Does this have something to do with using test specific beans xml with the junit runner?

Another strange thing to us: how is no one else having this problem? Is no one else using component scanning + mock beans in their unit tests?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 14, 2014

Michael Moossen commented

I agree that the else statement is the problem. And as i said the only scenario i have this problem is trying to override a single bean definition from a configuration bean with a mock.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 2, 2014

Clint Checketts commented

Would a PR be welcome for this? It appears that this would be a quick fix in the isOverriddenByExistingDefinition() method could just be improved by before the last 'return true' checking if the setting is set in the Registry:

if(registry instanceof DefaultListableBeanFactory && !((DefaultListableBeanFactory)registry).allowAliasOverriding()){
  throw new FatalBeanException("Useful message here")
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 13, 2015

Juergen Hoeller commented

This has been addressed for 4.2 along with #14201 now.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.