Skip to content

Commit

Permalink
Polish "Fix caching issues with map property sources"
Browse files Browse the repository at this point in the history
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
  • Loading branch information
philwebb committed Jun 6, 2018
1 parent c556d2b commit dc1c459
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -129,23 +131,18 @@ private PropertyMapping[] getPropertyMappings(Cache cache) {
}

private Cache getCache() {
Object cacheKey = getCacheKey();
CacheKey cacheKey = CacheKey.get(getPropertySource());
if (cacheKey == null) {
return null;
}
if (ObjectUtils.nullSafeEquals(cacheKey, this.cacheKey)) {
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();
Expand Down Expand Up @@ -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());
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down

0 comments on commit dc1c459

Please sign in to comment.