Commit 3e1b163c authored by Phillip Webb's avatar Phillip Webb

Merge pull request #13344 from fahimfarookme

* pr/13344:
  Polish "Fix caching issues with map property sources"
  Fix caching issues with map property sources
parents 461202bc dc1c459c
...@@ -18,8 +18,10 @@ package org.springframework.boot.context.properties.source; ...@@ -18,8 +18,10 @@ package org.springframework.boot.context.properties.source;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Set;
import java.util.stream.Stream; import java.util.stream.Stream;
import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.EnumerablePropertySource;
...@@ -129,7 +131,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -129,7 +131,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
} }
private Cache getCache() { private Cache getCache() {
Object cacheKey = getCacheKey(); CacheKey cacheKey = CacheKey.get(getPropertySource());
if (cacheKey == null) { if (cacheKey == null) {
return null; return null;
} }
...@@ -137,17 +139,10 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -137,17 +139,10 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
return this.cache; return this.cache;
} }
this.cache = new Cache(); this.cache = new Cache();
this.cacheKey = cacheKey; this.cacheKey = cacheKey.copy();
return this.cache; return this.cache;
} }
private Object getCacheKey() {
if (getPropertySource() instanceof MapPropertySource) {
return ((MapPropertySource) getPropertySource()).getSource().keySet();
}
return getPropertySource().getPropertyNames();
}
@Override @Override
protected EnumerablePropertySource<?> getPropertySource() { protected EnumerablePropertySource<?> getPropertySource() {
return (EnumerablePropertySource<?>) super.getPropertySource(); return (EnumerablePropertySource<?>) super.getPropertySource();
...@@ -177,4 +172,48 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ...@@ -177,4 +172,48 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
} }
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());
}
}
} }
...@@ -76,6 +76,7 @@ import static org.hamcrest.Matchers.not; ...@@ -76,6 +76,7 @@ import static org.hamcrest.Matchers.not;
* @author Andy Wilkinson * @author Andy Wilkinson
* @author Stephane Nicoll * @author Stephane Nicoll
* @author Ben Hale * @author Ben Hale
* @author Fahim Farook
*/ */
@RunWith(ModifiedClassPathRunner.class) @RunWith(ModifiedClassPathRunner.class)
@ClassPathExclusions("log4j*.jar") @ClassPathExclusions("log4j*.jar")
...@@ -500,12 +501,23 @@ public class LoggingApplicationListenerTests { ...@@ -500,12 +501,23 @@ public class LoggingApplicationListenerTests {
@Test @Test
public void environmentPropertiesIgnoreUnresolvablePlaceholders() { public void environmentPropertiesIgnoreUnresolvablePlaceholders() {
// gh-7719 // 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, TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.context,
"logging.pattern.console=console ${pid}"); "logging.pattern.console=console ${pid}");
this.initializer.initialize(this.context.getEnvironment(), this.initializer.initialize(this.context.getEnvironment(),
this.context.getClassLoader()); this.context.getClassLoader());
assertThat(System.getProperty(LoggingSystemProperties.CONSOLE_LOG_PATTERN)) assertThat(System.getProperty(LoggingSystemProperties.CONSOLE_LOG_PATTERN))
.isEqualTo("console ${pid}"); .isEqualTo(this.context.getEnvironment()
.getProperty("logging.pattern.console"));
} }
@Test @Test
......
...@@ -37,6 +37,7 @@ import static org.mockito.Mockito.mock; ...@@ -37,6 +37,7 @@ import static org.mockito.Mockito.mock;
* *
* @author Phillip Webb * @author Phillip Webb
* @author Madhura Bhave * @author Madhura Bhave
* @author Fahim Farook
*/ */
public class SpringIterableConfigurationPropertySourceTests { public class SpringIterableConfigurationPropertySourceTests {
...@@ -157,6 +158,20 @@ public class SpringIterableConfigurationPropertySourceTests { ...@@ -157,6 +158,20 @@ public class SpringIterableConfigurationPropertySourceTests {
.isEqualTo(ConfigurationPropertyState.ABSENT); .isEqualTo(ConfigurationPropertyState.ABSENT);
} }
@Test
public void propertySourceKeyDataChangeInvalidatesCache() {
// gh-13344
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.put("key3", "value3");
assertThat(adapter.stream().count()).isEqualTo(3);
}
/** /**
* Test {@link PropertySource} that's also an {@link OriginLookup}. * Test {@link PropertySource} that's also an {@link OriginLookup}.
*/ */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment