Fix same name configuration properties adapting

Update `SpringConfigurationPropertySources` adapter logic so that and
exact instance match is used when checking if the cache can be used.

Prior to this commit if a property source was replaced with one that
had exactly the same name, the adapted version would not change.

Fixes gh-12451

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

@@ -124,7 +124,8 @@ public final class ConfigurationPropertySources {
 	 * This method will flatten any nested property sources and will filter all
 	 * {@link StubPropertySource stub property sources}. Updates to the underlying source,
 	 * identified by changes in the sources returned by its iterator, will be
-	 * automatically tracked.
+	 * automatically tracked. The underlying source should be thread safe, for example a
+	 * {@link MutablePropertySources}
 	 * @param sources the Spring property sources to adapt
 	 * @return an {@link Iterable} containing newly adapted
 	 * {@link SpringConfigurationPropertySource} instances

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -16,19 +16,20 @@
 
 package org.springframework.boot.context.properties.source;
 
-import java.util.ArrayList;
+import java.util.ArrayDeque;
+import java.util.Deque;
 import java.util.Iterator;
-import java.util.List;
+import java.util.Map;
-import java.util.stream.Collectors;
+import java.util.NoSuchElementException;
-import java.util.stream.Stream;
+import java.util.function.Function;
-import java.util.stream.StreamSupport;
 
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.MutablePropertySources;
 import org.springframework.core.env.PropertySource;
 import org.springframework.core.env.PropertySource.StubPropertySource;
 import org.springframework.util.Assert;
-import org.springframework.util.ObjectUtils;
+import org.springframework.util.ConcurrentReferenceHashMap;
+import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType;
 
 /**
  * Adapter to convert Spring's {@link MutablePropertySources} to
@@ -41,9 +42,8 @@ class SpringConfigurationPropertySources
 
 	private final Iterable<PropertySource<?>> sources;
 
-	private volatile PropertySourcesKey lastKey;
+	private final Map<PropertySource<?>, ConfigurationPropertySource> cache = new ConcurrentReferenceHashMap<>(
-
+			16, ReferenceType.SOFT);
-	private volatile List<ConfigurationPropertySource> adaptedSources;
 
 	SpringConfigurationPropertySources(Iterable<PropertySource<?>> sources) {
 		Assert.notNull(sources, "Sources must not be null");
@@ -52,107 +52,81 @@ class SpringConfigurationPropertySources
 
 	@Override
 	public Iterator<ConfigurationPropertySource> iterator() {
-		checkForChanges();
+		return new SourcesIterator(this.sources.iterator(), this::adapt);
-		return this.adaptedSources.iterator();
-	}
-
-	private void checkForChanges() {
-		PropertySourcesKey lastKey = this.lastKey;
-		PropertySourcesKey currentKey = new PropertySourcesKey(this.sources);
-		if (!currentKey.equals(lastKey)) {
-			onChange(this.sources);
-			this.lastKey = currentKey;
-		}
-	}
-
-	private void onChange(Iterable<PropertySource<?>> sources) {
-		this.adaptedSources = streamPropertySources(sources)
-				.map(SpringConfigurationPropertySource::from)
-				.collect(Collectors.toList());
 	}
 
-	private Stream<PropertySource<?>> streamPropertySources(
+	private ConfigurationPropertySource adapt(PropertySource<?> source) {
-			Iterable<PropertySource<?>> sources) {
+		ConfigurationPropertySource result = this.cache.get(source);
-		return StreamSupport.stream(sources.spliterator(), false).flatMap(this::flatten)
+		// Most PropertySource test quality only using the source name, we need to
-				.filter(this::isIncluded);
+		// check the actual source hasn't also changed.
-	}
+		if (result != null && result.getUnderlyingSource() == source) {
-
+			return result;
-	private Stream<PropertySource<?>> flatten(PropertySource<?> source) {
-		if (source.getSource() instanceof ConfigurableEnvironment) {
-			return streamPropertySources(
-					((ConfigurableEnvironment) source.getSource()).getPropertySources());
 		}
-		return Stream.of(source);
+		result = SpringConfigurationPropertySource.from(source);
+		this.cache.put(source, result);
+		return result;
 	}
 
-	private boolean isIncluded(PropertySource<?> source) {
+	private static class SourcesIterator
-		return !(source instanceof StubPropertySource)
+			implements Iterator<ConfigurationPropertySource> {
-				&& !(source instanceof ConfigurationPropertySourcesPropertySource);
-	}
 
-	private static class PropertySourcesKey {
+		private Deque<Iterator<PropertySource<?>>> iterators;
 
-		private final List<PropertySourceKey> keys = new ArrayList<>();
+		private ConfigurationPropertySource next;
 
-		PropertySourcesKey(Iterable<PropertySource<?>> sources) {
+		private final Function<PropertySource<?>, ConfigurationPropertySource> adapter;
-			sources.forEach(this::addKey);
-		}
 
-		private void addKey(PropertySource<?> source) {
+		SourcesIterator(Iterator<PropertySource<?>> iterator,
-			this.keys.add(new PropertySourceKey(source));
+				Function<PropertySource<?>, ConfigurationPropertySource> adapter) {
+			this.iterators = new ArrayDeque<>(4);
+			this.iterators.push(iterator);
+			this.adapter = adapter;
 		}
 
 		@Override
-		public int hashCode() {
+		public boolean hasNext() {
-			return this.keys.hashCode();
+			return fetchNext() != null;
 		}
 
 		@Override
-		public boolean equals(Object obj) {
+		public ConfigurationPropertySource next() {
-			if (this == obj) {
+			ConfigurationPropertySource next = fetchNext();
-				return true;
+			this.next = null;
-			}
+			if (next == null) {
-			if (obj == null || getClass() != obj.getClass()) {
+				throw new NoSuchElementException();
-				return false;
 			}
-			return this.keys.equals(((PropertySourcesKey) obj).keys);
+			return next;
 		}
 
-	}
+		private ConfigurationPropertySource fetchNext() {
-
+			if (this.next == null) {
-	private static class PropertySourceKey {
+				if (this.iterators.isEmpty()) {
-
+					return null;
-		private final String name;
+				}
-
+				if (!this.iterators.peek().hasNext()) {
-		private final Class<?> type;
+					this.iterators.pop();
-
+					return fetchNext();
-		PropertySourceKey(PropertySource<?> source) {
+				}
-			this.name = source.getName();
+				PropertySource<?> candidate = this.iterators.peek().next();
-			this.type = source.getClass();
+				if (candidate.getSource() instanceof ConfigurableEnvironment) {
+					push((ConfigurableEnvironment) candidate.getSource());
+					return fetchNext();
+				}
+				if (isIgnored(candidate)) {
+					return fetchNext();
+				}
+				this.next = this.adapter.apply(candidate);
+			}
+			return this.next;
 		}
 
-		@Override
+		private void push(ConfigurableEnvironment environment) {
-		public int hashCode() {
+			this.iterators.push(environment.getPropertySources().iterator());
-			final int prime = 31;
-			int result = 1;
-			result = prime * result + ObjectUtils.nullSafeHashCode(this.name);
-			result = prime * result + ObjectUtils.nullSafeHashCode(this.type);
-			return result;
 		}
 
-		@Override
+		private boolean isIgnored(PropertySource<?> candidate) {
-		public boolean equals(Object obj) {
+			return (candidate instanceof StubPropertySource
-			if (this == obj) {
+					|| candidate instanceof ConfigurationPropertySourcesPropertySource);
-				return true;
-			}
-			if (obj == null || getClass() != obj.getClass()) {
-				return false;
-			}
-			PropertySourceKey other = (PropertySourceKey) obj;
-			boolean result = true;
-			result = result && ObjectUtils.nullSafeEquals(this.name, other.name);
-			result = result && ObjectUtils.nullSafeEquals(this.type, other.type);
-			return result;
 		}
 
 	}

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

@@ -152,12 +152,36 @@ public class SpringConfigurationPropertySourcesTests {
 	@Test
 	public void shouldTrackChanges() {
 		MutablePropertySources sources = new MutablePropertySources();
-		sources.addLast(
+		SpringConfigurationPropertySources configurationSources = new SpringConfigurationPropertySources(
-				new MapPropertySource("test1", Collections.singletonMap("a", "b")));
+				sources);
-		assertThat(new SpringConfigurationPropertySources(sources).iterator()).hasSize(1);
+		assertThat(configurationSources.iterator()).hasSize(0);
-		sources.addLast(
+		MapPropertySource source1 = new MapPropertySource("test1",
-				new MapPropertySource("test2", Collections.singletonMap("b", "c")));
+				Collections.singletonMap("a", "b"));
-		assertThat(new SpringConfigurationPropertySources(sources).iterator()).hasSize(2);
+		sources.addLast(source1);
+		assertThat(configurationSources.iterator()).hasSize(1);
+		MapPropertySource source2 = new MapPropertySource("test2",
+				Collections.singletonMap("b", "c"));
+		sources.addLast(source2);
+		assertThat(configurationSources.iterator()).hasSize(2);
+	}
+
+	@Test
+	public void shouldTrackWhenSourceHasIdenticalName() {
+		MutablePropertySources sources = new MutablePropertySources();
+		SpringConfigurationPropertySources configurationSources = new SpringConfigurationPropertySources(
+				sources);
+		ConfigurationPropertyName name = ConfigurationPropertyName.of("a");
+		MapPropertySource source1 = new MapPropertySource("test",
+				Collections.singletonMap("a", "s1"));
+		sources.addLast(source1);
+		assertThat(configurationSources.iterator().next().getConfigurationProperty(name)
+				.getValue()).isEqualTo("s1");
+		MapPropertySource source2 = new MapPropertySource("test",
+				Collections.singletonMap("a", "s2"));
+		sources.remove("test");
+		sources.addLast(source2);
+		assertThat(configurationSources.iterator().next().getConfigurationProperty(name)
+				.getValue()).isEqualTo("s2");
 	}
 
 }