From eb6b48fff03e5bce063a88a10ad4ec2b8c9510a9 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 11 Jan 2022 19:08:54 -0800 Subject: [PATCH] Use side-effect free environment with tests rather than converting Refine the logic introduced in 64270eca to use a side-effect free Environment implementation rather than converting the Environment early. Early conversion can cause condition evaluation issues if `src/test/resources/application.properties` files are bound to the `SpringApplication`. Specifically the `spring.main.web-application-type` property can change the `Environment` type which must happen before conditions are evaluated. Fixes gh-29169 --- .../test/context/SpringBootContextLoader.java | 45 ++++++++++++------- .../boot/SpringApplication.java | 2 + .../build.gradle | 3 +- .../profile/SampleProfileApplication.java | 16 +++++-- .../profile/AttributeInjectionTests.java | 42 +++++++++++++++++ .../SampleProfileApplicationTests.java | 10 ++--- .../src/test/resources/application.properties | 1 + 7 files changed, 93 insertions(+), 26 deletions(-) create mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/AttributeInjectionTests.java create mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/resources/application.properties 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 0485ddfab6f..f5ca6e50387 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 @@ -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. @@ -39,6 +39,7 @@ import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; import org.springframework.core.annotation.Order; import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.Environment; import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.ResourceLoader; @@ -110,7 +111,8 @@ public class SpringBootContextLoader extends AbstractContextLoader { application.setWebApplicationType(WebApplicationType.NONE); } application.setInitializers(initializers); - ConfigurableEnvironment environment = getEnvironment(application, config.getActiveProfiles()); + ConfigurableEnvironment environment = getEnvironment(); + setActiveProfiles(environment, config.getActiveProfiles()); ResourceLoader resourceLoader = (application.getResourceLoader() != null) ? application.getResourceLoader() : new DefaultResourceLoader(null); TestPropertySourceUtils.addPropertiesFilesToEnvironment(environment, resourceLoader, @@ -121,23 +123,11 @@ public class SpringBootContextLoader extends AbstractContextLoader { 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) { + private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles) { if (ObjectUtils.isEmpty(profiles)) { return; } - if (!applicationEnvironment) { + if (!(environment instanceof TestEnvironment)) { environment.setActiveProfiles(profiles); } String[] pairs = new String[profiles.length]; @@ -162,7 +152,7 @@ public class SpringBootContextLoader extends AbstractContextLoader { * @return a {@link ConfigurableEnvironment} instance */ protected ConfigurableEnvironment getEnvironment() { - return new StandardEnvironment(); + return new TestEnvironment(); } protected String[] getInlinedProperties(MergedContextConfiguration config) { @@ -293,6 +283,9 @@ public class SpringBootContextLoader extends AbstractContextLoader { } + /** + * {@link ApplicationContextInitializer} used to set the parent context. + */ @Order(Ordered.HIGHEST_PRECEDENCE) private static class ParentContextApplicationContextInitializer implements ApplicationContextInitializer { @@ -310,4 +303,22 @@ public class SpringBootContextLoader extends AbstractContextLoader { } + /** + * Side-effect free {@link Environment} that doesn't set profiles directly from + * properties. + */ + static class TestEnvironment extends StandardEnvironment { + + @Override + protected String doGetActiveProfilesProperty() { + return null; + } + + @Override + protected String doGetDefaultProfilesProperty() { + return null; + } + + } + } 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 6fc125e6f10..593d6a18c3c 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 @@ -389,7 +389,9 @@ public class SpringApplication { * @param environment the environment to convert * @return the converted environment * @since 2.5.7 + * @deprecated since 2.5.8 for removal in 2.7.0 */ + @Deprecated public StandardEnvironment convertEnvironment(ConfigurableEnvironment environment) { return new EnvironmentConverter(getClassLoader()).convertEnvironmentIfNecessary(environment, deduceEnvironmentClass()); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/build.gradle b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/build.gradle index e85bae25e21..879107d65dd 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/build.gradle +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/build.gradle @@ -6,7 +6,8 @@ plugins { description = "Spring Boot profile smoke test" dependencies { - implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter")) + implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-web")) + implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-webflux")) testImplementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-test")) } diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/SampleProfileApplication.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/SampleProfileApplication.java index 03487f96a12..c64eb7e7a98 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/SampleProfileApplication.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/main/java/smoketest/profile/SampleProfileApplication.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 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. @@ -21,7 +21,9 @@ import smoketest.profile.service.MessageService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.CommandLineRunner; import org.springframework.boot.SpringApplication; +import org.springframework.boot.WebApplicationType; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.core.env.ConfigurableEnvironment; @SpringBootApplication public class SampleProfileApplication implements CommandLineRunner { @@ -38,8 +40,16 @@ public class SampleProfileApplication implements CommandLineRunner { System.out.println(this.helloWorldService.getMessage()); } - public static void main(String[] args) { - SpringApplication.run(SampleProfileApplication.class, args); + public static void main(String... args) { + SpringApplication application = new SpringApplication(SampleProfileApplication.class) { + + @Override + protected void bindToSpringApplication(ConfigurableEnvironment environment) { + } + + }; + application.setWebApplicationType(WebApplicationType.NONE); + application.run(args); } } diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/AttributeInjectionTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/AttributeInjectionTests.java new file mode 100644 index 00000000000..42a26eefee5 --- /dev/null +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/AttributeInjectionTests.java @@ -0,0 +1,42 @@ +/* + * 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. + * 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.test.context.SpringBootTest; + +import static org.assertj.core.api.Assertions.assertThat; + +// gh-29169 +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +class AttributeInjectionTests { + + @Autowired(required = false) + private org.springframework.boot.web.servlet.error.ErrorAttributes errorAttributesServlet; + + @Autowired(required = false) + private org.springframework.boot.web.reactive.error.ErrorAttributes errorAttributesReactive; + + @Test + void contextLoads() { + assertThat(this.errorAttributesServlet).isNull(); + assertThat(this.errorAttributesReactive).isNotNull(); + } + +} diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/SampleProfileApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/SampleProfileApplicationTests.java index 0232a2ca9a2..d4dd213b0c3 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/SampleProfileApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/java/smoketest/profile/SampleProfileApplicationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 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. @@ -48,14 +48,14 @@ class SampleProfileApplicationTests { @Test void testDefaultProfile(CapturedOutput output) { - SampleProfileApplication.main(new String[0]); + SampleProfileApplication.main(); assertThat(output).contains("Hello Phil"); } @Test void testGoodbyeProfile(CapturedOutput output) { System.setProperty("spring.profiles.active", "goodbye"); - SampleProfileApplication.main(new String[0]); + SampleProfileApplication.main(); assertThat(output).contains("Goodbye Everyone"); } @@ -68,13 +68,13 @@ class SampleProfileApplicationTests { * "name" property. */ System.setProperty("spring.profiles.active", "generic"); - SampleProfileApplication.main(new String[0]); + SampleProfileApplication.main(); assertThat(output).contains("Bonjour Phil"); } @Test void testGoodbyeProfileFromCommandline(CapturedOutput output) { - SampleProfileApplication.main(new String[] { "--spring.profiles.active=goodbye" }); + SampleProfileApplication.main("--spring.profiles.active=goodbye"); assertThat(output).contains("Goodbye Everyone"); } diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/resources/application.properties b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/resources/application.properties new file mode 100644 index 00000000000..66403d21ae9 --- /dev/null +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-profile/src/test/resources/application.properties @@ -0,0 +1 @@ +spring.main.web-application-type=reactive \ No newline at end of file