@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

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

@@ -156,7 +156,7 @@ class ConfigurationClassBeanDefinitionReader {
 		ConfigurationClass configClass = beanMethod.getConfigurationClass();
 		MethodMetadata metadata = beanMethod.getMetadata();
 
-		RootBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass);
+		ConfigurationClassBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass);
 		beanDef.setResource(configClass.getResource());
 		beanDef.setSource(this.sourceExtractor.extractSource(metadata, configClass.getResource()));
 		if (metadata.isStatic()) {
@@ -188,9 +188,18 @@ class ConfigurationClassBeanDefinitionReader {
 
 		// has this already been overridden (e.g. via XML)?
 		if (this.registry.containsBeanDefinition(beanName)) {
-			BeanDefinition existingBeanDef = registry.getBeanDefinition(beanName);
-			// is the existing bean definition one that was created from a configuration class?
-			if (!(existingBeanDef instanceof ConfigurationClassBeanDefinition)) {
+			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.
+			// However, if the bean method is an overloaded case on the same configuration class,
+			// preserve the existing bean definition.
+			if (existingBeanDef instanceof ConfigurationClassBeanDefinition) {
+				ConfigurationClassBeanDefinition ccbd = (ConfigurationClassBeanDefinition) existingBeanDef;
+				if (ccbd.getMetadata().getClassName().equals(beanMethod.getConfigurationClass().getMetadata().getClassName())) {
+					return;
+				}
+			}
+			else {
 				// no -> then it's an external override, probably XML
 				// overriding is legal, return immediately
 				if (logger.isDebugEnabled()) {
@@ -238,7 +247,7 @@ class ConfigurationClassBeanDefinitionReader {
 			beanDef.setDestroyMethodName(destroyMethodName);
 		}
 
-		// consider scoping
+		// Consider scoping
 		ScopedProxyMode proxyMode = ScopedProxyMode.NO;
 		AnnotationAttributes scope = attributesFor(metadata, Scope.class);
 		if (scope != null) {
@@ -249,7 +258,7 @@ class ConfigurationClassBeanDefinitionReader {
 			}
 		}
 
-		// replace the original bean definition with the target one, if necessary
+		// Replace the original bean definition with the target one, if necessary
 		BeanDefinition beanDefToRegister = beanDef;
 		if (proxyMode != ScopedProxyMode.NO) {
 			BeanDefinitionHolder proxyDef = ScopedProxyCreator.createScopedProxy(
@@ -259,7 +268,8 @@ class ConfigurationClassBeanDefinitionReader {
 		}
 
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Registering bean definition for @Bean method %s.%s()", configClass.getMetadata().getClassName(), beanName));
+			logger.debug(String.format("Registering bean definition for @Bean method %s.%s()",
+					configClass.getMetadata().getClassName(), beanName));
 		}
 
 		registry.registerBeanDefinition(beanName, beanDefToRegister);

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

@@ -16,7 +16,6 @@
 
 package org.springframework.context.annotation;
 
-import java.lang.annotation.Inherited;
 import java.util.List;
 
 import org.junit.Rule;
@@ -24,8 +23,6 @@ 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;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
@@ -34,25 +31,33 @@ 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 beanMethodDetectedOnSuperClass() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class);
+		ctx.getBean("testBean", TestBean.class);
+	}
+
+	@Test
+	public void beanMethodOverriding() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.register(OverridingConfig.class);
+		ctx.setAllowBeanDefinitionOverriding(false);
+		ctx.refresh();
+		assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("testBean"));
+		assertEquals("overridden", ctx.getBean("testBean", TestBean.class).toString());
+		assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("testBean"));
+	}
+
 	@Test
 	public void beanMethodOverloadingWithoutInheritance() {
 
@@ -69,15 +74,25 @@ public class BeanMethodPolymorphismTests {
 
 	@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,20 +106,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();
-		beanFactory.registerBeanDefinition("config", new RootBeanDefinition(Config.class));
-		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
-		pp.postProcessBeanFactory(beanFactory);
-		beanFactory.getBean("testBean", TestBean.class);
-	}
-
 
 	@Configuration
 	static class BaseConfig {
@@ -113,7 +114,6 @@ public class BeanMethodPolymorphismTests {
 		public TestBean testBean() {
 			return new TestBean();
 		}
-
 	}
 
 
@@ -122,6 +122,22 @@ public class BeanMethodPolymorphismTests {
 	}
 
 
+	@Configuration
+	static class OverridingConfig extends BaseConfig {
+
+		@Bean @Lazy
+		@Override
+		public TestBean testBean() {
+			return new TestBean() {
+				@Override
+				public String toString() {
+					return "overridden";
+				}
+			};
+		}
+	}
+
+
 	@Configuration
 	static class SuperConfig {
 
@@ -140,7 +156,7 @@ public class BeanMethodPolymorphismTests {
 			return 5;
 		}
 
-		@Bean
+		@Bean @Lazy
 		String aString(Integer dependency) {
 			return "overloaded" + dependency;
 		}
@@ -155,7 +171,7 @@ public class BeanMethodPolymorphismTests {
 			return 5;
 		}
 
-		@Bean
+		@Bean @Lazy
 		String aString(List<Integer> dependency) {
 			return "overloaded" + dependency.get(0);
 		}