Ensure @conditions consider super classes

Fix @condition evaluation to also consider super classes for both
@configuration classes and regular @components. This change allows
@conditional annotations to be inherited and restores the previous
behavior of @Profile.

Issue: SPR-10840

Author: Phillip Webb

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

@@ -134,10 +134,10 @@ public void registerBean(Class<?> annotatedClass, Class<? extends Annotation>...
 	}
 
 	public void registerBean(Class<?> annotatedClass, String name, Class<? extends Annotation>... qualifiers) {
-		AnnotatedGenericBeanDefinition abd = new AnnotatedGenericBeanDefinition(annotatedClass);
-		if (conditionEvaluator.shouldSkip(abd.getMetadata())) {
+		if (shouldSkip(annotatedClass)) {
 			return;
 		}
+		AnnotatedGenericBeanDefinition abd = new AnnotatedGenericBeanDefinition(annotatedClass);
 		ScopeMetadata scopeMetadata = this.scopeMetadataResolver.resolveScopeMetadata(abd);
 		abd.setScope(scopeMetadata.getScopeName());
 		String beanName = (name != null ? name : this.beanNameGenerator.generateBeanName(abd, this.registry));
@@ -161,6 +161,17 @@ else if (Lazy.class.equals(qualifier)) {
 	}
 
 
+	private boolean shouldSkip(Class<?> annotatedClass) {
+		while(annotatedClass != null) {
+			AnnotatedGenericBeanDefinition abd = new AnnotatedGenericBeanDefinition(annotatedClass);
+			if(conditionEvaluator.shouldSkip(abd.getMetadata())) {
+				return true;
+			}
+			annotatedClass = annotatedClass.getSuperclass();
+		}
+		return false;
+	}
+
 	/**
 	 * Get the Environment from the given registry if possible, otherwise return a new
 	 * StandardEnvironment.

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

@@ -38,6 +38,7 @@
 import org.springframework.core.io.support.PathMatchingResourcePatternResolver;
 import org.springframework.core.io.support.ResourcePatternResolver;
 import org.springframework.core.io.support.ResourcePatternUtils;
+import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.core.type.classreading.CachingMetadataReaderFactory;
 import org.springframework.core.type.classreading.MetadataReader;
 import org.springframework.core.type.classreading.MetadataReaderFactory;
@@ -341,24 +342,29 @@ protected boolean isCandidateComponent(MetadataReader metadataReader) throws IOE
 		}
 		for (TypeFilter tf : this.includeFilters) {
 			if (tf.match(metadataReader, this.metadataReaderFactory)) {
-				return isConditionMatch(metadataReader);
+				return !shouldSkip(metadataReader);
 			}
 		}
 		return false;
 	}
 
-	/**
-	 * Determine whether the given class is a candidate component based on any
-	 * {@code @Conditional} annotations.
-	 * @param metadataReader the ASM ClassReader for the class
-	 * @return whether the class qualifies as a candidate component
-	 */
-	private boolean isConditionMatch(MetadataReader metadataReader) {
+	private boolean shouldSkip(MetadataReader metadataReader) throws IOException {
 		if (this.conditionEvaluator == null) {
 			this.conditionEvaluator = new ConditionEvaluator(getRegistry(),
 					getEnvironment(), null, null, getResourceLoader());
 		}
-		return !conditionEvaluator.shouldSkip(metadataReader.getAnnotationMetadata());
+
+		while(metadataReader != null) {
+			AnnotationMetadata metadata = metadataReader.getAnnotationMetadata();
+			if(this.conditionEvaluator.shouldSkip(metadata)) {
+				return true;
+			}
+			metadataReader = (metadata.hasSuperClass() ?
+					this.metadataReaderFactory.getMetadataReader(metadata.getSuperClassName())
+					: null);
+		}
+
+		return false;
 	}
 
 	/**

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

@@ -50,7 +50,6 @@ class ConditionEvaluator {
 	 */
 	public ConditionEvaluator(BeanDefinitionRegistry registry, Environment environment,
 			ApplicationContext applicationContext, ClassLoader classLoader, ResourceLoader resourceLoader) {
-
 		this.context = new ConditionContextImpl(registry, environment, applicationContext, classLoader, resourceLoader);
 	}
 
@@ -73,10 +72,6 @@ public boolean shouldSkip(AnnotatedTypeMetadata metadata) {
 	 * @return if the item should be skipped
 	 */
 	public boolean shouldSkip(AnnotatedTypeMetadata metadata, ConfigurationPhase phase) {
-		if (metadata == null || !metadata.isAnnotated(Conditional.class.getName())) {
-			return false;
-		}
-
 		if (phase == null) {
 			if (metadata instanceof AnnotationMetadata &&
 					ConfigurationClassUtils.isConfigurationCandidate((AnnotationMetadata) metadata)) {
@@ -99,6 +94,7 @@ public boolean shouldSkip(AnnotatedTypeMetadata metadata, ConfigurationPhase pha
 				}
 			}
 		}
+
 		return false;
 	}
 

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

@@ -16,10 +16,12 @@
 
 package org.springframework.context.annotation;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -51,6 +53,8 @@ final class ConfigurationClass {
 
 	private final AnnotationMetadata metadata;
 
+	private final List<AnnotationMetadata> metadataHierarchy = new ArrayList<AnnotationMetadata>();
+
 	private final Resource resource;
 
 	private String beanName;
@@ -129,6 +133,14 @@ public AnnotationMetadata getMetadata() {
 		return this.metadata;
 	}
 
+	public List<AnnotationMetadata> getMetadataHierarchy() {
+		return Collections.unmodifiableList(metadataHierarchy);
+	}
+
+	public void addMetadataHierarchy(AnnotationMetadata metadata) {
+		this.metadataHierarchy.add(metadata);
+	}
+
 	public Resource getResource() {
 		return this.resource;
 	}

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

@@ -380,13 +380,22 @@ public boolean shouldSkip(ConfigurationClass configClass) {
 					}
 				}
 				if (skip == null) {
-					skip = conditionEvaluator.shouldSkip(configClass.getMetadata(),
-							ConfigurationPhase.REGISTER_BEAN);
+					skip = shouldSkipConsideringHierarchy(configClass);
 				}
 				this.skipped.put(configClass, skip);
 			}
 			return skip;
 		}
+
+		private boolean shouldSkipConsideringHierarchy(ConfigurationClass configClass) {
+			for (AnnotationMetadata metadata : configClass.getMetadataHierarchy()) {
+				if (conditionEvaluator.shouldSkip(metadata,
+						ConfigurationPhase.REGISTER_BEAN)) {
+					return true;
+				}
+			}
+			return false;
+		}
 	}
 
 }

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

