From dc1c459cde74e73b621358ddf30c62110372595a Mon Sep 17 00:00:00 2001
From: Phillip Webb <pwebb@pivotal.io>
Date: Wed, 6 Jun 2018 14:55:20 -0700
Subject: [PATCH] Polish "Fix caching issues with map property sources"

Refine the property source cache key fix so that a copy of the
key is only taken when the values change. This allows us to
retain the previous performance optimization of not creating
unnecessary string arrays.

Closes gh-13344
---
 ...ngIterableConfigurationPropertySource.java | 55 ++++++++++++++++---
 ...rableConfigurationPropertySourceTests.java | 20 +++----
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java
index f4292fdf5481..f300fc045d45 100644
--- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java
+++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java
@@ -18,8 +18,10 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Stream;
 
 import org.springframework.core.env.EnumerablePropertySource;
@@ -129,7 +131,7 @@ private PropertyMapping[] getPropertyMappings(Cache cache) {
 	}
 
 	private Cache getCache() {
-		Object cacheKey = getCacheKey();
+		CacheKey cacheKey = CacheKey.get(getPropertySource());
 		if (cacheKey == null) {
 			return null;
 		}
@@ -137,15 +139,10 @@ private Cache getCache() {
 			return this.cache;
 		}
 		this.cache = new Cache();
-		this.cacheKey = cacheKey;
+		this.cacheKey = cacheKey.copy();
 		return this.cache;
 	}
 
-	private Object getCacheKey() {
-		// gh-13344
-		return getPropertySource().getPropertyNames();
-	}
-
 	@Override
 	protected EnumerablePropertySource<?> getPropertySource() {
 		return (EnumerablePropertySource<?>) super.getPropertySource();
@@ -175,4 +172,48 @@ public void setMappings(PropertyMapping[] mappings) {
 
 	}
 
+	private static final class CacheKey {
+
+		private Object key;
+
+		private CacheKey(Object key) {
+			this.key = key;
+		}
+
+		public CacheKey copy() {
+			return new CacheKey(copyKey(this.key));
+		}
+
+		private Object copyKey(Object key) {
+			if (key instanceof Set) {
+				return new HashSet<Object>((Set<?>) key);
+			}
+			return ((String[]) key).clone();
+		}
+
+		@Override
+		public int hashCode() {
+			return this.key.hashCode();
+		}
+
+		@Override
+		public boolean equals(Object obj) {
+			if (this == obj) {
+				return true;
+			}
+			if (obj == null || getClass() != obj.getClass()) {
+				return false;
+			}
+			return ObjectUtils.nullSafeEquals(this.key, ((CacheKey) obj).key);
+		}
+
+		public static CacheKey get(EnumerablePropertySource<?> source) {
+			if (source instanceof MapPropertySource) {
+				return new CacheKey(((MapPropertySource) source).getSource().keySet());
+			}
+			return new CacheKey(source.getPropertyNames());
+		}
+
+	}
+
 }
diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java
index abd59f3c2bc0..f75fe647f4ac 100644
--- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java
+++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySourceTests.java
@@ -158,21 +158,17 @@ public void containsDescendantOfShouldCheckSourceNames() {
 				.isEqualTo(ConfigurationPropertyState.ABSENT);
 	}
 
-	@SuppressWarnings("unchecked")
 	@Test
-	public void propertySourceChangeReflects() {
+	public void propertySourceKeyDataChangeInvalidatesCache() {
 		// gh-13344
-		final Map<String, Object> source = new LinkedHashMap<>();
-		source.put("key1", "value1");
-		source.put("key2", "value2");
-		final EnumerablePropertySource<?> propertySource = new MapPropertySource("test",
-				source);
-		final SpringIterableConfigurationPropertySource adapter = new SpringIterableConfigurationPropertySource(
-				propertySource, DefaultPropertyMapper.INSTANCE);
+		Map<String, Object> map = new LinkedHashMap<>();
+		map.put("key1", "value1");
+		map.put("key2", "value2");
+		EnumerablePropertySource<?> source = new MapPropertySource("test", map);
+		SpringIterableConfigurationPropertySource adapter = new SpringIterableConfigurationPropertySource(
+				source, DefaultPropertyMapper.INSTANCE);
 		assertThat(adapter.stream().count()).isEqualTo(2);
-
-		((Map<String, Object>) adapter.getPropertySource().getSource()).put("key3",
-				"value3");
+		map.put("key3", "value3");
 		assertThat(adapter.stream().count()).isEqualTo(3);
 	}