ConfigurationClassParser uses unified ImportStack with chained import analysis

Issue: SPR-14517
(cherry picked from commit d96a66a)
(cherry picked from commit ff878ea)

Author: jhoeller

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -103,7 +103,7 @@ public ConfigurationClass(MetadataReader metadataReader, ConfigurationClass impo
 	public ConfigurationClass(Class<?> clazz, String beanName) {
 		Assert.hasText(beanName, "Bean name must not be null");
 		this.metadata = new StandardAnnotationMetadata(clazz, true);
-		this.resource = new DescriptiveResource(clazz.toString());
+		this.resource = new DescriptiveResource(clazz.getName());
 		this.beanName = beanName;
 	}
 
@@ -117,7 +117,7 @@ public ConfigurationClass(Class<?> clazz, String beanName) {
 	 */
 	public ConfigurationClass(Class<?> clazz, ConfigurationClass importedBy) {
 		this.metadata = new StandardAnnotationMetadata(clazz, true);
-		this.resource = new DescriptiveResource(clazz.toString());
+		this.resource = new DescriptiveResource(clazz.getName());
 		this.importedBy.add(importedBy);
 	}
 
@@ -233,7 +233,7 @@ public int hashCode() {
 
 	@Override
 	public String toString() {
-		return "ConfigurationClass:beanName=" + this.beanName + ",resource=" + this.resource;
+		return "ConfigurationClass: beanName '" + this.beanName + "', " + this.resource;
 	}
 
 

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

@@ -134,7 +134,7 @@ public int compare(DeferredImportSelectorHolder o1, DeferredImportSelectorHolder
 
 	private final List<String> propertySourceNames = new ArrayList<String>();
 
-	private ImportStack importStack = new ImportStack();
+	private final ImportStack importStack = new ImportStack();
 
 	private List<DeferredImportSelectorHolder> deferredImportSelectors;
 
@@ -177,7 +177,7 @@ else if (bd instanceof AbstractBeanDefinition && ((AbstractBeanDefinition) bd).h
 			catch (BeanDefinitionStoreException ex) {
 				throw ex;
 			}
-			catch (Exception ex) {
+			catch (Throwable ex) {
 				throw new BeanDefinitionStoreException(
 						"Failed to parse configuration class [" + bd.getBeanClassName() + "]", ex);
 			}
@@ -269,16 +269,7 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
 			// Check the set of scanned definitions for any further config classes and parse recursively if necessary
 			for (BeanDefinitionHolder holder : scannedBeanDefinitions) {
 				if (ConfigurationClassUtils.checkConfigurationClassCandidate(holder.getBeanDefinition(), this.metadataReaderFactory)) {
-					// Provide isolated circular import detection for scanned classes,
-					// since the initial registration did not come explicitly.
-					ImportStack previousStack = this.importStack;
-					this.importStack = new ImportStack();
-					try {
-						parse(holder.getBeanDefinition().getBeanClassName(), holder.getBeanName());
-					}
-					finally {
-						this.importStack = previousStack;
-					}
+					parse(holder.getBeanDefinition().getBeanClassName(), holder.getBeanName());
 				}
 			}
 		}
@@ -472,7 +463,7 @@ private void processDeferredImportSelectors() {
 			catch (BeanDefinitionStoreException ex) {
 				throw ex;
 			}
-			catch (Exception ex) {
+			catch (Throwable ex) {
 				throw new BeanDefinitionStoreException("Failed to process import candidates for configuration class [" +
 						configClass.getMetadata().getClassName() + "]", ex);
 			}
@@ -486,7 +477,7 @@ private void processImports(ConfigurationClass configClass, SourceClass currentS
 			return;
 		}
 
-		if (checkForCircularImports && this.importStack.contains(configClass)) {
+		if (checkForCircularImports && isChainedImportOnStack(configClass)) {
 			this.problemReporter.error(new CircularImportProblem(configClass, this.importStack));
 		}
 		else {
@@ -529,7 +520,7 @@ else if (candidate.isAssignable(ImportBeanDefinitionRegistrar.class)) {
 			catch (BeanDefinitionStoreException ex) {
 				throw ex;
 			}
-			catch (Exception ex) {
+			catch (Throwable ex) {
 				throw new BeanDefinitionStoreException("Failed to process import candidates for configuration class [" +
 						configClass.getMetadata().getClassName() + "]", ex);
 			}
@@ -539,6 +530,20 @@ else if (candidate.isAssignable(ImportBeanDefinitionRegistrar.class)) {
 		}
 	}
 
+	private boolean isChainedImportOnStack(ConfigurationClass configClass) {
+		if (this.importStack.contains(configClass)) {
+			String configClassName = configClass.getMetadata().getClassName();
+			AnnotationMetadata importingClass = this.importStack.getImportingClassFor(configClassName);
+			while (importingClass != null) {
+				if (configClassName.equals(importingClass.getClassName())) {
+					return true;
+				}
+				importingClass = this.importStack.getImportingClassFor(importingClass.getClassName());
+			}
+		}
+		return false;
+	}
+
 	/**
 	 * Invoke {@link ResourceLoaderAware}, {@link BeanClassLoaderAware} and
 	 * {@link BeanFactoryAware} contracts if implemented by the given {@code bean}.