Affects: 5.2.9.RELEASE
When allow-bean-definition-overriding=true, the @Primary annotation for overriden bean names is not behaving as expected. If the configuration class with the overridden bean name and @Primary annotation is processed before the configuration providing the same bean name and without the @Primary annotation, that definition would be used.
In fact, in DefaultListableBeanFactory.registerBeanDefinition(String beanName, BeanDefinition beanDefinition), there is no handling to check if the exisitingDefinition.isPrimary().
The expected behaviour is that the configuration with the @Primary annotation on the overridden bean name is the one that is used for that bean type and name. I would also expect that an exception is raised if more than one @Primary ovveriding configuration is found.
Comment From: snicoll
If the configuration class with the overridden bean name and @Primary annotation is processed before the configuration providing the same bean name and without the @Primary annotation, that definition would be used.
That's the expected behaviour. If you are overriding the bean definition, that's exactly what should be happening as @Primary is just one attribute of the bean definition. It would be weird to override some things and not others.
I would also expect that an exception is raised if more than one @Primary ovveriding configuration is found.
Overriding means that only one bean definition will be used for that bean id. By opting-in for bean overriding that's what should happen.
Comment From: alienisty
Hi @snicoll, I should specify that the problem is arising during testing, normal code allow-bean-definition-overriding=false. In what I'm experiencing, the order of the processing of configuration classes is affected by the classpath entries order and using @Order(LOWEST_PRECEDENCE) on the class I want to be processed last, does not work. You say that what is happening is the exepected behaviour but I could not find any specific mention in the documentation. My understanding is that if I have two instances of a bean of type A, a1 and a2, if a2 is marked as @Primary, then BeanFactory.getBean(A.class) should return a2. As an extension to the concept, when I allow bean name overriding, why shouldn't BeanFactory.getBean() return the bean with the same name but marked as @Primary? If such annotation is establishing a preference, such preference should also be applied to overriding, considering that the override operation would otherwise be unpredictable due to classpath ordering sensitivity.
Comment From: snicoll
Overriding and @Primary are orthogonal.
My understanding is that if I have two instances of a bean of type A, a1 and a2, if a2 is marked as @Primary, then BeanFactory.getBean(A.class) should return a2.
That is correct but none of that has anything to do with overriding. With overriding the last bean definition that is processed is the one that ends up in the bean factory. So rather than a1 and a2 that would be a and a. You therefore end up with only one bean definition in the context. And @Primary has no role in that whatsoever.
It looks like there's some confusion on your end about those concepts. If you believe you've found a bug, please take the time to share a small sample we can run ourselves that demonstrates it and we can take another look. You can do so by attaching a zip to this issue or sharing the link to a GitHub repository.
Comment From: alienisty
Hi @snicoll, I appreciate what you are saying about bean override, but, arguably, the two instances with the same name could be both considered "candidates" in terms of what is described in the @Primary documentation. In any case the important point to understand here is that the override logic, as it is, has an unpredictable behaviour because we don't have control over the order of the processing of the configuration classes, which seems determined only by the oder of the entries in the classpath. As I said in previous comments, I tried using @Order to no avail. I'm not saying that @Primary is the right tool, but we should definitely have a way of making the override deterministic. I'll see if I can manage to create a minimal demonstration project.
Comment From: snicoll
arguably, the two instances with the same name could be both considered "candidates" in terms of what is described in the @Primary documentation.
I don't see why. Bean definitions are uniquely identified by their id. If you have two bean definitions with the same id (for instance two @Bean methods with the same name) then one will override the other. The Javadoc of @Primary starts with:
Indicates that a bean should be given preference when multiple candidates are qualified to autowire a single-valued dependency. If exactly one 'primary' bean exists among the candidates, it will be the autowired value.
We're talking about beans here, not bean definitions so your assumption is inaccurate. @Order is in the same category, it order bean instances, not bean definitions. As I've indicated, if two bean definitions have the same ID, one will override the other and only the one is kept with its associated metadata (@Order, @Primary, etc).
the override logic, as it is, has an unpredictable behaviour because we don't have control over the order of the processing of the configuration classes, which seems determined only by the oder of the entries in the classpath.
You shouldn't be using bean overriding without a deterministic import of the related components indeed. You could use @Import with a deterministic order or ordered profiles. If you need support with such arrangement, please follow-up on StackOverflow, as mentioned in the guidelines for contributing, we prefer to use the issue tracker only for bugs and enhancements.
Comment From: alienisty
Hi @snicoll, I did follow the guidelines and searched for StackOverflow but didn't find much on the topic which, by the way, is almost totally undocumented (I could not find anything with a google search nor in the reference documentation). I could definitely use imports as you suggest to control order, but that is not always possible, especially when you use scanning and find other configuration classes in such fashion for supporting dynamic modules. So the fact remains that you don't have any control when allowing bean override while using package scanning. I believe that is a gap in the framework because two of its fetaures are at odds, but I'll take in the limitation and use corrective actions accordingly. I'll try to find the time to dig into the issue more and see if there are other tools provided by the framework to make override predictable in my setup.