Skip to content

Commit 044d68b

Browse files
committed
@bean method metadata is always being picked from the most concrete subclass
As a side effect, @bean overrides and overloads work with 'allowBeanDefinitionOverriding'=false as well now. Issue: SPR-10992
1 parent a95eb1d commit 044d68b

File tree

2 files changed

+66
-40
lines changed

2 files changed

+66
-40
lines changed

spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java

+17-7
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ private void loadBeanDefinitionsForBeanMethod(BeanMethod beanMethod) {
156156
ConfigurationClass configClass = beanMethod.getConfigurationClass();
157157
MethodMetadata metadata = beanMethod.getMetadata();
158158

159-
RootBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass);
159+
ConfigurationClassBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass);
160160
beanDef.setResource(configClass.getResource());
161161
beanDef.setSource(this.sourceExtractor.extractSource(metadata, configClass.getResource()));
162162
if (metadata.isStatic()) {
@@ -188,9 +188,18 @@ private void loadBeanDefinitionsForBeanMethod(BeanMethod beanMethod) {
188188

189189
// has this already been overridden (e.g. via XML)?
190190
if (this.registry.containsBeanDefinition(beanName)) {
191-
BeanDefinition existingBeanDef = registry.getBeanDefinition(beanName);
192-
// is the existing bean definition one that was created from a configuration class?
193-
if (!(existingBeanDef instanceof ConfigurationClassBeanDefinition)) {
191+
BeanDefinition existingBeanDef = this.registry.getBeanDefinition(beanName);
192+
// Is the existing bean definition one that was created from a configuration class?
193+
// -> allow the current bean method to override, since both are at second-pass level.
194+
// However, if the bean method is an overloaded case on the same configuration class,
195+
// preserve the existing bean definition.
196+
if (existingBeanDef instanceof ConfigurationClassBeanDefinition) {
197+
ConfigurationClassBeanDefinition ccbd = (ConfigurationClassBeanDefinition) existingBeanDef;
198+
if (ccbd.getMetadata().getClassName().equals(beanMethod.getConfigurationClass().getMetadata().getClassName())) {
199+
return;
200+
}
201+
}
202+
else {
194203
// no -> then it's an external override, probably XML
195204
// overriding is legal, return immediately
196205
if (logger.isDebugEnabled()) {
@@ -238,7 +247,7 @@ else if (configClass.getMetadata().isAnnotated(Lazy.class.getName())){
238247
beanDef.setDestroyMethodName(destroyMethodName);
239248
}
240249

241-
// consider scoping
250+
// Consider scoping
242251
ScopedProxyMode proxyMode = ScopedProxyMode.NO;
243252
AnnotationAttributes scope = attributesFor(metadata, Scope.class);
244253
if (scope != null) {
@@ -249,7 +258,7 @@ else if (configClass.getMetadata().isAnnotated(Lazy.class.getName())){
249258
}
250259
}
251260

252-
// replace the original bean definition with the target one, if necessary
261+
// Replace the original bean definition with the target one, if necessary
253262
BeanDefinition beanDefToRegister = beanDef;
254263
if (proxyMode != ScopedProxyMode.NO) {
255264
BeanDefinitionHolder proxyDef = ScopedProxyCreator.createScopedProxy(
@@ -259,7 +268,8 @@ else if (configClass.getMetadata().isAnnotated(Lazy.class.getName())){
259268
}
260269

261270
if (logger.isDebugEnabled()) {
262-
logger.debug(String.format("Registering bean definition for @Bean method %s.%s()", configClass.getMetadata().getClassName(), beanName));
271+
logger.debug(String.format("Registering bean definition for @Bean method %s.%s()",
272+
configClass.getMetadata().getClassName(), beanName));
263273
}
264274

265275
registry.registerBeanDefinition(beanName, beanDefToRegister);

spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java

+49-33
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,13 @@
1616

1717
package org.springframework.context.annotation;
1818

19-
import java.lang.annotation.Inherited;
2019
import java.util.List;
2120

2221
import org.junit.Rule;
2322
import org.junit.Test;
2423
import org.junit.rules.ExpectedException;
2524

2625
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
27-
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
28-
import org.springframework.beans.factory.support.RootBeanDefinition;
2926

3027
import static org.hamcrest.CoreMatchers.*;
3128
import static org.junit.Assert.*;
@@ -34,25 +31,33 @@
3431
* Tests regarding overloading and overriding of bean methods.
3532
* Related to SPR-6618.
3633
*
37-
* Bean-annotated methods should be able to be overridden, just as any regular
38-
* method. This is straightforward.
39-
*
40-
* Bean-annotated methods should be able to be overloaded, though supporting this
41-
* is more subtle. Essentially, it must be unambiguous to the container which bean
42-
* method to call. A simple way to think about this is that no one Configuration
43-
* class may declare two bean methods with the same name. In the case of inheritance,
44-
* the most specific subclass bean method will always be the one that is invoked.
45-
*
4634
* @author Chris Beams
4735
* @author Phillip Webb
36+
* @author Juergen Hoeller
4837
*/
49-
@SuppressWarnings("resource")
5038
public class BeanMethodPolymorphismTests {
5139

5240
@Rule
5341
public ExpectedException thrown = ExpectedException.none();
5442

5543

44+
@Test
45+
public void beanMethodDetectedOnSuperClass() {
46+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class);
47+
ctx.getBean("testBean", TestBean.class);
48+
}
49+
50+
@Test
51+
public void beanMethodOverriding() {
52+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
53+
ctx.register(OverridingConfig.class);
54+
ctx.setAllowBeanDefinitionOverriding(false);
55+
ctx.refresh();
56+
assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("testBean"));
57+
assertEquals("overridden", ctx.getBean("testBean", TestBean.class).toString());
58+
assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("testBean"));
59+
}
60+
5661
@Test
5762
public void beanMethodOverloadingWithoutInheritance() {
5863

@@ -69,15 +74,25 @@ public void beanMethodOverloadingWithoutInheritance() {
6974

7075
@Test
7176
public void beanMethodOverloadingWithInheritance() {
72-
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfig.class);
77+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
78+
ctx.register(SubConfig.class);
79+
ctx.setAllowBeanDefinitionOverriding(false);
80+
ctx.refresh();
81+
assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
7382
assertThat(ctx.getBean(String.class), equalTo("overloaded5"));
83+
assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
7484
}
7585

86+
// SPR-11025
7687
@Test
7788
public void beanMethodOverloadingWithInheritanceAndList() {
78-
// SPR-11025
79-
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfigWithList.class);
89+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
90+
ctx.register(SubConfigWithList.class);
91+
ctx.setAllowBeanDefinitionOverriding(false);
92+
ctx.refresh();
93+
assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
8094
assertThat(ctx.getBean(String.class), equalTo("overloaded5"));
95+
assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
8196
}
8297

8398
/**
@@ -91,20 +106,6 @@ public void beanMethodShadowing() {
91106
assertThat(ctx.getBean(String.class), equalTo("shadow"));
92107
}
93108

94-
/**
95-
* Tests that polymorphic Configuration classes need not explicitly redeclare the
96-
* {@link Configuration} annotation. This respects the {@link Inherited} nature
97-
* of the Configuration annotation, even though it's being detected via ASM.
98-
*/
99-
@Test
100-
public void beanMethodsDetectedOnSuperClass() {
101-
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
102-
beanFactory.registerBeanDefinition("config", new RootBeanDefinition(Config.class));
103-
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
104-
pp.postProcessBeanFactory(beanFactory);
105-
beanFactory.getBean("testBean", TestBean.class);
106-
}
107-
108109

109110
@Configuration
110111
static class BaseConfig {
@@ -113,7 +114,6 @@ static class BaseConfig {
113114
public TestBean testBean() {
114115
return new TestBean();
115116
}
116-
117117
}
118118

119119

@@ -122,6 +122,22 @@ static class Config extends BaseConfig {
122122
}
123123

124124

125+
@Configuration
126+
static class OverridingConfig extends BaseConfig {
127+
128+
@Bean @Lazy
129+
@Override
130+
public TestBean testBean() {
131+
return new TestBean() {
132+
@Override
133+
public String toString() {
134+
return "overridden";
135+
}
136+
};
137+
}
138+
}
139+
140+
125141
@Configuration
126142
static class SuperConfig {
127143

@@ -140,7 +156,7 @@ Integer anInt() {
140156
return 5;
141157
}
142158

143-
@Bean
159+
@Bean @Lazy
144160
String aString(Integer dependency) {
145161
return "overloaded" + dependency;
146162
}
@@ -155,7 +171,7 @@ Integer anInt() {
155171
return 5;
156172
}
157173

158-
@Bean
174+
@Bean @Lazy
159175
String aString(List<Integer> dependency) {
160176
return "overloaded" + dependency.get(0);
161177
}

0 commit comments

Comments
 (0)