SPR-10630

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

@@ -27,6 +27,8 @@
 import org.springframework.beans.factory.parsing.Problem;
 import org.springframework.beans.factory.parsing.ProblemReporter;
 import org.springframework.beans.factory.support.BeanDefinitionReader;
+import org.springframework.beans.factory.support.BeanDefinitionRegistry;
+import org.springframework.core.env.Environment;
 import org.springframework.core.io.DescriptiveResource;
 import org.springframework.core.io.Resource;
 import org.springframework.core.type.AnnotationMetadata;
@@ -65,6 +67,8 @@ final class ConfigurationClass {
 	private final Set<ImportBeanDefinitionRegistrar> importBeanDefinitionRegistrars =
 			new LinkedHashSet<ImportBeanDefinitionRegistrar>();
 
+	private Boolean shouldSkip;
+
 
 	/**
 	 * Create a new {@link ConfigurationClass} with the given name.
@@ -219,6 +223,16 @@ public void validate(ProblemReporter problemReporter) {
 		}
 	}
 
+	public boolean evaluateConditionals(BeanDefinitionRegistry registry, Environment environment) {
+		if(isImported() && getImportedBy().evaluateConditionals(registry, environment)) {
+			return true;
+		}
+		if(this.shouldSkip == null) {
+			this.shouldSkip = ConditionEvaluator.get(getMetadata(), false).shouldSkip(
+					registry, environment);
+		}
+		return this.shouldSkip;
+	}
 
 	@Override
 	public boolean equals(Object other) {

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

@@ -109,20 +109,18 @@ public ConfigurationClassBeanDefinitionReader(
 	 * based on its contents.
 	 */
 	public void loadBeanDefinitions(Set<ConfigurationClass> configurationModel) {
-		TrackedConditionEvaluator conditionEvaluator = new TrackedConditionEvaluator();
 		for (ConfigurationClass configClass : configurationModel) {
-			loadBeanDefinitionsForConfigurationClass(configClass, conditionEvaluator);
+			loadBeanDefinitionsForConfigurationClass(configClass);
 		}
 	}
 
 	/**
 	 * Read a particular {@link ConfigurationClass}, registering bean definitions for the
 	 * class itself, all its {@link Bean} methods
 	 */
-	private void loadBeanDefinitionsForConfigurationClass(ConfigurationClass configClass,
-			TrackedConditionEvaluator conditionEvaluator) {
+	private void loadBeanDefinitionsForConfigurationClass(ConfigurationClass configClass) {
 
-		if(conditionEvaluator.shouldSkip(configClass)) {
+		if(configClass.evaluateConditionals(this.registry, this.environment)) {
 			removeBeanDefinition(configClass);
 			return;
 		}
@@ -384,32 +382,5 @@ public InvalidConfigurationImportProblem(String className, Resource resource, An
 		}
 	}
 
-	/**
-	 * Evaluate {@Code @Conditional} annotations, tracking results and taking into
-	 * account 'imported by'.
-	 */
-	private class TrackedConditionEvaluator {
-
-		private final Map<ConfigurationClass, Boolean> skipped = new HashMap<ConfigurationClass, Boolean>();
-
-		public boolean shouldSkip(ConfigurationClass configClass) {
-			Boolean skip = this.skipped.get(configClass);
-			if (skip == null) {
-				if (configClass.isImported()) {
-					if (shouldSkip(configClass.getImportedBy())) {
-						// The config that imported this one was skipped, therefore we are skipped
-						skip = true;
-					}
-				}
-				if (skip == null) {
-					skip = ConditionEvaluator.get(configClass.getMetadata(), false).shouldSkip(
-							registry, environment);
-				}
-				this.skipped.put(configClass, skip);
-			}
-			return skip;
-		}
-
-	}
 
 }

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

@@ -221,8 +221,7 @@ protected AnnotationMetadata doProcessConfigurationClass(
 		AnnotationAttributes componentScan = attributesFor(metadata, ComponentScan.class);
 		if (componentScan != null) {
 			// the config class is annotated with @ComponentScan -> perform the scan immediately
-			if (!ConditionEvaluator.get(configClass.getMetadata(), false).shouldSkip(
-					this.registry, this.environment)) {
+			if (!configClass.evaluateConditionals(this.registry, this.environment)) {
 				Set<BeanDefinitionHolder> scannedBeanDefinitions =
 						this.componentScanParser.parse(componentScan, metadata.getClassName());
 
@@ -280,8 +279,16 @@ else if (superclass.startsWith("java")) {
 					}
 				}
 				else {
-					MetadataReader reader = this.metadataReaderFactory.getMetadataReader(superclass);
-					return reader.getAnnotationMetadata();
+					try {
+						MetadataReader reader = this.metadataReaderFactory.getMetadataReader(superclass);
+						return reader.getAnnotationMetadata();
+					}
+					catch (IOException ex) {
+						// Loading failed, evaluate the conditionals to see if it is important
+						if (!configClass.evaluateConditionals(this.registry, this.environment)) {
+							throw ex;
+						}
+					}
 				}
 			}
 		}
@@ -425,40 +432,9 @@ private void processImport(ConfigurationClass configClass, Collection<?> classes
 		}
 		else {
 			this.importStack.push(configClass);
-			AnnotationMetadata importingClassMetadata = configClass.getMetadata();
 			try {
 				for (Object candidate : classesToImport) {
-					Object candidateToCheck = (candidate instanceof Class ? (Class) candidate :
-							this.metadataReaderFactory.getMetadataReader((String) candidate));
-					if (checkAssignability(ImportSelector.class, candidateToCheck)) {
-						// the candidate class is an ImportSelector -> delegate to it to determine imports
-						Class<?> candidateClass = (candidate instanceof Class ? (Class) candidate :
-								this.resourceLoader.getClassLoader().loadClass((String) candidate));
-						ImportSelector selector = BeanUtils.instantiateClass(candidateClass, ImportSelector.class);
-						invokeAwareMethods(selector);
-						if(selector instanceof DeferredImportSelector) {
-							this.deferredImportSelectors.add(new DeferredImportSelectorHolder(configClass, (DeferredImportSelector) selector));
-						}
-						else {
-							processImport(configClass, Arrays.asList(selector.selectImports(importingClassMetadata)), false);
-						}
-					}
-					else if (checkAssignability(ImportBeanDefinitionRegistrar.class, candidateToCheck)) {
-						// the candidate class is an ImportBeanDefinitionRegistrar -> delegate to it to register additional bean definitions
-						Class<?> candidateClass = (candidate instanceof Class ? (Class) candidate :
-								this.resourceLoader.getClassLoader().loadClass((String) candidate));
-						ImportBeanDefinitionRegistrar registrar = BeanUtils.instantiateClass(candidateClass, ImportBeanDefinitionRegistrar.class);
-						invokeAwareMethods(registrar);
-						configClass.addImportBeanDefinitionRegistrar(registrar);
-					}
-					else {
-						// candidate class not an ImportSelector or ImportBeanDefinitionRegistrar -> process it as a @Configuration class
-						this.importStack.registerImport(importingClassMetadata.getClassName(),
-								(candidate instanceof Class ? ((Class) candidate).getName() : (String) candidate));
-						processConfigurationClass((candidateToCheck instanceof Class ?
-								new ConfigurationClass((Class) candidateToCheck, configClass) :
-								new ConfigurationClass((MetadataReader) candidateToCheck, configClass)));
-					}
+					processImportCandidate(configClass, candidate);
 				}
 			}
 			catch (ClassNotFoundException ex) {
@@ -470,12 +446,64 @@ else if (checkAssignability(ImportBeanDefinitionRegistrar.class, candidateToChec
 		}
 	}
 
-	private boolean checkAssignability(Class<?> clazz, Object candidate) throws IOException {
-		if (candidate instanceof Class) {
-			return clazz.isAssignableFrom((Class) candidate);
+	private void processImportCandidate(ConfigurationClass configClass, Object candidate)
+			throws IOException, ClassNotFoundException {
+
+		ImportType importType;
+		try {
+			importType = candidate instanceof Class ?
+					ImportType.get((Class) candidate) :
+						ImportType.get(
+							this.metadataReaderFactory.getMetadataReader((String) candidate),
+							this.metadataReaderFactory);
+		}
+		catch(IOException ex) {
+			if(configClass.evaluateConditionals(this.registry, this.environment)) {
+				return;
+			}
+			throw ex;
+		}
+
+		if (importType == ImportType.SELECTOR) {
+			if (!configClass.evaluateConditionals(this.registry, this.environment)) {
+				// the candidate class is an ImportSelector -> delegate to it to determine imports
+				Class<?> candidateClass = (candidate instanceof Class ? (Class) candidate :
+						this.resourceLoader.getClassLoader().loadClass((String) candidate));
+				ImportSelector selector = BeanUtils.instantiateClass(candidateClass, ImportSelector.class);
+				invokeAwareMethods(selector);
+				if(selector instanceof DeferredImportSelector) {
+					this.deferredImportSelectors.add(new DeferredImportSelectorHolder(configClass, (DeferredImportSelector) selector));
+				}
+				else {
+					processImport(configClass, Arrays.asList(selector.selectImports(configClass.getMetadata())), false);
+				}
+			}
+			return;
+		}
+
+		if (importType == ImportType.REGISTRAR) {
+			if (!configClass.evaluateConditionals(this.registry, this.environment)) {
+				// the candidate class is an ImportBeanDefinitionRegistrar -> delegate to it to register additional bean definitions
+				Class<?> candidateClass = (candidate instanceof Class ? (Class) candidate :
+					this.resourceLoader.getClassLoader().loadClass((String) candidate));
+				ImportBeanDefinitionRegistrar registrar = BeanUtils.instantiateClass(candidateClass, ImportBeanDefinitionRegistrar.class);
+				invokeAwareMethods(registrar);
+				configClass.addImportBeanDefinitionRegistrar(registrar);
+			}
+			return;
+		}
+
+		// candidate class not an ImportSelector or ImportBeanDefinitionRegistrar -> process it as a @Configuration class
+		if(candidate instanceof Class) {
+			Class candidateClass = (Class) candidate;
+			this.importStack.registerImport(configClass.getMetadata().getClassName(), candidateClass.getName());
+			processConfigurationClass(new ConfigurationClass(candidateClass, configClass));
 		}
 		else {
-			return new AssignableTypeFilter(clazz).match((MetadataReader) candidate, this.metadataReaderFactory);
+			String candidateClassName = (String) candidate;
+			this.importStack.registerImport(configClass.getMetadata().getClassName(), candidateClassName);
+			processConfigurationClass(new ConfigurationClass(
+					this.metadataReaderFactory.getMetadataReader(candidateClassName), configClass));
 		}
 	}
 
@@ -623,4 +651,44 @@ public CircularImportProblem(ConfigurationClass attemptedImport, Stack<Configura
 		}
 	}
 
+
+	private static enum ImportType {
+
+		SELECTOR(ImportSelector.class),
+
+		REGISTRAR(ImportBeanDefinitionRegistrar.class),
+
+		CONFIGURATION(null);
+
+
+		private Class<?> typeClass;
+
+
+		private ImportType(Class<?> typeClass) {
+			this.typeClass = typeClass;
+		}
+
+
+		public static ImportType get(Class<?> candidate) {
+			for (ImportType type : values()) {
+				if (type.typeClass == null || type.typeClass.isAssignableFrom(candidate)) {
+					return type;
+				}
+			}
+			throw new IllegalStateException();
+		}
+
+		public static ImportType get(MetadataReader candidate,
+				MetadataReaderFactory metadataReaderFactory) throws IOException {
+			for (ImportType type : values()) {
+				if (type.typeClass == null
+						|| new AssignableTypeFilter(type.typeClass).match(candidate,
+								metadataReaderFactory)) {
+					return type;
+				}
+			}
+			throw new IllegalStateException();
+		}
+	};
+
 }

spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java

@@ -635,18 +635,10 @@ protected void invokeBeanFactoryPostProcessors(ConfigurableListableBeanFactory b
 					regularPostProcessors.add(postProcessor);
 				}
 			}
-			Map<String, BeanDefinitionRegistryPostProcessor> beanMap =
-					beanFactory.getBeansOfType(BeanDefinitionRegistryPostProcessor.class, true, false);
-			List<BeanDefinitionRegistryPostProcessor> registryPostProcessorBeans =
-					new ArrayList<BeanDefinitionRegistryPostProcessor>(beanMap.values());
-			OrderComparator.sort(registryPostProcessorBeans);
-			for (BeanDefinitionRegistryPostProcessor postProcessor : registryPostProcessorBeans) {
-				postProcessor.postProcessBeanDefinitionRegistry(registry);
-			}
 			invokeBeanFactoryPostProcessors(registryPostProcessors, beanFactory);
-			invokeBeanFactoryPostProcessors(registryPostProcessorBeans, beanFactory);
+			processedBeans.addAll(
+					invokeBeanDefinitionRegistryPostProcessorsBeans(registry, beanFactory));
 			invokeBeanFactoryPostProcessors(regularPostProcessors, beanFactory);
-			processedBeans.addAll(beanMap.keySet());
 		}
 		else {
 			// Invoke factory processors registered with the context instance.
@@ -698,6 +690,34 @@ else if (isTypeMatch(ppName, Ordered.class)) {
 		invokeBeanFactoryPostProcessors(nonOrderedPostProcessors, beanFactory);
 	}
 
+	/**
+	 * Process {@link BeanDefinitionRegistryPostProcessor} beans.
+	 */
+	private Collection<String> invokeBeanDefinitionRegistryPostProcessorsBeans(
+			BeanDefinitionRegistry registry, ConfigurableListableBeanFactory beanFactory) {
+		Set<String> processed = new HashSet<String>();
+
+		Map<String, BeanDefinitionRegistryPostProcessor> beanMap;
+
+		do {
+			beanMap = beanFactory.getBeansOfType(BeanDefinitionRegistryPostProcessor.class, true, false);
+			beanMap.keySet().removeAll(processed);
+
+			List<BeanDefinitionRegistryPostProcessor> beans =
+					new ArrayList<BeanDefinitionRegistryPostProcessor>(beanMap.values());
+			OrderComparator.sort(beans);
+			for (BeanDefinitionRegistryPostProcessor postProcessor : beans) {
+				postProcessor.postProcessBeanDefinitionRegistry(registry);
+			}
+
+			invokeBeanFactoryPostProcessors(beans, beanFactory);
+			processed.addAll(beanMap.keySet());
+		}
+		while (!beanMap.isEmpty()); // Loop in case further BDRPPs were added
+
+		return processed;
+	}
+
 	/**
 	 * Invoke the given BeanFactoryPostProcessor beans.
 	 */