Revised @Bean processing rules
@Bean method metadata is always being picked from the most concrete subclass; @Bean method overloads are allowed within the same config class as well; and @Bean overrides and overloads work with 'allowBeanDefinitionOverriding'=false now. Issue: SPR-10992 Issue: SPR-11025
This commit is contained in:
@@ -17,7 +17,6 @@
|
||||
package org.springframework.context.annotation;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.Map;
|
||||
@@ -156,8 +155,8 @@ final class ConfigurationClass {
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the configuration class that imported this class or {@code null} if
|
||||
* this configuration was not imported.
|
||||
* Return the configuration class that imported this class,
|
||||
* or {@code null} if this configuration was not imported.
|
||||
* @since 4.0
|
||||
* @see #isImported()
|
||||
*/
|
||||
@@ -182,7 +181,7 @@ final class ConfigurationClass {
|
||||
}
|
||||
|
||||
public Set<ImportBeanDefinitionRegistrar> getImportBeanDefinitionRegistrars() {
|
||||
return Collections.unmodifiableSet(importBeanDefinitionRegistrars);
|
||||
return Collections.unmodifiableSet(this.importBeanDefinitionRegistrars);
|
||||
}
|
||||
|
||||
public Map<String, Class<? extends BeanDefinitionReader>> getImportedResources() {
|
||||
@@ -197,23 +196,6 @@ final class ConfigurationClass {
|
||||
}
|
||||
}
|
||||
|
||||
// An @Bean method may only be overloaded through inheritance. No single
|
||||
// @Configuration class may declare two @Bean methods with the same name.
|
||||
Map<String, Integer> methodNameCounts = new HashMap<String, Integer>();
|
||||
for (BeanMethod beanMethod : this.beanMethods) {
|
||||
String fqMethodName = beanMethod.getFullyQualifiedMethodName();
|
||||
Integer currentCount = methodNameCounts.get(fqMethodName);
|
||||
int newCount = (currentCount != null ? currentCount + 1 : 1);
|
||||
methodNameCounts.put(fqMethodName, newCount);
|
||||
}
|
||||
for (String fqMethodName : methodNameCounts.keySet()) {
|
||||
int count = methodNameCounts.get(fqMethodName);
|
||||
if (count > 1) {
|
||||
String shortMethodName = ConfigurationMethod.getShortMethodName(fqMethodName);
|
||||
problemReporter.error(new BeanMethodOverloadingProblem(shortMethodName, count));
|
||||
}
|
||||
}
|
||||
|
||||
for (BeanMethod beanMethod : this.beanMethods) {
|
||||
beanMethod.validate(problemReporter);
|
||||
}
|
||||
@@ -232,7 +214,7 @@ final class ConfigurationClass {
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return String.format("[ConfigurationClass:beanName=%s,resource=%s]", this.beanName, this.resource);
|
||||
return "ConfigurationClass:beanName=" + this.beanName + ",resource=" + this.resource;
|
||||
}
|
||||
|
||||
|
||||
@@ -247,17 +229,4 @@ final class ConfigurationClass {
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Bean methods on configuration classes may only be overloaded through inheritance.
|
||||
*/
|
||||
private class BeanMethodOverloadingProblem extends Problem {
|
||||
|
||||
public BeanMethodOverloadingProblem(String methodName, int count) {
|
||||
super(String.format("@Configuration class '%s' has %s overloaded @Bean methods named '%s'. " +
|
||||
"Only one @Bean method of a given name is allowed within each @Configuration class.",
|
||||
getSimpleName(), count, methodName), new Location(getResource(), getMetadata()));
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -174,6 +174,7 @@ class ConfigurationClassBeanDefinitionReader {
|
||||
if (this.conditionEvaluator.shouldSkip(beanMethod.getMetadata(), ConfigurationPhase.REGISTER_BEAN)) {
|
||||
return;
|
||||
}
|
||||
|
||||
ConfigurationClass configClass = beanMethod.getConfigurationClass();
|
||||
MethodMetadata metadata = beanMethod.getMetadata();
|
||||
|
||||
@@ -258,9 +259,12 @@ class ConfigurationClassBeanDefinitionReader {
|
||||
BeanDefinition existingBeanDef = this.registry.getBeanDefinition(beanName);
|
||||
|
||||
// Is the existing bean definition one that was created from a configuration class?
|
||||
// -> allow the current bean method to override, since both are at second-pass level
|
||||
// -> allow the current bean method to override, since both are at second-pass level.
|
||||
// However, if the bean method is an overloaded case on the same configuration class,
|
||||
// preserve the existing bean definition.
|
||||
if (existingBeanDef instanceof ConfigurationClassBeanDefinition) {
|
||||
return false;
|
||||
ConfigurationClassBeanDefinition ccbd = (ConfigurationClassBeanDefinition) existingBeanDef;
|
||||
return (ccbd.getMetadata().getClassName().equals(beanMethod.getConfigurationClass().getMetadata().getClassName()));
|
||||
}
|
||||
|
||||
// Has the existing bean definition bean marked as a framework-generated bean?
|
||||
|
||||
@@ -16,14 +16,10 @@
|
||||
|
||||
package org.springframework.context.annotation;
|
||||
|
||||
import java.lang.annotation.Inherited;
|
||||
import java.util.List;
|
||||
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.junit.rules.ExpectedException;
|
||||
|
||||
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
|
||||
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
|
||||
import org.springframework.beans.factory.support.RootBeanDefinition;
|
||||
|
||||
@@ -34,50 +30,75 @@ import static org.junit.Assert.*;
|
||||
* Tests regarding overloading and overriding of bean methods.
|
||||
* Related to SPR-6618.
|
||||
*
|
||||
* Bean-annotated methods should be able to be overridden, just as any regular
|
||||
* method. This is straightforward.
|
||||
*
|
||||
* Bean-annotated methods should be able to be overloaded, though supporting this
|
||||
* is more subtle. Essentially, it must be unambiguous to the container which bean
|
||||
* method to call. A simple way to think about this is that no one Configuration
|
||||
* class may declare two bean methods with the same name. In the case of inheritance,
|
||||
* the most specific subclass bean method will always be the one that is invoked.
|
||||
*
|
||||
* @author Chris Beams
|
||||
* @author Phillip Webb
|
||||
* @author Juergen Hoeller
|
||||
*/
|
||||
@SuppressWarnings("resource")
|
||||
public class BeanMethodPolymorphismTests {
|
||||
|
||||
@Rule
|
||||
public ExpectedException thrown = ExpectedException.none();
|
||||
|
||||
|
||||
@Test
|
||||
public void beanMethodOverloadingWithoutInheritance() {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
|
||||
ctx.register(ConfigWithOverloading.class);
|
||||
ctx.setAllowBeanDefinitionOverriding(false);
|
||||
ctx.refresh();
|
||||
assertThat(ctx.getBean(String.class), equalTo("regular"));
|
||||
}
|
||||
|
||||
@SuppressWarnings({ "hiding" })
|
||||
@Configuration class Config {
|
||||
@Bean String aString() { return "na"; }
|
||||
@Bean String aString(Integer dependency) { return "na"; }
|
||||
}
|
||||
@Test
|
||||
public void beanMethodOverloadingWithoutInheritanceAndExtraDependency() {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
|
||||
ctx.register(ConfigWithOverloading.class);
|
||||
ctx.getDefaultListableBeanFactory().registerSingleton("anInt", 5);
|
||||
ctx.setAllowBeanDefinitionOverriding(false);
|
||||
ctx.refresh();
|
||||
assertThat(ctx.getBean(String.class), equalTo("overloaded5"));
|
||||
}
|
||||
|
||||
this.thrown.expect(BeanDefinitionParsingException.class);
|
||||
this.thrown.expectMessage("overloaded @Bean methods named 'aString'");
|
||||
new AnnotationConfigApplicationContext(Config.class);
|
||||
@Test
|
||||
public void beanMethodOverloadingWithAdditionalMetadata() {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
|
||||
ctx.register(ConfigWithOverloadingAndAdditionalMetadata.class);
|
||||
ctx.setAllowBeanDefinitionOverriding(false);
|
||||
ctx.refresh();
|
||||
assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
assertThat(ctx.getBean(String.class), equalTo("regular"));
|
||||
assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void beanMethodOverloadingWithAdditionalMetadataButOtherMethodExecuted() {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
|
||||
ctx.register(ConfigWithOverloadingAndAdditionalMetadata.class);
|
||||
ctx.getDefaultListableBeanFactory().registerSingleton("anInt", 5);
|
||||
ctx.setAllowBeanDefinitionOverriding(false);
|
||||
ctx.refresh();
|
||||
assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
assertThat(ctx.getBean(String.class), equalTo("overloaded5"));
|
||||
assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void beanMethodOverloadingWithInheritance() {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfig.class);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
|
||||
ctx.register(SubConfig.class);
|
||||
ctx.setAllowBeanDefinitionOverriding(false);
|
||||
ctx.refresh();
|
||||
assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
assertThat(ctx.getBean(String.class), equalTo("overloaded5"));
|
||||
assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
}
|
||||
|
||||
// SPR-11025
|
||||
@Test
|
||||
public void beanMethodOverloadingWithInheritanceAndList() {
|
||||
// SPR-11025
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfigWithList.class);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
|
||||
ctx.register(SubConfigWithList.class);
|
||||
ctx.setAllowBeanDefinitionOverriding(false);
|
||||
ctx.refresh();
|
||||
assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
assertThat(ctx.getBean(String.class), equalTo("overloaded5"));
|
||||
assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString"));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -91,11 +112,6 @@ public class BeanMethodPolymorphismTests {
|
||||
assertThat(ctx.getBean(String.class), equalTo("shadow"));
|
||||
}
|
||||
|
||||
/**
|
||||
* Tests that polymorphic Configuration classes need not explicitly redeclare the
|
||||
* {@link Configuration} annotation. This respects the {@link Inherited} nature
|
||||
* of the Configuration annotation, even though it's being detected via ASM.
|
||||
*/
|
||||
@Test
|
||||
public void beanMethodsDetectedOnSuperClass() {
|
||||
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
|
||||
@@ -122,6 +138,36 @@ public class BeanMethodPolymorphismTests {
|
||||
}
|
||||
|
||||
|
||||
@Configuration
|
||||
static class ConfigWithOverloading {
|
||||
|
||||
@Bean
|
||||
String aString() {
|
||||
return "regular";
|
||||
}
|
||||
|
||||
@Bean
|
||||
String aString(Integer dependency) {
|
||||
return "overloaded" + dependency;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@Configuration
|
||||
static class ConfigWithOverloadingAndAdditionalMetadata {
|
||||
|
||||
@Bean @Lazy
|
||||
String aString() {
|
||||
return "regular";
|
||||
}
|
||||
|
||||
@Bean
|
||||
String aString(Integer dependency) {
|
||||
return "overloaded" + dependency;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@Configuration
|
||||
static class SuperConfig {
|
||||
|
||||
@@ -140,7 +186,7 @@ public class BeanMethodPolymorphismTests {
|
||||
return 5;
|
||||
}
|
||||
|
||||
@Bean
|
||||
@Bean @Lazy
|
||||
String aString(Integer dependency) {
|
||||
return "overloaded" + dependency;
|
||||
}
|
||||
@@ -155,7 +201,7 @@ public class BeanMethodPolymorphismTests {
|
||||
return 5;
|
||||
}
|
||||
|
||||
@Bean
|
||||
@Bean @Lazy
|
||||
String aString(List<Integer> dependency) {
|
||||
return "overloaded" + dependency.get(0);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user