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

Reconsider whether to generally allow bean overriding by name [SPR-10808] #15434

Closed
spring-projects-issues opened this issue Aug 6, 2013 · 5 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another type: task A general task

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Aug 6, 2013

Emerson Farrugia opened SPR-10808 and commented

I've hit an issue where a single component scan was finding two @Beans with the same name. The beans weren't intentionally named the same, it was just an oversight. The container initialised one bean and ignored the other, logging at info instead of warn. The logging level is the first problem, since logging at info makes it much more likely the message is missed. The second problem is that the ignored bean was explicitly marked as @Primary. I've attached files with an example.

As far as I can tell, there's no way to control which bean gets picked in this situation. @Order on the @Configuration doesn't help.

In terms of fixes, I suggest the info message gets changed to warning. I'd personally even go one further. Given the functionality of @Profile, @Primary, and that autowiring by type is encouraged, does bean overriding still make sense in 3.1+.x? @Profile and @Primary give us a deterministic way of controlling autowire candidates. In contrast, quiet non-deterministic overrides by name seem too fragile to fit.


Affects: 3.2.3

Attachments:

Issue Links:

2 votes, 7 watchers

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I'm afraid that bean definition overriding is being used by intention quite frequently, which is why we're logging at info level there... Warn level would be inappropriate for an intended override.

Effectively, even if it may be surprising when they're happening by accident, overrides themselves are deterministic. The non-deterministic twist comes in via component scanning where the order of registration is up to the classpath structure or even specific ClassLoader behavior. As an alternative, if you register your configuration classes explicitly (through XML bean class entries or AnnotationConfigApplicationContext register calls), they have a clear order and therefore also well-defined overriding behavior.

@Primary is semantically only meant to apply for by-type matching, not for bean definition overriding. We could extend its definition to overriding but I'm not sure we'd do ourselves a favor with it.

All of that said, I personally tend to agree that overriding should be avoided wherever possible these days, preferring other means to express special registration arrangements. Note that you may call setAllowBeanDefinitionOverriding(false) on DefaultListableBeanFactory and also on the common ApplicationContext implementation classes...

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Emerson Farrugia commented

Thanks for the explanation. I think it's worth exploring whether overriding is intentionally being used because it offers value over @Order, @Profile, and @Primary, or whether it's simply a carryover from when those annotations weren't available.

Are there any other circumstances where XML bean definition order matters in Spring, or is that only a side effect of overrides? If the latter, that's quite a cost for a feature whose benefit may be outdated.

Allow me to outline the problem I hit, to see if the weirdness is as jarring as I'm making it out to be. Assume you have a system which uses Hiberate, JDBC, or a mixture of the two. You organised your code into two packages com.foo.persistence.hibernate and com.foo.persistence.jdbc, containing configuration classes

HibernateTransactionManagerConfiguration (defines HibernateTransactionManager as transactionManager)
HibernateConfiguration (defines SessionFactory)

and

DataSourceTransactionManagerConfiguration (defines DataSourceTransactionManager as transactionManager)
JdbcConfiguration (defines JdbcTemplate)

respectively. Both *TransactionManagerConfiguration define a bean transactionManager, which must be named that way because other infrastructure classes wire it in by name. The idea is that an application that needs JDBC will component scan the jdbc package, one that needs Hibernate will scan the hibernate package, and one that needs both scans both.

Assume your code needs to use both Hibernate and JDBC, and therefore must use the HibernateTransactionManager. A simple component scan like

<context:component-scan base-package="com.foo.persistence.hibernate"/>
<context:component-scan base-package="com.foo.persistence.jdbc"/>

fails when the JDBC transaction manager is picked up because it appears last, but if defined as

<context:component-scan base-package="com.foo.persistence.jdbc"/>
<context:component-scan base-package="com.foo.persistence.hibernate"/>

it works. That's not obvious by any stretch, and the problem only manifests itself as a runtime exception when a Hibernate session isn't found, unless using setAllowBeanDefinitionOverriding (not straightforward on web application contexts).

IMO, a @Primary annotation on the HibernateTransactionManager, or an @Order annotation to have more flexibility are better ways to disambiguate.

@spring-projects-issues
Copy link
Collaborator Author

Modestas Kažinauskas commented

Hi, I think I found the same issue in 1.4.0.BUILD-SNAPSHOT

Bean name is taken as init method name

Please see my code below

@Configuration
public class ConfigA {
    @Bean
    @Primary
    public MyObject init() {
        return new MyObject("A");
    }
}
@Configuration
public class ConfigB {
    @Bean
    public MyObject init() {
        return new MyObject("B");
    }
}
public class MyObject {
    private String name;

    public MyObject(String name) {
        this.name = name;
    }
    ...
}
@Configuration
public class Runner {
    @Autowired
    private MyObject myObject;

    @PostConstruct
    void init() {
        System.out.println(myObject.getName()); //Output is B, but primary bean is A
    }
}

2016-02-25 08:39:49.351 INFO 1196 --- [ main] o.s.b.f.s.DefaultListableBeanFactory : Overriding bean definition for bean 'init' with a different definition: replacing
[Root bean: class [null]; scope=; abstract=false; lazyInit=false; autowireMode=3; dependencyCheck=0; autowireCandidate=true; primary=true; factoryBeanName=configA; factoryMethodName=init; initMethodName=null; destroyMethodName=(inferred); defined in class path resource [com/modzo/ConfigA.class]]
with
[Root bean: class [null]; scope=; abstract=false; lazyInit=false; autowireMode=3; dependencyCheck=0; autowireCandidate=true; primary=false; factoryBeanName=configB; factoryMethodName=init; initMethodName=null; destroyMethodName=(inferred); defined in class path resource [com/modzo/ConfigB.class]]

@bclozel
Copy link
Member

bclozel commented Sep 20, 2021

See spring-projects/spring-boot#13609 - bean overriding has been disabled by default in Spring Boot 2.1.

@jhoeller
Copy link
Contributor

Superseded by #31288.

@jhoeller jhoeller added the status: superseded An issue that has been superseded by another label Dec 28, 2023
@jhoeller jhoeller removed this from the General Backlog milestone Dec 28, 2023
@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Dec 29, 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: superseded An issue that has been superseded by another type: task A general task
Projects
None yet
Development

No branches or pull requests

4 participants