Polish 'Fix inconsistent ordering of config imports'

See gh-49324

Author: philwebb

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java

@@ -17,7 +17,6 @@
 package org.springframework.boot.context.config;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -199,8 +198,7 @@ ConfigDataEnvironmentContributors getContributors() {
 
 	private List<ConfigDataEnvironmentContributor> getInitialImportContributors(Binder binder) {
 		List<ConfigDataEnvironmentContributor> initialContributors = new ArrayList<>();
-		addInitialImportPropertyContributors(initialContributors,
-				bindLocations(binder, IMPORT_PROPERTY, EMPTY_LOCATIONS));
+		addInitialImportContributors(initialContributors, bindLocations(binder, IMPORT_PROPERTY, EMPTY_LOCATIONS));
 		addInitialImportContributors(initialContributors,
 				bindLocations(binder, ADDITIONAL_LOCATION_PROPERTY, EMPTY_LOCATIONS));
 		addInitialImportContributors(initialContributors,
@@ -212,27 +210,21 @@ private ConfigDataLocation[] bindLocations(Binder binder, String propertyName, C
 		return binder.bind(propertyName, CONFIG_DATA_LOCATION_ARRAY).orElse(other);
 	}
 
-	private void addInitialImportPropertyContributors(List<ConfigDataEnvironmentContributor> initialContributors,
+	private void addInitialImportContributors(List<ConfigDataEnvironmentContributor> initialContributors,
 			ConfigDataLocation[] locations) {
-		List<ConfigDataEnvironmentContributor> initialPropertiesContributors = new ArrayList<>();
-		addInitialImportContributors(initialPropertiesContributors, locations);
-		initialContributors.add(ConfigDataEnvironmentContributor.ofInitialImportProperty(
-			initialPropertiesContributors, this.environment.getConversionService(), Arrays.asList(locations))
-		);
+		addInitialImportContributors(initialContributors, List.of(locations));
 	}
 
 	private void addInitialImportContributors(List<ConfigDataEnvironmentContributor> initialContributors,
-			ConfigDataLocation[] locations) {
-		for (int i = locations.length - 1; i >= 0; i--) {
-			initialContributors.add(createInitialImportContributor(locations[i]));
+			List<ConfigDataLocation> locations) {
+		if (!locations.isEmpty()) {
+			this.logger.trace(LogMessage.format("Adding initial config data import from locations %s", locations));
+			ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImports(locations,
+					this.environment.getConversionService());
+			initialContributors.add(contributor);
 		}
 	}
 
-	private ConfigDataEnvironmentContributor createInitialImportContributor(ConfigDataLocation location) {
-		this.logger.trace(LogMessage.format("Adding initial config data import from location '%s'", location));
-		return ConfigDataEnvironmentContributor.ofInitialImport(location, this.environment.getConversionService());
-	}
-
 	/**
 	 * Process all contributions and apply any newly imported property sources to the
 	 * {@link Environment}.

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentContributor.java

@@ -393,39 +393,17 @@ static ConfigDataEnvironmentContributor of(List<ConfigDataEnvironmentContributor
 	 * Factory method to create a {@link Kind#INITIAL_IMPORT initial import} contributor.
 	 * This contributor is used to trigger initial imports of additional contributors. It
 	 * does not contribute any properties itself.
-	 * @param initialImport the initial import location (with placeholders resolved)
+	 * @param initialImports the initial import locations (with placeholders resolved)
 	 * @param conversionService the conversion service to use
 	 * @return a new {@link ConfigDataEnvironmentContributor} instance
 	 */
-	static ConfigDataEnvironmentContributor ofInitialImport(ConfigDataLocation initialImport,
+	static ConfigDataEnvironmentContributor ofInitialImports(List<ConfigDataLocation> initialImports,
 			ConversionService conversionService) {
-		List<ConfigDataLocation> imports = Collections.singletonList(initialImport);
-		ConfigDataProperties properties = new ConfigDataProperties(imports, null);
+		ConfigDataProperties properties = new ConfigDataProperties(initialImports, null);
 		return new ConfigDataEnvironmentContributor(Kind.INITIAL_IMPORT, null, null, false, null, null, properties,
 				null, null, conversionService);
 	}
 
-	/**
-	 * Factory method to create a {@link Kind#INITIAL_IMPORT_PROPERTY initial import property}
-	 * contributor. This contributor is a container that wraps initial imports from
-	 * {@code spring.config.import} property, allowing profile-specific imports to take
-	 * precedence over the imported locations.
-	 * @param contributors the contributors created from the import locations
-	 * @param conversionService the conversion service to use
-	 * @param locations the original import locations from the property
-	 * @return a new {@link ConfigDataEnvironmentContributor} instance
-	 */
-	static ConfigDataEnvironmentContributor ofInitialImportProperty(
-			List<ConfigDataEnvironmentContributor> contributors,
-			ConversionService conversionService,
-			List<ConfigDataLocation> locations) {
-		Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children = new LinkedHashMap<>();
-		ConfigDataProperties properties = new ConfigDataProperties(locations, null);
-		children.put(ImportPhase.BEFORE_PROFILE_ACTIVATION, Collections.unmodifiableList(contributors));
-		return new ConfigDataEnvironmentContributor(Kind.INITIAL_IMPORT_PROPERTY, null, null, false, null, null, properties, null, children,
-				conversionService);
-	}
-
 	/**
 	 * Factory method to create a contributor that wraps an {@link Kind#EXISTING existing}
 	 * property source. The contributor provides access to existing properties, but
@@ -499,11 +477,6 @@ enum Kind {
 		 */
 		INITIAL_IMPORT,
 
-		/**
-		 * A container contributor that wraps initial imports from spring.config.import property.
-		 */
-		INITIAL_IMPORT_PROPERTY,
-
 		/**
 		 * An existing property source that contributes properties but no imports.
 		 */

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentContributorTests.java

@@ -50,21 +50,23 @@ class ConfigDataEnvironmentContributorTests {
 
 	private static final ConfigDataLocation TEST_LOCATION = ConfigDataLocation.of("test");
 
+	private static final List<ConfigDataLocation> TEST_LOCATIONS = List.of(TEST_LOCATION);
+
 	private final ConfigDataActivationContext activationContext = new ConfigDataActivationContext(
 			CloudPlatform.KUBERNETES, null);
 
 	private final ConversionService conversionService = DefaultConversionService.getSharedInstance();
 
 	@Test
 	void getKindReturnsKind() {
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(TEST_LOCATION,
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImports(TEST_LOCATIONS,
 				this.conversionService);
 		assertThat(contributor.getKind()).isEqualTo(Kind.INITIAL_IMPORT);
 	}
 
 	@Test
 	void isActiveWhenPropertiesIsNullReturnsTrue() {
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(TEST_LOCATION,
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImports(TEST_LOCATIONS,
 				this.conversionService);
 		assertThat(contributor.isActive(null)).isTrue();
 	}
@@ -287,33 +289,18 @@ void ofCreatesRootContributor() {
 	}
 
 	@Test
-	void ofInitialImportCreatedInitialImportContributor() {
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(TEST_LOCATION,
+	void ofInitialImportsCreatedInitialImportContributor() {
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImports(TEST_LOCATIONS,
 				this.conversionService);
 		assertThat(contributor.getKind()).isEqualTo(Kind.INITIAL_IMPORT);
 		assertThat(contributor.getResource()).isNull();
-		assertThat(contributor.getImports()).containsExactly(TEST_LOCATION);
+		assertThat(contributor.getImports()).isEqualTo(TEST_LOCATIONS);
 		assertThat(contributor.isActive(this.activationContext)).isTrue();
 		assertThat(contributor.getPropertySource()).isNull();
 		assertThat(contributor.getConfigurationPropertySource()).isNull();
 		assertThat(contributor.getChildren(ImportPhase.BEFORE_PROFILE_ACTIVATION)).isEmpty();
 	}
 
-	@Test
-	void ofInitialImportPropertyCreatedInitialImportPropertyContributor() {
-		ConfigDataEnvironmentContributor innerContributor = ConfigDataEnvironmentContributor.ofInitialImport(TEST_LOCATION,
-				this.conversionService);
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImportProperty(
-				Collections.singletonList(innerContributor), this.conversionService, Arrays.asList(TEST_LOCATION));
-		assertThat(contributor.getKind()).isEqualTo(Kind.INITIAL_IMPORT_PROPERTY);
-		assertThat(contributor.getResource()).isNull();
-		assertThat(contributor.getImports()).containsExactly(TEST_LOCATION);
-		assertThat(contributor.isActive(this.activationContext)).isTrue();
-		assertThat(contributor.getPropertySource()).isNull();
-		assertThat(contributor.getConfigurationPropertySource()).isNull();
-		assertThat(contributor.getChildren(ImportPhase.BEFORE_PROFILE_ACTIVATION)).containsExactly(innerContributor);
-	}
-
 	@Test
 	void ofExistingCreatesExistingContributor() {
 		MockPropertySource propertySource = new MockPropertySource();

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentContributorsTests.java

@@ -91,8 +91,8 @@ binder, new DefaultResourceLoader(getClass().getClassLoader()),
 
 	@Test
 	void createCreatesWithInitialContributors() {
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(LOCATION_1,
-				this.conversionService);
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor
+			.ofInitialImports(List.of(LOCATION_1), this.conversionService);
 		ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
 				this.bootstrapContext, List.of(contributor), this.conversionService,
 				ConfigDataEnvironmentUpdateListener.NONE);
@@ -123,8 +123,8 @@ void withProcessedImportsResolvesAndLoads() {
 				new ConfigData(List.of(propertySource)));
 		given(this.importer.resolveAndLoad(eq(this.activationContext), any(), any(), eq(locations)))
 			.willReturn(imported);
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(LOCATION_1,
-				this.conversionService);
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor
+			.ofInitialImports(List.of(LOCATION_1), this.conversionService);
 		ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
 				this.bootstrapContext, List.of(contributor), this.conversionService,
 				ConfigDataEnvironmentUpdateListener.NONE);
@@ -155,8 +155,8 @@ void withProcessedImportsResolvesAndLoadsChainedImports() {
 				new ConfigData(List.of(secondPropertySource)));
 		given(this.importer.resolveAndLoad(eq(this.activationContext), any(), any(), eq(secondLocations)))
 			.willReturn(secondImported);
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(LOCATION_1,
-				this.conversionService);
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor
+			.ofInitialImports(List.of(LOCATION_1), this.conversionService);
 		ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
 				this.bootstrapContext, List.of(contributor), this.conversionService,
 				ConfigDataEnvironmentUpdateListener.NONE);
@@ -184,8 +184,8 @@ void withProcessedImportsProvidesLocationResolverContextWithAccessToBinder() {
 				new ConfigData(List.of(propertySource)));
 		given(this.importer.resolveAndLoad(eq(this.activationContext), any(), any(), eq(locations)))
 			.willReturn(imported);
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(LOCATION_1,
-				this.conversionService);
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor
+			.ofInitialImports(List.of(LOCATION_1), this.conversionService);
 		ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
 				this.bootstrapContext, Arrays.asList(existingContributor, contributor), this.conversionService,
 				ConfigDataEnvironmentUpdateListener.NONE);
@@ -215,8 +215,8 @@ void withProcessedImportsProvidesLocationResolverContextWithAccessToParent() {
 				new ConfigData(List.of(secondPropertySource)));
 		given(this.importer.resolveAndLoad(eq(this.activationContext), any(), any(), eq(secondLocations)))
 			.willReturn(secondImported);
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(LOCATION_1,
-				this.conversionService);
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor
+			.ofInitialImports(List.of(LOCATION_1), this.conversionService);
 		ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
 				this.bootstrapContext, List.of(contributor), this.conversionService,
 				ConfigDataEnvironmentUpdateListener.NONE);
@@ -243,8 +243,8 @@ void withProcessedImportsProvidesLocationResolverContextWithAccessToBootstrapReg
 				new ConfigData(List.of(propertySource)));
 		given(this.importer.resolveAndLoad(eq(this.activationContext), any(), any(), eq(locations)))
 			.willReturn(imported);
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(LOCATION_1,
-				this.conversionService);
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor
+			.ofInitialImports(List.of(LOCATION_1), this.conversionService);
 		ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
 				this.bootstrapContext, Arrays.asList(existingContributor, contributor), this.conversionService,
 				ConfigDataEnvironmentUpdateListener.NONE);
@@ -269,8 +269,8 @@ void withProcessedImportsProvidesLoaderContextWithAccessToBootstrapRegistry() {
 				new ConfigData(List.of(propertySource)));
 		given(this.importer.resolveAndLoad(eq(this.activationContext), any(), any(), eq(locations)))
 			.willReturn(imported);
-		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofInitialImport(LOCATION_1,
-				this.conversionService);
+		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor
+			.ofInitialImports(List.of(LOCATION_1), this.conversionService);
 		ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
 				this.bootstrapContext, Arrays.asList(existingContributor, contributor), this.conversionService,
 				ConfigDataEnvironmentUpdateListener.NONE);

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentTests.java

@@ -144,7 +144,7 @@ void createCreatesInitialImportContributorsInCorrectOrder() {
 			.map(ConfigDataEnvironmentContributor::getImports)
 			.map(Object::toString)
 			.toArray();
-		assertThat(imports).containsExactly("[i2]", "[i1]", "[a2]", "[a1]", "[l2]", "[l1]");
+		assertThat(imports).containsExactly("[i1, i2]", "[a1, a2]", "[l1, l2]");
 	}
 
 	@Test