Refactor ConfigurationPropertiesRebinder a bit

It's not super expensive, but it all adds up, and rebinding beans
that don't need to be rebound is wasteful. This change should improve
things in three places:

The ConfigurationPropertiesRebinder
has always done a pre-emptive re-bind on startup, which was a bit
of a blunt instrument, since all we really wanted as to rebind the
beans in the parent context (to the current context's Environment).

The rebinding always happened twice for every bean because we
explicitly called into the ConfigurationPropertiesPostProcessor
directly as well as indirectly through the initialization callbacks.

Rebinding in response to an EnvironmentChangeEvent was not able to
distinguish between events published locally and by child contexts.
The child context always binds the parent's beans anyway, so you
don't need to do it twice.

spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/ConfigurationPropertiesRebinderAutoConfiguration.java

@@ -16,8 +16,6 @@
 
 package org.springframework.cloud.autoconfigure;
 
-import java.util.Collections;
-
 import org.springframework.beans.factory.SmartInitializingSingleton;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
@@ -25,7 +23,6 @@ import org.springframework.boot.autoconfigure.condition.SearchStrategy;
 import org.springframework.boot.context.properties.ConfigurationBeanFactoryMetaData;
 import org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor;
 import org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessorRegistrar;
-import org.springframework.cloud.context.environment.EnvironmentChangeEvent;
 import org.springframework.cloud.context.properties.ConfigurationPropertiesBeans;
 import org.springframework.cloud.context.properties.ConfigurationPropertiesRebinder;
 import org.springframework.context.ApplicationContext;
@@ -64,19 +61,27 @@ public class ConfigurationPropertiesRebinderAutoConfiguration
 	@Bean
 	@ConditionalOnMissingBean(search = SearchStrategy.CURRENT)
 	public ConfigurationPropertiesRebinder configurationPropertiesRebinder(
-			ConfigurationPropertiesBeans beans,
-			ConfigurationPropertiesBindingPostProcessor binder) {
+			ConfigurationPropertiesBeans beans) {
 		ConfigurationPropertiesRebinder rebinder = new ConfigurationPropertiesRebinder(
-				binder, beans);
+				beans);
 		return rebinder;
 	}
 
 	@Override
 	public void afterSingletonsInstantiated() {
-		// After all beans are initialized send a pre-emptive EnvironmentChangeEvent
-		// so that anything that needs to rebind gets a chance now (especially for
-		// beans in the parent context)
-		this.context.publishEvent(
-				new EnvironmentChangeEvent(Collections.<String> emptySet()));
+		// After all beans are initialized explicitly rebind beans from the parent
+		// so that changes during the initialization of the current context are
+		// reflected. In particular this can be important when low level services like
+		// decryption are bootstrapped in the parent, but need to change their
+		// configuration before the child context is processed.
+		if (this.context.getParent() != null) {
+			// TODO: make this optional? (E.g. when creating child contexts that prefer to
+			// be isolated.)
+			ConfigurationPropertiesRebinder rebinder = context
+					.getBean(ConfigurationPropertiesRebinder.class);
+			for (String name : context.getParent().getBeanDefinitionNames()) {
+				rebinder.rebind(name);
+			}
+		}
 	}
 }

spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java

@@ -26,6 +26,7 @@ import java.util.TreeSet;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.bind.PropertySourcesPropertyValues;
 import org.springframework.boot.bind.RelaxedDataBinder;
@@ -107,7 +108,7 @@ public class PropertySourceBootstrapConfiguration implements
 			}
 			insertPropertySources(propertySources, composite);
 			reinitializeLoggingSystem(environment, logConfig, logFile);
-			setLogLevels(environment);
+			setLogLevels(applicationContext, environment);
 			handleIncludedProfiles(environment);
 		}
 	}
@@ -139,13 +140,14 @@ public class PropertySourceBootstrapConfiguration implements
 		}
 	}
 
