From c556d2b58f90bac11d2d87a4f16b27e4592d80a2 Mon Sep 17 00:00:00 2001 From: Fahim Farook Date: Mon, 4 Jun 2018 00:06:13 +0530 Subject: [PATCH] Fix caching issues with map property sources Update `SpringIterableConfigurationPropertySource` so that the cache key from a `MapPropertySource` is invalidated when the map contents changes. Prior to this commit, the actual keys of the map were used as the key. This meant that if the underlying map changed, they key wouldn't be invalidated because it ultimately pointed to the same object instance. See gh-13344 --- ...ngIterableConfigurationPropertySource.java | 4 +--- .../LoggingApplicationListenerTests.java | 14 +++++++++++++- ...rableConfigurationPropertySourceTests.java | 19 +++++++++++++++++++ 3 files changed, 33 insertions(+), 4 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 04ee92ff8b..f4292fdf54 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 @@ -142,9 +142,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope } private Object getCacheKey() { - if (getPropertySource() instanceof MapPropertySource) { - return ((MapPropertySource) getPropertySource()).getSource().keySet(); - } + // gh-13344 return getPropertySource().getPropertyNames(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java index f05cb1f5ec..276fd8ac68 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java @@ -76,6 +76,7 @@ import static org.hamcrest.Matchers.not; * @author Andy Wilkinson * @author Stephane Nicoll * @author Ben Hale + * @author Fahim Farook */ @RunWith(ModifiedClassPathRunner.class) @ClassPathExclusions("log4j*.jar") @@ -500,12 +501,23 @@ public class LoggingApplicationListenerTests { @Test public void environmentPropertiesIgnoreUnresolvablePlaceholders() { // gh-7719 + TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.context, + "logging.pattern.console=console ${doesnotexist}"); + this.initializer.initialize(this.context.getEnvironment(), + this.context.getClassLoader()); + assertThat(System.getProperty(LoggingSystemProperties.CONSOLE_LOG_PATTERN)) + .isEqualTo("console ${doesnotexist}"); + } + + @Test + public void environmentPropertiesResolvePlaceholders() { TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.context, "logging.pattern.console=console ${pid}"); this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertThat(System.getProperty(LoggingSystemProperties.CONSOLE_LOG_PATTERN)) - .isEqualTo("console ${pid}"); + .isEqualTo(this.context.getEnvironment() + .getProperty("logging.pattern.console")); } @Test 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 fd9a9d59b0..abd59f3c2b 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 @@ -37,6 +37,7 @@ import static org.mockito.Mockito.mock; * * @author Phillip Webb * @author Madhura Bhave + * @author Fahim Farook */ public class SpringIterableConfigurationPropertySourceTests { @@ -157,6 +158,24 @@ public class SpringIterableConfigurationPropertySourceTests { .isEqualTo(ConfigurationPropertyState.ABSENT); } + @SuppressWarnings("unchecked") + @Test + public void propertySourceChangeReflects() { + // gh-13344 + final Map 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); + assertThat(adapter.stream().count()).isEqualTo(2); + + ((Map) adapter.getPropertySource().getSource()).put("key3", + "value3"); + assertThat(adapter.stream().count()).isEqualTo(3); + } + /** * Test {@link PropertySource} that's also an {@link OriginLookup}. */