@@ -183,7 +183,7 @@ protected final void parse(Class<?> clazz, String beanName) throws IOException {
 
 
 	protected void processConfigurationClass(ConfigurationClass configClass) throws IOException {
-		if (this.conditionEvaluator.shouldSkip(configClass.getMetadata(), ConfigurationPhase.PARSE_CONFIGURATION)) {
+		if (shouldSkip(asSourceClass(configClass), ConfigurationPhase.PARSE_CONFIGURATION)) {
 			return;
 		}
 
@@ -201,6 +201,7 @@ protected void processConfigurationClass(ConfigurationClass configClass) throws
 		// Recursively process the configuration class and its superclass hierarchy.
 		SourceClass sourceClass = asSourceClass(configClass);
 		do {
+			configClass.addMetadataHierarchy(sourceClass.getMetadata());
 			sourceClass = doProcessConfigurationClass(configClass, sourceClass);
 		}
 		while (sourceClass != null);
@@ -230,7 +231,7 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
 		AnnotationAttributes componentScan = AnnotationConfigUtils.attributesFor(sourceClass.getMetadata(), ComponentScan.class);
 		if (componentScan != null) {
 			// the config class is annotated with @ComponentScan -> perform the scan immediately
-			if (!conditionEvaluator.shouldSkip(sourceClass.getMetadata(), ConfigurationPhase.REGISTER_BEAN)) {
+			if (!shouldSkip(sourceClass, ConfigurationPhase.REGISTER_BEAN)) {
 				Set<BeanDefinitionHolder> scannedBeanDefinitions =
 						this.componentScanParser.parse(componentScan, sourceClass.getMetadata().getClassName());
 
@@ -269,19 +270,25 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
 			if (!this.knownSuperclasses.containsKey(superclass)) {
 				this.knownSuperclasses.put(superclass, configClass);
 				// superclass found, return its annotation metadata and recurse
-				try {
-					return sourceClass.getSuperClass();
-				}
-				catch (ClassNotFoundException ex) {
-					throw new IllegalStateException(ex);
-				}
+				return sourceClass.getSuperClass();
 			}
 		}
 
 		// no superclass, processing is complete
 		return null;
 	}
 
+	private boolean shouldSkip(SourceClass sourceClass, ConfigurationPhase phase)
+			throws IOException {
+		while (sourceClass != null) {
+			if (conditionEvaluator.shouldSkip(sourceClass.getMetadata(), phase)) {
+				return true;
+			}
+			sourceClass = sourceClass.getSuperClass();
+		}
+		return false;
+	}
+
 	/**
 	 * Register member (nested) classes that happen to be configuration classes themselves.
 	 * @param sourceClass the source class to process
@@ -692,11 +699,19 @@ public Collection<SourceClass> getMemberClasses() throws IOException {
 			return members;
 		}
 
-		public SourceClass getSuperClass() throws IOException, ClassNotFoundException {
-			if (this.source instanceof Class<?>) {
-				return asSourceClass(((Class<?>) this.source).getSuperclass());
+		public SourceClass getSuperClass() throws IOException {
+			if (!getMetadata().hasSuperClass()) {
+				return null;
+			}
+			try {
+				if (this.source instanceof Class<?>) {
+					return asSourceClass(((Class<?>) this.source).getSuperclass());
+				}
+				return asSourceClass(((MetadataReader) this.source).getClassMetadata().getSuperClassName());
+			}
+			catch (ClassNotFoundException ex) {
+				throw new IllegalStateException(ex);
 			}
-			return asSourceClass(((MetadataReader) this.source).getClassMetadata().getSuperClassName());
 		}
 
 		public Set<SourceClass> getAnnotations() throws IOException, ClassNotFoundException {

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

@@ -54,6 +54,21 @@ public void closeContext() {
 		ctx.close();
 	}
 
+	@Test
+	public void conditionalOnConfiguration() throws Exception {
+		ctx.register(ConditionOnConfiguration.class);
+		ctx.refresh();
+		assertThat(ctx.getBeansOfType(ConditionOnConfiguration.class).size(), equalTo(0));
+	}
+
+	@Test
+	public void inheritedConditionalOnConfiguration() throws Exception {
+		ctx.register(InheritedConditionOnConfiguration.class);
+		ctx.refresh();
+		assertThat(ctx.getBeansOfType(ConditionOnConfiguration.class).size(), equalTo(0));
+		assertThat(ctx.getBeansOfType(InheritedConditionOnConfiguration.class).size(), equalTo(0));
+	}
+
 	@Test
 	public void conditionalOnMissingBeanMatch() throws Exception {
 		ctx.register(BeanOneConfiguration.class, BeanTwoConfiguration.class);
@@ -103,6 +118,14 @@ public void nonConfigurationClass() throws Exception {
 		assertNull(ctx.getBean(NonConfigurationClass.class));
 	}
 
+	@Test
+	public void inheritedNonConfigurationClass() throws Exception {
+		ctx.register(InheritedNonConfigurationClass.class);
+		ctx.refresh();
+		thrown.expect(NoSuchBeanDefinitionException.class);
+		assertNull(ctx.getBean(InheritedNonConfigurationClass.class));
+	}
+
 	@Test
 	public void methodConditional() throws Exception {
 		ctx.register(ConditionOnMethodConfiguration.class);
@@ -118,6 +141,13 @@ public void importsNotCreated() throws Exception {
 		ctx.refresh();
 	}
 
+	@Test
+	public void inheritedImportsNotCreated() throws Exception {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.register(InheritedImportsNotCreated.class);
+		ctx.refresh();
+	}
+
 	@Test
 	public void importsNotLoaded() throws Exception {
 		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
@@ -228,6 +258,10 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata)
 	static class NonConfigurationClass {
 	}
 
+	@Component
+	static class InheritedNonConfigurationClass extends NonConfigurationClass {
+	}
+
 	@Configuration
 	static class ConditionOnMethodConfiguration {
 
@@ -247,6 +281,21 @@ static class ImportsNotCreated {
 		}
 	}
 
+	@Configuration
+	@Never
+	static class ConditionOnConfiguration {
+	}
+
+	@Configuration
+	static class InheritedConditionOnConfiguration extends ConditionOnConfiguration {
+	}
+
+
+	@Import({ ConfigurationNotCreated.class, RegistrarNotCreated.class, ImportSelectorNotCreated.class })
+	static class InheritedImportsNotCreated extends ConditionOnConfiguration {
+
+	}
+
 	@Configuration
 	static class ConfigurationNotCreated {
 		static {