Restore original override behavior when override allowed
Closes gh-33920
This commit is contained in:
parent
68d6cb9d35
commit
66da5d7ab9
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2022 the original author or authors.
|
* Copyright 2002-2024 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -54,6 +54,22 @@ public class BeanDefinitionOverrideException extends BeanDefinitionStoreExceptio
|
||||||
this.existingDefinition = existingDefinition;
|
this.existingDefinition = existingDefinition;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a new BeanDefinitionOverrideException for the given new and existing definition.
|
||||||
|
* @param beanName the name of the bean
|
||||||
|
* @param beanDefinition the newly registered bean definition
|
||||||
|
* @param existingDefinition the existing bean definition for the same name
|
||||||
|
* @param msg the detail message to include
|
||||||
|
* @since 6.2.1
|
||||||
|
*/
|
||||||
|
public BeanDefinitionOverrideException(
|
||||||
|
String beanName, BeanDefinition beanDefinition, BeanDefinition existingDefinition, String msg) {
|
||||||
|
|
||||||
|
super(beanDefinition.getResourceDescription(), beanName, msg);
|
||||||
|
this.beanDefinition = beanDefinition;
|
||||||
|
this.existingDefinition = existingDefinition;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return the description of the resource that the bean definition came from.
|
* Return the description of the resource that the bean definition came from.
|
||||||
|
|
|
@ -36,10 +36,10 @@ import org.springframework.beans.factory.groovy.GroovyBeanDefinitionReader;
|
||||||
import org.springframework.beans.factory.parsing.SourceExtractor;
|
import org.springframework.beans.factory.parsing.SourceExtractor;
|
||||||
import org.springframework.beans.factory.support.AbstractBeanDefinition;
|
import org.springframework.beans.factory.support.AbstractBeanDefinition;
|
||||||
import org.springframework.beans.factory.support.AbstractBeanDefinitionReader;
|
import org.springframework.beans.factory.support.AbstractBeanDefinitionReader;
|
||||||
|
import org.springframework.beans.factory.support.BeanDefinitionOverrideException;
|
||||||
import org.springframework.beans.factory.support.BeanDefinitionReader;
|
import org.springframework.beans.factory.support.BeanDefinitionReader;
|
||||||
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
|
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
|
||||||
import org.springframework.beans.factory.support.BeanNameGenerator;
|
import org.springframework.beans.factory.support.BeanNameGenerator;
|
||||||
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
|
|
||||||
import org.springframework.beans.factory.support.RootBeanDefinition;
|
import org.springframework.beans.factory.support.RootBeanDefinition;
|
||||||
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
|
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
|
||||||
import org.springframework.context.annotation.ConfigurationCondition.ConfigurationPhase;
|
import org.springframework.context.annotation.ConfigurationCondition.ConfigurationPhase;
|
||||||
|
@ -297,13 +297,21 @@ class ConfigurationClassBeanDefinitionReader {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
BeanDefinition existingBeanDef = this.registry.getBeanDefinition(beanName);
|
BeanDefinition existingBeanDef = this.registry.getBeanDefinition(beanName);
|
||||||
|
ConfigurationClass configClass = beanMethod.getConfigurationClass();
|
||||||
|
|
||||||
// If the bean method is an overloaded case on the same configuration class,
|
// If the bean method is an overloaded case on the same configuration class,
|
||||||
// preserve the existing bean definition and mark it as overloaded.
|
// preserve the existing bean definition and mark it as overloaded.
|
||||||
if (existingBeanDef instanceof ConfigurationClassBeanDefinition ccbd) {
|
if (existingBeanDef instanceof ConfigurationClassBeanDefinition ccbd) {
|
||||||
if (ccbd.getMetadata().getClassName().equals(beanMethod.getConfigurationClass().getMetadata().getClassName()) &&
|
if (ccbd.getMetadata().getClassName().equals(configClass.getMetadata().getClassName())) {
|
||||||
ccbd.getFactoryMethodMetadata().getMethodName().equals(beanMethod.getMetadata().getMethodName())) {
|
if (ccbd.getFactoryMethodMetadata().getMethodName().equals(beanMethod.getMetadata().getMethodName())) {
|
||||||
ccbd.setNonUniqueFactoryMethodName(ccbd.getFactoryMethodMetadata().getMethodName());
|
ccbd.setNonUniqueFactoryMethodName(ccbd.getFactoryMethodMetadata().getMethodName());
|
||||||
|
}
|
||||||
|
else if (!this.registry.isBeanDefinitionOverridable(beanName)) {
|
||||||
|
throw new BeanDefinitionOverrideException(beanName,
|
||||||
|
new ConfigurationClassBeanDefinition(configClass, beanMethod.getMetadata(), beanName),
|
||||||
|
existingBeanDef,
|
||||||
|
"@Bean method override with same bean name but different method name: " + existingBeanDef);
|
||||||
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
|
@ -329,9 +337,11 @@ class ConfigurationClassBeanDefinitionReader {
|
||||||
|
|
||||||
// At this point, it's a top-level override (probably XML), just having been parsed
|
// At this point, it's a top-level override (probably XML), just having been parsed
|
||||||
// before configuration class processing kicks in...
|
// before configuration class processing kicks in...
|
||||||
if (this.registry instanceof DefaultListableBeanFactory dlbf && !dlbf.isBeanDefinitionOverridable(beanName)) {
|
if (!this.registry.isBeanDefinitionOverridable(beanName)) {
|
||||||
throw new BeanDefinitionStoreException(beanMethod.getConfigurationClass().getResource().getDescription(),
|
throw new BeanDefinitionOverrideException(beanName,
|
||||||
beanName, "@Bean definition illegally overridden by existing bean definition: " + existingBeanDef);
|
new ConfigurationClassBeanDefinition(configClass, beanMethod.getMetadata(), beanName),
|
||||||
|
existingBeanDef,
|
||||||
|
"@Bean definition illegally overridden by existing bean definition: " + existingBeanDef);
|
||||||
}
|
}
|
||||||
if (logger.isDebugEnabled()) {
|
if (logger.isDebugEnabled()) {
|
||||||
logger.debug(String.format("Skipping bean definition for %s: a definition for bean '%s' " +
|
logger.debug(String.format("Skipping bean definition for %s: a definition for bean '%s' " +
|
||||||
|
|
|
@ -110,7 +110,7 @@ class ConfigurationClassProcessingTests {
|
||||||
|
|
||||||
private void aliasesAreRespected(Class<?> testClass, Supplier<TestBean> testBeanSupplier, String beanName) {
|
private void aliasesAreRespected(Class<?> testClass, Supplier<TestBean> testBeanSupplier, String beanName) {
|
||||||
TestBean testBean = testBeanSupplier.get();
|
TestBean testBean = testBeanSupplier.get();
|
||||||
BeanFactory factory = initBeanFactory(testClass);
|
BeanFactory factory = initBeanFactory(false, testClass);
|
||||||
|
|
||||||
assertThat(factory.getBean(beanName)).isSameAs(testBean);
|
assertThat(factory.getBean(beanName)).isSameAs(testBean);
|
||||||
Arrays.stream(factory.getAliases(beanName)).map(factory::getBean).forEach(alias -> assertThat(alias).isSameAs(testBean));
|
Arrays.stream(factory.getAliases(beanName)).map(factory::getBean).forEach(alias -> assertThat(alias).isSameAs(testBean));
|
||||||
|
@ -141,30 +141,30 @@ class ConfigurationClassProcessingTests {
|
||||||
@Test
|
@Test
|
||||||
void finalBeanMethod() {
|
void finalBeanMethod() {
|
||||||
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
|
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
|
||||||
initBeanFactory(ConfigWithFinalBean.class));
|
initBeanFactory(false, ConfigWithFinalBean.class));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void finalBeanMethodWithoutProxy() {
|
void finalBeanMethodWithoutProxy() {
|
||||||
initBeanFactory(ConfigWithFinalBeanWithoutProxy.class);
|
initBeanFactory(false, ConfigWithFinalBeanWithoutProxy.class);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test // gh-31007
|
@Test // gh-31007
|
||||||
void voidBeanMethod() {
|
void voidBeanMethod() {
|
||||||
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
|
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
|
||||||
initBeanFactory(ConfigWithVoidBean.class));
|
initBeanFactory(false, ConfigWithVoidBean.class));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void simplestPossibleConfig() {
|
void simplestPossibleConfig() {
|
||||||
BeanFactory factory = initBeanFactory(SimplestPossibleConfig.class);
|
BeanFactory factory = initBeanFactory(false, SimplestPossibleConfig.class);
|
||||||
String stringBean = factory.getBean("stringBean", String.class);
|
String stringBean = factory.getBean("stringBean", String.class);
|
||||||
assertThat(stringBean).isEqualTo("foo");
|
assertThat(stringBean).isEqualTo("foo");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void configWithObjectReturnType() {
|
void configWithObjectReturnType() {
|
||||||
BeanFactory factory = initBeanFactory(ConfigWithNonSpecificReturnTypes.class);
|
BeanFactory factory = initBeanFactory(false, ConfigWithNonSpecificReturnTypes.class);
|
||||||
assertThat(factory.getType("stringBean")).isEqualTo(Object.class);
|
assertThat(factory.getType("stringBean")).isEqualTo(Object.class);
|
||||||
assertThat(factory.isTypeMatch("stringBean", String.class)).isFalse();
|
assertThat(factory.isTypeMatch("stringBean", String.class)).isFalse();
|
||||||
String stringBean = factory.getBean("stringBean", String.class);
|
String stringBean = factory.getBean("stringBean", String.class);
|
||||||
|
@ -173,7 +173,7 @@ class ConfigurationClassProcessingTests {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void configWithFactoryBeanReturnType() {
|
void configWithFactoryBeanReturnType() {
|
||||||
ListableBeanFactory factory = initBeanFactory(ConfigWithNonSpecificReturnTypes.class);
|
ListableBeanFactory factory = initBeanFactory(false, ConfigWithNonSpecificReturnTypes.class);
|
||||||
assertThat(factory.getType("factoryBean")).isEqualTo(List.class);
|
assertThat(factory.getType("factoryBean")).isEqualTo(List.class);
|
||||||
assertThat(factory.isTypeMatch("factoryBean", List.class)).isTrue();
|
assertThat(factory.isTypeMatch("factoryBean", List.class)).isTrue();
|
||||||
assertThat(factory.getType("&factoryBean")).isEqualTo(FactoryBean.class);
|
assertThat(factory.getType("&factoryBean")).isEqualTo(FactoryBean.class);
|
||||||
|
@ -201,7 +201,7 @@ class ConfigurationClassProcessingTests {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void configurationWithPrototypeScopedBeans() {
|
void configurationWithPrototypeScopedBeans() {
|
||||||
BeanFactory factory = initBeanFactory(ConfigWithPrototypeBean.class);
|
BeanFactory factory = initBeanFactory(false, ConfigWithPrototypeBean.class);
|
||||||
|
|
||||||
TestBean foo = factory.getBean("foo", TestBean.class);
|
TestBean foo = factory.getBean("foo", TestBean.class);
|
||||||
ITestBean bar = factory.getBean("bar", ITestBean.class);
|
ITestBean bar = factory.getBean("bar", ITestBean.class);
|
||||||
|
@ -213,7 +213,7 @@ class ConfigurationClassProcessingTests {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void configurationWithNullReference() {
|
void configurationWithNullReference() {
|
||||||
BeanFactory factory = initBeanFactory(ConfigWithNullReference.class);
|
BeanFactory factory = initBeanFactory(false, ConfigWithNullReference.class);
|
||||||
|
|
||||||
TestBean foo = factory.getBean("foo", TestBean.class);
|
TestBean foo = factory.getBean("foo", TestBean.class);
|
||||||
assertThat(factory.getBean("bar")).isEqualTo(null);
|
assertThat(factory.getBean("bar")).isEqualTo(null);
|
||||||
|
@ -223,7 +223,15 @@ class ConfigurationClassProcessingTests {
|
||||||
@Test // gh-33330
|
@Test // gh-33330
|
||||||
void configurationWithMethodNameMismatch() {
|
void configurationWithMethodNameMismatch() {
|
||||||
assertThatExceptionOfType(BeanDefinitionOverrideException.class)
|
assertThatExceptionOfType(BeanDefinitionOverrideException.class)
|
||||||
.isThrownBy(() -> initBeanFactory(ConfigWithMethodNameMismatch.class));
|
.isThrownBy(() -> initBeanFactory(false, ConfigWithMethodNameMismatch.class));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test // gh-33920
|
||||||
|
void configurationWithMethodNameMismatchAndOverridingAllowed() {
|
||||||
|
BeanFactory factory = initBeanFactory(true, ConfigWithMethodNameMismatch.class);
|
||||||
|
|
||||||
|
SpousyTestBean foo = factory.getBean("foo", SpousyTestBean.class);
|
||||||
|
assertThat(foo.getName()).isEqualTo("foo1");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -353,13 +361,13 @@ class ConfigurationClassProcessingTests {
|
||||||
* When complete, the factory is ready to service requests for any {@link Bean} methods
|
* When complete, the factory is ready to service requests for any {@link Bean} methods
|
||||||
* declared by {@code configClasses}.
|
* declared by {@code configClasses}.
|
||||||
*/
|
*/
|
||||||
private DefaultListableBeanFactory initBeanFactory(Class<?>... configClasses) {
|
private DefaultListableBeanFactory initBeanFactory(boolean allowOverriding, Class<?>... configClasses) {
|
||||||
DefaultListableBeanFactory factory = new DefaultListableBeanFactory();
|
DefaultListableBeanFactory factory = new DefaultListableBeanFactory();
|
||||||
for (Class<?> configClass : configClasses) {
|
for (Class<?> configClass : configClasses) {
|
||||||
String configBeanName = configClass.getName();
|
String configBeanName = configClass.getName();
|
||||||
factory.registerBeanDefinition(configBeanName, new RootBeanDefinition(configClass));
|
factory.registerBeanDefinition(configBeanName, new RootBeanDefinition(configClass));
|
||||||
}
|
}
|
||||||
factory.setAllowBeanDefinitionOverriding(false);
|
factory.setAllowBeanDefinitionOverriding(allowOverriding);
|
||||||
ConfigurationClassPostProcessor ccpp = new ConfigurationClassPostProcessor();
|
ConfigurationClassPostProcessor ccpp = new ConfigurationClassPostProcessor();
|
||||||
ccpp.postProcessBeanDefinitionRegistry(factory);
|
ccpp.postProcessBeanDefinitionRegistry(factory);
|
||||||
ccpp.postProcessBeanFactory(factory);
|
ccpp.postProcessBeanFactory(factory);
|
||||||
|
@ -537,12 +545,12 @@ class ConfigurationClassProcessingTests {
|
||||||
@Configuration
|
@Configuration
|
||||||
static class ConfigWithMethodNameMismatch {
|
static class ConfigWithMethodNameMismatch {
|
||||||
|
|
||||||
@Bean(name = "foo") public TestBean foo() {
|
@Bean(name = "foo") public TestBean foo1() {
|
||||||
return new SpousyTestBean("foo");
|
return new SpousyTestBean("foo1");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Bean(name = "foo") public TestBean fooX() {
|
@Bean(name = "foo") public TestBean foo2() {
|
||||||
return new SpousyTestBean("fooX");
|
return new SpousyTestBean("foo2");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue