From 6f266145c7a9dce46872947b27a068ef66fbf49c Mon Sep 17 00:00:00 2001 From: nguyensach Date: Tue, 30 Mar 2021 12:31:09 +0900 Subject: [PATCH] Process additional profiles before config files processing Additional profiles were being processed after config file processing when legacy processing was used. This commit also restores the order in which additional profiles are added when legacy processing is used. Active profiles take precedence over additional profiles. See gh-25817 --- .../boot/SpringApplication.java | 12 +----- .../ConfigDataEnvironmentPostProcessor.java | 15 +++++++ .../boot/SpringApplicationTests.java | 12 ++++++ ...nfigDataEnvironmentPostProcessorTests.java | 19 +++++++++ ...leApplicationListenerLegacyReproTests.java | 12 ++++++ .../ConfigFileApplicationListenerTests.java | 41 +++++++++++++++++++ 6 files changed, 100 insertions(+), 11 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index f4ca8084f02..080d056ba78 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -154,6 +154,7 @@ import org.springframework.web.context.support.StandardServletEnvironment; * @author Madhura Bhave * @author Brian Clozel * @author Ethan Rubinson + * @author Nguyen Bao Sach * @since 1.0.0 * @see #run(Class, String[]) * @see #run(Class[], String[]) @@ -373,7 +374,6 @@ public class SpringApplication { ConfigurationPropertySources.attach(environment); listeners.environmentPrepared(bootstrapContext, environment); DefaultPropertiesPropertySource.moveToEnd(environment); - configureAdditionalProfiles(environment); Assert.state(!environment.containsProperty("spring.main.environment-prefix"), "Environment prefix cannot be set via properties."); bindToSpringApplication(environment); @@ -558,16 +558,6 @@ public class SpringApplication { protected void configureProfiles(ConfigurableEnvironment environment, String[] args) { } - private void configureAdditionalProfiles(ConfigurableEnvironment environment) { - if (!CollectionUtils.isEmpty(this.additionalProfiles)) { - Set profiles = new LinkedHashSet<>(Arrays.asList(environment.getActiveProfiles())); - if (!profiles.containsAll(this.additionalProfiles)) { - profiles.addAll(this.additionalProfiles); - environment.setActiveProfiles(StringUtils.toStringArray(profiles)); - } - } - } - private void configureIgnoreBeanInfo(ConfigurableEnvironment environment) { if (System.getProperty(CachedIntrospectionResults.IGNORE_BEANINFO_PROPERTY_NAME) == null) { Boolean ignore = environment.getProperty("spring.beaninfo.ignore", Boolean.class, Boolean.TRUE); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessor.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessor.java index 2c11110fc10..7df92a25482 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessor.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessor.java @@ -19,6 +19,8 @@ package org.springframework.boot.context.config; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Set; import java.util.function.Supplier; import org.apache.commons.logging.Log; @@ -34,6 +36,8 @@ import org.springframework.core.env.Environment; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.ResourceLoader; import org.springframework.core.log.LogMessage; +import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; /** * {@link EnvironmentPostProcessor} that loads and applies {@link ConfigData} to Spring's @@ -41,6 +45,7 @@ import org.springframework.core.log.LogMessage; * * @author Phillip Webb * @author Madhura Bhave + * @author Nguyen Bao Sach * @since 2.4.0 */ public class ConfigDataEnvironmentPostProcessor implements EnvironmentPostProcessor, Ordered { @@ -99,6 +104,7 @@ public class ConfigDataEnvironmentPostProcessor implements EnvironmentPostProces catch (UseLegacyConfigProcessingException ex) { this.logger.debug(LogMessage.format("Switching to legacy config file processing [%s]", ex.getConfigurationProperty())); + configureAdditionalProfiles(environment, additionalProfiles); postProcessUsingLegacyApplicationListener(environment, resourceLoader); } } @@ -109,6 +115,15 @@ public class ConfigDataEnvironmentPostProcessor implements EnvironmentPostProces additionalProfiles, this.environmentUpdateListener); } + private void configureAdditionalProfiles(ConfigurableEnvironment environment, + Collection additionalProfiles) { + if (!CollectionUtils.isEmpty(additionalProfiles)) { + Set profiles = new LinkedHashSet<>(additionalProfiles); + profiles.addAll(Arrays.asList(environment.getActiveProfiles())); + environment.setActiveProfiles(StringUtils.toStringArray(profiles)); + } + } + private void postProcessUsingLegacyApplicationListener(ConfigurableEnvironment environment, ResourceLoader resourceLoader) { getLegacyListener().addPropertySources(environment, resourceLoader); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index 8b8d437746d..109f9fb8aad 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -147,6 +147,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; * @author Brian Clozel * @author Artsiom Yudovin * @author Marten Deinum + * @author Nguyen Bao Sach */ @ExtendWith(OutputCaptureExtension.class) class SpringApplicationTests { @@ -604,6 +605,17 @@ class SpringApplicationTests { assertThat(environment.getActiveProfiles()).containsExactly("bar", "spam", "foo"); } + @Test + void includeProfilesOrder() { + SpringApplication application = new SpringApplication(ExampleConfig.class); + application.setWebApplicationType(WebApplicationType.NONE); + ConfigurableEnvironment environment = new StandardEnvironment(); + application.setEnvironment(environment); + this.context = application.run("--spring.profiles.active=bar,spam", "--spring.profiles.include=foo"); + // Since Boot 2.4 included profiles should always be last + assertThat(environment.getActiveProfiles()).containsExactly("bar", "spam", "foo"); + } + @Test void addProfilesOrderWithProperties() { SpringApplication application = new SpringApplication(ExampleConfig.class); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorTests.java index a3320116eb0..6bec0e36248 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataEnvironmentPostProcessorTests.java @@ -48,6 +48,7 @@ import static org.mockito.Mockito.verifyNoInteractions; * * @author Phillip Webb * @author Madhura Bhave + * @author Nguyen Bao Sach */ @ExtendWith(MockitoExtension.class) class ConfigDataEnvironmentPostProcessorTests { @@ -82,6 +83,7 @@ class ConfigDataEnvironmentPostProcessorTests { verify(this.postProcessor).getConfigDataEnvironment(any(), this.resourceLoaderCaptor.capture(), any()); verify(this.configDataEnvironment).processAndApply(); assertThat(this.resourceLoaderCaptor.getValue()).isInstanceOf(DefaultResourceLoader.class); + assertThat(this.environment.getActiveProfiles()).isEmpty(); } @Test @@ -93,6 +95,7 @@ class ConfigDataEnvironmentPostProcessorTests { verify(this.postProcessor).getConfigDataEnvironment(any(), this.resourceLoaderCaptor.capture(), any()); verify(this.configDataEnvironment).processAndApply(); assertThat(this.resourceLoaderCaptor.getValue()).isSameAs(resourceLoader); + assertThat(this.environment.getActiveProfiles()).isEmpty(); } @Test @@ -103,6 +106,7 @@ class ConfigDataEnvironmentPostProcessorTests { verify(this.postProcessor).getConfigDataEnvironment(any(), any(), this.additionalProfilesCaptor.capture()); verify(this.configDataEnvironment).processAndApply(); assertThat(this.additionalProfilesCaptor.getValue()).containsExactly("dev"); + assertThat(this.environment.getActiveProfiles()).isEmpty(); } @Test @@ -115,6 +119,21 @@ class ConfigDataEnvironmentPostProcessorTests { this.postProcessor.postProcessEnvironment(this.environment, this.application); verifyNoInteractions(this.configDataEnvironment); verify(legacyListener).addPropertySources(eq(this.environment), any(DefaultResourceLoader.class)); + assertThat(this.environment.getActiveProfiles()).isEmpty(); + } + + @Test + void postProcessEnvironmentWhenHasAdditionalProfilesViaProgrammaticallySettingAndUseLegacyProcessing() { + this.application.setAdditionalProfiles("dev"); + ConfigDataEnvironmentPostProcessor.LegacyConfigFileApplicationListener legacyListener = mock( + ConfigDataEnvironmentPostProcessor.LegacyConfigFileApplicationListener.class); + willThrow(new UseLegacyConfigProcessingException(null)).given(this.postProcessor) + .getConfigDataEnvironment(any(), any(), any()); + willReturn(legacyListener).given(this.postProcessor).getLegacyListener(); + this.postProcessor.postProcessEnvironment(this.environment, this.application); + verifyNoInteractions(this.configDataEnvironment); + verify(legacyListener).addPropertySources(eq(this.environment), any(DefaultResourceLoader.class)); + assertThat(this.environment.getActiveProfiles()).containsExactly("dev"); } @Test diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerLegacyReproTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerLegacyReproTests.java index 9c27a0f61f9..a11173f2fe0 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerLegacyReproTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerLegacyReproTests.java @@ -33,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Phillip Webb * @author Dave Syer + * @author Nguyen Bao Sach */ @ExtendWith(UseLegacyProcessing.class) class ConfigFileApplicationListenerLegacyReproTests { @@ -167,6 +168,17 @@ class ConfigFileApplicationListenerLegacyReproTests { assertVersionProperty(this.context, "A", "C", "A"); } + @Test + void additionalProfilesViaProgrammaticallySetting() { + // gh-25704 + SpringApplication application = new SpringApplication(Config.class); + application.setWebApplicationType(WebApplicationType.NONE); + application.setAdditionalProfiles("dev"); + this.context = application.run(); + assertThat(this.context.getEnvironment().acceptsProfiles(Profiles.of("dev"))).isTrue(); + assertThat(this.context.getEnvironment().getProperty("my.property")).isEqualTo("fromdevpropertiesfile"); + } + private void assertVersionProperty(ConfigurableApplicationContext context, String expectedVersion, String... expectedActiveProfiles) { assertThat(context.getEnvironment().getActiveProfiles()).isEqualTo(expectedActiveProfiles); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java index f0ffcc0dc6b..7c420c126ae 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java @@ -70,6 +70,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; * @author Eddú Meléndez * @author Madhura Bhave * @author Scott Frederick + * @author Nguyen Bao Sach */ @Deprecated @ExtendWith({ OutputCaptureExtension.class, UseLegacyProcessing.class }) @@ -1150,6 +1151,46 @@ class ConfigFileApplicationListenerTests { assertThat(this.environment.getProperty("fourth.property")).isNull(); } + @Test + void additionalProfilesCanBeIncludedFromProgrammaticallySetting() { + // gh-25704 + SpringApplication application = new SpringApplication(Config.class); + application.setWebApplicationType(WebApplicationType.NONE); + application.setAdditionalProfiles("dev"); + this.context = application.run(); + // Active profile should win over default + assertThat(this.context.getEnvironment().getProperty("my.property")).isEqualTo("fromdevpropertiesfile"); + } + + @Test + void twoAdditionalProfilesCanBeIncludedFromProgrammaticallySetting() { + // gh-25704 + SpringApplication application = new SpringApplication(Config.class); + application.setWebApplicationType(WebApplicationType.NONE); + application.setAdditionalProfiles("other", "dev"); + this.context = application.run(); + assertThat(this.context.getEnvironment().getProperty("my.property")).isEqualTo("fromdevpropertiesfile"); + } + + @Test + void includeProfilesOrder() { + SpringApplication application = new SpringApplication(Config.class); + application.setWebApplicationType(WebApplicationType.NONE); + this.context = application.run("--spring.profiles.active=bar,spam", "--spring.profiles.include=foo"); + // Before Boot 2.4 included profiles should always be first + assertThat(this.context.getEnvironment().getActiveProfiles()).containsExactly("foo", "bar", "spam"); + } + + @Test + void addProfilesOrder() { + SpringApplication application = new SpringApplication(Config.class); + application.setWebApplicationType(WebApplicationType.NONE); + application.setAdditionalProfiles("foo"); + this.context = application.run("--spring.profiles.active=bar,spam"); + // Before Boot 2.4 additional profiles should always be first + assertThat(this.context.getEnvironment().getActiveProfiles()).containsExactly("foo", "bar", "spam"); + } + private Condition matchingPropertySource(final String sourceName) { return new Condition("environment containing property source " + sourceName) {