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
This commit is contained in:
parent
1c6471ef60
commit
3d46b06e8d
|
|
@ -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 @@ class ConfigDataEnvironment {
|
|||
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();
|
||||
|
|
|
|||
|
|
@ -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.Stream;
|
|||
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 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
|
|||
* @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 @@ class ConfigDataEnvironmentContributor implements Iterable<ConfigDataEnvironment
|
|||
/**
|
||||
* 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)) {
|
||||
|
|
|
|||
|
|
@ -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 @@ class ConfigDataEnvironmentContributorPlaceholdersResolver implements Placeholde
|
|||
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 @@ class ConfigDataEnvironmentContributorPlaceholdersResolver implements Placeholde
|
|||
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);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 @@ class ConfigDataEnvironmentContributors implements Iterable<ConfigDataEnvironmen
|
|||
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 @@ class ConfigDataEnvironmentContributors implements Iterable<ConfigDataEnvironmen
|
|||
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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 @@ class ConfigDataEnvironmentContributorPlaceholdersResolverTests {
|
|||
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 @@ class ConfigDataEnvironmentContributorPlaceholdersResolverTests {
|
|||
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 @@ class ConfigDataEnvironmentContributorPlaceholdersResolverTests {
|
|||
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");
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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.Option;
|
|||
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 @@ class ConfigDataEnvironmentContributorTests {
|
|||
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 @@ class ConfigDataEnvironmentContributorTests {
|
|||
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 @@ class ConfigDataEnvironmentContributorTests {
|
|||
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) {
|
||||
|
|
|
|||
|
|
@ -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 @@ class ConfigDataEnvironmentContributorsTests {
|
|||
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 {
|
||||
|
|
|
|||
|
|
@ -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 @@ class ConfigDataEnvironmentPostProcessorIntegrationTests {
|
|||
.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(
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
my.value=application-import-with-placeholder-in-earlier-document-imported
|
||||
|
|
@ -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
|
||||
|
|
@ -0,0 +1 @@
|
|||
my.value=application-import-with-placeholder-in-earlier-profile-document-imported
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
my.value=application-import-with-placeholder-in-earlier-profile-document
|
||||
#---
|
||||
my.import=application-import-with-placeholder-in-earlier-profile-document-imported
|
||||
spring.config.activate.on-profile=missing
|
||||
#---
|
||||
spring.config.import=classpath:${my.import}.properties
|
||||
Loading…
Reference in New Issue