Skip to content

Commit

Permalink
Fix caching issues with map property sources
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fahimfarookme authored and philwebb committed Jun 6, 2018
1 parent 461202b commit c556d2b
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,7 @@ private Cache getCache() {
}

private Object getCacheKey() {
if (getPropertySource() instanceof MapPropertySource) {
return ((MapPropertySource) getPropertySource()).getSource().keySet();
}
// gh-13344
return getPropertySource().getPropertyNames();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
* @author Andy Wilkinson
* @author Stephane Nicoll
* @author Ben Hale
* @author Fahim Farook
*/
@RunWith(ModifiedClassPathRunner.class)
@ClassPathExclusions("log4j*.jar")
Expand Down Expand Up @@ -500,12 +501,23 @@ public void systemPropertiesAreSetForLoggingConfiguration() {
@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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
*
* @author Phillip Webb
* @author Madhura Bhave
* @author Fahim Farook
*/
public class SpringIterableConfigurationPropertySourceTests {

Expand Down Expand Up @@ -157,6 +158,24 @@ public void containsDescendantOfShouldCheckSourceNames() {
.isEqualTo(ConfigurationPropertyState.ABSENT);
}

@SuppressWarnings("unchecked")
@Test
public void propertySourceChangeReflects() {
// 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);
assertThat(adapter.stream().count()).isEqualTo(2);

((Map<String, Object>) adapter.getPropertySource().getSource()).put("key3",
"value3");
assertThat(adapter.stream().count()).isEqualTo(3);
}

/**
* Test {@link PropertySource} that's also an {@link OriginLookup}.
*/
Expand Down

0 comments on commit c556d2b

Please sign in to comment.