From 9e88dced888b5f176573579a392ff65d94b61b0b Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 12 Aug 2015 12:14:12 +0100 Subject: [PATCH] Update devtools to customise environment before app context is refreshed Prior to this commit, the devtools used bean factory post processors to configure the environment with custom, development-time properties. This meant that the environment was configured as part of the application context being refreshed. Crucially, this happened after any property conditions were evaluated making it impossible for the devtools to change the default auto-configuration behaviour for a bean or configuration class that was conditional on a property. This commit moves the configuration of the environment into an ApplicationListener that listens for the ApplicationEnvironmentPreparedEvent which is published as soon as the Environment has been prepared and before the application context is refreshed. Closes gh-3726 --- .../devtools/RemoteUrlPropertyExtractor.java | 9 ++- .../LocalDevToolsAutoConfiguration.java | 13 --- .../DevToolsHomePropertiesPostProcessor.java} | 49 ++++------- ...DevToolsPropertyDefaultsPostProcessor.java | 38 ++++----- .../client/RemoteClientConfiguration.java | 6 -- .../main/resources/META-INF/spring.factories | 5 ++ .../RemoteUrlPropertyExtractorTests.java | 5 +- .../DevToolPropertiesIntegrationTests.java | 81 +++++++++++++++++++ ...oolsHomePropertiesPostProcessorTests.java} | 26 +++--- 9 files changed, 142 insertions(+), 90 deletions(-) rename spring-boot-devtools/src/main/java/org/springframework/boot/devtools/{autoconfigure/DevToolHomePropertiesPostProcessor.java => env/DevToolsHomePropertiesPostProcessor.java} (57%) rename spring-boot-devtools/src/main/java/org/springframework/boot/devtools/{autoconfigure => env}/DevToolsPropertyDefaultsPostProcessor.java (57%) create mode 100644 spring-boot-devtools/src/test/java/org/springframework/boot/devtools/env/DevToolPropertiesIntegrationTests.java rename spring-boot-devtools/src/test/java/org/springframework/boot/devtools/{autoconfigure/DevToolHomePropertiesPostProcessorTests.java => env/DevToolsHomePropertiesPostProcessorTests.java} (72%) diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractor.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractor.java index b5d5b938fe..2c1331c331 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractor.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractor.java @@ -23,6 +23,7 @@ import java.util.Map; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; import org.springframework.context.ApplicationListener; +import org.springframework.core.Ordered; import org.springframework.core.env.CommandLinePropertySource; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.MapPropertySource; @@ -35,9 +36,10 @@ import org.springframework.util.StringUtils; * {@link RemoteSpringApplication} to use. * * @author Phillip Webb + * @author Andy Wilkinson */ class RemoteUrlPropertyExtractor implements - ApplicationListener { + ApplicationListener, Ordered { private static final String NON_OPTION_ARGS = CommandLinePropertySource.DEFAULT_NON_OPTION_ARGS_PROPERTY_NAME; @@ -58,4 +60,9 @@ class RemoteUrlPropertyExtractor implements environment.getPropertySources().addLast(propertySource); } + @Override + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; + } + } diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java index eb2f8a4581..482f5c5835 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java @@ -53,19 +53,6 @@ import org.springframework.util.StringUtils; @EnableConfigurationProperties(DevToolsProperties.class) public class LocalDevToolsAutoConfiguration { - @Autowired - private DevToolsProperties properties; - - @Bean - public static DevToolsPropertyDefaultsPostProcessor devToolsPropertyDefaultsPostProcessor() { - return new DevToolsPropertyDefaultsPostProcessor(); - } - - @Bean - public static DevToolHomePropertiesPostProcessor devToolHomePropertiesPostProcessor() { - return new DevToolHomePropertiesPostProcessor(); - } - /** * Local LiveReload configuration. */ diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/DevToolHomePropertiesPostProcessor.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsHomePropertiesPostProcessor.java similarity index 57% rename from spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/DevToolHomePropertiesPostProcessor.java rename to spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsHomePropertiesPostProcessor.java index 445d743d55..5b5d26764d 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/DevToolHomePropertiesPostProcessor.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsHomePropertiesPostProcessor.java @@ -14,47 +14,44 @@ * limitations under the License. */ -package org.springframework.boot.devtools.autoconfigure; +package org.springframework.boot.devtools.env; import java.io.File; import java.io.IOException; import java.util.Properties; -import org.springframework.beans.BeansException; -import org.springframework.beans.factory.config.BeanFactoryPostProcessor; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.context.EnvironmentAware; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.env.EnvironmentPostProcessor; import org.springframework.core.env.ConfigurableEnvironment; -import org.springframework.core.env.Environment; import org.springframework.core.env.PropertiesPropertySource; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.support.PropertiesLoaderUtils; import org.springframework.util.StringUtils; /** - * {@link BeanFactoryPostProcessor} to add devtools properties from the users home folder. + * {@link EnvironmentPostProcessor} to add devtools properties from the user's home + * folder. * * @author Phillip Webb + * @author Andy Wilkinson * @since 1.3.0 */ -public class DevToolHomePropertiesPostProcessor implements BeanFactoryPostProcessor, - EnvironmentAware { +public class DevToolsHomePropertiesPostProcessor implements EnvironmentPostProcessor { private static final String FILE_NAME = ".spring-boot-devtools.properties"; - private Environment environment; - - @Override - public void setEnvironment(Environment environment) { - this.environment = environment; - } - @Override - public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) - throws BeansException { - if (this.environment instanceof ConfigurableEnvironment) { + public void postProcessEnvironment(ConfigurableEnvironment environment, + SpringApplication application) { + File home = getHomeFolder(); + File propertyFile = (home == null ? null : new File(home, FILE_NAME)); + if (propertyFile != null && propertyFile.exists() && propertyFile.isFile()) { + FileSystemResource resource = new FileSystemResource(propertyFile); + Properties properties; try { - postProcessEnvironment((ConfigurableEnvironment) this.environment); + properties = PropertiesLoaderUtils.loadProperties(resource); + environment.getPropertySources().addFirst( + new PropertiesPropertySource("devtools-local", properties)); } catch (IOException ex) { throw new IllegalStateException("Unable to load " + FILE_NAME, ex); @@ -62,18 +59,6 @@ public class DevToolHomePropertiesPostProcessor implements BeanFactoryPostProces } } - private void postProcessEnvironment(ConfigurableEnvironment environment) - throws IOException { - File home = getHomeFolder(); - File propertyFile = (home == null ? null : new File(home, FILE_NAME)); - if (propertyFile != null && propertyFile.exists() && propertyFile.isFile()) { - FileSystemResource resource = new FileSystemResource(propertyFile); - Properties properties = PropertiesLoaderUtils.loadProperties(resource); - environment.getPropertySources().addFirst( - new PropertiesPropertySource("devtools-local", properties)); - } - } - protected File getHomeFolder() { String home = System.getProperty("user.home"); if (StringUtils.hasLength(home)) { diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/DevToolsPropertyDefaultsPostProcessor.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsPropertyDefaultsPostProcessor.java similarity index 57% rename from spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/DevToolsPropertyDefaultsPostProcessor.java rename to spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsPropertyDefaultsPostProcessor.java index 785b65a12c..d72361a950 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/DevToolsPropertyDefaultsPostProcessor.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsPropertyDefaultsPostProcessor.java @@ -14,29 +14,27 @@ * limitations under the License. */ -package org.springframework.boot.devtools.autoconfigure; +package org.springframework.boot.devtools.env; import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.springframework.beans.BeansException; -import org.springframework.beans.factory.config.BeanFactoryPostProcessor; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.context.EnvironmentAware; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.env.EnvironmentPostProcessor; import org.springframework.core.env.ConfigurableEnvironment; -import org.springframework.core.env.Environment; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.PropertySource; /** - * {@link BeanFactoryPostProcessor} to add properties that make sense when working at + * {@link EnvironmentPostProcessor} to add properties that make sense when working at * development time. * * @author Phillip Webb + * @author Andy Wilkinson + * @since 1.3.0 */ -class DevToolsPropertyDefaultsPostProcessor implements BeanFactoryPostProcessor, - EnvironmentAware { +public class DevToolsPropertyDefaultsPostProcessor implements EnvironmentPostProcessor { private static final Map PROPERTIES; static { @@ -51,24 +49,18 @@ class DevToolsPropertyDefaultsPostProcessor implements BeanFactoryPostProcessor, PROPERTIES = Collections.unmodifiableMap(properties); } - private Environment environment; - - @Override - public void setEnvironment(Environment environment) { - this.environment = environment; - } - @Override - public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) - throws BeansException { - if (this.environment instanceof ConfigurableEnvironment) { - postProcessEnvironment((ConfigurableEnvironment) this.environment); + public void postProcessEnvironment(ConfigurableEnvironment environment, + SpringApplication application) { + if (isLocalApplication(environment)) { + PropertySource propertySource = new MapPropertySource("refresh", + PROPERTIES); + environment.getPropertySources().addFirst(propertySource); } } - private void postProcessEnvironment(ConfigurableEnvironment environment) { - PropertySource propertySource = new MapPropertySource("refresh", PROPERTIES); - environment.getPropertySources().addFirst(propertySource); + private boolean isLocalApplication(ConfigurableEnvironment environment) { + return environment.getPropertySources().get("remoteUrl") == null; } } diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/remote/client/RemoteClientConfiguration.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/remote/client/RemoteClientConfiguration.java index e103270251..65479679cd 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/remote/client/RemoteClientConfiguration.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/remote/client/RemoteClientConfiguration.java @@ -33,7 +33,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.boot.devtools.autoconfigure.DevToolHomePropertiesPostProcessor; import org.springframework.boot.devtools.autoconfigure.DevToolsProperties; import org.springframework.boot.devtools.autoconfigure.DevToolsProperties.Restart; import org.springframework.boot.devtools.autoconfigure.OptionalLiveReloadServer; @@ -88,11 +87,6 @@ public class RemoteClientConfiguration { return new PropertySourcesPlaceholderConfigurer(); } - @Bean - public static DevToolHomePropertiesPostProcessor devToolHomePropertiesPostProcessor() { - return new DevToolHomePropertiesPostProcessor(); - } - @Bean public ClientHttpRequestFactory clientHttpRequestFactory() { List interceptors = Arrays diff --git a/spring-boot-devtools/src/main/resources/META-INF/spring.factories b/spring-boot-devtools/src/main/resources/META-INF/spring.factories index 03c80fd6cb..c5923c0b13 100644 --- a/spring-boot-devtools/src/main/resources/META-INF/spring.factories +++ b/spring-boot-devtools/src/main/resources/META-INF/spring.factories @@ -10,3 +10,8 @@ org.springframework.boot.devtools.restart.RestartApplicationListener org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ org.springframework.boot.devtools.autoconfigure.LocalDevToolsAutoConfiguration,\ org.springframework.boot.devtools.autoconfigure.RemoteDevToolsAutoConfiguration + +# Environment Post Processors +org.springframework.boot.env.EnvironmentPostProcessor=\ +org.springframework.boot.devtools.env.DevToolsHomePropertiesPostProcessor,\ +org.springframework.boot.devtools.env.DevToolsPropertyDefaultsPostProcessor diff --git a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractorTests.java b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractorTests.java index 5e1ece5fd5..ade48a0d9a 100644 --- a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractorTests.java +++ b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/RemoteUrlPropertyExtractorTests.java @@ -20,11 +20,12 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.springframework.boot.SpringApplication; -import org.springframework.boot.devtools.RemoteUrlPropertyExtractor; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Configuration; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; /** @@ -64,6 +65,8 @@ public class RemoteUrlPropertyExtractorTests { ApplicationContext context = doTest("http://localhost:8080"); assertThat(context.getEnvironment().getProperty("remoteUrl"), equalTo("http://localhost:8080")); + assertThat(context.getEnvironment().getProperty("spring.thymeleaf.cache"), + is(nullValue())); } private ApplicationContext doTest(String... args) { diff --git a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/env/DevToolPropertiesIntegrationTests.java b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/env/DevToolPropertiesIntegrationTests.java new file mode 100644 index 0000000000..02dcbdad1f --- /dev/null +++ b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/env/DevToolPropertiesIntegrationTests.java @@ -0,0 +1,81 @@ +/* + * Copyright 2012-2015 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 + * + * http://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.devtools.env; + +import org.junit.After; +import org.junit.Test; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * Integration tests for the configuration of development-time properties + * + * @author Andy Wilkinson + */ +public class DevToolPropertiesIntegrationTests { + + private ConfigurableApplicationContext context; + + @After + public void cleanup() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void classPropertyConditionIsAffectedByDevToolProperties() { + SpringApplication application = new SpringApplication( + ClassConditionConfiguration.class); + application.setWebEnvironment(false); + this.context = application.run(); + this.context.getBean(ClassConditionConfiguration.class); + } + + @Test + public void beanMethodPropertyConditionIsAffectedByDevToolProperties() { + SpringApplication application = new SpringApplication( + BeanConditionConfiguration.class); + application.setWebEnvironment(false); + this.context = application.run(); + this.context.getBean(MyBean.class); + } + + @Configuration + @ConditionalOnProperty("spring.h2.console.enabled") + static class ClassConditionConfiguration { + + } + + @Configuration + static class BeanConditionConfiguration { + + @Bean + @ConditionalOnProperty("spring.h2.console.enabled") + public MyBean myBean() { + return new MyBean(); + } + } + + static class MyBean { + + } + +} diff --git a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/autoconfigure/DevToolHomePropertiesPostProcessorTests.java b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/env/DevToolsHomePropertiesPostProcessorTests.java similarity index 72% rename from spring-boot-devtools/src/test/java/org/springframework/boot/devtools/autoconfigure/DevToolHomePropertiesPostProcessorTests.java rename to spring-boot-devtools/src/test/java/org/springframework/boot/devtools/env/DevToolsHomePropertiesPostProcessorTests.java index 330997bf96..9a71f1e8f9 100644 --- a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/autoconfigure/DevToolHomePropertiesPostProcessorTests.java +++ b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/env/DevToolsHomePropertiesPostProcessorTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.boot.devtools.autoconfigure; +package org.springframework.boot.devtools.env; import java.io.File; import java.io.FileOutputStream; @@ -26,21 +26,21 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.core.env.Environment; +import org.springframework.boot.devtools.env.DevToolsHomePropertiesPostProcessor; +import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.mock.env.MockEnvironment; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.mock; /** - * Tests for {@link DevToolHomePropertiesPostProcessor}. + * Tests for {@link DevToolsHomePropertiesPostProcessor}. * * @author Phillip Webb + * @author Andy Wilkinson */ -public class DevToolHomePropertiesPostProcessorTests { +public class DevToolsHomePropertiesPostProcessorTests { @Rule public TemporaryFolder temp = new TemporaryFolder(); @@ -60,28 +60,26 @@ public class DevToolHomePropertiesPostProcessorTests { ".spring-boot-devtools.properties")); properties.store(out, null); out.close(); - Environment environment = new MockEnvironment(); + ConfigurableEnvironment environment = new MockEnvironment(); MockDevToolHomePropertiesPostProcessor postProcessor = new MockDevToolHomePropertiesPostProcessor(); - postProcessor.setEnvironment(environment); - postProcessor.postProcessBeanFactory(mock(ConfigurableListableBeanFactory.class)); + postProcessor.postProcessEnvironment(environment, null); assertThat(environment.getProperty("abc"), equalTo("def")); } @Test public void ignoresMissingHomeProperties() throws Exception { - Environment environment = new MockEnvironment(); + ConfigurableEnvironment environment = new MockEnvironment(); MockDevToolHomePropertiesPostProcessor postProcessor = new MockDevToolHomePropertiesPostProcessor(); - postProcessor.setEnvironment(environment); - postProcessor.postProcessBeanFactory(mock(ConfigurableListableBeanFactory.class)); + postProcessor.postProcessEnvironment(environment, null); assertThat(environment.getProperty("abc"), nullValue()); } private class MockDevToolHomePropertiesPostProcessor extends - DevToolHomePropertiesPostProcessor { + DevToolsHomePropertiesPostProcessor { @Override protected File getHomeFolder() { - return DevToolHomePropertiesPostProcessorTests.this.home; + return DevToolsHomePropertiesPostProcessorTests.this.home; } }