Fix config data placeholder resolution active document logic

Update `ConfigDataEnvironmentContributor.isActive` so that unbound
imports are no longer considered active. Prior to this commit, any
`ConfigDataEnvironmentContributor` that had `null` properties was
considered active. This is incorrect for `Kind.UNBOUND_IMPORT`
contributors since we haven't yet bound the `spring.config.*`
properties.

The `ConfigDataEnvironmentContributorPlaceholdersResolver` has been
updated to handle the refined logic. A placeholder can now be resolved
from the current contributor, or from an unbound contributor by binding
it on the fly.

Fixes gh-29386

Author: philwebb

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -286,7 +286,7 @@ private ConfigDataActivationContext withProfiles(ConfigDataEnvironmentContributo
 	private Collection<? extends String> getIncludedProfiles(ConfigDataEnvironmentContributors contributors,
 			ConfigDataActivationContext activationContext) {
 		PlaceholdersResolver placeholdersResolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
-				contributors, activationContext, true);
+				contributors, activationContext, null, true);
 		Set<String> result = new LinkedHashSet<>();
 		for (ConfigDataEnvironmentContributor contributor : contributors) {
 			ConfigurationPropertySource source = contributor.getConfigurationPropertySource();

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -27,6 +27,7 @@
 import java.util.stream.StreamSupport;
 
 import org.springframework.boot.context.properties.bind.Binder;
+import org.springframework.boot.context.properties.bind.PlaceholdersResolver;
 import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
 import org.springframework.core.env.Environment;
 import org.springframework.core.env.PropertySource;
@@ -120,6 +121,9 @@ ConfigDataLocation getLocation() {
 	 * @return if the contributor is active
 	 */
 	boolean isActive(ConfigDataActivationContext activationContext) {
+		if (this.kind == Kind.UNBOUND_IMPORT) {
+			return false;
+		}
 		return this.properties == null || this.properties.isActive(activationContext);
 	}
 
@@ -223,10 +227,16 @@ public Iterator<ConfigDataEnvironmentContributor> iterator() {
 	/**
 	 * Create an new {@link ConfigDataEnvironmentContributor} with bound
 	 * {@link ConfigDataProperties}.
-	 * @param binder the binder to use
+	 * @param contributors the contributors used for binding
+	 * @param activationContext the activation context
 	 * @return a new contributor instance
 	 */
-	ConfigDataEnvironmentContributor withBoundProperties(Binder binder) {
+	ConfigDataEnvironmentContributor withBoundProperties(Iterable<ConfigDataEnvironmentContributor> contributors,
+			ConfigDataActivationContext activationContext) {
+		Iterable<ConfigurationPropertySource> sources = Collections.singleton(getConfigurationPropertySource());
+		PlaceholdersResolver placeholdersResolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
+				contributors, activationContext, this, true);
+		Binder binder = new Binder(sources, placeholdersResolver, null, null, null);
 		UseLegacyConfigProcessingException.throwIfRequested(binder);
 		ConfigDataProperties properties = ConfigDataProperties.get(binder);
 		if (properties != null && this.configDataOptions.contains(ConfigData.Option.IGNORE_IMPORTS)) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2022 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.context.config;
 
+import org.springframework.boot.context.config.ConfigDataEnvironmentContributor.Kind;
 import org.springframework.boot.context.properties.bind.PlaceholdersResolver;
 import org.springframework.boot.origin.Origin;
 import org.springframework.boot.origin.OriginLookup;
@@ -40,10 +41,14 @@ class ConfigDataEnvironmentContributorPlaceholdersResolver implements Placeholde
 
 	private final PropertyPlaceholderHelper helper;
 
+	private final ConfigDataEnvironmentContributor activeContributor;
+
 	ConfigDataEnvironmentContributorPlaceholdersResolver(Iterable<ConfigDataEnvironmentContributor> contributors,
-			ConfigDataActivationContext activationContext, boolean failOnResolveFromInactiveContributor) {
+			ConfigDataActivationContext activationContext, ConfigDataEnvironmentContributor activeContributor,
+			boolean failOnResolveFromInactiveContributor) {
 		this.contributors = contributors;
 		this.activationContext = activationContext;
+		this.activeContributor = activeContributor;
 		this.failOnResolveFromInactiveContributor = failOnResolveFromInactiveContributor;
 		this.helper = new PropertyPlaceholderHelper(SystemPropertyUtils.PLACEHOLDER_PREFIX,
 				SystemPropertyUtils.PLACEHOLDER_SUFFIX, SystemPropertyUtils.VALUE_SEPARATOR, true);
@@ -62,7 +67,7 @@ private String resolvePlaceholder(String placeholder) {
 		for (ConfigDataEnvironmentContributor contributor : this.contributors) {
 			PropertySource<?> propertySource = contributor.getPropertySource();
 			Object value = (propertySource != null) ? propertySource.getProperty(placeholder) : null;
-			if (value != null && !contributor.isActive(this.activationContext)) {
+			if (value != null && !isActive(contributor)) {
 				if (this.failOnResolveFromInactiveContributor) {
 					ConfigDataResource resource = contributor.getResource();
 					Origin origin = OriginLookup.getOrigin(propertySource, placeholder);
@@ -75,4 +80,15 @@ private String resolvePlaceholder(String placeholder) {
 		return (result != null) ? String.valueOf(result) : null;
 	}
 
+	private boolean isActive(ConfigDataEnvironmentContributor contributor) {
+		if (contributor == this.activeContributor) {
+			return true;
+		}
+		if (contributor.getKind() != Kind.UNBOUND_IMPORT) {
+			return contributor.isActive(this.activationContext);
+		}
+		return contributor.withBoundProperties(this.contributors, this.activationContext)
+				.isActive(this.activationContext);
+	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -103,12 +103,7 @@ ConfigDataEnvironmentContributors withProcessedImports(ConfigDataImporter import
 				return result;
 			}
 			if (contributor.getKind() == Kind.UNBOUND_IMPORT) {
-				Iterable<ConfigurationPropertySource> sources = Collections
-						.singleton(contributor.getConfigurationPropertySource());
-				PlaceholdersResolver placeholdersResolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
-						result, activationContext, true);
-				Binder binder = new Binder(sources, placeholdersResolver, null, null, null);
-				ConfigDataEnvironmentContributor bound = contributor.withBoundProperties(binder);
+				ConfigDataEnvironmentContributor bound = contributor.withBoundProperties(result, activationContext);
 				result = new ConfigDataEnvironmentContributors(this.logger, this.bootstrapContext,
 						result.getRoot().withReplacement(contributor, bound));
 				continue;
@@ -220,7 +215,7 @@ private Binder getBinder(ConfigDataActivationContext activationContext,
 		Iterable<ConfigurationPropertySource> sources = () -> getBinderSources(
 				filter.and((contributor) -> failOnInactiveSource || contributor.isActive(activationContext)));
 		PlaceholdersResolver placeholdersResolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(this.root,
-				activationContext, failOnInactiveSource);
+				activationContext, null, failOnInactiveSource);
 		BindHandler bindHandler = !failOnInactiveSource ? null : new InactiveSourceChecker(activationContext);
 		return new Binder(sources, placeholdersResolver, null, null, bindHandler);
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -44,14 +44,14 @@ class ConfigDataEnvironmentContributorPlaceholdersResolverTests {
 	@Test
 	void resolvePlaceholdersWhenNotStringReturnsResolved() {
 		ConfigDataEnvironmentContributorPlaceholdersResolver resolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
-				Collections.emptyList(), null, false);
+				Collections.emptyList(), null, null, false);
 		assertThat(resolver.resolvePlaceholders(123)).isEqualTo(123);
 	}
 
 	@Test
 	void resolvePlaceholdersWhenNotFoundReturnsOriginal() {
 		ConfigDataEnvironmentContributorPlaceholdersResolver resolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
-				Collections.emptyList(), null, false);
+				Collections.emptyList(), null, null, false);
 		assertThat(resolver.resolvePlaceholders("${test}")).isEqualTo("${test}");
 	}
 
@@ -62,7 +62,7 @@ void resolvePlaceholdersWhenFoundReturnsFirstMatch() {
 		contributors.add(new TestConfigDataEnvironmentContributor(new TestPropertySource("s2", "test", "t2"), true));
 		contributors.add(new TestConfigDataEnvironmentContributor(new TestPropertySource("s3", "test", "t3"), true));
 		ConfigDataEnvironmentContributorPlaceholdersResolver resolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
-				contributors, null, true);
+				contributors, null, null, true);
 		assertThat(resolver.resolvePlaceholders("${test}")).isEqualTo("t2");
 	}
 
@@ -73,7 +73,7 @@ void resolvePlaceholdersWhenFoundInInactiveThrowsException() {
 		contributors.add(new TestConfigDataEnvironmentContributor(new TestPropertySource("s2", "test", "t2"), true));
 		contributors.add(new TestConfigDataEnvironmentContributor(new TestPropertySource("s3", "test", "t3"), false));
 		ConfigDataEnvironmentContributorPlaceholdersResolver resolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
-				contributors, null, true);
+				contributors, null, null, true);
 		assertThatExceptionOfType(InactiveConfigDataAccessException.class)
 				.isThrownBy(() -> resolver.resolvePlaceholders("${test}"))
 				.satisfies(propertyNameAndOriginOf("test", "s3"));
@@ -86,7 +86,7 @@ void resolvePlaceholderWhenFoundInInactiveAndIgnoringReturnsResolved() {
 		contributors.add(new TestConfigDataEnvironmentContributor(new TestPropertySource("s2", "test", "t2"), true));
 		contributors.add(new TestConfigDataEnvironmentContributor(new TestPropertySource("s3", "test", "t3"), false));
 		ConfigDataEnvironmentContributorPlaceholdersResolver resolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(
-				contributors, null, false);
+				contributors, null, null, false);
 		assertThat(resolver.resolvePlaceholders("${test}")).isEqualTo("t2");
 	}
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -30,7 +30,6 @@
 import org.springframework.boot.context.config.ConfigData.PropertySourceOptions;
 import org.springframework.boot.context.config.ConfigDataEnvironmentContributor.ImportPhase;
 import org.springframework.boot.context.config.ConfigDataEnvironmentContributor.Kind;
-import org.springframework.boot.context.properties.bind.Binder;
 import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
 import org.springframework.mock.env.MockPropertySource;
 
@@ -316,7 +315,7 @@ void ofUnboundImportCreatesImportedContributor() {
 		assertThat(contributor.getKind()).isEqualTo(Kind.UNBOUND_IMPORT);
 		assertThat(contributor.getResource()).isSameAs(resource);
 		assertThat(contributor.getImports()).isEmpty();
-		assertThat(contributor.isActive(this.activationContext)).isTrue();
+		assertThat(contributor.isActive(this.activationContext)).isFalse();
 		assertThat(contributor.getPropertySource()).isEqualTo(propertySource);
 		assertThat(contributor.getConfigurationPropertySource()).isNotNull();
 		assertThat(contributor.getChildren(ImportPhase.BEFORE_PROFILE_ACTIVATION)).isEmpty();
@@ -368,8 +367,8 @@ void withBoundPropertiesWhenIgnoringImportsAndNothingBound() {
 		ConfigData configData = new ConfigData(Collections.singleton(new MockPropertySource()), Option.IGNORE_IMPORTS);
 		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofUnboundImport(TEST_LOCATION,
 				resource, false, configData, 0);
-		Binder binder = new Binder(contributor.getConfigurationPropertySource());
-		ConfigDataEnvironmentContributor bound = contributor.withBoundProperties(binder);
+		ConfigDataEnvironmentContributor bound = contributor.withBoundProperties(Collections.singleton(contributor),
+				null);
 		assertThat(bound).isNotNull();
 	}
 
@@ -382,8 +381,7 @@ private ConfigDataEnvironmentContributor createBoundContributor(ConfigDataResour
 			int propertySourceIndex) {
 		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofUnboundImport(TEST_LOCATION,
 				resource, false, configData, propertySourceIndex);
-		Binder binder = new Binder(contributor.getConfigurationPropertySource());
-		return contributor.withBoundProperties(binder);
+		return contributor.withBoundProperties(Collections.singleton(contributor), null);
 	}
 
 	private List<String> asLocationsList(Iterator<ConfigDataEnvironmentContributor> iterator) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -17,6 +17,7 @@
 package org.springframework.boot.context.config;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -389,8 +390,7 @@ private ConfigDataEnvironmentContributor createBoundImportContributor(ConfigData
 			int propertySourceIndex) {
 		ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofUnboundImport(null, null,
 				false, configData, propertySourceIndex);
-		Binder binder = new Binder(contributor.getConfigurationPropertySource());
-		return contributor.withBoundProperties(binder);
+		return contributor.withBoundProperties(Collections.singleton(contributor), null);
 	}
 
 	private static class TestConfigDataResource extends ConfigDataResource {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -636,6 +636,21 @@ void runWhenHasPropertyInProfileDocumentThrowsException() {
 				.withCauseInstanceOf(InactiveConfigDataAccessException.class);
 	}
 
+	@Test // gh-29386
+	void runWhenHasPropertyInEarlierProfileDocumentThrowsException() {
+		assertThatExceptionOfType(BindException.class).isThrownBy(() -> this.application.run(
+				"--spring.config.location=classpath:application-import-with-placeholder-in-earlier-profile-document.properties"))
+				.withCauseInstanceOf(InactiveConfigDataAccessException.class);
+	}
+
+	@Test // gh-29386
+	void runWhenHasPropertyInEarlierDocumentLoads() {
+		ConfigurableApplicationContext context = this.application.run(
+				"--spring.config.location=classpath:application-import-with-placeholder-in-earlier-document.properties");
+		assertThat(context.getEnvironment().getProperty("my.value"))
+				.isEqualTo("application-import-with-placeholder-in-earlier-document-imported");
+	}
+
 	@Test
 	void runWhenHasNonOptionalImportThrowsException() {
 		assertThatExceptionOfType(ConfigDataResourceNotFoundException.class).isThrownBy(

spring-boot-project/spring-boot/src/test/resources/application-import-with-placeholder-in-earlier-document-imported.properties

@@ -0,0 +1 @@
+my.value=application-import-with-placeholder-in-earlier-document-imported

spring-boot-project/spring-boot/src/test/resources/application-import-with-placeholder-in-earlier-document.properties

@@ -0,0 +1,6 @@
+my.import=application-import-with-placeholder-in-earlier-document-imported
+#---
+my.value=should-be-ignored
+spring.config.activate.on-profile=missing
+#---
+spring.config.import=classpath:${my.import}.properties