Skip to content

Commit 66da5d7

Browse files
jhoellerjhoeller
committed
Restore original override behavior when override allowed
Closes gh-33920
1 parent 68d6cb9 commit 66da5d7

File tree

3 files changed

+58
-24
lines changed

3 files changed

+58
-24
lines changed

‎spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionOverrideException.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -54,6 +54,22 @@ public BeanDefinitionOverrideException(
5454
this.existingDefinition = existingDefinition;
5555
}
5656

57+
/**
58+
* Create a new BeanDefinitionOverrideException for the given new and existing definition.
59+
* @param beanName the name of the bean
60+
* @param beanDefinition the newly registered bean definition
61+
* @param existingDefinition the existing bean definition for the same name
62+
* @param msg the detail message to include
63+
* @since 6.2.1
64+
*/
65+
public BeanDefinitionOverrideException(
66+
String beanName, BeanDefinition beanDefinition, BeanDefinition existingDefinition, String msg) {
67+
68+
super(beanDefinition.getResourceDescription(), beanName, msg);
69+
this.beanDefinition = beanDefinition;
70+
this.existingDefinition = existingDefinition;
71+
}
72+
5773

5874
/**
5975
* Return the description of the resource that the bean definition came from.

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

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@
3636
import org.springframework.beans.factory.parsing.SourceExtractor;
3737
import org.springframework.beans.factory.support.AbstractBeanDefinition;
3838
import org.springframework.beans.factory.support.AbstractBeanDefinitionReader;
39+
import org.springframework.beans.factory.support.BeanDefinitionOverrideException;
3940
import org.springframework.beans.factory.support.BeanDefinitionReader;
4041
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
4142
import org.springframework.beans.factory.support.BeanNameGenerator;
42-
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
4343
import org.springframework.beans.factory.support.RootBeanDefinition;
4444
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
4545
import org.springframework.context.annotation.ConfigurationCondition.ConfigurationPhase;
@@ -297,13 +297,21 @@ protected boolean isOverriddenByExistingDefinition(BeanMethod beanMethod, String
297297
return false;
298298
}
299299
BeanDefinition existingBeanDef = this.registry.getBeanDefinition(beanName);
300+
ConfigurationClass configClass = beanMethod.getConfigurationClass();
300301

301302
// If the bean method is an overloaded case on the same configuration class,
302303
// preserve the existing bean definition and mark it as overloaded.
303304
if (existingBeanDef instanceof ConfigurationClassBeanDefinition ccbd) {
304-
if (ccbd.getMetadata().getClassName().equals(beanMethod.getConfigurationClass().getMetadata().getClassName()) &&
305-
ccbd.getFactoryMethodMetadata().getMethodName().equals(beanMethod.getMetadata().getMethodName())) {
306-
ccbd.setNonUniqueFactoryMethodName(ccbd.getFactoryMethodMetadata().getMethodName());
305+
if (ccbd.getMetadata().getClassName().equals(configClass.getMetadata().getClassName())) {
306+
if (ccbd.getFactoryMethodMetadata().getMethodName().equals(beanMethod.getMetadata().getMethodName())) {
307+
ccbd.setNonUniqueFactoryMethodName(ccbd.getFactoryMethodMetadata().getMethodName());
308+
}
309+
else if (!this.registry.isBeanDefinitionOverridable(beanName)) {
310+
throw new BeanDefinitionOverrideException(beanName,
311+
new ConfigurationClassBeanDefinition(configClass, beanMethod.getMetadata(), beanName),
312+
existingBeanDef,
313+
"@Bean method override with same bean name but different method name: " + existingBeanDef);
314+
}
307315
return true;
308316
}
309317
else {
@@ -329,9 +337,11 @@ protected boolean isOverriddenByExistingDefinition(BeanMethod beanMethod, String
329337

330338
// At this point, it's a top-level override (probably XML), just having been parsed
331339
// before configuration class processing kicks in...
332-
if (this.registry instanceof DefaultListableBeanFactory dlbf && !dlbf.isBeanDefinitionOverridable(beanName)) {
333-
throw new BeanDefinitionStoreException(beanMethod.getConfigurationClass().getResource().getDescription(),
334-
beanName, "@Bean definition illegally overridden by existing bean definition: " + existingBeanDef);
340+
if (!this.registry.isBeanDefinitionOverridable(beanName)) {
341+
throw new BeanDefinitionOverrideException(beanName,
342+
new ConfigurationClassBeanDefinition(configClass, beanMethod.getMetadata(), beanName),
343+
existingBeanDef,
344+
"@Bean definition illegally overridden by existing bean definition: " + existingBeanDef);
335345
}
336346
if (logger.isDebugEnabled()) {
337347
logger.debug(String.format("Skipping bean definition for %s: a definition for bean '%s' " +

‎spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ void aliasesAreRespectedWhenConfiguredViaValueAttribute() {
110110

111111
private void aliasesAreRespected(Class<?> testClass, Supplier<TestBean> testBeanSupplier, String beanName) {
112112
TestBean testBean = testBeanSupplier.get();
113-
BeanFactory factory = initBeanFactory(testClass);
113+
BeanFactory factory = initBeanFactory(false, testClass);
114114

115115
assertThat(factory.getBean(beanName)).isSameAs(testBean);
116116
Arrays.stream(factory.getAliases(beanName)).map(factory::getBean).forEach(alias -> assertThat(alias).isSameAs(testBean));
@@ -141,30 +141,30 @@ void configWithSetWithProviderImplementation() {
141141
@Test
142142
void finalBeanMethod() {
143143
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
144-
initBeanFactory(ConfigWithFinalBean.class));
144+
initBeanFactory(false, ConfigWithFinalBean.class));
145145
}
146146

147147
@Test
148148
void finalBeanMethodWithoutProxy() {
149-
initBeanFactory(ConfigWithFinalBeanWithoutProxy.class);
149+
initBeanFactory(false, ConfigWithFinalBeanWithoutProxy.class);
150150
}
151151

152152
@Test // gh-31007
153153
void voidBeanMethod() {
154154
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
155-
initBeanFactory(ConfigWithVoidBean.class));
155+
initBeanFactory(false, ConfigWithVoidBean.class));
156156
}
157157

158158
@Test
159159
void simplestPossibleConfig() {
160-
BeanFactory factory = initBeanFactory(SimplestPossibleConfig.class);
160+
BeanFactory factory = initBeanFactory(false, SimplestPossibleConfig.class);
161161
String stringBean = factory.getBean("stringBean", String.class);
162162
assertThat(stringBean).isEqualTo("foo");
163163
}
164164

165165
@Test
166166
void configWithObjectReturnType() {
167-
BeanFactory factory = initBeanFactory(ConfigWithNonSpecificReturnTypes.class);
167+
BeanFactory factory = initBeanFactory(false, ConfigWithNonSpecificReturnTypes.class);
168168
assertThat(factory.getType("stringBean")).isEqualTo(Object.class);
169169
assertThat(factory.isTypeMatch("stringBean", String.class)).isFalse();
170170
String stringBean = factory.getBean("stringBean", String.class);
@@ -173,7 +173,7 @@ void configWithObjectReturnType() {
173173

174174
@Test
175175
void configWithFactoryBeanReturnType() {
176-
ListableBeanFactory factory = initBeanFactory(ConfigWithNonSpecificReturnTypes.class);
176+
ListableBeanFactory factory = initBeanFactory(false, ConfigWithNonSpecificReturnTypes.class);
177177
assertThat(factory.getType("factoryBean")).isEqualTo(List.class);
178178
assertThat(factory.isTypeMatch("factoryBean", List.class)).isTrue();
179179
assertThat(factory.getType("&factoryBean")).isEqualTo(FactoryBean.class);
@@ -201,7 +201,7 @@ void configWithFactoryBeanReturnType() {
201201

202202
@Test
203203
void configurationWithPrototypeScopedBeans() {
204-
BeanFactory factory = initBeanFactory(ConfigWithPrototypeBean.class);
204+
BeanFactory factory = initBeanFactory(false, ConfigWithPrototypeBean.class);
205205

206206
TestBean foo = factory.getBean("foo", TestBean.class);
207207
ITestBean bar = factory.getBean("bar", ITestBean.class);
@@ -213,7 +213,7 @@ void configurationWithPrototypeScopedBeans() {
213213

214214
@Test
215215
void configurationWithNullReference() {
216-
BeanFactory factory = initBeanFactory(ConfigWithNullReference.class);
216+
BeanFactory factory = initBeanFactory(false, ConfigWithNullReference.class);
217217

218218
TestBean foo = factory.getBean("foo", TestBean.class);
219219
assertThat(factory.getBean("bar")).isEqualTo(null);
@@ -223,7 +223,15 @@ void configurationWithNullReference() {
223223
@Test // gh-33330
224224
void configurationWithMethodNameMismatch() {
225225
assertThatExceptionOfType(BeanDefinitionOverrideException.class)
226-
.isThrownBy(() -> initBeanFactory(ConfigWithMethodNameMismatch.class));
226+
.isThrownBy(() -> initBeanFactory(false, ConfigWithMethodNameMismatch.class));
227+
}
228+
229+
@Test // gh-33920
230+
void configurationWithMethodNameMismatchAndOverridingAllowed() {
231+
BeanFactory factory = initBeanFactory(true, ConfigWithMethodNameMismatch.class);
232+
233+
SpousyTestBean foo = factory.getBean("foo", SpousyTestBean.class);
234+
assertThat(foo.getName()).isEqualTo("foo1");
227235
}
228236

229237
@Test
@@ -353,13 +361,13 @@ void autowiringWithDynamicPrototypeBeanClass() {
353361
* When complete, the factory is ready to service requests for any {@link Bean} methods
354362
* declared by {@code configClasses}.
355363
*/
356-
private DefaultListableBeanFactory initBeanFactory(Class<?>... configClasses) {
364+
private DefaultListableBeanFactory initBeanFactory(boolean allowOverriding, Class<?>... configClasses) {
357365
DefaultListableBeanFactory factory = new DefaultListableBeanFactory();
358366
for (Class<?> configClass : configClasses) {
359367
String configBeanName = configClass.getName();
360368
factory.registerBeanDefinition(configBeanName, new RootBeanDefinition(configClass));
361369
}
362-
factory.setAllowBeanDefinitionOverriding(false);
370+
factory.setAllowBeanDefinitionOverriding(allowOverriding);
363371
ConfigurationClassPostProcessor ccpp = new ConfigurationClassPostProcessor();
364372
ccpp.postProcessBeanDefinitionRegistry(factory);
365373
ccpp.postProcessBeanFactory(factory);
@@ -537,12 +545,12 @@ public TestBean bar() {
537545
@Configuration
538546
static class ConfigWithMethodNameMismatch {
539547

540-
@Bean(name = "foo") public TestBean foo() {
541-
return new SpousyTestBean("foo");
548+
@Bean(name = "foo") public TestBean foo1() {
549+
return new SpousyTestBean("foo1");
542550
}
543551

544-
@Bean(name = "foo") public TestBean fooX() {
545-
return new SpousyTestBean("fooX");
552+
@Bean(name = "foo") public TestBean foo2() {
553+
return new SpousyTestBean("foo2");
546554
}
547555
}
548556

0 commit comments

Comments
 (0)