Recursively process DeferredImportSelector properly

Previously, if a DeferredImportSelector was identified at a later stage
as part of processing the collected set of deferred imports, such
selector was processed as a regular import selector.

Semantically, it makes sense as we already are in the deferred phase at
this point and it doesn't make much sense to bother about the extra
contract.

However, Spring Framework 5 introduced the notion of Group that a
deferred import selector can define. When it does, the container has to
honour the contract of the Group rather than calling the simpler
ImportSelector parent contract.

This commit restructures the processing of such case. When a deferred
import selector is detected while processing deferred import selectors,
a group is created with only that selector and the group API is invoked.

Issue: SPR-17351

Author: snicoll

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

@@ -30,7 +30,6 @@
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -138,8 +137,7 @@ class ConfigurationClassParser {
 
 	private final ImportStack importStack = new ImportStack();
 
-	@Nullable
-	private List<DeferredImportSelectorHolder> deferredImportSelectors;
+	private final DeferredImportSelectorHandler deferredImportSelectorHandler = new DeferredImportSelectorHandler();
 
 
 	/**
@@ -162,8 +160,6 @@ public ConfigurationClassParser(MetadataReaderFactory metadataReaderFactory,
 
 
 	public void parse(Set<BeanDefinitionHolder> configCandidates) {
-		this.deferredImportSelectors = new LinkedList<>();
-
 		for (BeanDefinitionHolder holder : configCandidates) {
 			BeanDefinition bd = holder.getBeanDefinition();
 			try {
@@ -186,7 +182,7 @@ else if (bd instanceof AbstractBeanDefinition && ((AbstractBeanDefinition) bd).h
 			}
 		}
 
-		processDeferredImportSelectors();
+		this.deferredImportSelectorHandler.process();
 	}
 
 	protected final void parse(@Nullable String className, String beanName) throws IOException {
@@ -543,53 +539,7 @@ private void collectImports(SourceClass sourceClass, Set<SourceClass> imports, S
 		}
 	}
 
-	private void processDeferredImportSelectors() {
-		List<DeferredImportSelectorHolder> deferredImports = this.deferredImportSelectors;
-		this.deferredImportSelectors = null;
-		if (deferredImports == null) {
-			return;
-		}
-
-		deferredImports.sort(DEFERRED_IMPORT_COMPARATOR);
-		Map<Object, DeferredImportSelectorGrouping> groupings = new LinkedHashMap<>();
-		Map<AnnotationMetadata, ConfigurationClass> configurationClasses = new HashMap<>();
-		for (DeferredImportSelectorHolder deferredImport : deferredImports) {
-			Class<? extends Group> group = deferredImport.getImportSelector().getImportGroup();
-			DeferredImportSelectorGrouping grouping = groupings.computeIfAbsent(
-					(group != null ? group : deferredImport),
-					key -> new DeferredImportSelectorGrouping(createGroup(group)));
-			grouping.add(deferredImport);
-			configurationClasses.put(deferredImport.getConfigurationClass().getMetadata(),
-					deferredImport.getConfigurationClass());
-		}
-		for (DeferredImportSelectorGrouping grouping : groupings.values()) {
-			grouping.getImports().forEach(entry -> {
-				ConfigurationClass configurationClass = configurationClasses.get(entry.getMetadata());
-				try {
-					processImports(configurationClass, asSourceClass(configurationClass),
-							asSourceClasses(entry.getImportClassName()), false);
-				}
-				catch (BeanDefinitionStoreException ex) {
-					throw ex;
-				}
-				catch (Throwable ex) {
-					throw new BeanDefinitionStoreException(
-							"Failed to process import candidates for configuration class [" +
-							configurationClass.getMetadata().getClassName() + "]", ex);
-				}
-			});
-		}
-	}
 
-	private Group createGroup(@Nullable Class<? extends Group> type) {
-		Class<? extends Group> effectiveType = (type != null ? type : DefaultDeferredImportSelectorGroup.class);
-		Group group = BeanUtils.instantiateClass(effectiveType);
-		ParserStrategyUtils.invokeAwareMethods(group,
-				ConfigurationClassParser.this.environment,
-				ConfigurationClassParser.this.resourceLoader,
-				ConfigurationClassParser.this.registry);
-		return group;
-	}
 
 	private void processImports(ConfigurationClass configClass, SourceClass currentSourceClass,
 			Collection<SourceClass> importCandidates, boolean checkForCircularImports) {
@@ -611,9 +561,9 @@ private void processImports(ConfigurationClass configClass, SourceClass currentS
 						ImportSelector selector = BeanUtils.instantiateClass(candidateClass, ImportSelector.class);
 						ParserStrategyUtils.invokeAwareMethods(
 								selector, this.environment, this.resourceLoader, this.registry);
-						if (this.deferredImportSelectors != null && selector instanceof DeferredImportSelector) {
-							this.deferredImportSelectors.add(
-									new DeferredImportSelectorHolder(configClass, (DeferredImportSelector) selector));
+						if (selector instanceof DeferredImportSelector) {
+							this.deferredImportSelectorHandler.handle(
+									configClass, (DeferredImportSelector) selector);
 						}
 						else {
 							String[] importClassNames = selector.selectImports(currentSourceClass.getMetadata());
@@ -787,6 +737,103 @@ public String toString() {
 	}
 
 
+	private class DeferredImportSelectorHandler {
+
+		@Nullable
+		private List<DeferredImportSelectorHolder> deferredImportSelectors = new ArrayList<>();
+
+		/**
+		 * Handle the specified {@link DeferredImportSelector}. If deferred import
+		 * selectors are being collected, this registers this instance to the list. If
+		 * they are being processed, the {@link DeferredImportSelector} is also processed
+		 * immediately according to its {@link DeferredImportSelector.Group}.
+		 * @param configClass the source configuration class
+		 * @param importSelector the selector to handle
+		 */
+		public void handle(ConfigurationClass configClass, DeferredImportSelector importSelector) {
+			DeferredImportSelectorHolder holder = new DeferredImportSelectorHolder(
+					configClass, importSelector);
+			if (this.deferredImportSelectors == null) {
+				DeferredImportSelectorGroupingHandler handler = new DeferredImportSelectorGroupingHandler();
+				handler.register(holder);
+				handler.processGroupImports();
+			}
+			else {
+				this.deferredImportSelectors.add(holder);
+			}
+		}
+
+		public void process() {
+			List<DeferredImportSelectorHolder> deferredImports = this.deferredImportSelectors;
+			this.deferredImportSelectors = null;
+			try {
+				if (deferredImports != null) {
+					DeferredImportSelectorGroupingHandler handler = new DeferredImportSelectorGroupingHandler();
+					deferredImports.sort(DEFERRED_IMPORT_COMPARATOR);
+					deferredImports.forEach(handler::register);
+					handler.processGroupImports();
+				}
+			}
+			finally {
+				this.deferredImportSelectors = new ArrayList<>();
+			}
+		}
+		
+	}
+
+
+	private class DeferredImportSelectorGroupingHandler {
+
+		private final Map<Object, DeferredImportSelectorGrouping> groupings = new LinkedHashMap<>();
+
+		private final Map<AnnotationMetadata, ConfigurationClass> configurationClasses = new HashMap<>();
+
+		public void register(DeferredImportSelectorHolder deferredImport) {
+			Class<? extends Group> group = deferredImport.getImportSelector()
+					.getImportGroup();
+			DeferredImportSelectorGrouping grouping = this.groupings.computeIfAbsent(
+					(group != null ? group : deferredImport),
+					key -> new DeferredImportSelectorGrouping(createGroup(group)));
+			grouping.add(deferredImport);
+			this.configurationClasses.put(deferredImport.getConfigurationClass().getMetadata(),
+					deferredImport.getConfigurationClass());
+		}
+
+		public void processGroupImports() {
+			for (DeferredImportSelectorGrouping grouping : this.groupings.values()) {
+				grouping.getImports().forEach(entry -> {
+					ConfigurationClass configurationClass = this.configurationClasses.get(
+							entry.getMetadata());
+					try {
+						processImports(configurationClass, asSourceClass(configurationClass),
+								asSourceClasses(entry.getImportClassName()), false);
+					}
+					catch (BeanDefinitionStoreException ex) {
+						throw ex;
+					}
+					catch (Throwable ex) {
+						throw new BeanDefinitionStoreException(
+								"Failed to process import candidates for configuration class [" +
+										configurationClass.getMetadata().getClassName() + "]", ex);
+					}
+				});
+			}
+		}
+
+		private Group createGroup(@Nullable Class<? extends Group> type) {
+			Class<? extends Group> effectiveType = (type != null ? type
+					: DefaultDeferredImportSelectorGroup.class);
+			Group group = BeanUtils.instantiateClass(effectiveType);
+			ParserStrategyUtils.invokeAwareMethods(group,
+					ConfigurationClassParser.this.environment,
+					ConfigurationClassParser.this.resourceLoader,
+					ConfigurationClassParser.this.registry);
+			return group;
+		}
+
+	}
+
+
 	private static class DeferredImportSelectorHolder {
 
 		private final ConfigurationClass configurationClass;