Skip to content

Commit d9d206a

Browse files
philwebbphilwebb
committed
Bypass SystemEnvironmentPropertySource.resolvePropertyName
Update `ConfigurationPropertySource` adapters so accessing the `SystemEnvironmentPropertySource` is handled by directly calling the source. This saves potentially expensive calls to `resolvePropertyName` which are unnecessary since mappings are handled directly. Closes gh-44862
1 parent 54afa69 commit d9d206a

File tree

5 files changed

+116
-75
lines changed

5 files changed

+116
-75
lines changed

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/MapConfigurationPropertySource.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public MapConfigurationPropertySource() {
5656
public MapConfigurationPropertySource(Map<?, ?> map) {
5757
this.source = new LinkedHashMap<>();
5858
MapPropertySource mapPropertySource = new MapPropertySource("source", this.source);
59-
this.delegate = new SpringIterableConfigurationPropertySource(mapPropertySource, DEFAULT_MAPPERS);
59+
this.delegate = new SpringIterableConfigurationPropertySource(mapPropertySource, false, DEFAULT_MAPPERS);
6060
putAll(map);
6161
}
6262

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java

+33-17
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,22 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource {
5959

6060
private final PropertySource<?> propertySource;
6161

62+
private final boolean systemEnvironmentSource;
63+
6264
private final PropertyMapper[] mappers;
6365

6466
/**
6567
* Create a new {@link SpringConfigurationPropertySource} implementation.
6668
* @param propertySource the source property source
69+
* @param systemEnvironmentSource if the source is from the system environment
6770
* @param mappers the property mappers
6871
*/
69-
SpringConfigurationPropertySource(PropertySource<?> propertySource, PropertyMapper... mappers) {
72+
SpringConfigurationPropertySource(PropertySource<?> propertySource, boolean systemEnvironmentSource,
73+
PropertyMapper... mappers) {
7074
Assert.notNull(propertySource, "'propertySource' must not be null");
7175
Assert.isTrue(mappers.length > 0, "'mappers' must contain at least one item");
7276
this.propertySource = propertySource;
77+
this.systemEnvironmentSource = systemEnvironmentSource;
7378
this.mappers = mappers;
7479
}
7580

@@ -81,7 +86,7 @@ public ConfigurationProperty getConfigurationProperty(ConfigurationPropertyName
8186
for (PropertyMapper mapper : this.mappers) {
8287
try {
8388
for (String candidate : mapper.map(name)) {
84-
Object value = getPropertySource().getProperty(candidate);
89+
Object value = getPropertySourceProperty(candidate);
8590
if (value != null) {
8691
Origin origin = PropertySourceOrigin.get(this.propertySource, candidate);
8792
return ConfigurationProperty.of(this, name, value, origin);
@@ -95,6 +100,18 @@ public ConfigurationProperty getConfigurationProperty(ConfigurationPropertyName
95100
return null;
96101
}
97102

103+
protected final Object getPropertySourceProperty(String name) {
104+
// Save calls to SystemEnvironmentPropertySource.resolvePropertyName(...)
105+
// since we've already done the mapping
106+
PropertySource<?> propertySource = getPropertySource();
107+
return (!this.systemEnvironmentSource) ? propertySource.getProperty(name)
108+
: getSystemEnvironmentProperty(((SystemEnvironmentPropertySource) propertySource).getSource(), name);
109+
}
110+
111+
Object getSystemEnvironmentProperty(Map<String, Object> systemEnvironment, String name) {
112+
return systemEnvironment.get(name);
113+
}
114+
98115
@Override
99116
public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) {
100117
PropertySource<?> source = getPropertySource();
@@ -127,6 +144,10 @@ protected PropertySource<?> getPropertySource() {
127144
return this.propertySource;
128145
}
129146

147+
protected final boolean isSystemEnvironmentSource() {
148+
return this.systemEnvironmentSource;
149+
}
150+
130151
protected final PropertyMapper[] getMappers() {
131152
return this.mappers;
132153
}
@@ -145,24 +166,19 @@ public String toString() {
145166
*/
146167
static SpringConfigurationPropertySource from(PropertySource<?> source) {
147168
Assert.notNull(source, "'source' must not be null");
148-
PropertyMapper[] mappers = getPropertyMappers(source);
149-
if (isFullEnumerable(source)) {
150-
return new SpringIterableConfigurationPropertySource((EnumerablePropertySource<?>) source, mappers);
151-
}
152-
return new SpringConfigurationPropertySource(source, mappers);
153-
}
154-
155-
private static PropertyMapper[] getPropertyMappers(PropertySource<?> source) {
156-
if (source instanceof SystemEnvironmentPropertySource && hasSystemEnvironmentName(source)) {
157-
return SYSTEM_ENVIRONMENT_MAPPERS;
158-
}
159-
return DEFAULT_MAPPERS;
169+
boolean systemEnvironmentSource = isSystemEnvironmentPropertySource(source);
170+
PropertyMapper[] mappers = (!systemEnvironmentSource) ? DEFAULT_MAPPERS : SYSTEM_ENVIRONMENT_MAPPERS;
171+
return (!isFullEnumerable(source))
172+
? new SpringConfigurationPropertySource(source, systemEnvironmentSource, mappers)
173+
: new SpringIterableConfigurationPropertySource((EnumerablePropertySource<?>) source,
174+
systemEnvironmentSource, mappers);
160175
}
161176

162-
private static boolean hasSystemEnvironmentName(PropertySource<?> source) {
177+
private static boolean isSystemEnvironmentPropertySource(PropertySource<?> source) {
163178
String name = source.getName();
164-
return StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME.equals(name)
165-
|| name.endsWith("-" + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME);
179+
return (source instanceof SystemEnvironmentPropertySource)
180+
&& (StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME.equals(name)
181+
|| name.endsWith("-" + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME));
166182
}
167183

168184
private static boolean isFullEnumerable(PropertySource<?> source) {

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java

+61-39
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.Arrays;
2020
import java.util.Collections;
2121
import java.util.ConcurrentModificationException;
22+
import java.util.HashMap;
2223
import java.util.HashSet;
2324
import java.util.Iterator;
2425
import java.util.LinkedHashMap;
@@ -27,7 +28,6 @@
2728
import java.util.Objects;
2829
import java.util.Set;
2930
import java.util.function.BiPredicate;
30-
import java.util.function.Supplier;
3131
import java.util.stream.Stream;
3232

3333
import org.springframework.boot.origin.Origin;
@@ -59,11 +59,13 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
5959

6060
private volatile ConfigurationPropertyName[] configurationPropertyNames;
6161

62-
SpringIterableConfigurationPropertySource(EnumerablePropertySource<?> propertySource, PropertyMapper... mappers) {
63-
super(propertySource, mappers);
62+
SpringIterableConfigurationPropertySource(EnumerablePropertySource<?> propertySource,
63+
boolean systemEnvironmentSource, PropertyMapper... mappers) {
64+
super(propertySource, systemEnvironmentSource, mappers);
6465
assertEnumerablePropertySource();
66+
boolean immutable = isImmutablePropertySource();
6567
this.ancestorOfCheck = getAncestorOfCheck(mappers);
66-
this.cache = new SoftReferenceConfigurationPropertyCache<>(isImmutablePropertySource());
68+
this.cache = new SoftReferenceConfigurationPropertyCache<>(immutable);
6769
}
6870

6971
private BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> getAncestorOfCheck(
@@ -102,7 +104,7 @@ public ConfigurationProperty getConfigurationProperty(ConfigurationPropertyName
102104
return configurationProperty;
103105
}
104106
for (String candidate : getCache().getMapped(name)) {
105-
Object value = getPropertySource().getProperty(candidate);
107+
Object value = getPropertySourceProperty(candidate);
106108
if (value != null) {
107109
Origin origin = PropertySourceOrigin.get(getPropertySource(), candidate);
108110
return ConfigurationProperty.of(this, name, value, origin);
@@ -111,6 +113,11 @@ public ConfigurationProperty getConfigurationProperty(ConfigurationPropertyName
111113
return null;
112114
}
113115

116+
@Override
117+
protected Object getSystemEnvironmentProperty(Map<String, Object> systemEnvironment, String name) {
118+
return getCache().getSystemEnvironmentProperty(name);
119+
}
120+
114121
@Override
115122
public Stream<ConfigurationPropertyName> stream() {
116123
ConfigurationPropertyName[] names = getConfigurationPropertyNames();
@@ -129,7 +136,14 @@ public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName
129136
return result;
130137
}
131138
if (this.ancestorOfCheck == PropertyMapper.DEFAULT_ANCESTOR_OF_CHECK) {
132-
return getCache().containsDescendantOf(name, this.ancestorOfCheck);
139+
Set<ConfigurationPropertyName> descendants = getCache().getDescendants();
140+
if (descendants != null) {
141+
if (name.isEmpty() && !descendants.isEmpty()) {
142+
return ConfigurationPropertyState.PRESENT;
143+
}
144+
return !descendants.contains(name) ? ConfigurationPropertyState.ABSENT
145+
: ConfigurationPropertyState.PRESENT;
146+
}
133147
}
134148
ConfigurationPropertyName[] candidates = getConfigurationPropertyNames();
135149
for (ConfigurationPropertyName candidate : candidates) {
@@ -158,12 +172,13 @@ private Cache getCache() {
158172
}
159173

160174
private Cache createCache() {
161-
return new Cache(getMappers(), isImmutablePropertySource(),
162-
this.ancestorOfCheck == PropertyMapper.DEFAULT_ANCESTOR_OF_CHECK);
175+
boolean immutable = isImmutablePropertySource();
176+
boolean captureDescendants = this.ancestorOfCheck == PropertyMapper.DEFAULT_ANCESTOR_OF_CHECK;
177+
return new Cache(getMappers(), immutable, captureDescendants, isSystemEnvironmentSource());
163178
}
164179

165180
private Cache updateCache(Cache cache) {
166-
cache.update(getPropertySource()::getPropertyNames);
181+
cache.update(getPropertySource());
167182
return cache;
168183
}
169184

@@ -193,20 +208,24 @@ private static class Cache {
193208

194209
private final boolean captureDescendants;
195210

211+
private final boolean systemEnvironmentSource;
212+
196213
private volatile Data data;
197214

198-
Cache(PropertyMapper[] mappers, boolean immutable, boolean captureDescendants) {
215+
Cache(PropertyMapper[] mappers, boolean immutable, boolean captureDescendants,
216+
boolean systemEnvironmentSource) {
199217
this.mappers = mappers;
200218
this.immutable = immutable;
201219
this.captureDescendants = captureDescendants;
220+
this.systemEnvironmentSource = systemEnvironmentSource;
202221
}
203222

204-
void update(Supplier<String[]> propertyNames) {
223+
void update(EnumerablePropertySource<?> propertySource) {
205224
if (this.data == null || !this.immutable) {
206225
int count = 0;
207226
while (true) {
208227
try {
209-
tryUpdate(propertyNames.get());
228+
tryUpdate(propertySource);
210229
return;
211230
}
212231
catch (ConcurrentModificationException ex) {
@@ -218,9 +237,10 @@ void update(Supplier<String[]> propertyNames) {
218237
}
219238
}
220239

221-
private void tryUpdate(String[] propertyNames) {
240+
private void tryUpdate(EnumerablePropertySource<?> propertySource) {
222241
Data data = this.data;
223242
String[] lastUpdated = (data != null) ? data.lastUpdated() : null;
243+
String[] propertyNames = propertySource.getPropertyNames();
224244
if (lastUpdated != null && Arrays.equals(lastUpdated, propertyNames)) {
225245
return;
226246
}
@@ -229,37 +249,46 @@ private void tryUpdate(String[] propertyNames) {
229249
(data != null) ? data.mappings() : null, size);
230250
Map<String, ConfigurationPropertyName> reverseMappings = cloneOrCreate(
231251
(data != null) ? data.reverseMappings() : null, size);
232-
Map<ConfigurationPropertyName, Set<ConfigurationPropertyName>> descendants = cloneOrCreate(
233-
(data != null) ? data.descendants() : null, size);
252+
Set<ConfigurationPropertyName> descendants = (!this.captureDescendants) ? null : new HashSet<>();
253+
Map<String, Object> systemEnvironmentCopy = (!this.systemEnvironmentSource) ? null
254+
: copySource(propertySource);
234255
for (PropertyMapper propertyMapper : this.mappers) {
235256
for (String propertyName : propertyNames) {
236257
if (!reverseMappings.containsKey(propertyName)) {
237258
ConfigurationPropertyName configurationPropertyName = propertyMapper.map(propertyName);
238259
if (configurationPropertyName != null && !configurationPropertyName.isEmpty()) {
239260
add(mappings, configurationPropertyName, propertyName);
240261
reverseMappings.put(propertyName, configurationPropertyName);
241-
if (this.captureDescendants) {
242-
addParents(descendants, configurationPropertyName);
243-
}
262+
addParents(descendants, configurationPropertyName);
244263
}
245264
}
246265
}
247266
}
248267
ConfigurationPropertyName[] configurationPropertyNames = this.immutable
249268
? reverseMappings.values().toArray(new ConfigurationPropertyName[0]) : null;
250269
lastUpdated = this.immutable ? null : propertyNames;
251-
this.data = new Data(mappings, reverseMappings, descendants, configurationPropertyNames, lastUpdated);
270+
this.data = new Data(mappings, reverseMappings, descendants, configurationPropertyNames,
271+
systemEnvironmentCopy, lastUpdated);
272+
}
273+
274+
@SuppressWarnings("unchecked")
275+
private HashMap<String, Object> copySource(EnumerablePropertySource<?> propertySource) {
276+
return new HashMap<>((Map<String, Object>) propertySource.getSource());
252277
}
253278

254279
private <K, V> Map<K, V> cloneOrCreate(Map<K, V> source, int size) {
255280
return (source != null) ? new LinkedHashMap<>(source) : new LinkedHashMap<>(size);
256281
}
257282

258-
private void addParents(Map<ConfigurationPropertyName, Set<ConfigurationPropertyName>> descendants,
259-
ConfigurationPropertyName name) {
260-
ConfigurationPropertyName parent = name;
283+
private void addParents(Set<ConfigurationPropertyName> descendants, ConfigurationPropertyName name) {
284+
if (descendants == null || name.isEmpty()) {
285+
return;
286+
}
287+
ConfigurationPropertyName parent = name.getParent();
261288
while (!parent.isEmpty()) {
262-
add(descendants, parent, name);
289+
if (!descendants.add(parent)) {
290+
return;
291+
}
263292
parent = parent.getParent();
264293
}
265294
}
@@ -289,25 +318,18 @@ ConfigurationPropertyName[] getConfigurationPropertyNames(String[] propertyNames
289318
return names;
290319
}
291320

292-
ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name,
293-
BiPredicate<ConfigurationPropertyName, ConfigurationPropertyName> ancestorOfCheck) {
294-
Data data = this.data;
295-
if (name.isEmpty() && !data.descendants().isEmpty()) {
296-
return ConfigurationPropertyState.PRESENT;
297-
}
298-
Set<ConfigurationPropertyName> candidates = data.descendants().getOrDefault(name, Collections.emptySet());
299-
for (ConfigurationPropertyName candidate : candidates) {
300-
if (ancestorOfCheck.test(name, candidate)) {
301-
return ConfigurationPropertyState.PRESENT;
302-
}
303-
}
304-
return ConfigurationPropertyState.ABSENT;
321+
Set<ConfigurationPropertyName> getDescendants() {
322+
return this.data.descendants();
323+
}
324+
325+
Object getSystemEnvironmentProperty(String name) {
326+
return this.data.systemEnvironmentCopy().get(name);
305327
}
306328

307329
private record Data(Map<ConfigurationPropertyName, Set<String>> mappings,
308-
Map<String, ConfigurationPropertyName> reverseMappings,
309-
Map<ConfigurationPropertyName, Set<ConfigurationPropertyName>> descendants,
310-
ConfigurationPropertyName[] configurationPropertyNames, String[] lastUpdated) {
330+
Map<String, ConfigurationPropertyName> reverseMappings, Set<ConfigurationPropertyName> descendants,
331+
ConfigurationPropertyName[] configurationPropertyNames, Map<String, Object> systemEnvironmentCopy,
332+
String[] lastUpdated) {
311333

312334
}
313335

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySourceTests.java

+8-5
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class SpringConfigurationPropertySourceTests {
4343
@Test
4444
void createWhenPropertySourceIsNullShouldThrowException() {
4545
assertThatIllegalArgumentException()
46-
.isThrownBy(() -> new SpringConfigurationPropertySource(null, mock(PropertyMapper.class)))
46+
.isThrownBy(() -> new SpringConfigurationPropertySource(null, false, mock(PropertyMapper.class)))
4747
.withMessageContaining("'propertySource' must not be null");
4848
}
4949

@@ -57,7 +57,8 @@ void getValueShouldUseDirectMapping() {
5757
TestPropertyMapper mapper = new TestPropertyMapper();
5858
ConfigurationPropertyName name = ConfigurationPropertyName.of("my.key");
5959
mapper.addFromConfigurationProperty(name, "key2");
60-
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource, mapper);
60+
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource, false,
61+
mapper);
6162
assertThat(adapter.getConfigurationProperty(name).getValue()).isEqualTo("value2");
6263
}
6364

@@ -69,7 +70,8 @@ void getValueOriginAndPropertySource() {
6970
TestPropertyMapper mapper = new TestPropertyMapper();
7071
ConfigurationPropertyName name = ConfigurationPropertyName.of("my.key");
7172
mapper.addFromConfigurationProperty(name, "key");
72-
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource, mapper);
73+
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource, false,
74+
mapper);
7375
ConfigurationProperty configurationProperty = adapter.getConfigurationProperty(name);
7476
assertThat(configurationProperty.getOrigin()).hasToString("\"key\" from property source \"test\"");
7577
assertThat(configurationProperty.getSource()).isEqualTo(adapter);
@@ -83,7 +85,8 @@ void getValueWhenOriginCapableShouldIncludeSourceOrigin() {
8385
TestPropertyMapper mapper = new TestPropertyMapper();
8486
ConfigurationPropertyName name = ConfigurationPropertyName.of("my.key");
8587
mapper.addFromConfigurationProperty(name, "key");
86-
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource, mapper);
88+
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource, false,
89+
mapper);
8790
assertThat(adapter.getConfigurationProperty(name).getOrigin()).hasToString("TestOrigin key");
8891
}
8992

@@ -92,7 +95,7 @@ void containsDescendantOfShouldReturnEmpty() {
9295
Map<String, Object> source = new LinkedHashMap<>();
9396
source.put("foo.bar", "value");
9497
PropertySource<?> propertySource = new MapPropertySource("test", source);
95-
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource,
98+
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(propertySource, false,
9699
DefaultPropertyMapper.INSTANCE);
97100
assertThat(adapter.containsDescendantOf(ConfigurationPropertyName.of("foo")))
98101
.isEqualTo(ConfigurationPropertyState.UNKNOWN);

0 commit comments

Comments
 (0)