From bc7db86c8c0e58c7dc1c5f02e03aa0c816d2ab9b Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 19 Jul 2018 09:58:29 +0200 Subject: [PATCH] Consistently apply exclude on auto-configuration Previously, exclude of an import selector was applied only locally. In other words, if one import selector imports `AcmeAutoConfiguration` and another one exclude it, it would still be imported because exclude were applied separately This commit collects the outcome of all auto-configuration import selectors and then apply exclusions in a single pass. Closes gh-12586 --- .../AutoConfigurationImportSelector.java | 126 +++++++++++++++--- .../condition/ConditionEvaluationReport.java | 13 +- .../ImportAutoConfigurationTests.java | 29 +++- ...tAutoConfigurationImportListenerTests.java | 32 ++++- 4 files changed, 177 insertions(+), 23 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationImportSelector.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationImportSelector.java index dadd0faba1..6cb236f124 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationImportSelector.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationImportSelector.java @@ -18,7 +18,9 @@ package org.springframework.boot.autoconfigure; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -40,6 +42,7 @@ import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.boot.context.properties.bind.Binder; import org.springframework.context.EnvironmentAware; import org.springframework.context.ResourceLoaderAware; +import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.DeferredImportSelector; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAttributes; @@ -70,6 +73,8 @@ public class AutoConfigurationImportSelector implements DeferredImportSelector, BeanClassLoaderAware, ResourceLoaderAware, BeanFactoryAware, EnvironmentAware, Ordered { + private static final AutoConfigurationEntry EMPTY_ENTRY = new AutoConfigurationEntry(); + private static final String[] NO_IMPORTS = {}; private static final Log logger = LogFactory @@ -92,6 +97,24 @@ public class AutoConfigurationImportSelector } AutoConfigurationMetadata autoConfigurationMetadata = AutoConfigurationMetadataLoader .loadMetadata(this.beanClassLoader); + AutoConfigurationEntry autoConfigurationEntry = getAutoConfigurationEntry( + autoConfigurationMetadata, annotationMetadata); + return StringUtils.toStringArray(autoConfigurationEntry.getConfigurations()); + } + + /** + * Return the {@link AutoConfigurationEntry} based on the {@link AnnotationMetadata} + * of the importing @{@link Configuration} class. + * @param autoConfigurationMetadata the auto-configuration metadata + * @param annotationMetadata the annotation metadata of the configuration class + * @return the auto-configurations that should be imported + */ + protected AutoConfigurationEntry getAutoConfigurationEntry( + AutoConfigurationMetadata autoConfigurationMetadata, + AnnotationMetadata annotationMetadata) { + if (!isEnabled(annotationMetadata)) { + return EMPTY_ENTRY; + } AnnotationAttributes attributes = getAttributes(annotationMetadata); List configurations = getCandidateConfigurations(annotationMetadata, attributes); @@ -101,7 +124,7 @@ public class AutoConfigurationImportSelector configurations.removeAll(exclusions); configurations = filter(configurations, autoConfigurationMetadata); fireAutoConfigurationImportEvents(configurations, exclusions); - return StringUtils.toStringArray(configurations); + return new AutoConfigurationEntry(configurations, exclusions); } @Override @@ -357,13 +380,17 @@ public class AutoConfigurationImportSelector private static class AutoConfigurationGroup implements DeferredImportSelector.Group, BeanClassLoaderAware, BeanFactoryAware, ResourceLoaderAware { + private final Map entries = new LinkedHashMap<>(); + + private final List autoConfigurationEntries = new ArrayList<>(); + private ClassLoader beanClassLoader; private BeanFactory beanFactory; private ResourceLoader resourceLoader; - private final Map entries = new LinkedHashMap<>(); + private AutoConfigurationMetadata autoConfigurationMetadata; @Override public void setBeanClassLoader(ClassLoader classLoader) { @@ -383,29 +410,63 @@ public class AutoConfigurationImportSelector @Override public void process(AnnotationMetadata annotationMetadata, DeferredImportSelector deferredImportSelector) { - String[] imports = deferredImportSelector.selectImports(annotationMetadata); - for (String importClassName : imports) { - this.entries.put(importClassName, annotationMetadata); + Assert.state( + deferredImportSelector instanceof AutoConfigurationImportSelector, + String.format( + "AutoConfigurationImportSelector only supports %s implementations, got %s", + AutoConfigurationImportSelector.class.getSimpleName(), + deferredImportSelector.getClass().getName())); + AutoConfigurationEntry autoConfigurationEntry = ((AutoConfigurationImportSelector) deferredImportSelector) + .getAutoConfigurationEntry(getAutoConfigurationMetadata(), + annotationMetadata); + this.autoConfigurationEntries.add(autoConfigurationEntry); + for (String importClassName : autoConfigurationEntry.getConfigurations()) { + this.entries.putIfAbsent(importClassName, annotationMetadata); } } @Override public Iterable selectImports() { - return sortAutoConfigurations().stream() - .map((importClassName) -> new Entry(this.entries.get(importClassName), - importClassName)) - .collect(Collectors.toList()); + if (this.autoConfigurationEntries.isEmpty()) { + return Collections.emptyList(); + } + Set allExclusions = this.autoConfigurationEntries.stream() + .map(AutoConfigurationEntry::getExclusions) + .flatMap(Collection::stream).collect(Collectors.toSet()); + Set processedConfigurations = new LinkedHashSet<>(); + Set processedExclusions = new LinkedHashSet<>(); + this.autoConfigurationEntries.forEach((entry) -> { + List configurations = new ArrayList<>(entry.getConfigurations()); + configurations.removeAll(allExclusions); + configurations.removeIf(processedConfigurations::contains); + Set exclusions = new HashSet<>(entry.getExclusions()); + exclusions.removeIf(processedExclusions::contains); + // This now represents the exact state of this entry based on the + // state of all other entries + processedConfigurations.addAll(configurations); + processedExclusions.addAll(exclusions); + }); + + return sortAutoConfigurations(processedConfigurations, + getAutoConfigurationMetadata()) + .stream() + .map((importClassName) -> new Entry( + this.entries.get(importClassName), importClassName)) + .collect(Collectors.toList()); } - private List sortAutoConfigurations() { - List autoConfigurations = new ArrayList<>(this.entries.keySet()); - if (this.entries.size() <= 1) { - return autoConfigurations; + private AutoConfigurationMetadata getAutoConfigurationMetadata() { + if (this.autoConfigurationMetadata == null) { + this.autoConfigurationMetadata = AutoConfigurationMetadataLoader + .loadMetadata(this.beanClassLoader); } - AutoConfigurationMetadata autoConfigurationMetadata = AutoConfigurationMetadataLoader - .loadMetadata(this.beanClassLoader); + return this.autoConfigurationMetadata; + } + + private List sortAutoConfigurations(Set configurations, + AutoConfigurationMetadata autoConfigurationMetadata) { return new AutoConfigurationSorter(getMetadataReaderFactory(), - autoConfigurationMetadata).getInPriorityOrder(autoConfigurations); + autoConfigurationMetadata).getInPriorityOrder(configurations); } private MetadataReaderFactory getMetadataReaderFactory() { @@ -421,4 +482,37 @@ public class AutoConfigurationImportSelector } + protected static class AutoConfigurationEntry { + + private final List configurations; + + private final Set exclusions; + + private AutoConfigurationEntry() { + this.configurations = Collections.EMPTY_LIST; + this.exclusions = Collections.EMPTY_SET; + } + + /** + * Create an entry with the configurations that were contributed and their + * exclusions. + * @param configurations the configurations that should be imported + * @param exclusions the exclusions that were applied to the original list + */ + AutoConfigurationEntry(Collection configurations, + Collection exclusions) { + this.configurations = new ArrayList<>(configurations); + this.exclusions = new HashSet<>(exclusions); + } + + public List getConfigurations() { + return this.configurations; + } + + public Set getExclusions() { + return this.exclusions; + } + + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java index 128b7cf399..9823a2f029 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReport.java @@ -43,6 +43,7 @@ import org.springframework.util.ObjectUtils; * @author Dave Syer * @author Phillip Webb * @author Andy Wilkinson + * @author Stephane Nicoll */ public final class ConditionEvaluationReport { @@ -56,9 +57,9 @@ public final class ConditionEvaluationReport { private ConditionEvaluationReport parent; - private List exclusions = Collections.emptyList(); + private final List exclusions = new ArrayList<>(); - private Set unconditionalClasses = new HashSet<>(); + private final Set unconditionalClasses = new HashSet<>(); /** * Private constructor. @@ -92,7 +93,7 @@ public final class ConditionEvaluationReport { */ public void recordExclusions(Collection exclusions) { Assert.notNull(exclusions, "exclusions must not be null"); - this.exclusions = new ArrayList<>(exclusions); + this.exclusions.addAll(exclusions); } /** @@ -102,7 +103,7 @@ public final class ConditionEvaluationReport { */ public void recordEvaluationCandidates(List evaluationCandidates) { Assert.notNull(evaluationCandidates, "evaluationCandidates must not be null"); - this.unconditionalClasses = new HashSet<>(evaluationCandidates); + this.unconditionalClasses.addAll(evaluationCandidates); } /** @@ -145,7 +146,9 @@ public final class ConditionEvaluationReport { * @return the names of the unconditional classes */ public Set getUnconditionalClasses() { - return Collections.unmodifiableSet(this.unconditionalClasses); + Set filtered = new HashSet<>(this.unconditionalClasses); + filtered.removeIf(this.exclusions::contains); + return Collections.unmodifiableSet(filtered); } /** diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationTests.java index bc9421747a..1934f91be5 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationTests.java @@ -56,7 +56,19 @@ public class ImportAutoConfigurationTests { .containsExactly("ConfigA", "ConfigB", "ConfigD"); } - private List getImportedConfigBeans(Class config) { + @Test + public void excludeAppliedGlobally() { + assertThat(getImportedConfigBeans(ExcludeDConfig.class, ImportADConfig.class)) + .containsExactly("ConfigA"); + } + + @Test + public void excludeWithRedundancy() { + assertThat(getImportedConfigBeans(ExcludeADConfig.class, ExcludeDConfig.class, + ImportADConfig.class)).isEmpty(); + } + + private List getImportedConfigBeans(Class... config) { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( config); String shortName = ClassUtils.getShortName(ImportAutoConfigurationTests.class); @@ -97,6 +109,21 @@ public class ImportAutoConfigurationTests { } + @ImportAutoConfiguration(classes = { ConfigA.class, ConfigD.class }) + static class ImportADConfig { + + } + + @ImportAutoConfiguration(exclude = { ConfigA.class, ConfigD.class }) + static class ExcludeADConfig { + + } + + @ImportAutoConfiguration(exclude = ConfigD.class) + static class ExcludeDConfig { + + } + @Retention(RetentionPolicy.RUNTIME) @ImportAutoConfiguration({ ConfigC.class, ConfigA.class }) @interface MetaImportAutoConfiguration { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReportAutoConfigurationImportListenerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReportAutoConfigurationImportListenerTests.java index 0f7e839685..224295cebd 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReportAutoConfigurationImportListenerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReportAutoConfigurationImportListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -16,6 +16,7 @@ package org.springframework.boot.autoconfigure.condition; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Set; @@ -35,6 +36,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link ConditionEvaluationReportAutoConfigurationImportListener}. * * @author Phillip Webb + * @author Stephane Nicoll */ public class ConditionEvaluationReportAutoConfigurationImportListenerTests { @@ -81,4 +83,32 @@ public class ConditionEvaluationReportAutoConfigurationImportListenerTests { assertThat(report.getExclusions()).containsExactlyElementsOf(exclusions); } + @Test + public void onAutoConfigurationImportEventShouldApplyExclusionsGlobally() { + AutoConfigurationImportEvent event = new AutoConfigurationImportEvent(this, + Arrays.asList("First", "Second"), Collections.emptySet()); + this.listener.onAutoConfigurationImportEvent(event); + AutoConfigurationImportEvent anotherEvent = new AutoConfigurationImportEvent(this, + Collections.emptyList(), Collections.singleton("First")); + this.listener.onAutoConfigurationImportEvent(anotherEvent); + ConditionEvaluationReport report = ConditionEvaluationReport + .get(this.beanFactory); + assertThat(report.getUnconditionalClasses()).containsExactly("Second"); + assertThat(report.getExclusions()).containsExactly("First"); + } + + @Test + public void onAutoConfigurationImportEventShouldApplyExclusionsGloballyWhenExclusionIsAlreadyApplied() { + AutoConfigurationImportEvent excludeEvent = new AutoConfigurationImportEvent(this, + Collections.emptyList(), Collections.singleton("First")); + this.listener.onAutoConfigurationImportEvent(excludeEvent); + AutoConfigurationImportEvent event = new AutoConfigurationImportEvent(this, + Arrays.asList("First", "Second"), Collections.emptySet()); + this.listener.onAutoConfigurationImportEvent(event); + ConditionEvaluationReport report = ConditionEvaluationReport + .get(this.beanFactory); + assertThat(report.getUnconditionalClasses()).containsExactly("Second"); + assertThat(report.getExclusions()).containsExactly("First"); + } + }