Skip to content

Commit

Permalink
Merge pull request #16717 from dreis2211
Browse files Browse the repository at this point in the history
* pr/16717:
  Polish "Optimize CacheKey handling for immutable sources"
  Optimize CacheKey handling for immutable sources

Closes gh-16717
  • Loading branch information
philwebb committed Jun 4, 2019
2 parents 071410c + 3bfc923 commit 05ad955
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@
import java.util.Set;
import java.util.stream.Stream;

import org.springframework.boot.env.OriginTrackedMapPropertySource;
import org.springframework.core.env.EnumerablePropertySource;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource;
import org.springframework.core.env.StandardEnvironment;
import org.springframework.core.env.SystemEnvironmentPropertySource;
import org.springframework.util.ObjectUtils;

Expand Down Expand Up @@ -190,13 +192,19 @@ public void setMappings(PropertyMapping[] mappings) {

private static final class CacheKey {

private static final CacheKey IMMUTABLE_PROPERTY_SOURCE = new CacheKey(
new Object[0]);

private final Object key;

private CacheKey(Object key) {
this.key = key;
}

public CacheKey copy() {
if (this == IMMUTABLE_PROPERTY_SOURCE) {
return IMMUTABLE_PROPERTY_SOURCE;
}
return new CacheKey(copyKey(this.key));
}

Expand All @@ -215,7 +223,8 @@ public boolean equals(Object obj) {
if (obj == null || getClass() != obj.getClass()) {
return false;
}
return ObjectUtils.nullSafeEquals(this.key, ((CacheKey) obj).key);
CacheKey otherCacheKey = (CacheKey) obj;
return ObjectUtils.nullSafeEquals(this.key, otherCacheKey.key);
}

@Override
Expand All @@ -224,12 +233,27 @@ public int hashCode() {
}

public static CacheKey get(EnumerablePropertySource<?> source) {
if (isImmutable(source)) {
return IMMUTABLE_PROPERTY_SOURCE;
}
if (source instanceof MapPropertySource) {
return new CacheKey(((MapPropertySource) source).getSource().keySet());
MapPropertySource mapPropertySource = (MapPropertySource) source;
return new CacheKey(mapPropertySource.getSource().keySet());
}
return new CacheKey(source.getPropertyNames());
}

private static boolean isImmutable(EnumerablePropertySource<?> source) {
if (source instanceof OriginTrackedMapPropertySource) {
return ((OriginTrackedMapPropertySource) source).isImmutable();
}
if (StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME
.equals(source.getName())) {
return source.getSource() == System.getenv();
}
return false;
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.springframework.boot.origin.OriginLookup;
import org.springframework.boot.origin.OriginTrackedValue;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource;

/**
* {@link OriginLookup} backed by a {@link Map} containing {@link OriginTrackedValue
Expand All @@ -35,9 +36,28 @@
public final class OriginTrackedMapPropertySource extends MapPropertySource
implements OriginLookup<String> {

@SuppressWarnings({ "unchecked", "rawtypes" })
private final boolean immutable;

/**
* Create a new {@link OriginTrackedMapPropertySource} instance.
* @param name the property source name
* @param source the underlying map source
*/
@SuppressWarnings("rawtypes")
public OriginTrackedMapPropertySource(String name, Map source) {
this(name, source, false);
}

/**
* Create a new {@link OriginTrackedMapPropertySource} instance.
* @param name the property source name
* @param source the underlying map source
* @param immutable if the underlying source is immutable and guaranteed not to change
*/
@SuppressWarnings({ "unchecked", "rawtypes" })
public OriginTrackedMapPropertySource(String name, Map source, boolean immutable) {
super(name, source);
this.immutable = immutable;
}

@Override
Expand All @@ -58,4 +78,13 @@ public Origin getOrigin(String name) {
return null;
}

/**
* Return {@code true} if this {@link PropertySource} is immutable and has contents
* that will never change.
* @return if the property source is read only
*/
public boolean isImmutable() {
return this.immutable;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public List<PropertySource<?>> load(String name, Resource resource)
if (properties.isEmpty()) {
return Collections.emptyList();
}
return Collections
.singletonList(new OriginTrackedMapPropertySource(name, properties));
return Collections.singletonList(new OriginTrackedMapPropertySource(name,
Collections.unmodifiableMap(properties), true));
}

@SuppressWarnings({ "unchecked", "rawtypes" })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public List<PropertySource<?>> load(String name, Resource resource)
for (int i = 0; i < loaded.size(); i++) {
String documentNumber = (loaded.size() != 1) ? " (document #" + i + ")" : "";
propertySources.add(new OriginTrackedMapPropertySource(name + documentNumber,
loaded.get(i)));
Collections.unmodifiableMap(loaded.get(i)), true));
}
return propertySources;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import org.junit.Test;

import org.springframework.boot.env.OriginTrackedMapPropertySource;
import org.springframework.boot.origin.Origin;
import org.springframework.boot.origin.OriginLookup;
import org.springframework.core.env.EnumerablePropertySource;
Expand Down Expand Up @@ -160,7 +161,7 @@ public void containsDescendantOfShouldCheckSourceNames() {
}

@Test
public void propertySourceKeyDataChangeInvalidatesCache() {
public void simpleMapPropertySourceKeyDataChangeInvalidatesCache() {
// gh-13344
Map<String, Object> map = new LinkedHashMap<>();
map.put("key1", "value1");
Expand All @@ -184,10 +185,36 @@ public void concurrentModificationExceptionInvalidatesCache() {
source, DefaultPropertyMapper.INSTANCE);
assertThat(adapter.stream().count()).isEqualTo(2);
map.setThrowException(true);
}

public void originTrackedMapPropertySourceKeyAdditionInvalidatesCache() {
// gh-13344
Map<String, Object> map = new LinkedHashMap<>();
map.put("key1", "value1");
map.put("key2", "value2");
EnumerablePropertySource<?> source = new OriginTrackedMapPropertySource("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);
}

public void readOnlyOriginTrackedMapPropertySourceKeyAdditionDoesNotInvalidateCache() {
// gh-16717
Map<String, Object> map = new LinkedHashMap<>();
map.put("key1", "value1");
map.put("key2", "value2");
EnumerablePropertySource<?> source = new OriginTrackedMapPropertySource("test",
map, true);
SpringIterableConfigurationPropertySource adapter = new SpringIterableConfigurationPropertySource(
source, DefaultPropertyMapper.INSTANCE);
assertThat(adapter.stream().count()).isEqualTo(2);
map.put("key3", "value3");
assertThat(adapter.stream().count()).isEqualTo(2);
}

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

0 comments on commit 05ad955

Please sign in to comment.