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

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<String> 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<String, AnnotationMetadata> entries = new LinkedHashMap<>();
+
+		private final List<AutoConfigurationEntry> autoConfigurationEntries = new ArrayList<>();
+
 		private ClassLoader beanClassLoader;
 
 		private BeanFactory beanFactory;
 
 		private ResourceLoader resourceLoader;
 
-		private final Map<String, AnnotationMetadata> 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<Entry> selectImports() {
-			return sortAutoConfigurations().stream()
-					.map((importClassName) -> new Entry(this.entries.get(importClassName),
-							importClassName))
-					.collect(Collectors.toList());
+			if (this.autoConfigurationEntries.isEmpty()) {
+				return Collections.emptyList();
+			}
+			Set<String> allExclusions = this.autoConfigurationEntries.stream()
+					.map(AutoConfigurationEntry::getExclusions)
+					.flatMap(Collection::stream).collect(Collectors.toSet());
+			Set<String> processedConfigurations = new LinkedHashSet<>();
+			Set<String> processedExclusions = new LinkedHashSet<>();
+			this.autoConfigurationEntries.forEach((entry) -> {
+				List<String> configurations = new ArrayList<>(entry.getConfigurations());
+				configurations.removeAll(allExclusions);
+				configurations.removeIf(processedConfigurations::contains);
+				Set<String> 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<String> sortAutoConfigurations() {
-			List<String> 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<String> sortAutoConfigurations(Set<String> 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<String> configurations;
+
+		private final Set<String> 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<String> configurations,
+				Collection<String> exclusions) {
+			this.configurations = new ArrayList<>(configurations);
+			this.exclusions = new HashSet<>(exclusions);
+		}
+
+		public List<String> getConfigurations() {
+			return this.configurations;
+		}
+
+		public Set<String> getExclusions() {
+			return this.exclusions;
+		}
+
+	}
+
 }

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<String> exclusions = Collections.emptyList();
+	private final List<String> exclusions = new ArrayList<>();
 
-	private Set<String> unconditionalClasses = new HashSet<>();
+	private final Set<String> unconditionalClasses = new HashSet<>();
 
 	/**
 	 * Private constructor.
@@ -92,7 +93,7 @@ public final class ConditionEvaluationReport {
 	 */
 	public void recordExclusions(Collection<String> 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<String> 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<String> getUnconditionalClasses() {
-		return Collections.unmodifiableSet(this.unconditionalClasses);
+		Set<String> filtered = new HashSet<>(this.unconditionalClasses);
+		filtered.removeIf(this.exclusions::contains);
+		return Collections.unmodifiableSet(filtered);
 	}
 
 	/**

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<String> 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<String> 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 {

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionEvaluationReportAutoConfigurationImportListenerTests.java

 /*
- * 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");
+	}
+
 }