From 64270eca51a48dfcf5ca753206aee0a86fd34267 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 10 Nov 2021 16:09:19 -0800 Subject: [PATCH] Convert environment used by SpringBootTestContextLoader This commit aligns `SpringBootTest`s to also use `ApplicationEnvironment` instead of `StandardEnvironment`. This prevents the side-effect of active profiles from `@ActiveProfiles` from being added to the environment when doGetActiveProfiles is called. In this case, calling `addActiveProfiles()` in the environment post processor would result in `@ActiveProfiles` being added to the environment first, resulting in the wrong order. The additional call to `setActiveProfiles()` is also not necessary when using ApplicationEnvironment because that call was put in place to prevent the side-effect which `ApplicationEnvironment` does not have. Fixes gh-28530 --- .../test/context/SpringBootContextLoader.java | 50 ++++++++------ ...ingBootTestWithCustomEnvironmentTests.java | 65 +++++++++++++++++++ .../boot/SpringApplication.java | 15 ++++- ...ctiveProfilesEnvironmentPostProcessor.java | 40 ++++++++++++ .../main/resources/META-INF/spring.factories | 2 + .../profile/ActiveProfilesTests.java | 47 ++++++++++++++ 6 files changed, 197 insertions(+), 22 deletions(-) create mode 100644 spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootTestWithCustomEnvironmentTests.java create mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/ActiveProfilesEnvironmentPostProcessor.java create mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/resources/META-INF/spring.factories create mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/ActiveProfilesTests.java diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java index 131cf8ca92..4990c8f14c 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java @@ -91,16 +91,6 @@ public class SpringBootContextLoader extends AbstractContextLoader { application.setMainApplicationClass(config.getTestClass()); application.addPrimarySources(Arrays.asList(configClasses)); application.getSources().addAll(Arrays.asList(configLocations)); - ConfigurableEnvironment environment = getEnvironment(); - if (!ObjectUtils.isEmpty(config.getActiveProfiles())) { - setActiveProfiles(environment, config.getActiveProfiles()); - } - ResourceLoader resourceLoader = (application.getResourceLoader() != null) ? application.getResourceLoader() - : new DefaultResourceLoader(null); - TestPropertySourceUtils.addPropertiesFilesToEnvironment(environment, resourceLoader, - config.getPropertySourceLocations()); - TestPropertySourceUtils.addInlinedPropertiesToEnvironment(environment, getInlinedProperties(config)); - application.setEnvironment(environment); List> initializers = getInitializers(config, application); if (config instanceof WebMergedContextConfiguration) { application.setWebApplicationType(WebApplicationType.SERVLET); @@ -119,10 +109,40 @@ public class SpringBootContextLoader extends AbstractContextLoader { application.setWebApplicationType(WebApplicationType.NONE); } application.setInitializers(initializers); + ConfigurableEnvironment environment = getEnvironment(application, config.getActiveProfiles()); + ResourceLoader resourceLoader = (application.getResourceLoader() != null) ? application.getResourceLoader() + : new DefaultResourceLoader(null); + TestPropertySourceUtils.addPropertiesFilesToEnvironment(environment, resourceLoader, + config.getPropertySourceLocations()); + TestPropertySourceUtils.addInlinedPropertiesToEnvironment(environment, getInlinedProperties(config)); + application.setEnvironment(environment); String[] args = SpringBootTestArgs.get(config.getContextCustomizers()); return application.run(args); } + private ConfigurableEnvironment getEnvironment(SpringApplication application, String[] activeProfiles) { + ConfigurableEnvironment environment = getEnvironment(); + boolean applicationEnvironment = false; + if (environment.getClass() == StandardEnvironment.class) { + environment = application.convertEnvironment(environment); + applicationEnvironment = true; + } + setActiveProfiles(environment, activeProfiles, applicationEnvironment); + return environment; + } + + private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles, + boolean applicationEnvironment) { + if (!applicationEnvironment) { + environment.setActiveProfiles(profiles); + } + String[] pairs = new String[profiles.length]; + for (int i = 0; i < profiles.length; i++) { + pairs[i] = "spring.profiles.active[" + i + "]=" + profiles[i]; + } + TestPropertyValues.of(pairs).applyTo(environment); + } + /** * Builds new {@link org.springframework.boot.SpringApplication} instance. You can * override this method to add custom behavior @@ -141,16 +161,6 @@ public class SpringBootContextLoader extends AbstractContextLoader { return new StandardEnvironment(); } - private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles) { - environment.setActiveProfiles(profiles); - // Also add as properties to override any application.properties - String[] pairs = new String[profiles.length]; - for (int i = 0; i < profiles.length; i++) { - pairs[i] = "spring.profiles.active[" + i + "]=" + profiles[i]; - } - TestPropertyValues.of(pairs).applyTo(environment); - } - protected String[] getInlinedProperties(MergedContextConfiguration config) { ArrayList properties = new ArrayList<>(); // JMX bean names will clash if the same bean is used in multiple contexts diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootTestWithCustomEnvironmentTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootTestWithCustomEnvironmentTests.java new file mode 100644 index 0000000000..24258f5af0 --- /dev/null +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootTestWithCustomEnvironmentTests.java @@ -0,0 +1,65 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.test.context; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.Environment; +import org.springframework.mock.env.MockEnvironment; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.ContextConfiguration; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests for {@link SpringBootTest @SpringBootTest} with a custom + * {@link Environment}. + * + * @author Madhura Bhave + */ +@SpringBootTest +@ActiveProfiles({ "test1", "test2" }) +@ContextConfiguration(loader = SpringBootTestWithCustomEnvironmentTests.Loader.class) +class SpringBootTestWithCustomEnvironmentTests { + + @Autowired + private Environment environment; + + @Test + void getActiveProfiles() { + assertThat(this.environment).isInstanceOf(MockEnvironment.class); + assertThat(this.environment.getActiveProfiles()).containsOnly("test1", "test2"); + } + + @Configuration + static class Config { + + } + + static class Loader extends SpringBootContextLoader { + + @Override + protected ConfigurableEnvironment getEnvironment() { + return new MockEnvironment(); + } + + } + +} 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 5effd35a9d..6fc125e6f1 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 @@ -377,13 +377,24 @@ public class SpringApplication { "Environment prefix cannot be set via properties."); bindToSpringApplication(environment); if (!this.isCustomEnvironment) { - environment = new EnvironmentConverter(getClassLoader()).convertEnvironmentIfNecessary(environment, - deduceEnvironmentClass()); + environment = convertEnvironment(environment); } ConfigurationPropertySources.attach(environment); return environment; } + /** + * Convert the given {@link ConfigurableEnvironment environment} to an application + * environment that doesn't attempt to resolve profile properties directly. + * @param environment the environment to convert + * @return the converted environment + * @since 2.5.7 + */ + public StandardEnvironment convertEnvironment(ConfigurableEnvironment environment) { + return new EnvironmentConverter(getClassLoader()).convertEnvironmentIfNecessary(environment, + deduceEnvironmentClass()); + } + private Class deduceEnvironmentClass() { switch (this.webApplicationType) { case SERVLET: diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/ActiveProfilesEnvironmentPostProcessor.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/ActiveProfilesEnvironmentPostProcessor.java new file mode 100644 index 0000000000..52161c8b06 --- /dev/null +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/ActiveProfilesEnvironmentPostProcessor.java @@ -0,0 +1,40 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package smoketest.profile; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.env.EnvironmentPostProcessor; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.core.env.ConfigurableEnvironment; + +/** + * {@link EnvironmentPostProcessor} that adds an active profile. + * + * @author Madhura Bhave + */ +@Order(Ordered.HIGHEST_PRECEDENCE) +class ActiveProfilesEnvironmentPostProcessor implements EnvironmentPostProcessor { + + @Override + public void postProcessEnvironment(ConfigurableEnvironment environment, SpringApplication application) { + if (environment.getProperty("enableEnvironmentPostProcessor") != null) { + environment.addActiveProfile("dev"); + } + } + +} diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/resources/META-INF/spring.factories b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/resources/META-INF/spring.factories new file mode 100644 index 0000000000..ac0822ce5f --- /dev/null +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/resources/META-INF/spring.factories @@ -0,0 +1,2 @@ +org.springframework.boot.env.EnvironmentPostProcessor=\ +smoketest.profile.ActiveProfilesEnvironmentPostProcessor \ No newline at end of file diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/ActiveProfilesTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/ActiveProfilesTests.java new file mode 100644 index 0000000000..e9d8aee7c2 --- /dev/null +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/ActiveProfilesTests.java @@ -0,0 +1,47 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package smoketest.profile; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.env.EnvironmentPostProcessor; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.core.env.Environment; +import org.springframework.test.context.ActiveProfiles; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests that profiles are activited in the correct order from an + * {@link EnvironmentPostProcessor}. + * + * @author Madhura Bhave + */ +@SpringBootTest(properties = "enableEnvironmentPostProcessor=true") // gh-28530 +@ActiveProfiles("hello") +class ActiveProfilesTests { + + @Autowired + private Environment environment; + + @Test + void activeProfileShouldTakePrecedenceOverProgrammaticallySetProfile() { + assertThat(this.environment.getActiveProfiles()).containsExactly("dev", "hello"); + } + +}