diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java index 38f6aaac2b..63299fa486 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java @@ -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 getImportBeanDefinitionRegistrars() { - return Collections.unmodifiableSet(importBeanDefinitionRegistrars); + return Collections.unmodifiableSet(this.importBeanDefinitionRegistrars); } public Map> 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 methodNameCounts = new HashMap(); - 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())); - } - } - } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java index 497fb4cb21..9dd371278e 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java @@ -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? diff --git a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java index b666f8be92..c11e66e71f 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java @@ -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 dependency) { return "overloaded" + dependency.get(0); }