From 3f7bf7d34f85dc67b71dc8626612a89fedac0174 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 13 Jan 2022 12:50:33 -0800 Subject: [PATCH] Modify SpringApplication Environment rather than setting it Update `SpringBootContextLoader` so that when possible the `SpringApplication` remains in control of creating the `Environment` instance. Prior to this commit, we would always create the `Environment` in the `SpringBootContextLoader` and then call `setEnvironment` on the `SpringApplication`. This meant that the `ApplicationEnvironment` classes were not used and that `isCustomEnvironment` was set to `true` so no conversion was applied. With the updated code, an `ApplicationListener` is used to mutate the `Environment` instance and add the required test property sources. Fixes gh-29169 --- .../test/context/SpringBootContextLoader.java | 75 +++++++++++++++---- .../context/SpringBootContextLoaderTests.java | 25 +++++++ .../profile/ActiveProfilesTests.java | 5 +- 3 files changed, 87 insertions(+), 18 deletions(-) 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 f5ca6e5038..95b27ca8ce 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 @@ -18,12 +18,15 @@ package org.springframework.boot.test.context; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.springframework.beans.BeanUtils; import org.springframework.boot.ApplicationContextFactory; +import org.springframework.boot.DefaultPropertiesPropertySource; import org.springframework.boot.SpringApplication; import org.springframework.boot.WebApplicationType; +import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.test.mock.web.SpringBootMockServletContext; import org.springframework.boot.test.util.TestPropertyValues; @@ -32,14 +35,18 @@ import org.springframework.boot.web.reactive.context.GenericReactiveWebApplicati import org.springframework.boot.web.servlet.support.ServletContextApplicationContextInitializer; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ApplicationListener; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.core.Ordered; +import org.springframework.core.PriorityOrdered; import org.springframework.core.SpringVersion; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; import org.springframework.core.annotation.Order; +import org.springframework.core.env.CommandLinePropertySource; import org.springframework.core.env.ConfigurableEnvironment; -import org.springframework.core.env.Environment; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.ResourceLoader; @@ -53,6 +60,7 @@ import org.springframework.test.context.support.TestPropertySourceUtils; import org.springframework.test.context.web.WebMergedContextConfiguration; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.context.support.GenericWebApplicationContext; @@ -81,6 +89,9 @@ import org.springframework.web.context.support.GenericWebApplicationContext; */ public class SpringBootContextLoader extends AbstractContextLoader { + private static final String[] PRIORITY_PROPERTY_SOURCES = { "configurationProperties", + DefaultPropertiesPropertySource.NAME, CommandLinePropertySource.COMMAND_LINE_PROPERTY_SOURCE_NAME }; + @Override public ApplicationContext loadContext(MergedContextConfiguration config) throws Exception { Class[] configClasses = config.getClasses(); @@ -111,23 +122,49 @@ public class SpringBootContextLoader extends AbstractContextLoader { application.setWebApplicationType(WebApplicationType.NONE); } application.setInitializers(initializers); - ConfigurableEnvironment environment = getEnvironment(); - setActiveProfiles(environment, config.getActiveProfiles()); + boolean customEnvironent = ReflectionUtils.findMethod(getClass(), "getEnvironment") + .getDeclaringClass() != SpringBootContextLoader.class; + if (customEnvironent) { + ConfigurableEnvironment environment = getEnvironment(); + prepareEnvironment(config, application, environment, false); + application.setEnvironment(environment); + } + else { + application.addListeners(new PrepareEnvironmentListener(config)); + } + String[] args = SpringBootTestArgs.get(config.getContextCustomizers()); + return application.run(args); + } + + private void prepareEnvironment(MergedContextConfiguration config, SpringApplication application, + ConfigurableEnvironment environment, boolean applicationEnvironment) { + MutablePropertySources propertySources = environment.getPropertySources(); + List> priorityPropertySources = new ArrayList<>(); + if (applicationEnvironment) { + for (String priorityPropertySourceName : PRIORITY_PROPERTY_SOURCES) { + PropertySource priorityPropertySource = propertySources.get(priorityPropertySourceName); + if (priorityPropertySource != null) { + priorityPropertySources.add(priorityPropertySource); + propertySources.remove(priorityPropertySourceName); + } + } + } + setActiveProfiles(environment, config.getActiveProfiles(), applicationEnvironment); 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); + Collections.reverse(priorityPropertySources); + priorityPropertySources.forEach(propertySources::addFirst); } - private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles) { + private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles, + boolean applicationEnvironment) { if (ObjectUtils.isEmpty(profiles)) { return; } - if (!(environment instanceof TestEnvironment)) { + if (!applicationEnvironment) { environment.setActiveProfiles(profiles); } String[] pairs = new String[profiles.length]; @@ -152,7 +189,7 @@ public class SpringBootContextLoader extends AbstractContextLoader { * @return a {@link ConfigurableEnvironment} instance */ protected ConfigurableEnvironment getEnvironment() { - return new TestEnvironment(); + return new StandardEnvironment(); } protected String[] getInlinedProperties(MergedContextConfiguration config) { @@ -304,19 +341,25 @@ public class SpringBootContextLoader extends AbstractContextLoader { } /** - * Side-effect free {@link Environment} that doesn't set profiles directly from - * properties. + * {@link ApplicationListener} used to prepare the application created environment. */ - static class TestEnvironment extends StandardEnvironment { + private class PrepareEnvironmentListener + implements ApplicationListener, PriorityOrdered { + + private final MergedContextConfiguration config; + + PrepareEnvironmentListener(MergedContextConfiguration config) { + this.config = config; + } @Override - protected String doGetActiveProfilesProperty() { - return null; + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; } @Override - protected String doGetDefaultProfilesProperty() { - return null; + public void onApplicationEvent(ApplicationEnvironmentPreparedEvent event) { + prepareEnvironment(this.config, event.getSpringApplication(), event.getEnvironment(), true); } } diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java index 2cf23eec3c..c0554f6799 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/SpringBootContextLoaderTests.java @@ -16,7 +16,9 @@ package org.springframework.boot.test.context; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @@ -24,11 +26,14 @@ import org.junit.jupiter.api.Test; import org.springframework.boot.test.util.TestPropertyValues; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.PropertySource; import org.springframework.core.env.StandardEnvironment; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.MergedContextConfiguration; import org.springframework.test.context.TestContext; import org.springframework.test.context.TestContextManager; +import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.support.TestPropertySourceUtils; import org.springframework.test.util.ReflectionTestUtils; @@ -127,6 +132,20 @@ class SpringBootContextLoaderTests { assertThat(environment.getPropertySources().get("active-test-profiles")).isNotNull(); } + @Test + void propertySourceOrdering() throws Exception { + TestContext context = new ExposedTestContextManager(PropertySourceOrdering.class).getExposedTestContext(); + ConfigurableEnvironment environment = (ConfigurableEnvironment) context.getApplicationContext() + .getEnvironment(); + List names = environment.getPropertySources().stream().map(PropertySource::getName) + .collect(Collectors.toList()); + String last = names.remove(names.size() - 1); + assertThat(names).containsExactly("configurationProperties", "commandLineArgs", "Inlined Test Properties", + "servletConfigInitParams", "servletContextInitParams", "systemProperties", "systemEnvironment", + "random"); + assertThat(last).startsWith("Config resource"); + } + private String[] getActiveProfiles(Class testClass) { TestContext testContext = new ExposedTestContextManager(testClass).getExposedTestContext(); ApplicationContext applicationContext = testContext.getApplicationContext(); @@ -198,6 +217,12 @@ class SpringBootContextLoaderTests { } + @SpringBootTest(classes = Config.class, args = "args", properties = "one=1") + @TestPropertySource(properties = "two=2") + static class PropertySourceOrdering { + + } + @Configuration(proxyBeanMethods = false) static class Config { 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 index e9d8aee7c2..bc4b1cde61 100644 --- 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 @@ -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. @@ -21,6 +21,7 @@ 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.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.core.env.Environment; import org.springframework.test.context.ActiveProfiles; @@ -32,7 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Madhura Bhave */ -@SpringBootTest(properties = "enableEnvironmentPostProcessor=true") // gh-28530 +@SpringBootTest(webEnvironment = WebEnvironment.NONE, properties = "enableEnvironmentPostProcessor=true") // gh-28530 @ActiveProfiles("hello") class ActiveProfilesTests {