-	private void setLogLevels(ConfigurableEnvironment environment) {
+	private void setLogLevels(ConfigurableApplicationContext applicationContext,
+			ConfigurableEnvironment environment) {
 		LoggingRebinder rebinder = new LoggingRebinder();
 		rebinder.setEnvironment(environment);
 		// We can't fire the event in the ApplicationContext here (too early), but we can
 		// create our own listener and poke it (it doesn't need the key changes)
-		rebinder.onApplicationEvent(
-				new EnvironmentChangeEvent(Collections.<String> emptySet()));
+		rebinder.onApplicationEvent(new EnvironmentChangeEvent(applicationContext,
+				Collections.<String>emptySet()));
 	}
 
 	private void insertPropertySources(MutablePropertySources propertySources,

spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java

@@ -25,6 +25,7 @@ import java.util.regex.Pattern;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.cloud.bootstrap.BootstrapApplicationListener;
 import org.springframework.cloud.context.environment.EnvironmentChangeEvent;
 import org.springframework.context.ApplicationContext;
@@ -96,8 +97,8 @@ public class EnvironmentDecryptApplicationInitializer implements
 		if (!map.isEmpty()) {
 			// We have some decrypted properties
 			found.addAll(map.keySet());
-			insert(applicationContext,
-					new SystemEnvironmentPropertySource(DECRYPTED_PROPERTY_SOURCE_NAME, map));
+			insert(applicationContext, new SystemEnvironmentPropertySource(
+					DECRYPTED_PROPERTY_SOURCE_NAME, map));
 		}
 		PropertySource<?> bootstrap = propertySources
 				.get(BootstrapApplicationListener.BOOTSTRAP_PROPERTY_SOURCE_NAME);
@@ -115,7 +116,7 @@ public class EnvironmentDecryptApplicationInitializer implements
 				// The parent is actually the bootstrap context, and it is fully
 				// initialized, so we can fire an EnvironmentChangeEvent there to rebind
 				// @ConfigurationProperties, in case they were encrypted.
-				parent.publishEvent(new EnvironmentChangeEvent(found));
+				parent.publishEvent(new EnvironmentChangeEvent(parent, found));
 			}
 
 		}
@@ -214,8 +215,10 @@ public class EnvironmentDecryptApplicationInitializer implements
 						if (COLLECTION_PROPERTY.matcher(key).matches()) {
 							sourceHasDecryptedCollection = true;
 						}
-					} else if (COLLECTION_PROPERTY.matcher(key).matches()){
-						// put non-ecrypted properties so merging of index properties happens correctly
+					}
+					else if (COLLECTION_PROPERTY.matcher(key).matches()) {
+						// put non-ecrypted properties so merging of index properties
+						// happens correctly
 						otherCollectionProperties.put(key, value);
 					}
 				}

spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentChangeEvent.java

