From efad44be43d23b0d6e4a423968d09f444ba89a98 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 6 May 2021 18:02:17 -0700 Subject: [PATCH] Refine mandatory location checking logic Update `ConfigDataEnvironment.checkMandatoryLocations` to use the actual locations that were imported, including those that were skipped because the related `ConfigDataResource` had already been imported by a different location. Prior to this commit, any location that was skipped because it had already been imported would throw a `ConfigDataNotFoundException`. Closes gh-26147 Co-authored-by: Scott Frederick Co-authored-by: Madhura Bhave --- .../boot/context/config/ConfigDataEnvironment.java | 9 +++++---- .../boot/context/config/ConfigDataImporter.java | 13 ++++++++++++- ...ataEnvironmentPostProcessorIntegrationTests.java | 8 ++++++++ ...variant-and-direct-profile-import-dev.properties | 2 ++ ...ile-variant-and-direct-profile-import.properties | 2 ++ 5 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import-dev.properties create mode 100644 spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import.properties diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java index 8155e9ab23..3340d9b759 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java @@ -233,7 +233,7 @@ class ConfigDataEnvironment { contributors = processWithoutProfiles(contributors, importer, activationContext); activationContext = withProfiles(contributors, activationContext); contributors = processWithProfiles(contributors, importer, activationContext); - applyToEnvironment(contributors, activationContext); + applyToEnvironment(contributors, activationContext, importer.getLoadedLocations()); } private ConfigDataEnvironmentContributors processInitial(ConfigDataEnvironmentContributors contributors, @@ -320,9 +320,9 @@ class ConfigDataEnvironment { } private void applyToEnvironment(ConfigDataEnvironmentContributors contributors, - ConfigDataActivationContext activationContext) { + ConfigDataActivationContext activationContext, Set loadedLocations) { checkForInvalidProperties(contributors); - checkMandatoryLocations(contributors, activationContext); + checkMandatoryLocations(contributors, activationContext, loadedLocations); MutablePropertySources propertySources = this.environment.getPropertySources(); this.logger.trace("Applying config data environment contributions"); for (ConfigDataEnvironmentContributor contributor : contributors) { @@ -357,7 +357,7 @@ class ConfigDataEnvironment { } private void checkMandatoryLocations(ConfigDataEnvironmentContributors contributors, - ConfigDataActivationContext activationContext) { + ConfigDataActivationContext activationContext, Set loadedLocations) { Set mandatoryLocations = new LinkedHashSet<>(); for (ConfigDataEnvironmentContributor contributor : contributors) { if (contributor.isActive(activationContext)) { @@ -369,6 +369,7 @@ class ConfigDataEnvironment { mandatoryLocations.remove(contributor.getLocation()); } } + mandatoryLocations.removeAll(loadedLocations); if (!mandatoryLocations.isEmpty()) { for (ConfigDataLocation mandatoryLocation : mandatoryLocations) { this.notFoundAction.handle(this.logger, new ConfigDataLocationNotFoundException(mandatoryLocation)); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataImporter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataImporter.java index 133be9f8c2..4369880f29 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataImporter.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataImporter.java @@ -49,6 +49,8 @@ class ConfigDataImporter { private final Set loaded = new HashSet<>(); + private final Set loadedLocations = new HashSet<>(); + /** * Create a new {@link ConfigDataImporter} instance. * @param logFactory the log factory @@ -113,10 +115,15 @@ class ConfigDataImporter { ConfigDataResolutionResult candidate = candidates.get(i); ConfigDataLocation location = candidate.getLocation(); ConfigDataResource resource = candidate.getResource(); - if (this.loaded.add(resource)) { + if (this.loaded.contains(resource)) { + this.loadedLocations.add(location); + } + else { try { ConfigData loaded = this.loaders.load(loaderContext, resource); if (loaded != null) { + this.loaded.add(resource); + this.loadedLocations.add(location); result.put(candidate, loaded); } } @@ -139,4 +146,8 @@ class ConfigDataImporter { return (!location.isOptional()) ? this.notFoundAction : ConfigDataNotFoundAction.IGNORE; } + Set getLoadedLocations() { + return this.loadedLocations; + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java index 1d8b523220..a018b2e946 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorIntegrationTests.java @@ -604,6 +604,14 @@ class ConfigDataEnvironmentPostProcessorIntegrationTests { assertThat(context.getEnvironment().getProperty("my.value")).isEqualTo("iwasimported-dev"); } + @Test + void runWhenImportWithProfileVariantAndDirectProfileImportOrdersPropertySourcesCorrectly() { + this.application.setAdditionalProfiles("dev"); + ConfigurableApplicationContext context = this.application.run( + "--spring.config.location=classpath:application-import-with-profile-variant-and-direct-profile-import.properties"); + assertThat(context.getEnvironment().getProperty("my.value")).isEqualTo("iwasimported-dev"); + } + @Test void runWhenHasPropertyInProfileDocumentThrowsException() { assertThatExceptionOfType(BindException.class).isThrownBy(() -> this.application.run( diff --git a/spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import-dev.properties b/spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import-dev.properties new file mode 100644 index 0000000000..7cc417b789 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import-dev.properties @@ -0,0 +1,2 @@ +spring.config.import=classpath:application-import-with-profile-variant-imported-dev.properties +my.value=notimported-dev diff --git a/spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import.properties b/spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import.properties new file mode 100644 index 0000000000..0dee106b62 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/resources/application-import-with-profile-variant-and-direct-profile-import.properties @@ -0,0 +1,2 @@ +spring.config.import=classpath:application-import-with-profile-variant-imported.properties +my.value=notimported