Skip to content

Disable bean overriding by default and provide a configuration property to enable it #13609

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
wilkinsona opened this issue Jun 28, 2018 · 13 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

wilkinsona commented Jun 28, 2018

From what we've seen, overriding one bean definition with another (by providing two definitions with the same name) is most often done accidentally. When it is done, the info log message is almost always missed, leading to hard to diagnose behaviour. We should disable bean overriding by default and provide a configuration property to turn it back on along with failure analysis of the exception that clearly describes the problem and suggests setting the property as a possible action.

@wilkinsona wilkinsona added the type: enhancement A general enhancement label Jun 28, 2018
@wilkinsona
Copy link
Member Author

Currently, a rejected override attempt results in a BeanDefinitionStoreException. It would be very useful for failure analysis if a specific exception was thrown that provided access to both the existing and the overriding bean definitions. I've opened https://jira.spring.io/browse/SPR-16982.

@spencergibb
Copy link
Member

We've similarly dealt with some issues with this in spring cloud

@wilkinsona wilkinsona self-assigned this Jun 29, 2018
@wilkinsona wilkinsona added this to the 2.1.0.M1 milestone Jul 9, 2018
@wilkinsona wilkinsona reopened this Jul 9, 2018
@wilkinsona
Copy link
Member Author

This broke the build and fixing it is blocked by #13737.

@wilkinsona wilkinsona added the status: blocked An issue that's blocked on an external project change label Jul 9, 2018
@wilkinsona wilkinsona modified the milestones: 2.1.0.M1, 2.0.x Jul 9, 2018
@wilkinsona
Copy link
Member Author

With #13737 fixed, this is still blocked but by Spring Data REST now. It tries to override a bean that's already been defined by Spring MVC.

@wilkinsona wilkinsona modified the milestones: 2.0.x, 2.1.0.M1 Jul 10, 2018
@wilkinsona wilkinsona removed the status: blocked An issue that's blocked on an external project change label Jul 10, 2018
@wilkinsona
Copy link
Member Author

The problem in Spring Data REST has been fixed in its latest snapshots.

@duke-cliff
Copy link

@wilkinsona Hi Andy, just noticed this changed. We often use the Primary annotation to override some bean for integration test(I guess it's a normal approach). Took me hours to figure out this was disabled by default from 2.1.0. : (

@wilkinsona
Copy link
Member Author

I'm sorry to hear that. The change to the default is one of the first things that is described in the release notes for 2.1. They're always worth reading before upgrading and again if you hit a problem when you do.

@maccac
Copy link

maccac commented Jan 3, 2019

It's nice to know when you are accidentally overriding beans, but it would also be nice to be able to specifically mark specific beans as deliberately overriding others so they will be allowed.

When I've made heavy use of spring auto-configuration I've often find that there are beans that need customisation so I get the benefits of it being mostly auto-configured with only a few variations. In these situations it's pretty useful for it to error out when I accidently 'override' a bean, but it would be nice to be able to deliberately tell the framework that this particular bean is still something I want to override. That way, this feature could catch future mistakes I make whilst letting me retain the earlier decisions I made.

Perhaps I'm making a feature request or suggestion... maybe a new annotation like @OverridesExistingBean or an additional parameter on @'Bean' with a similar name.

@inletfetch
Copy link

inletfetch commented Jan 8, 2019

This issue is causing a problem for me because I broke my single bean definition XML file into multiple files many months ago. I'm getting the error because I'm redefining THE SAME BEAN multiple times. This happens because of multiple XML files including the same other XML file to get the definitions of the beans it references from the other file. I do this so that can use different mixes of my bean definition files and be sure that the dependent bean definitions are always available for reference. It also helps in my IDE, where it doesn't have the context of one XML file being included after another, and so it thinks the latter file is missing definitions for referenced beans. What I am doing worked fine until 2.1.

My work-around is to set spring.main.allow-bean-definition-overriding=true. This fixes the problem. But I'd love to have the benefit of being warned if I'm truly overriding one bean with a DIFFERENT bean. I am going to look to see if I can reorganize my bean files to avoid the need to disable the check and still get the functionality I want, but it's a pain. I just wanted to put this particular problem on record. I'm not saying what I'm doing is a proper practice.

I wonder if an exception could be made if the offending overriding bean has the same source coordinates as the original, or if the framework can otherwise recognize this situation...a duplicate definition of the same bean.

@snicoll
Copy link
Member

snicoll commented Jan 8, 2019

If your bean definition has an id the framework should not redefine the same bean definition multiple times, which should automatically fix your problem. If that does not fix it, please consider creating a Spring Framework issue with a small sample that reproduces the behavior you've described.

@inletfetch
Copy link

Thanks Stephane for your response. I will attempt to reproduce this with a simple example. I hope it's ok for me to put more info here in case it can shed some light before I enter a new issue:

My bean defs all have ids. Here's the first one I hit:

<bean id="identity" class="com.filethis.common.bluegreen.InstanceIdentity">
    <property name="name" value="${filethis.identity.name}"/>
    <property name="service" value="${filethis.identity.service}"/>
    <property name="cluster" value="${filethis.identity.cluster}"/>
    <property name="baseDomain" value="${filethis.identity.baseDomain}"/>
    <property name="address" value="${filethis.identity.address}"/>
</bean>

Here's the error I get:

Caused by: org.springframework.beans.factory.support.BeanDefinitionOverrideException: 
   Invalid bean definition with name 'identity' 
      defined in URL [file:/Users/steve/Development/inlet/inletfetch_if2/common/common-jutil/target/classes/beanDefinitionFiles/identity.xml]: 
   Cannot register bean definition [Generic bean: class [com.filethis.common.bluegreen.InstanceIdentity];
      scope=; abstract=false; lazyInit=false; autowireMode=0; dependencyCheck=0; autowireCandidate=true; primary=false; factoryBeanName=null; factoryMethodName=null; initMethodName=null; destroyMethodName=null;
      defined in URL [file:/Users/steve/Development/inlet/inletfetch_if2/common/common-jutil/target/classes/beanDefinitionFiles/identity.xml]] for bean 'identity':
   There is already [Generic bean: class [com.filethis.common.bluegreen.InstanceIdentity];
      scope=; abstract=false; lazyInit=false; autowireMode=0; dependencyCheck=0; autowireCandidate=true; primary=false; factoryBeanName=null; factoryMethodName=null; initMethodName=null; destroyMethodName=null;
      defined in URL [file:/Users/steve/Development/inlet/inletfetch_if2/common/common-jutil/target/classes/beanDefinitionFiles/identity.xml]] bound.

@snicoll
Copy link
Member

snicoll commented Jan 8, 2019

@inletfetch thanks for the feedback. It looks odd to me that the core container throws a BeanDefinitionOverrideException in such a case. Could you please create a Spring Framework issue?

@inletfetch
Copy link

will do

tomcruise81 added a commit to tomcruise81/spring-security-oauth2-boot that referenced this issue Mar 1, 2019
Seems to be necessary as a result of the changes related to spring-projects/spring-boot#13609
jzheaux pushed a commit to spring-attic/spring-security-oauth2-boot that referenced this issue Apr 4, 2019
Seems to be necessary as a result of the changes related to spring-projects/spring-boot#13609
jzheaux pushed a commit to spring-attic/spring-security-oauth2-boot that referenced this issue Apr 4, 2019
Seems to be necessary as a result of the changes related to spring-projects/spring-boot#13609
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants