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

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<ApplicationEnvironmentPreparedEvent> {
+		ApplicationListener<ApplicationEnvironmentPreparedEvent>, 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;
+	}
+
 }

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.
 	 */

spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsHomePropertiesPostProcessor.java

@@ -14,63 +14,48 @@
  * 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) {
-			try {
-				postProcessEnvironment((ConfigurableEnvironment) this.environment);
-			}
-			catch (IOException ex) {
-				throw new IllegalStateException("Unable to load " + FILE_NAME, ex);
-			}
-		}
-	}
-
-	private void postProcessEnvironment(ConfigurableEnvironment environment)
-			throws IOException {
+	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 = PropertiesLoaderUtils.loadProperties(resource);
-			environment.getPropertySources().addFirst(
-					new PropertiesPropertySource("devtools-local", properties));
+			Properties properties;
+			try {
+				properties = PropertiesLoaderUtils.loadProperties(resource);
+				environment.getPropertySources().addFirst(
+						new PropertiesPropertySource("devtools-local", properties));
+			}
+			catch (IOException ex) {
+				throw new IllegalStateException("Unable to load " + FILE_NAME, ex);
+			}
 		}
 	}
 

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<String, Object> 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;
 	}
 
 }

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<ClientHttpRequestInterceptor> interceptors = Arrays

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

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) {

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 {
+
+	}
+
+}

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;
 		}
 	}