Properly honor conditions on overriding bean methods

Issue: SPR-12694

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 
 package org.springframework.context.annotation;
 
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -63,6 +64,8 @@ final class ConfigurationClass {
 	private final Map<ImportBeanDefinitionRegistrar, AnnotationMetadata> importBeanDefinitionRegistrars =
 			new LinkedHashMap<ImportBeanDefinitionRegistrar, AnnotationMetadata>();
 
+	final Set<String> skippedBeans = new HashSet<String>();
+
 
 	/**
 	 * Create a new {@link ConfigurationClass} with the given name.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -182,16 +182,37 @@ class ConfigurationClassBeanDefinitionReader {
 	 * with the BeanDefinitionRegistry based on its contents.
 	 */
 	private void loadBeanDefinitionsForBeanMethod(BeanMethod beanMethod) {
-		if (this.conditionEvaluator.shouldSkip(beanMethod.getMetadata(), ConfigurationPhase.REGISTER_BEAN)) {
+		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());
+
+		// Do we need to mark the bean as skipped by its condition?
+		if (this.conditionEvaluator.shouldSkip(metadata, ConfigurationPhase.REGISTER_BEAN)) {
+			configClass.skippedBeans.add(beanName);
+			return;
+		}
+		if (configClass.skippedBeans.contains(beanName)) {
 			return;
 		}
 
-		ConfigurationClass configClass = beanMethod.getConfigurationClass();
+		// Register aliases even when overridden
-		MethodMetadata metadata = beanMethod.getMetadata();
+		for (String alias : names) {
+			this.registry.registerAlias(beanName, alias);
+		}
+
+		// Has this effectively been overridden before (e.g. via XML)?
+		if (isOverriddenByExistingDefinition(beanMethod, beanName)) {
+			return;
+		}
 
 		ConfigurationClassBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass, metadata);
 		beanDef.setResource(configClass.getResource());
 		beanDef.setSource(this.sourceExtractor.extractSource(metadata, configClass.getResource()));
+
 		if (metadata.isStatic()) {
 			// static @Bean method
 			beanDef.setBeanClassName(configClass.getMetadata().getClassName());
@@ -205,19 +226,6 @@ class ConfigurationClassBeanDefinitionReader {
 		beanDef.setAutowireMode(RootBeanDefinition.AUTOWIRE_CONSTRUCTOR);
 		beanDef.setAttribute(RequiredAnnotationBeanPostProcessor.SKIP_REQUIRED_CHECK_ATTRIBUTE, Boolean.TRUE);
 
-		// 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());
-		for (String alias : names) {
-			this.registry.registerAlias(beanName, alias);
-		}
-
-		// Has this effectively been overridden before (e.g. via XML)?
-		if (isOverriddenByExistingDefinition(beanMethod, beanName)) {
-			return;
-		}
-
 		AnnotationConfigUtils.processCommonDefinitionAnnotations(beanDef, metadata);
 
 		Autowire autowire = bean.getEnum("autowire");

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@ import static org.junit.Assert.*;
  * Test for {@link Conditional} beans.
  *
  * @author Phillip Webb
+ * @author Juergen Hoeller
  */
 @SuppressWarnings("resource")
 public class ConfigurationClassWithConditionTests {
@@ -117,6 +118,19 @@ public class ConfigurationClassWithConditionTests {
 		ctx.refresh();
 	}
 
+	@Test
+	public void conditionOnOverriddenMethodHonored() {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithBeanSkipped.class);
+		assertEquals(0, context.getBeansOfType(ExampleBean.class).size());
+	}
+
+	@Test
+	public void noConditionOnOverriddenMethodHonored() {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithBeanReactivated.class);
+		assertEquals(1, context.getBeansOfType(ExampleBean.class).size());
+	}
+
+
 	@Configuration
 	static class BeanOneConfiguration {
 		@Bean
@@ -197,6 +211,7 @@ public class ConfigurationClassWithConditionTests {
 	}
 
 	static class NeverCondition implements Condition {
+
 		@Override
 		public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
 			return false;
@@ -261,4 +276,32 @@ public class ConfigurationClassWithConditionTests {
 	static class ExampleBean {
 	}
 
+	@Configuration
+	private static class ConfigWithBeanActive {
+
+		@Bean
+		public ExampleBean baz() {
+			return new ExampleBean();
+		}
+	}
+
+	private static class ConfigWithBeanSkipped extends ConfigWithBeanActive {
+
+		@Override
+		@Bean
+		@Conditional(NeverCondition.class)
+		public ExampleBean baz() {
+			return new ExampleBean();
+		}
+	}
+
+	private static class ConfigWithBeanReactivated extends ConfigWithBeanSkipped {
+
+		@Override
+		@Bean
+		public ExampleBean baz() {
+			return new ExampleBean();
+		}
+	}
+
 }