diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 5158867c46f..5547f729947 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -30,7 +30,6 @@ import java.util.HashMap; 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 deferredImportSelectors; + private final DeferredImportSelectorHandler deferredImportSelectorHandler = new DeferredImportSelectorHandler(); /** @@ -162,8 +160,6 @@ class ConfigurationClassParser { public void parse(Set configCandidates) { - this.deferredImportSelectors = new LinkedList<>(); - for (BeanDefinitionHolder holder : configCandidates) { BeanDefinition bd = holder.getBeanDefinition(); try { @@ -186,7 +182,7 @@ class ConfigurationClassParser { } } - processDeferredImportSelectors(); + this.deferredImportSelectorHandler.process(); } protected final void parse(@Nullable String className, String beanName) throws IOException { @@ -543,53 +539,7 @@ class ConfigurationClassParser { } } - private void processDeferredImportSelectors() { - List deferredImports = this.deferredImportSelectors; - this.deferredImportSelectors = null; - if (deferredImports == null) { - return; - } - - deferredImports.sort(DEFERRED_IMPORT_COMPARATOR); - Map groupings = new LinkedHashMap<>(); - Map configurationClasses = new HashMap<>(); - for (DeferredImportSelectorHolder deferredImport : deferredImports) { - Class 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 type) { - Class 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 importCandidates, boolean checkForCircularImports) { @@ -611,9 +561,9 @@ class ConfigurationClassParser { 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 @@ class ConfigurationClassParser { } + private class DeferredImportSelectorHandler { + + @Nullable + private List 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 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 groupings = new LinkedHashMap<>(); + + private final Map configurationClasses = new HashMap<>(); + + public void register(DeferredImportSelectorHolder deferredImport) { + Class 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 type) { + Class 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; diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ImportSelectorTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ImportSelectorTests.java index 45abc4afbac..f05116ca663 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ImportSelectorTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ImportSelectorTests.java @@ -20,15 +20,19 @@ import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; import org.hamcrest.Matcher; +import org.hamcrest.collection.IsIterableContainingInOrder; import org.junit.Before; import org.junit.Test; import org.mockito.InOrder; @@ -49,7 +53,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.any; import static org.mockito.Mockito.*; @@ -140,6 +144,47 @@ public class ImportSelectorTests { assertThat(iterator.next().getClassName(), equalTo(GroupedConfig1.class.getName())); } + @Test + public void importSelectorsWithNestedGroup() { + DefaultListableBeanFactory beanFactory = spy(new DefaultListableBeanFactory()); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(beanFactory); + context.register(ParentConfiguration1.class); + context.refresh(); + InOrder ordered = inOrder(beanFactory); + ordered.verify(beanFactory).registerBeanDefinition(eq("a"), any()); + ordered.verify(beanFactory).registerBeanDefinition(eq("e"), any()); + ordered.verify(beanFactory).registerBeanDefinition(eq("c"), any()); + assertThat(TestImportGroup.instancesCount.get(), equalTo(2)); + assertThat(TestImportGroup.imports.size(), equalTo(2)); + assertThat(TestImportGroup.allImports(), hasEntry( + is(ParentConfiguration1.class.getName()), + IsIterableContainingInOrder.contains(DeferredImportSelector1.class.getName(), + ChildConfiguration1.class.getName()))); + assertThat(TestImportGroup.allImports(), hasEntry( + is(ChildConfiguration1.class.getName()), + IsIterableContainingInOrder.contains(DeferredImportedSelector3.class.getName()))); + } + + @Test + public void importSelectorsWithNestedGroupSameDeferredImport() { + DefaultListableBeanFactory beanFactory = spy(new DefaultListableBeanFactory()); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(beanFactory); + context.register(ParentConfiguration2.class); + context.refresh(); + InOrder ordered = inOrder(beanFactory); + ordered.verify(beanFactory).registerBeanDefinition(eq("b"), any()); + ordered.verify(beanFactory).registerBeanDefinition(eq("d"), any()); + assertThat(TestImportGroup.instancesCount.get(), equalTo(2)); + assertThat(TestImportGroup.allImports().size(), equalTo(2)); + assertThat(TestImportGroup.allImports(), hasEntry( + is(ParentConfiguration2.class.getName()), + IsIterableContainingInOrder.contains(DeferredImportSelector2.class.getName(), + ChildConfiguration2.class.getName()))); + assertThat(TestImportGroup.allImports(), hasEntry( + is(ChildConfiguration2.class.getName()), + IsIterableContainingInOrder.contains(DeferredImportSelector2.class.getName()))); + } + @Test public void invokeAwareMethodsInImportGroup() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(GroupedConfig1.class); @@ -297,6 +342,15 @@ public class ImportSelectorTests { } } + @Configuration + public static class DeferredImportedSelector3 { + + @Bean + public String e() { + return "e"; + } + } + @Configuration @Import(IndirectImportSelector.class) @@ -360,6 +414,95 @@ public class ImportSelectorTests { } + @Configuration + @Import({ImportSelector1.class, ParentDeferredImportSelector1.class}) + public static class ParentConfiguration1 { + } + + + public static class ParentDeferredImportSelector1 implements DeferredImportSelector { + + @Override + public String[] selectImports(AnnotationMetadata importingClassMetadata) { + ImportSelectorTests.importFrom.put(getClass(), importingClassMetadata.getClassName()); + return new String[] { DeferredImportSelector1.class.getName(), ChildConfiguration1.class.getName() }; + } + + @Nullable + @Override + public Class getImportGroup() { + return TestImportGroup.class; + } + + } + + @Configuration + @Import({ImportSelector2.class, ParentDeferredImportSelector2.class}) + public static class ParentConfiguration2 { + } + + public static class ParentDeferredImportSelector2 implements DeferredImportSelector { + + @Override + public String[] selectImports(AnnotationMetadata importingClassMetadata) { + ImportSelectorTests.importFrom.put(getClass(), importingClassMetadata.getClassName()); + return new String[] { DeferredImportSelector2.class.getName(), ChildConfiguration2.class.getName() }; + } + + @Nullable + @Override + public Class getImportGroup() { + return TestImportGroup.class; + } + + } + + @Configuration + @Import(ChildDeferredImportSelector1.class) + public static class ChildConfiguration1 { + + } + + + public static class ChildDeferredImportSelector1 implements DeferredImportSelector { + + @Override + public String[] selectImports(AnnotationMetadata importingClassMetadata) { + ImportSelectorTests.importFrom.put(getClass(), importingClassMetadata.getClassName()); + return new String[] { DeferredImportedSelector3.class.getName() }; + } + + @Nullable + @Override + public Class getImportGroup() { + return TestImportGroup.class; + } + + } + + @Configuration + @Import(ChildDeferredImportSelector2.class) + public static class ChildConfiguration2 { + + } + + public static class ChildDeferredImportSelector2 implements DeferredImportSelector { + + @Override + public String[] selectImports(AnnotationMetadata importingClassMetadata) { + ImportSelectorTests.importFrom.put(getClass(), importingClassMetadata.getClassName()); + return new String[] { DeferredImportSelector2.class.getName() }; + } + + @Nullable + @Override + public Class getImportGroup() { + return TestImportGroup.class; + } + + } + + public static class TestImportGroup implements DeferredImportSelector.Group, BeanClassLoaderAware, ResourceLoaderAware, BeanFactoryAware, EnvironmentAware { @@ -384,17 +527,26 @@ public class ImportSelectorTests { TestImportGroup.imports.clear(); } + static Map> allImports() { + return TestImportGroup.imports.entrySet() + .stream() + .collect(Collectors.toMap((entry) -> entry.getKey().getClassName(), + Map.Entry::getValue)); + } + private final List instanceImports = new ArrayList<>(); + @Override public void process(AnnotationMetadata metadata, DeferredImportSelector selector) { + for (String importClassName : selector.selectImports(metadata)) { + this.instanceImports.add(new Entry(metadata, importClassName)); + } TestImportGroup.imports.addAll(metadata, Arrays.asList(selector.selectImports(metadata))); } @Override public Iterable selectImports() { - LinkedList content = new LinkedList<>(); - TestImportGroup.imports.forEach((metadata, values) -> - values.forEach(value -> content.add(new Entry(metadata, value)))); + LinkedList content = new LinkedList<>(this.instanceImports); Collections.reverse(content); return content; }