Commit 758df17c authored by Phillip Webb's avatar Phillip Webb

Allow placeholders to be used in imports

Allow `${..}` property placeholders to be used in `spring.config.import`
properties. Prior to this commit, placeholders were not resolved when
binding the `ConfigDataProperty` instance. Furthermore, binding happened
too early for all placeholders to be resolved correctly. The
`ConfigDataEnvironmentContributor` class now has two states for imported
contributors, `UNBOUND_IMPORT` has the initial unbound state and is
eventually replaced with a `BOUND_IMPORT`.

Closes gh-23020
parent 00cb5bbd
......@@ -235,7 +235,7 @@ class ConfigDataEnvironment {
MutablePropertySources propertySources = this.environment.getPropertySources();
this.logger.trace("Applying config data environment contributions");
for (ConfigDataEnvironmentContributor contributor : contributors) {
if (contributor.getKind() == ConfigDataEnvironmentContributor.Kind.IMPORTED
if (contributor.getKind() == ConfigDataEnvironmentContributor.Kind.BOUND_IMPORT
&& contributor.getPropertySource() != null) {
if (!contributor.isActive(activationContext)) {
this.logger.trace(LogMessage.format("Skipping inactive property source '%s'",
......
......@@ -59,6 +59,8 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
private final ConfigDataProperties properties;
private final boolean ignoreImports;
private final Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children;
private final Kind kind;
......@@ -71,16 +73,18 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
* @param configurationPropertySource the configuration property source for the data
* or {@code null}
* @param properties the config data properties or {@code null}
* @param ignoreImports if import properties should be ignored
* @param children the children of this contributor at each {@link ImportPhase}
*/
ConfigDataEnvironmentContributor(Kind kind, ConfigDataLocation location, PropertySource<?> propertySource,
ConfigurationPropertySource configurationPropertySource, ConfigDataProperties properties,
Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children) {
boolean ignoreImports, Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children) {
this.kind = kind;
this.location = location;
this.properties = properties;
this.propertySource = propertySource;
this.configurationPropertySource = configurationPropertySource;
this.ignoreImports = ignoreImports;
this.children = (children != null) ? children : Collections.emptyMap();
}
......@@ -175,6 +179,16 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
return new ContributorIterator();
}
ConfigDataEnvironmentContributor withBoundProperties(Binder binder) {
UseLegacyConfigProcessingException.throwIfRequested(binder);
ConfigDataProperties properties = ConfigDataProperties.get(binder);
if (this.ignoreImports) {
properties = properties.withoutImports();
}
return new ConfigDataEnvironmentContributor(Kind.BOUND_IMPORT, this.location, this.propertySource,
this.configurationPropertySource, properties, this.ignoreImports, null);
}
/**
* Create a new {@link ConfigDataEnvironmentContributor} instance with a new set of
* children for the given phase.
......@@ -187,7 +201,7 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
Map<ImportPhase, List<ConfigDataEnvironmentContributor>> updatedChildren = new LinkedHashMap<>(this.children);
updatedChildren.put(importPhase, children);
return new ConfigDataEnvironmentContributor(this.kind, this.location, this.propertySource,
this.configurationPropertySource, this.properties, updatedChildren);
this.configurationPropertySource, this.properties, this.ignoreImports, updatedChildren);
}
/**
......@@ -212,7 +226,7 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
updatedChildren.put(importPhase, Collections.unmodifiableList(updatedContributors));
});
return new ConfigDataEnvironmentContributor(this.kind, this.location, this.propertySource,
this.configurationPropertySource, this.properties, updatedChildren);
this.configurationPropertySource, this.properties, this.ignoreImports, updatedChildren);
}
/**
......@@ -223,20 +237,20 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
static ConfigDataEnvironmentContributor of(List<ConfigDataEnvironmentContributor> contributors) {
Map<ImportPhase, List<ConfigDataEnvironmentContributor>> children = new LinkedHashMap<>();
children.put(ImportPhase.BEFORE_PROFILE_ACTIVATION, Collections.unmodifiableList(contributors));
return new ConfigDataEnvironmentContributor(Kind.ROOT, null, null, null, null, children);
return new ConfigDataEnvironmentContributor(Kind.ROOT, null, null, null, null, false, children);
}
/**
* 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 importLocation the initial import location
* @param importLocation the initial import location (with placeholder resolved)
* @return a new {@link ConfigDataEnvironmentContributor} instance
*/
static ConfigDataEnvironmentContributor ofInitialImport(String importLocation) {
List<String> imports = Collections.singletonList(importLocation);
ConfigDataProperties properties = new ConfigDataProperties(imports, null);
return new ConfigDataEnvironmentContributor(Kind.INITIAL_IMPORT, null, null, null, properties, null);
return new ConfigDataEnvironmentContributor(Kind.INITIAL_IMPORT, null, null, null, properties, false, null);
}
/**
......@@ -248,31 +262,25 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
*/
static ConfigDataEnvironmentContributor ofExisting(PropertySource<?> propertySource) {
return new ConfigDataEnvironmentContributor(Kind.EXISTING, null, propertySource,
ConfigurationPropertySource.from(propertySource), null, null);
ConfigurationPropertySource.from(propertySource), null, false, null);
}
/**
* Factory method to create a {@link Kind#IMPORTED imported} contributor. This
* contributor has been actively imported from another contributor and may itself
* Factory method to create an {@link Kind#UNBOUND_IMPORT unbound import} contributor.
* This contributor has been actively imported from another contributor and may itself
* import further contributors later.
* @param location the location of imported config data
* @param configData the config data
* @param propertySourceIndex the index of the property source that should be used
* @param activationContext the current activation context
* @return a new {@link ConfigDataEnvironmentContributor} instance
*/
static ConfigDataEnvironmentContributor ofImported(ConfigDataLocation location, ConfigData configData,
int propertySourceIndex, ConfigDataActivationContext activationContext) {
static ConfigDataEnvironmentContributor ofUnboundImport(ConfigDataLocation location, ConfigData configData,
int propertySourceIndex) {
PropertySource<?> propertySource = configData.getPropertySources().get(propertySourceIndex);
ConfigurationPropertySource configurationPropertySource = ConfigurationPropertySource.from(propertySource);
Binder binder = new Binder(configurationPropertySource);
UseLegacyConfigProcessingException.throwIfRequested(binder);
ConfigDataProperties properties = ConfigDataProperties.get(binder);
if (configData.getOptions().contains(ConfigData.Option.IGNORE_IMPORTS)) {
properties = properties.withoutImports();
}
return new ConfigDataEnvironmentContributor(Kind.IMPORTED, location, propertySource,
configurationPropertySource, properties, null);
boolean ignoreImports = configData.getOptions().contains(ConfigData.Option.IGNORE_IMPORTS);
return new ConfigDataEnvironmentContributor(Kind.UNBOUND_IMPORT, location, propertySource,
configurationPropertySource, null, ignoreImports, null);
}
/**
......@@ -296,9 +304,16 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
EXISTING,
/**
* A contributor with {@link ConfigData} imported from another contributor.
* A contributor with {@link ConfigData} imported from another contributor but not
* yet bound.
*/
UNBOUND_IMPORT,
/**
* A contributor with {@link ConfigData} imported from another contributor that
* has been.
*/
IMPORTED;
BOUND_IMPORT;
}
......
......@@ -29,6 +29,7 @@ import java.util.stream.Stream;
import org.apache.commons.logging.Log;
import org.springframework.boot.context.config.ConfigDataEnvironmentContributor.ImportPhase;
import org.springframework.boot.context.config.ConfigDataEnvironmentContributor.Kind;
import org.springframework.boot.context.properties.bind.BindContext;
import org.springframework.boot.context.properties.bind.BindHandler;
import org.springframework.boot.context.properties.bind.Bindable;
......@@ -91,27 +92,38 @@ class ConfigDataEnvironmentContributors implements Iterable<ConfigDataEnvironmen
this.logger.trace(LogMessage.format("Processing imports for phase %s. %s", importPhase,
(activationContext != null) ? activationContext : "no activation context"));
ConfigDataEnvironmentContributors result = this;
int processedCount = 0;
int processed = 0;
while (true) {
ConfigDataEnvironmentContributor unprocessed = getFirstUnprocessed(result, activationContext, importPhase);
if (unprocessed == null) {
this.logger.trace(LogMessage.format("Processed imports for of %d contributors", processedCount));
ConfigDataEnvironmentContributor contributor = getNextToProcess(result, activationContext, importPhase);
if (contributor == null) {
this.logger.trace(LogMessage.format("Processed imports for of %d contributors", processed));
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);
result = new ConfigDataEnvironmentContributors(this.logger, this.bootstrapRegistry,
result.getRoot().withReplacement(contributor, bound));
continue;
}
ConfigDataLocationResolverContext locationResolverContext = new ContributorConfigDataLocationResolverContext(
result, unprocessed, activationContext);
result, contributor, activationContext);
ConfigDataLoaderContext loaderContext = new ContributorDataLoaderContext(this);
List<String> imports = unprocessed.getImports();
List<String> imports = contributor.getImports();
this.logger.trace(LogMessage.format("Processing imports %s", imports));
Map<ConfigDataLocation, ConfigData> imported = importer.resolveAndLoad(activationContext,
locationResolverContext, loaderContext, imports);
this.logger.trace(LogMessage.of(() -> imported.isEmpty() ? "Nothing imported" : "Imported "
+ imported.size() + " location " + ((imported.size() != 1) ? "s" : "") + imported.keySet()));
ConfigDataEnvironmentContributor processed = unprocessed.withChildren(importPhase,
asContributors(activationContext, imported));
ConfigDataEnvironmentContributor contributorAndChildren = contributor.withChildren(importPhase,
asContributors(imported));
result = new ConfigDataEnvironmentContributors(this.logger, this.bootstrapRegistry,
result.getRoot().withReplacement(unprocessed, processed));
processedCount++;
result.getRoot().withReplacement(contributor, contributorAndChildren));
processed++;
}
}
......@@ -119,22 +131,27 @@ class ConfigDataEnvironmentContributors implements Iterable<ConfigDataEnvironmen
return this.bootstrapRegistry;
}
private ConfigDataEnvironmentContributor getFirstUnprocessed(ConfigDataEnvironmentContributors contributors,
private ConfigDataEnvironmentContributor getNextToProcess(ConfigDataEnvironmentContributors contributors,
ConfigDataActivationContext activationContext, ImportPhase importPhase) {
for (ConfigDataEnvironmentContributor contributor : contributors.getRoot()) {
if (contributor.isActive(activationContext) && contributor.hasUnprocessedImports(importPhase)) {
if (contributor.getKind() == Kind.UNBOUND_IMPORT
|| isActiveWithUnprocessedImports(activationContext, importPhase, contributor)) {
return contributor;
}
}
return null;
}
private List<ConfigDataEnvironmentContributor> asContributors(ConfigDataActivationContext activationContext,
Map<ConfigDataLocation, ConfigData> imported) {
private boolean isActiveWithUnprocessedImports(ConfigDataActivationContext activationContext,
ImportPhase importPhase, ConfigDataEnvironmentContributor contributor) {
return contributor.isActive(activationContext) && contributor.hasUnprocessedImports(importPhase);
}
private List<ConfigDataEnvironmentContributor> asContributors(Map<ConfigDataLocation, ConfigData> imported) {
List<ConfigDataEnvironmentContributor> contributors = new ArrayList<>(imported.size() * 5);
imported.forEach((location, data) -> {
for (int i = data.getPropertySources().size() - 1; i >= 0; i--) {
contributors.add(ConfigDataEnvironmentContributor.ofImported(location, data, i, activationContext));
contributors.add(ConfigDataEnvironmentContributor.ofUnboundImport(location, data, i));
}
});
return Collections.unmodifiableList(contributors);
......@@ -155,14 +172,18 @@ class ConfigDataEnvironmentContributors implements Iterable<ConfigDataEnvironmen
* @return a binder instance
*/
Binder getBinder(ConfigDataActivationContext activationContext, BinderOption... options) {
return getBinder(activationContext, ObjectUtils.isEmpty(options) ? EnumSet.noneOf(BinderOption.class)
: EnumSet.copyOf(Arrays.asList(options)));
return getBinder(activationContext, asBinderOptionsSet(options));
}
private Set<BinderOption> asBinderOptionsSet(BinderOption... options) {
return ObjectUtils.isEmpty(options) ? EnumSet.noneOf(BinderOption.class)
: EnumSet.copyOf(Arrays.asList(options));
}
private Binder getBinder(ConfigDataActivationContext activationContext, Set<BinderOption> options) {
boolean failOnInactiveSource = options.contains(BinderOption.FAIL_ON_BIND_TO_INACTIVE_SOURCE);
Iterable<ConfigurationPropertySource> sources = () -> getBinderSources(activationContext,
!failOnInactiveSource);
!options.contains(BinderOption.FAIL_ON_BIND_TO_INACTIVE_SOURCE));
PlaceholdersResolver placeholdersResolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(this.root,
activationContext, failOnInactiveSource);
BindHandler bindHandler = !failOnInactiveSource ? null : new InactiveSourceChecker(activationContext);
......
......@@ -122,7 +122,7 @@ class ConfigDataEnvironmentContributorPlaceholdersResolverTests {
private final boolean active;
protected TestConfigDataEnvironmentContributor(PropertySource<?> propertySource, boolean active) {
super(Kind.ROOT, null, propertySource, null, null, null);
super(Kind.ROOT, null, propertySource, null, null, false, null);
this.active = active;
}
......
......@@ -268,10 +268,8 @@ class ConfigDataEnvironmentContributorsTests {
MockPropertySource secondPropertySource = new MockPropertySource();
secondPropertySource.setProperty("test", "two");
ConfigData configData = new ConfigData(Arrays.asList(firstPropertySource, secondPropertySource));
ConfigDataEnvironmentContributor firstContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 0, this.activationContext);
ConfigDataEnvironmentContributor secondContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 1, this.activationContext);
ConfigDataEnvironmentContributor firstContributor = createBoundImportContributor(configData, 0);
ConfigDataEnvironmentContributor secondContributor = createBoundImportContributor(configData, 1);
ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
this.bootstrapRegistry, Arrays.asList(firstContributor, secondContributor));
Binder binder = contributors.getBinder(this.activationContext);
......@@ -286,10 +284,8 @@ class ConfigDataEnvironmentContributorsTests {
MockPropertySource secondPropertySource = new MockPropertySource();
secondPropertySource.setProperty("test", "two");
ConfigData configData = new ConfigData(Arrays.asList(firstPropertySource, secondPropertySource));
ConfigDataEnvironmentContributor firstContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 0, this.activationContext);
ConfigDataEnvironmentContributor secondContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 1, this.activationContext);
ConfigDataEnvironmentContributor firstContributor = createBoundImportContributor(configData, 0);
ConfigDataEnvironmentContributor secondContributor = createBoundImportContributor(configData, 1);
ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
this.bootstrapRegistry, Arrays.asList(firstContributor, secondContributor));
Binder binder = contributors.getBinder(this.activationContext);
......@@ -317,10 +313,8 @@ class ConfigDataEnvironmentContributorsTests {
secondPropertySource.setProperty("other", "two");
secondPropertySource.setProperty("test", "${other}");
ConfigData configData = new ConfigData(Arrays.asList(firstPropertySource, secondPropertySource));
ConfigDataEnvironmentContributor firstContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 0, this.activationContext);
ConfigDataEnvironmentContributor secondContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 1, this.activationContext);
ConfigDataEnvironmentContributor firstContributor = createBoundImportContributor(configData, 0);
ConfigDataEnvironmentContributor secondContributor = createBoundImportContributor(configData, 1);
ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
this.bootstrapRegistry, Arrays.asList(firstContributor, secondContributor));
Binder binder = contributors.getBinder(this.activationContext);
......@@ -335,10 +329,8 @@ class ConfigDataEnvironmentContributorsTests {
MockPropertySource secondPropertySource = new MockPropertySource();
secondPropertySource.setProperty("test", "two");
ConfigData configData = new ConfigData(Arrays.asList(firstPropertySource, secondPropertySource));
ConfigDataEnvironmentContributor firstContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 0, this.activationContext);
ConfigDataEnvironmentContributor secondContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 1, this.activationContext);
ConfigDataEnvironmentContributor firstContributor = createBoundImportContributor(configData, 0);
ConfigDataEnvironmentContributor secondContributor = createBoundImportContributor(configData, 1);
ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
this.bootstrapRegistry, Arrays.asList(firstContributor, secondContributor));
Binder binder = contributors.getBinder(this.activationContext, BinderOption.FAIL_ON_BIND_TO_INACTIVE_SOURCE);
......@@ -354,10 +346,8 @@ class ConfigDataEnvironmentContributorsTests {
secondPropertySource.setProperty("spring.config.activate.on-profile", "production");
secondPropertySource.setProperty("test", "two");
ConfigData configData = new ConfigData(Arrays.asList(firstPropertySource, secondPropertySource));
ConfigDataEnvironmentContributor firstContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 0, this.activationContext);
ConfigDataEnvironmentContributor secondContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 1, this.activationContext);
ConfigDataEnvironmentContributor firstContributor = createBoundImportContributor(configData, 0);
ConfigDataEnvironmentContributor secondContributor = createBoundImportContributor(configData, 1);
ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
this.bootstrapRegistry, Arrays.asList(firstContributor, secondContributor));
Binder binder = contributors.getBinder(this.activationContext, BinderOption.FAIL_ON_BIND_TO_INACTIVE_SOURCE);
......@@ -374,10 +364,8 @@ class ConfigDataEnvironmentContributorsTests {
secondPropertySource.setProperty("test", "${other}");
secondPropertySource.setProperty("other", "one");
ConfigData configData = new ConfigData(Arrays.asList(firstPropertySource, secondPropertySource));
ConfigDataEnvironmentContributor firstContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 0, this.activationContext);
ConfigDataEnvironmentContributor secondContributor = ConfigDataEnvironmentContributor.ofImported(null,
configData, 1, this.activationContext);
ConfigDataEnvironmentContributor firstContributor = createBoundImportContributor(configData, 0);
ConfigDataEnvironmentContributor secondContributor = createBoundImportContributor(configData, 1);
ConfigDataEnvironmentContributors contributors = new ConfigDataEnvironmentContributors(this.logFactory,
this.bootstrapRegistry, Arrays.asList(firstContributor, secondContributor));
Binder binder = contributors.getBinder(this.activationContext, BinderOption.FAIL_ON_BIND_TO_INACTIVE_SOURCE);
......@@ -385,6 +373,14 @@ class ConfigDataEnvironmentContributorsTests {
.satisfies((ex) -> assertThat(ex.getCause()).isInstanceOf(InactiveConfigDataAccessException.class));
}
private ConfigDataEnvironmentContributor createBoundImportContributor(ConfigData configData,
int propertySourceIndex) {
ConfigDataEnvironmentContributor contributor = ConfigDataEnvironmentContributor.ofUnboundImport(null,
configData, propertySourceIndex);
Binder binder = new Binder(contributor.getConfigurationPropertySource());
return contributor.withBoundProperties(binder);
}
private static class TestConfigDataLocation extends ConfigDataLocation {
private final String value;
......
......@@ -35,6 +35,7 @@ import org.junit.jupiter.api.Test;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.WebApplicationType;
import org.springframework.boot.context.properties.bind.BindException;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.env.ConfigurableEnvironment;
......@@ -514,6 +515,27 @@ class ConfigDataEnvironmentPostProcessorIntegrationTests {
() -> this.application.run("--spring.config.location=classpath:invalidproperty.properties"));
}
@Test
void runWhenImportUsesPlaceholder() {
ConfigurableApplicationContext context = this.application
.run("--spring.config.location=classpath:application-import-with-placeholder.properties");
assertThat(context.getEnvironment().getProperty("my.value")).isEqualTo("iwasimported");
}
@Test
void runWhenImportFromEarlierDocumentUsesPlaceholder() {
ConfigurableApplicationContext context = this.application
.run("--spring.config.location=classpath:application-import-with-placeholder-in-document.properties");
assertThat(context.getEnvironment().getProperty("my.value")).isEqualTo("iwasimported");
}
@Test
void runWhenHasPropertyInProfileDocumentThrowsException() {
assertThatExceptionOfType(BindException.class).isThrownBy(() -> this.application.run(
"--spring.config.location=classpath:application-import-with-placeholder-in-profile-document.properties"))
.withCauseInstanceOf(InactiveConfigDataAccessException.class);
}
private Condition<ConfigurableEnvironment> matchingPropertySource(final String sourceName) {
return new Condition<ConfigurableEnvironment>("environment containing property source " + sourceName) {
......
my.import=application-import-with-placeholder-imported
#---
spring.config.import=classpath:${my.import}.properties
my.import=application-import-with-placeholder-imported
#---
spring.config.import=classpath:${my.import}.properties
#---
my.import=badbadbad
spring.config.activate.on-profile=missing
\ No newline at end of file
my.import=application-import-with-placeholder-imported
spring.config.import=classpath:${my.import}.properties
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment