Revised condition override check based on method names instead of bean names

Issue: SPR-12744

Author: jhoeller

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

@@ -64,7 +64,7 @@ final class ConfigurationClass {
 	private final Map<ImportBeanDefinitionRegistrar, AnnotationMetadata> importBeanDefinitionRegistrars =
 			new LinkedHashMap<ImportBeanDefinitionRegistrar, AnnotationMetadata>();
 
-	final Set<String> skippedBeans = new HashSet<String>();
+	final Set<String> skippedBeanMethods = new HashSet<String>();
 
 
 	/**

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

@@ -184,21 +184,22 @@ private void registerBeanDefinitionForImportedConfigurationClass(ConfigurationCl
 	private void loadBeanDefinitionsForBeanMethod(BeanMethod beanMethod) {
 		ConfigurationClass configClass = beanMethod.getConfigurationClass();
 		MethodMetadata metadata = beanMethod.getMetadata();
-
-		// Consider name and any aliases
-		AnnotationAttributes bean = AnnotationConfigUtils.attributesFor(metadata, Bean.class);
-		List<String> names = new ArrayList<String>(Arrays.asList(bean.getStringArray("name")));
-		String beanName = (names.size() > 0 ? names.remove(0) : beanMethod.getMetadata().getMethodName());
+		String methodName = metadata.getMethodName();
 
 		// Do we need to mark the bean as skipped by its condition?
 		if (this.conditionEvaluator.shouldSkip(metadata, ConfigurationPhase.REGISTER_BEAN)) {
-			configClass.skippedBeans.add(beanName);
+			configClass.skippedBeanMethods.add(methodName);
 			return;
 		}
-		if (configClass.skippedBeans.contains(beanName)) {
+		if (configClass.skippedBeanMethods.contains(methodName)) {
 			return;
 		}
 
+		// Consider name and any aliases
+		AnnotationAttributes bean = AnnotationConfigUtils.attributesFor(metadata, Bean.class);
+		List<String> names = new ArrayList<String>(Arrays.asList(bean.getStringArray("name")));
+		String beanName = (names.size() > 0 ? names.remove(0) : methodName);
+
 		// Register aliases even when overridden
 		for (String alias : names) {
 			this.registry.registerAlias(beanName, alias);
@@ -216,12 +217,12 @@ private void loadBeanDefinitionsForBeanMethod(BeanMethod beanMethod) {
 		if (metadata.isStatic()) {
 			// static @Bean method
 			beanDef.setBeanClassName(configClass.getMetadata().getClassName());
-			beanDef.setFactoryMethodName(metadata.getMethodName());
+			beanDef.setFactoryMethodName(methodName);
 		}
 		else {
 			// instance @Bean method
 			beanDef.setFactoryBeanName(configClass.getBeanName());
-			beanDef.setUniqueFactoryMethodName(metadata.getMethodName());
+			beanDef.setUniqueFactoryMethodName(methodName);
 		}
 		beanDef.setAutowireMode(RootBeanDefinition.AUTOWIRE_CONSTRUCTOR);
 		beanDef.setAttribute(RequiredAnnotationBeanPostProcessor.SKIP_REQUIRED_CHECK_ATTRIBUTE, Boolean.TRUE);

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

@@ -20,6 +20,7 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.util.Map;
 
 import org.junit.Rule;
 import org.junit.Test;
@@ -127,7 +128,17 @@ public void conditionOnOverriddenMethodHonored() {
 	@Test
 	public void noConditionOnOverriddenMethodHonored() {
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithBeanReactivated.class);
-		assertEquals(1, context.getBeansOfType(ExampleBean.class).size());
+		Map<String, ExampleBean> beans = context.getBeansOfType(ExampleBean.class);
+		assertEquals(1, beans.size());
+		assertEquals("baz", beans.keySet().iterator().next());
+	}
+
+	@Test
+	public void configWithAlternativeBeans() {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithAlternativeBeans.class);
+		Map<String, ExampleBean> beans = context.getBeansOfType(ExampleBean.class);
+		assertEquals(1, beans.size());
+		assertEquals("baz", beans.keySet().iterator().next());
 	}
 
 
@@ -218,6 +229,14 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata)
 		}
 	}
 
+	static class AlwaysCondition implements Condition {
+
+		@Override
+		public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
+			return true;
+		}
+	}
+
 	@Component
 	@Never
 	static class NonConfigurationClass {
@@ -277,15 +296,15 @@ static class ExampleBean {
 	}
 
 	@Configuration
-	private static class ConfigWithBeanActive {
+	static class ConfigWithBeanActive {
 
 		@Bean
 		public ExampleBean baz() {
 			return new ExampleBean();
 		}
 	}
 
-	private static class ConfigWithBeanSkipped extends ConfigWithBeanActive {
+	static class ConfigWithBeanSkipped extends ConfigWithBeanActive {
 
 		@Override
 		@Bean
@@ -295,7 +314,7 @@ public ExampleBean baz() {
 		}
 	}
 
-	private static class ConfigWithBeanReactivated extends ConfigWithBeanSkipped {
+	static class ConfigWithBeanReactivated extends ConfigWithBeanSkipped {
 
 		@Override
 		@Bean
@@ -304,4 +323,20 @@ public ExampleBean baz() {
 		}
 	}
 
+	@Configuration
+	static class ConfigWithAlternativeBeans {
+
+		@Bean(name = "baz")
+		@Conditional(AlwaysCondition.class)
+		public ExampleBean baz1() {
+			return new ExampleBean();
+		}
+
+		@Bean(name = "baz")
+		@Conditional(NeverCondition.class)
+		public ExampleBean baz2() {
+			return new ExampleBean();
+		}
+	}
+
 }