Make configuration properties binding always uses current environment

Previously, configuration properties binding would only see changes to
the environment if there was no PropertySourcesPlaceholderConfigurer
in the context. This happened because
PropertySourcesPlaceholderConfigurer wrapped the Environment in a
PropertySource, effectively hiding it from the change tracking
performed by SpringConfigurationPropertySources.

This commit updates ConfigurationPropertiesBindingPostProcessor so
that it ignores the environment property source contained by
PropertySourcesPlaceholderConfigurer and uses a composite of
the PropertySourcesPlaceholderConfigurer's other property sources and
the Environment's PropertySources instead.

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/CompositePropertySources.java

+/*
+ * Copyright 2012-2017 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.context.properties;
+
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+
+import org.springframework.core.env.PropertySource;
+import org.springframework.core.env.PropertySources;
+
+/**
+ * A composite {@link PropertySources} backed by one or more {@code PropertySources}.
+ * Changes to the backing {@code PropertySources} are automatically reflected in the
+ * composite.
+ *
+ * @author Andy Wilkinson
+ */
+final class CompositePropertySources implements PropertySources {
+
+	private final List<PropertySources> propertySources;
+
+	CompositePropertySources(PropertySources... propertySources) {
+		this.propertySources = Arrays.asList(propertySources);
+	}
+
+	@Override
+	public Iterator<PropertySource<?>> iterator() {
+		return this.propertySources.stream()
+				.flatMap((sources) -> StreamSupport.stream(sources.spliterator(), false))
+				.collect(Collectors.toList()).iterator();
+	}
+
+	@Override
+	public boolean contains(String name) {
+		for (PropertySources sources : this.propertySources) {
+			if (sources.contains(name)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	@Override
+	public PropertySource<?> get(String name) {
+		for (PropertySources sources : this.propertySources) {
+			PropertySource<?> source = sources.get(name);
+			if (source != null) {
+				return source;
+			}
+		}
+		return null;
+	}
+
+}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java

@@ -28,7 +28,6 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyS
 import org.springframework.boot.context.properties.source.UnboundElementsSourceFilter;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.core.convert.ConversionService;
-import org.springframework.core.env.MutablePropertySources;
 import org.springframework.core.env.PropertySource;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -59,14 +58,7 @@ public class ConfigurationPropertiesBinder {
 		this.propertySources = propertySources;
 		this.conversionService = conversionService;
 		this.validator = validator;
-		if (propertySources instanceof MutablePropertySources) {
-			this.configurationSources = ConfigurationPropertySources
-					.from((MutablePropertySources) propertySources);
-		}
-		else {
-			this.configurationSources = ConfigurationPropertySources
-					.from(propertySources);
-		}
+		this.configurationSources = ConfigurationPropertySources.from(propertySources);
 	}
 
 	/**

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java

@@ -38,6 +38,7 @@ import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.core.convert.ConversionService;
 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.PropertySources;
 import org.springframework.core.env.StandardEnvironment;
@@ -150,15 +151,30 @@ public class ConfigurationPropertiesBindingPostProcessor
 	}
 
 	private PropertySources deducePropertySources() {
+		MutablePropertySources environmentPropertySources = extractEnvironmentPropertySources();
 		PropertySourcesPlaceholderConfigurer configurer = getSinglePropertySourcesPlaceholderConfigurer();
-		if (configurer != null) {
-			return configurer.getAppliedPropertySources();
+		if (configurer == null) {
+			if (environmentPropertySources != null) {
+				return environmentPropertySources;
+			}
+			throw new IllegalStateException("Unable to obtain PropertySources from "
+					+ "PropertySourcesPlaceholderConfigurer or Environment");
 		}
+		PropertySources appliedPropertySources = configurer.getAppliedPropertySources();
+		return environmentPropertySources == null ? appliedPropertySources
+				: new CompositePropertySources(
+						new FilteredPropertySources(appliedPropertySources,
+								PropertySourcesPlaceholderConfigurer.ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME),
+						environmentPropertySources);
+	}
+
+	private MutablePropertySources extractEnvironmentPropertySources() {
+		MutablePropertySources environmentPropertySources = null;
 		if (this.environment instanceof ConfigurableEnvironment) {
-			return ((ConfigurableEnvironment) this.environment).getPropertySources();
+			environmentPropertySources = ((ConfigurableEnvironment) this.environment)
+					.getPropertySources();
 		}
-		throw new IllegalStateException("Unable to obtain PropertySources from "
-				+ "PropertySourcesPlaceholderConfigurer or Environment");
+		return environmentPropertySources;
 	}
 
 	private PropertySourcesPlaceholderConfigurer getSinglePropertySourcesPlaceholderConfigurer() {

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/FilteredPropertySources.java

+/*
+ * Copyright 2012-2017 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.context.properties;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+
+import org.springframework.core.env.PropertySource;
+import org.springframework.core.env.PropertySources;
+
+/**
+ * A {@link PropertySources} decorator that filters property sources by name.
+ *
+ * @author Andy Wilkinson
+ */
+final class FilteredPropertySources implements PropertySources {
+
+	private final Set<String> filtered;
+
+	private final PropertySources delegate;
+
+	FilteredPropertySources(PropertySources delegate, String... filtered) {
+		this.delegate = delegate;
+		this.filtered = new HashSet<>(Arrays.asList(filtered));
+	}
+
+	@Override
+	public Iterator<PropertySource<?>> iterator() {
+		return StreamSupport.stream(this.delegate.spliterator(), false)
+				.filter(this::included).collect(Collectors.toList()).iterator();
+	}
+
+	@Override
+	public boolean contains(String name) {
+		if (included(name)) {
+			return this.delegate.contains(name);
+		}
+		return false;
+	}
+
+	@Override
+	public PropertySource<?> get(String name) {
+		if (included(name)) {
+			return this.delegate.get(name);
+		}
+		return null;
+	}
+
+	private boolean included(PropertySource<?> propertySource) {
+		return included(propertySource.getName());
+	}
+
+	private boolean included(String name) {
+		return (!this.filtered.contains(name));
+	}
+
+}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertySources.java

@@ -17,7 +17,6 @@
 package org.springframework.boot.context.properties.source;
 
 import java.util.Collections;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
@@ -113,36 +112,21 @@ public final class ConfigurationPropertySources {
 		return Collections.singleton(SpringConfigurationPropertySource.from(source));
 	}
 
-	/**
-	 * Return {@link Iterable} containing new {@link ConfigurationPropertySource}
-	 * instances adapted from the given Spring {@link MutablePropertySources}.
-	 * <p>
-	 * This method will flatten any nested property sources and will filter all
-	 * {@link StubPropertySource stub property sources}. Updates to the underlying source
-	 * will be automatically tracked.
-	 * @param sources the Spring property sources to adapt
-	 * @return an {@link Iterable} containing newly adapted
-	 * {@link SpringConfigurationPropertySource} instances
-	 */
-	public static Iterable<ConfigurationPropertySource> from(
-			MutablePropertySources sources) {
-		return new SpringConfigurationPropertySources(sources);
-	}
-
 	/**
 	 * Return {@link Iterable} containing new {@link ConfigurationPropertySource}
 	 * instances adapted from the given Spring {@link PropertySource PropertySources}.
 	 * <p>
 	 * This method will flatten any nested property sources and will filter all
-	 * {@link StubPropertySource stub property sources}.
+	 * {@link StubPropertySource stub property sources}. Updates to the underlying source,
+	 * identified by changes in the sources returned by its iterator, will be
+	 * automatically tracked.
 	 * @param sources the Spring property sources to adapt
 	 * @return an {@link Iterable} containing newly adapted
 	 * {@link SpringConfigurationPropertySource} instances
 	 */
 	public static Iterable<ConfigurationPropertySource> from(
 			Iterable<PropertySource<?>> sources) {
-		return streamPropertySources(sources).map(SpringConfigurationPropertySource::from)
-				.collect(Collectors.toList());
+		return new SpringConfigurationPropertySources(sources);
 	}
 
 	private static Stream<PropertySource<?>> streamPropertySources(

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySources.java

@@ -39,13 +39,13 @@ import org.springframework.util.ObjectUtils;
 class SpringConfigurationPropertySources
 		implements Iterable<ConfigurationPropertySource> {
 
-	private final MutablePropertySources sources;
+	private final Iterable<PropertySource<?>> sources;
 
 	private volatile PropertySourcesKey lastKey;
 
 	private volatile List<ConfigurationPropertySource> adaptedSources;
 
-	SpringConfigurationPropertySources(MutablePropertySources sources) {
+	SpringConfigurationPropertySources(Iterable<PropertySource<?>> sources) {
 		Assert.notNull(sources, "Sources must not be null");
 		this.sources = sources;
 	}
@@ -65,7 +65,7 @@ class SpringConfigurationPropertySources
 		}
 	}
 
-	private void onChange(MutablePropertySources sources) {
+	private void onChange(Iterable<PropertySource<?>> sources) {
 		this.adaptedSources = streamPropertySources(sources)
 				.map(SpringConfigurationPropertySource::from)
 				.collect(Collectors.toList());
@@ -94,7 +94,7 @@ class SpringConfigurationPropertySources
 
 		private final List<PropertySourceKey> keys = new ArrayList<>();
 
-		PropertySourcesKey(MutablePropertySources sources) {
+		PropertySourcesKey(Iterable<PropertySource<?>> sources) {
 			sources.forEach(this::addKey);
 		}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/CompositePropertySourcesTests.java

+/*
+ * Copyright 2012-2017 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.context.properties;
+
+import java.util.Collections;
+
+import org.junit.Test;
+
+import org.springframework.core.env.MapPropertySource;
+import org.springframework.core.env.MutablePropertySources;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link CompositePropertySources}.
+ *
+ * @author Andy Wilkinson
+ */
+public class CompositePropertySourcesTests {
+
+	@Test
+	public void containsReturnsFalseWithNoBackingSources() {
+		assertThat(new CompositePropertySources().contains("foo")).isFalse();
+	}
+
+	@Test
+	public void getReturnsNullWithNoBackingSources() {
+		assertThat(new CompositePropertySources().get("foo")).isNull();
+	}
+
+	@Test
+	public void iteratorIsEmptyWithNoBackingSources() {
+		assertThat(new CompositePropertySources().iterator()).hasSize(0);
+	}
+
+	@Test
+	public void containsReturnsTrueForPropertySourceFoundInBackingSources() {
+		MutablePropertySources sources = new MutablePropertySources();
+		sources.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		assertThat(new CompositePropertySources(sources).contains("foo")).isTrue();
+	}
+
+	@Test
+	public void containsReturnsFalseForPropertySourceNotFoundInBackingSources() {
+		MutablePropertySources sources = new MutablePropertySources();
+		sources.addFirst(new MapPropertySource("bar", Collections.emptyMap()));
+		assertThat(new CompositePropertySources(sources).contains("foo")).isFalse();
+	}
+
+	@Test
+	public void getReturnsPropertySourceFoundInBackingSources() {
+		MutablePropertySources sources = new MutablePropertySources();
+		MapPropertySource fooSource = new MapPropertySource("foo",
+				Collections.emptyMap());
+		sources.addFirst(fooSource);
+		assertThat(new CompositePropertySources(sources).get("foo")).isEqualTo(fooSource);
+	}
+
+	@Test
+	public void getReturnsNullWhenPropertySourceNotFoundInBackingSources() {
+		MutablePropertySources sources = new MutablePropertySources();
+		sources.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		assertThat(new CompositePropertySources(sources).get("bar")).isNull();
+	}
+
+	@Test
+	public void iteratorContainsSingleEntryWithSingleBackingSource() {
+		MutablePropertySources sources = new MutablePropertySources();
+		MapPropertySource fooSource = new MapPropertySource("foo",
+				Collections.emptyMap());
+		sources.addFirst(fooSource);
+		assertThat(new CompositePropertySources(sources).iterator())
+				.containsExactly(fooSource);
+	}
+
+	@Test
+	public void iteratorReflectsOrderingOfSourcesAcrossMultipleBackingSources() {
+		MutablePropertySources sourcesOne = new MutablePropertySources();
+		MapPropertySource fooSource = new MapPropertySource("foo",
+				Collections.emptyMap());
+		sourcesOne.addFirst(fooSource);
+		MapPropertySource barSource = new MapPropertySource("bar",
+				Collections.emptyMap());
+		sourcesOne.addFirst(barSource);
+		MutablePropertySources sourcesTwo = new MutablePropertySources();
+		MapPropertySource bazSource = new MapPropertySource("baz",
+				Collections.emptyMap());
+		sourcesTwo.addFirst(bazSource);
+		assertThat(new CompositePropertySources(sourcesOne, sourcesTwo).iterator())
+				.containsExactly(barSource, fooSource, bazSource);
+	}
+
+	@Test
+	public void containsReflectsChangesInTheBackingSources() {
+		MutablePropertySources sources = new MutablePropertySources();
+		sources.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		assertThat(new CompositePropertySources(sources).contains("bar")).isFalse();
+		MapPropertySource barSource = new MapPropertySource("bar",
+				Collections.emptyMap());
+		sources.addFirst(barSource);
+		assertThat(new CompositePropertySources(sources).contains("bar")).isTrue();
+	}
+
+	@Test
+	public void getReflectsChangesInTheBackingSources() {
+		MutablePropertySources sources = new MutablePropertySources();
+		sources.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		assertThat(new CompositePropertySources(sources).get("bar")).isNull();
+		MapPropertySource barSource = new MapPropertySource("bar",
+				Collections.emptyMap());
+		sources.addFirst(barSource);
+		assertThat(new CompositePropertySources(sources).get("bar")).isEqualTo(barSource);
+	}
+
+	@Test
+	public void iteratorReflectsChangesInTheBackingSources() {
+		MutablePropertySources sources = new MutablePropertySources();
+		MapPropertySource fooSource = new MapPropertySource("foo",
+				Collections.emptyMap());
+		sources.addFirst(fooSource);
+		assertThat(new CompositePropertySources(sources).iterator())
+				.containsExactly(fooSource);
+		MapPropertySource barSource = new MapPropertySource("bar",
+				Collections.emptyMap());
+		sources.addFirst(barSource);
+		assertThat(new CompositePropertySources(sources).iterator())
+				.containsExactly(barSource, fooSource);
+	}
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessorTests.java

@@ -217,6 +217,28 @@ public class ConfigurationPropertiesBindingPostProcessorTests {
 		assertThat(second.getTwo()).isEqualTo("baz");
 	}
 
+	@Test
+	public void rebindableConfigurationPropertiesWithPropertySourcesPlaceholderConfigurer()
+			throws Exception {
+		this.context = new AnnotationConfigApplicationContext();
+		MutablePropertySources sources = this.context.getEnvironment()
+				.getPropertySources();
+		Map<String, Object> source = new LinkedHashMap<>();
+		source.put("example.one", "foo");
+		sources.addFirst(new MapPropertySource("test-source", source));
+		this.context.register(PrototypePropertiesConfig.class);
+		this.context.register(PropertySourcesPlaceholderConfigurerConfiguration.class);
+		this.context.refresh();
+		PrototypeBean first = this.context.getBean(PrototypeBean.class);
+		assertThat(first.getOne()).isEqualTo("foo");
+		source.put("example.one", "bar");
+		sources.addFirst(new MapPropertySource("extra",
+				Collections.singletonMap("example.two", "baz")));
+		PrototypeBean second = this.context.getBean(PrototypeBean.class);
+		assertThat(second.getOne()).isEqualTo("bar");
+		assertThat(second.getTwo()).isEqualTo("baz");
+	}
+
 	@Test
 	public void converterIsFound() {
 		prepareConverterContext(ConverterConfiguration.class, PersonProperty.class);
@@ -382,6 +404,16 @@ public class ConfigurationPropertiesBindingPostProcessorTests {
 
 	}
 
+	@Configuration
+	public static class PropertySourcesPlaceholderConfigurerConfiguration {
+
+		@Bean
+		public PropertySourcesPlaceholderConfigurer propertySourcesPlaceholderConfigurer() {
+			return new PropertySourcesPlaceholderConfigurer();
+		}
+
+	}
+
 	public static class PrototypeBean {
 
 		private String one;

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/FilteredPropertySourcesTests.java

+/*
+ * Copyright 2012-2017 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.context.properties;
+
+import java.util.Collections;
+
+import org.junit.Test;
+
+import org.springframework.core.env.MapPropertySource;
+import org.springframework.core.env.MutablePropertySources;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link FilteredPropertySources}.
+ *
+ * @author Andy Wilkinson
+ */
+public class FilteredPropertySourcesTests {
+
+	@Test
+	public void getReturnsNullForFilteredSource() {
+		MutablePropertySources delegate = new MutablePropertySources();
+		delegate.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		assertThat(new FilteredPropertySources(delegate, "foo").get("foo")).isNull();
+	}
+
+	@Test
+	public void getReturnsSourceThatIsNotFiltered() {
+		MutablePropertySources delegate = new MutablePropertySources();
+		delegate.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		MapPropertySource barSource = new MapPropertySource("bar",
+				Collections.emptyMap());
+		delegate.addFirst(barSource);
+		assertThat(new FilteredPropertySources(delegate, "foo").get("bar"))
+				.isEqualTo(barSource);
+	}
+
+	@Test
+	public void containsReturnsFalseForFilteredSource() {
+		MutablePropertySources delegate = new MutablePropertySources();
+		delegate.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		assertThat(new FilteredPropertySources(delegate, "foo").contains("foo"))
+				.isFalse();
+	}
+
+	@Test
+	public void containsReturnsTrueForSourceThatIsNotFiltered() {
+		MutablePropertySources delegate = new MutablePropertySources();
+		delegate.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		MapPropertySource barSource = new MapPropertySource("bar",
+				Collections.emptyMap());
+		delegate.addFirst(barSource);
+		assertThat(new FilteredPropertySources(delegate, "foo").contains("bar")).isTrue();
+	}
+
+	@Test
+	public void iteratorOmitsSourceThatIsFiltered() {
+		MutablePropertySources delegate = new MutablePropertySources();
+		MapPropertySource barSource = new MapPropertySource("bar",
+				Collections.emptyMap());
+		delegate.addFirst(barSource);
+		delegate.addFirst(new MapPropertySource("foo", Collections.emptyMap()));
+		MapPropertySource bazSource = new MapPropertySource("baz",
+				Collections.emptyMap());
+		delegate.addFirst(bazSource);
+		assertThat(new FilteredPropertySources(delegate, "foo").iterator())
+				.containsExactly(bazSource, barSource);
+	}
+
+}