@@ -32,10 +32,15 @@ public class EnvironmentChangeEvent extends ApplicationEvent {
 	private Set<String> keys;
 
 	public EnvironmentChangeEvent(Set<String> keys) {
-		super(keys);
+		// Backwards compatible constructor with less utility (practically no use at all)
+		this(keys, keys);
+	}
+
+	public EnvironmentChangeEvent(Object context, Set<String> keys) {
+		super(context);
 		this.keys = keys;
 	}
-	
+
 	/**
 	 * @return the keys
 	 */

spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentManager.java

@@ -68,7 +68,7 @@ public class EnvironmentManager implements ApplicationEventPublisherAware {
 		Map<String, Object> result = new LinkedHashMap<String, Object>(map);
 		if (!map.isEmpty()) {
 			map.clear();
-			publish(new EnvironmentChangeEvent(result.keySet()));
+			publish(new EnvironmentChangeEvent(publisher, result.keySet()));
 		}
 		return result;
 	}
@@ -88,7 +88,7 @@ public class EnvironmentManager implements ApplicationEventPublisherAware {
 
 		if (!value.equals(environment.getProperty(name))) {
 			map.put(name, value);
-			publish(new EnvironmentChangeEvent(Collections.singleton(name)));
+			publish(new EnvironmentChangeEvent(publisher, Collections.singleton(name)));
 		}
 
 	}

spring-cloud-context/src/main/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinder.java

@@ -24,7 +24,6 @@ import org.springframework.aop.framework.Advised;
 import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.BeansException;
 import org.springframework.boot.context.properties.ConfigurationProperties;
-import org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor;
 import org.springframework.cloud.context.config.annotation.RefreshScope;
 import org.springframework.cloud.context.environment.EnvironmentChangeEvent;
 import org.springframework.context.ApplicationContext;
@@ -55,16 +54,11 @@ public class ConfigurationPropertiesRebinder
 
 	private ConfigurationPropertiesBeans beans;
 
-	private ConfigurationPropertiesBindingPostProcessor binder;
-
 	private ApplicationContext applicationContext;
 
 	private Map<String, Exception> errors = new ConcurrentHashMap<>();
 
-	public ConfigurationPropertiesRebinder(
-			ConfigurationPropertiesBindingPostProcessor binder,
-			ConfigurationPropertiesBeans beans) {
-		this.binder = binder;
+	public ConfigurationPropertiesRebinder(ConfigurationPropertiesBeans beans) {
 		this.beans = beans;
 	}
 
@@ -102,7 +96,7 @@ public class ConfigurationPropertiesRebinder
 				if (AopUtils.isCglibProxy(bean)) {
 					bean = getTargetObject(bean);
 				}
-				this.binder.postProcessBeforeInitialization(bean, name);
+				this.applicationContext.getAutowireCapableBeanFactory().destroyBean(bean);
 				this.applicationContext.getAutowireCapableBeanFactory()
 						.initializeBean(bean, name);
 				return true;
@@ -135,7 +129,11 @@ public class ConfigurationPropertiesRebinder
 
 	@Override
 	public void onApplicationEvent(EnvironmentChangeEvent event) {
-		rebind();
+		if (this.applicationContext.equals(event.getSource())
+				// Backwards compatible
+				|| event.getKeys().equals(event.getSource())) {
+			rebind();
+		}
 	}
 
 }

spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java

@@ -34,9 +34,15 @@ public class ContextRefresher {
 
 	private static final String REFRESH_ARGS_PROPERTY_SOURCE = "refreshArgs";
 
-	private static final String[] DEFAULT_PROPERTY_SOURCES = new String[] { //order matters, cli args aren't first, things get messy
+	private static final String[] DEFAULT_PROPERTY_SOURCES = new String[] { // order
+																			// matters,
+																			// cli args
+																			// aren't
+																			// first,
+																			// things get
+																			// messy
 			CommandLinePropertySource.COMMAND_LINE_PROPERTY_SOURCE_NAME,
-			"defaultProperties"};
+			"defaultProperties" };
 
 	private Set<String> standardSources = new HashSet<>(
 			Arrays.asList(StandardEnvironment.SYSTEM_PROPERTIES_PROPERTY_SOURCE_NAME,
@@ -59,7 +65,7 @@ public class ContextRefresher {
 		addConfigFilesToEnvironment();
 		Set<String> keys = changes(before,
 				extract(this.context.getEnvironment().getPropertySources())).keySet();
-		this.context.publishEvent(new EnvironmentChangeEvent(keys));
+		this.context.publishEvent(new EnvironmentChangeEvent(context, keys));
 		this.scope.refreshAll();
 		return keys;
 	}

spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java

@@ -19,7 +19,9 @@ import javax.annotation.PostConstruct;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
+
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
@@ -33,17 +35,22 @@ import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.junit4.SpringRunner;
 
 import static org.junit.Assert.assertEquals;
 
 @RunWith(SpringRunner.class)
 @SpringBootTest(classes = TestConfiguration.class)
+@ActiveProfiles("config")
 public class ConfigurationPropertiesRebinderIntegrationTests {
 
 	@Autowired
 	private TestProperties properties;
 
+	@Autowired
+	private ConfigProperties config;
+
 	@Autowired
 	private ConfigurationPropertiesRebinder rebinder;
 
@@ -54,44 +61,55 @@ public class ConfigurationPropertiesRebinderIntegrationTests {
 	@DirtiesContext
 	public void testSimpleProperties() throws Exception {
 		assertEquals("Hello scope!", this.properties.getMessage());
-		assertEquals(2, this.properties.getCount());
+		assertEquals(1, this.properties.getCount());
 		// Change the dynamic property source...
 		EnvironmentTestUtils.addEnvironment(this.environment, "message:Foo");
 		// ...but don't refresh, so the bean stays the same:
 		assertEquals("Hello scope!", this.properties.getMessage());
-		assertEquals(2, this.properties.getCount());
+		assertEquals(1, this.properties.getCount());
+	}
+
+	@Test
+	@DirtiesContext
+	public void testRefreshInParent() throws Exception {
+		assertEquals("main", this.config.getName());
+		// Change the dynamic property source...
+		EnvironmentTestUtils.addEnvironment(this.environment, "config.name=foo");
+		// ...and then refresh, so the bean is re-initialized:
+		this.rebinder.rebind();
+		assertEquals("foo", this.config.getName());
 	}
 
 	@Test
 	@DirtiesContext
 	public void testRefresh() throws Exception {
-		assertEquals(2, this.properties.getCount());
+		assertEquals(1, this.properties.getCount());
 		assertEquals("Hello scope!", this.properties.getMessage());
 		// Change the dynamic property source...
 		EnvironmentTestUtils.addEnvironment(this.environment, "message:Foo");
 		// ...and then refresh, so the bean is re-initialized:
 		this.rebinder.rebind();
 		assertEquals("Foo", this.properties.getMessage());
-		assertEquals(3, this.properties.getCount());
+		assertEquals(2, this.properties.getCount());
 	}
 
 	@Test
 	@DirtiesContext
 	public void testRefreshByName() throws Exception {
-		assertEquals(2, this.properties.getCount());
+		assertEquals(1, this.properties.getCount());
 		assertEquals("Hello scope!", this.properties.getMessage());
 		// Change the dynamic property source...
 		EnvironmentTestUtils.addEnvironment(this.environment, "message:Foo");
 		// ...and then refresh, so the bean is re-initialized:
 		this.rebinder.rebind("properties");
 		assertEquals("Foo", this.properties.getMessage());
-		assertEquals(3, this.properties.getCount());
+		assertEquals(2, this.properties.getCount());
 	}
 
 	@Configuration
 	@EnableConfigurationProperties
 	@Import({ RefreshConfiguration.RebinderConfiguration.class,
-		PropertyPlaceholderAutoConfiguration.class })
+			PropertyPlaceholderAutoConfiguration.class })
 	protected static class TestConfiguration {
 
 		@Bean
@@ -104,8 +122,8 @@ public class ConfigurationPropertiesRebinderIntegrationTests {
 	// Hack out a protected inner class for testing
 	protected static class RefreshConfiguration extends RefreshAutoConfiguration {
 		@Configuration
-		protected static class RebinderConfiguration extends
-		ConfigurationPropertiesRebinderAutoConfiguration {
+		protected static class RebinderConfiguration
+				extends ConfigurationPropertiesRebinderAutoConfiguration {
 
 		}
 	}
@@ -142,4 +160,17 @@ public class ConfigurationPropertiesRebinderIntegrationTests {
 		}
 	}
 
+	@ConfigurationProperties("config")
+	@ConditionalOnMissingBean(ConfigProperties.class)
+	public static class ConfigProperties {
+		private String name;
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+	}
 }

spring-cloud-context/src/test/java/org/springframework/cloud/logging/LoggingRebinderTests.java

@@ -21,6 +21,7 @@ import org.junit.After;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import org.springframework.boot.logging.LogLevel;
 import org.springframework.boot.logging.LoggingSystem;
 import org.springframework.boot.test.util.EnvironmentTestUtils;
@@ -52,7 +53,7 @@ public class LoggingRebinderTests {
 		EnvironmentTestUtils.addEnvironment(environment,
 				"logging.level.org.springframework.web=TRACE");
 		this.rebinder.setEnvironment(environment);
-		this.rebinder.onApplicationEvent(new EnvironmentChangeEvent(
+		this.rebinder.onApplicationEvent(new EnvironmentChangeEvent(environment,
 				Collections.singleton("logging.level.org.springframework.web")));
 		assertTrue(this.logger.isTraceEnabled());
 	}
@@ -64,7 +65,7 @@ public class LoggingRebinderTests {
 		EnvironmentTestUtils.addEnvironment(environment,
 				"logging.level.org.springframework.web=trace");
 		this.rebinder.setEnvironment(environment);
-		this.rebinder.onApplicationEvent(new EnvironmentChangeEvent(
+		this.rebinder.onApplicationEvent(new EnvironmentChangeEvent(environment,
 				Collections.singleton("logging.level.org.springframework.web")));
 		assertTrue(this.logger.isTraceEnabled());
 	}

spring-cloud-context/src/test/resources/application-config.properties

@@ -0,0 +1 @@
+config.name: main

spring-cloud-context/src/test/resources/bootstrap-config.properties

@@ -0,0 +1,2 @@
+spring.main.sources: org.springframework.cloud.context.properties.ConfigurationPropertiesRebinderIntegrationTests.ConfigProperties
+config.name: parent