Skip to content

Commit 8f14dca

Browse files
philwebbphilwebb
committed
Restore IndexedElementsBinder knownIndexedChildren checking logic
Restore knownIndexedChildren logic in IndexedElementsBinder to that of Spring Boot 3.4, effectively reverting commit 93113a4. The performance improvements unfortunately caused the unwanted side-effect of no longer checking the all indexed elements were fully bound. Performance was also actually reduced for non iterable property sources that used deeply nested structures. In order to keep some performance improvements, the updated code also makes the following changes: * A `Set` is used rather than `LinkedMultiValueMap` for tracking. The full list of unbound properties are now only calculated when the exception is thrown. * If possible, the source is filtered early and passed down for child element binding. This should help reduce repeated full scans of all configuration property names. * The `FilteredIterableConfigurationPropertiesSource` has been optimized for immutable `SpringIterableConfigurationPropertySource` use. Fixes gh-45994 See gh-45970 See gh-44867
1 parent 34a8f37 commit 8f14dca

File tree

6 files changed

+169
-57
lines changed

6 files changed

+169
-57
lines changed

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java

Lines changed: 36 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818

1919
import java.lang.annotation.Annotation;
2020
import java.util.Collection;
21-
import java.util.List;
21+
import java.util.Collections;
22+
import java.util.HashSet;
23+
import java.util.Set;
2224
import java.util.TreeSet;
2325
import java.util.function.Supplier;
24-
import java.util.stream.Collectors;
2526

2627
import org.springframework.boot.context.properties.bind.Binder.Context;
2728
import org.springframework.boot.context.properties.source.ConfigurationProperty;
@@ -30,8 +31,6 @@
3031
import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
3132
import org.springframework.boot.context.properties.source.IterableConfigurationPropertySource;
3233
import org.springframework.core.ResolvableType;
33-
import org.springframework.util.LinkedMultiValueMap;
34-
import org.springframework.util.MultiValueMap;
3534

3635
/**
3736
* Base class for {@link AggregateBinder AggregateBinders} that read a sequential run of
@@ -106,65 +105,57 @@ private void bindValue(Bindable<?> target, Collection<Object> collection, Resolv
106105

107106
private void bindIndexed(ConfigurationPropertySource source, ConfigurationPropertyName root,
108107
AggregateElementBinder elementBinder, IndexedCollectionSupplier collection, ResolvableType elementType) {
109-
int firstUnboundIndex = 0;
110-
boolean hasBindingGap = false;
108+
Set<String> knownIndexedChildren = Collections.emptySet();
109+
if (source instanceof IterableConfigurationPropertySource iterableSource) {
110+
source = iterableSource.filter(root::isAncestorOf);
111+
knownIndexedChildren = getKnownIndexedChildren(iterableSource, root);
112+
}
111113
for (int i = 0; i < Integer.MAX_VALUE; i++) {
112114
ConfigurationPropertyName name = appendIndex(root, i);
113115
Object value = elementBinder.bind(name, Bindable.of(elementType), source);
114-
if (value != null) {
115-
collection.get().add(value);
116-
hasBindingGap = hasBindingGap || firstUnboundIndex > 0;
117-
continue;
118-
}
119-
firstUnboundIndex = (firstUnboundIndex <= 0) ? i : firstUnboundIndex;
120-
if (i - firstUnboundIndex > 10) {
116+
if (value == null) {
121117
break;
122118
}
119+
knownIndexedChildren.remove(name.getLastElement(Form.UNIFORM));
120+
collection.get().add(value);
123121
}
124-
if (hasBindingGap) {
125-
assertNoUnboundChildren(source, root, firstUnboundIndex);
122+
if (source instanceof IterableConfigurationPropertySource iterableSource) {
123+
assertNoUnboundChildren(knownIndexedChildren, iterableSource, root);
126124
}
127125
}
128126

129-
private ConfigurationPropertyName appendIndex(ConfigurationPropertyName root, int i) {
130-
return root.append((i < INDEXES.length) ? INDEXES[i] : "[" + i + "]");
131-
}
132-
133-
private void assertNoUnboundChildren(ConfigurationPropertySource source, ConfigurationPropertyName root,
134-
int firstUnboundIndex) {
135-
MultiValueMap<String, ConfigurationPropertyName> knownIndexedChildren = getKnownIndexedChildren(source, root);
136-
for (int i = 0; i < firstUnboundIndex; i++) {
137-
ConfigurationPropertyName name = appendIndex(root, i);
138-
knownIndexedChildren.remove(name.getLastElement(Form.UNIFORM));
127+
private Set<String> getKnownIndexedChildren(IterableConfigurationPropertySource filteredSource,
128+
ConfigurationPropertyName root) {
129+
Set<String> knownIndexedChildren = new HashSet<>();
130+
for (ConfigurationPropertyName name : filteredSource) {
131+
ConfigurationPropertyName choppedName = name.chop(root.getNumberOfElements() + 1);
132+
if (choppedName.isLastElementIndexed()) {
133+
knownIndexedChildren.add(choppedName.getLastElement(Form.UNIFORM));
134+
}
139135
}
140-
assertNoUnboundChildren(source, knownIndexedChildren);
136+
return knownIndexedChildren;
141137
}
142138

143-
private MultiValueMap<String, ConfigurationPropertyName> getKnownIndexedChildren(ConfigurationPropertySource source,
144-
ConfigurationPropertyName root) {
145-
MultiValueMap<String, ConfigurationPropertyName> children = new LinkedMultiValueMap<>();
146-
if (!(source instanceof IterableConfigurationPropertySource iterableSource)) {
147-
return children;
139+
private void assertNoUnboundChildren(Set<String> unboundIndexedChildren,
140+
IterableConfigurationPropertySource filteredSource, ConfigurationPropertyName root) {
141+
if (unboundIndexedChildren.isEmpty()) {
142+
return;
148143
}
149-
for (ConfigurationPropertyName name : iterableSource.filter(root::isAncestorOf)) {
144+
Set<ConfigurationProperty> unboundProperties = new TreeSet<>();
145+
for (ConfigurationPropertyName name : filteredSource) {
150146
ConfigurationPropertyName choppedName = name.chop(root.getNumberOfElements() + 1);
151-
if (choppedName.isLastElementIndexed()) {
152-
String key = choppedName.getLastElement(Form.UNIFORM);
153-
children.add(key, name);
147+
if (choppedName.isLastElementIndexed()
148+
&& unboundIndexedChildren.contains(choppedName.getLastElement(Form.UNIFORM))) {
149+
unboundProperties.add(filteredSource.getConfigurationProperty(name));
154150
}
155151
}
156-
return children;
152+
if (!unboundProperties.isEmpty()) {
153+
throw new UnboundConfigurationPropertiesException(unboundProperties);
154+
}
157155
}
158156

159-
private void assertNoUnboundChildren(ConfigurationPropertySource source,
160-
MultiValueMap<String, ConfigurationPropertyName> children) {
161-
if (!children.isEmpty()) {
162-
throw new UnboundConfigurationPropertiesException(children.values()
163-
.stream()
164-
.flatMap(List::stream)
165-
.map(source::getConfigurationProperty)
166-
.collect(Collectors.toCollection(TreeSet::new)));
167-
}
157+
private ConfigurationPropertyName appendIndex(ConfigurationPropertyName root, int i) {
158+
return root.append((i < INDEXES.length) ? INDEXES[i] : "[" + i + "]");
168159
}
169160

170161
private <C> C convert(Object value, ResolvableType type, Annotation... annotations) {

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,27 @@ static <T> ConfigurationPropertyState search(Iterable<T> source, Predicate<T> pr
6666
return ABSENT;
6767
}
6868

69+
/**
70+
* Search the given iterable using a predicate to determine if content is
71+
* {@link #PRESENT} or {@link #ABSENT}.
72+
* @param <T> the data type
73+
* @param source the source iterable to search
74+
* @param startInclusive the first index to cover
75+
* @param endExclusive index immediately past the last index to cover
76+
* @param predicate the predicate used to test for presence
77+
* @return {@link #PRESENT} if the iterable contains a matching item, otherwise
78+
* {@link #ABSENT}.
79+
*/
80+
static <T> ConfigurationPropertyState search(T[] source, int startInclusive, int endExclusive,
81+
Predicate<T> predicate) {
82+
Assert.notNull(source, "'source' must not be null");
83+
Assert.notNull(predicate, "'predicate' must not be null");
84+
for (int i = startInclusive; i < endExclusive; i++) {
85+
if (predicate.test(source[i])) {
86+
return PRESENT;
87+
}
88+
}
89+
return ABSENT;
90+
}
91+
6992
}

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.boot.context.properties.source;
1818

19+
import java.util.Arrays;
1920
import java.util.function.Predicate;
2021
import java.util.stream.Stream;
2122

@@ -28,13 +29,41 @@
2829
class FilteredIterableConfigurationPropertiesSource extends FilteredConfigurationPropertiesSource
2930
implements IterableConfigurationPropertySource {
3031

32+
private ConfigurationPropertyName[] filteredNames;
33+
34+
private int numerOfFilteredNames;
35+
3136
FilteredIterableConfigurationPropertiesSource(IterableConfigurationPropertySource source,
3237
Predicate<ConfigurationPropertyName> filter) {
3338
super(source, filter);
39+
ConfigurationPropertyName[] filterableNames = getFilterableNames(source);
40+
if (filterableNames != null) {
41+
this.filteredNames = new ConfigurationPropertyName[filterableNames.length];
42+
this.numerOfFilteredNames = 0;
43+
for (ConfigurationPropertyName name : filterableNames) {
44+
if (filter.test(name)) {
45+
this.filteredNames[this.numerOfFilteredNames++] = name;
46+
}
47+
}
48+
}
49+
}
50+
51+
private ConfigurationPropertyName[] getFilterableNames(IterableConfigurationPropertySource source) {
52+
if (source instanceof SpringIterableConfigurationPropertySource springPropertySource
53+
&& springPropertySource.isImmutablePropertySource()) {
54+
return springPropertySource.getConfigurationPropertyNames();
55+
}
56+
if (source instanceof FilteredIterableConfigurationPropertiesSource filteredSource) {
57+
return filteredSource.filteredNames;
58+
}
59+
return null;
3460
}
3561

3662
@Override
3763
public Stream<ConfigurationPropertyName> stream() {
64+
if (this.filteredNames != null) {
65+
return Arrays.stream(this.filteredNames, 0, this.numerOfFilteredNames);
66+
}
3867
return getSource().stream().filter(getFilter());
3968
}
4069

@@ -45,6 +74,10 @@ protected IterableConfigurationPropertySource getSource() {
4574

4675
@Override
4776
public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) {
77+
if (this.filteredNames != null) {
78+
return ConfigurationPropertyState.search(this.filteredNames, 0, this.numerOfFilteredNames,
79+
name::isAncestorOf);
80+
}
4881
return ConfigurationPropertyState.search(this, name::isAncestorOf);
4982
}
5083

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ private boolean ancestorOfCheck(ConfigurationPropertyName name) {
169169
return false;
170170
}
171171

172-
private ConfigurationPropertyName[] getConfigurationPropertyNames() {
172+
ConfigurationPropertyName[] getConfigurationPropertyNames() {
173173
if (!isImmutablePropertySource()) {
174174
return getCache().getConfigurationPropertyNames(getPropertySource().getPropertyNames());
175175
}
@@ -197,7 +197,7 @@ private Cache updateCache(Cache cache) {
197197
return cache;
198198
}
199199

200-
private boolean isImmutablePropertySource() {
200+
boolean isImmutablePropertySource() {
201201
EnumerablePropertySource<?> source = getPropertySource();
202202
if (source instanceof OriginLookup<?> originLookup) {
203203
return originLookup.isImmutable();

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -123,6 +123,26 @@ void bindToCollectionWhenNonSequentialShouldThrowException() {
123123
});
124124
}
125125

126+
@Test
127+
void bindToCollectionWhenNonKnownIndexedChildNotBoundThrowsException() {
128+
// gh-45994
129+
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
130+
source.put("foo[0].first", "Spring");
131+
source.put("foo[0].last", "Boot");
132+
source.put("foo[1].missing", "bad");
133+
this.sources.add(source);
134+
assertThatExceptionOfType(BindException.class)
135+
.isThrownBy(() -> this.binder.bind("foo", Bindable.listOf(Name.class)))
136+
.satisfies((ex) -> {
137+
Set<ConfigurationProperty> unbound = ((UnboundConfigurationPropertiesException) ex.getCause())
138+
.getUnboundProperties();
139+
assertThat(unbound).hasSize(1);
140+
ConfigurationProperty property = unbound.iterator().next();
141+
assertThat(property.getName()).hasToString("foo[1].missing");
142+
assertThat(property.getValue()).isEqualTo("bad");
143+
});
144+
}
145+
126146
@Test
127147
void bindToNonScalarCollectionWhenNonSequentialShouldThrowException() {
128148
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
@@ -562,4 +582,8 @@ List<EnumSet<ExampleEnum>> getValues() {
562582

563583
}
564584

585+
record Name(String first, String last) {
586+
587+
}
588+
565589
}

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

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,8 +16,15 @@
1616

1717
package org.springframework.boot.context.properties.source;
1818

19+
import java.util.LinkedHashMap;
20+
import java.util.Map;
21+
22+
import org.assertj.core.extractor.Extractors;
1923
import org.junit.jupiter.api.Test;
2024

25+
import org.springframework.boot.env.OriginTrackedMapPropertySource;
26+
import org.springframework.core.env.PropertySource;
27+
2128
import static org.assertj.core.api.Assertions.assertThat;
2229

2330
/**
@@ -29,21 +36,16 @@
2936
class FilteredIterableConfigurationPropertiesSourceTests extends FilteredConfigurationPropertiesSourceTests {
3037

3138
@Test
32-
void iteratorShouldFilterNames() {
39+
void iteratorFiltersNames() {
3340
MockConfigurationPropertySource source = (MockConfigurationPropertySource) createTestSource();
3441
IterableConfigurationPropertySource filtered = source.filter(this::noBrackets);
3542
assertThat(filtered.iterator()).toIterable()
3643
.extracting(ConfigurationPropertyName::toString)
3744
.containsExactly("a", "b", "c");
3845
}
3946

40-
@Override
41-
protected ConfigurationPropertySource convertSource(MockConfigurationPropertySource source) {
42-
return source;
43-
}
44-
4547
@Test
46-
void containsDescendantOfShouldUseContents() {
48+
void containsDescendantOfUsesContents() {
4749
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
4850
source.put("foo.bar.baz", "1");
4951
source.put("foo.bar[0]", "1");
@@ -55,6 +57,45 @@ void containsDescendantOfShouldUseContents() {
5557
.isEqualTo(ConfigurationPropertyState.ABSENT);
5658
}
5759

60+
@Test
61+
void iteratorWhenSpringPropertySourceFiltersNames() {
62+
IterableConfigurationPropertySource testSource = (IterableConfigurationPropertySource) createTestSource();
63+
Map<String, Object> map = new LinkedHashMap<>();
64+
for (ConfigurationPropertyName name : testSource) {
65+
map.put(name.toString(), testSource.getConfigurationProperty(name).getValue());
66+
}
67+
PropertySource<?> propertySource = new OriginTrackedMapPropertySource("test", map, true);
68+
SpringConfigurationPropertySource source = SpringConfigurationPropertySource.from(propertySource);
69+
IterableConfigurationPropertySource filtered = (IterableConfigurationPropertySource) source
70+
.filter(this::noBrackets);
71+
assertThat(Extractors.byName("filteredNames").apply(filtered)).isNotNull();
72+
assertThat(filtered.iterator()).toIterable()
73+
.extracting(ConfigurationPropertyName::toString)
74+
.containsExactly("a", "b", "c");
75+
}
76+
77+
@Test
78+
void containsDescendantOfWhenSpringPropertySourceUsesContents() {
79+
Map<String, Object> map = new LinkedHashMap<>();
80+
map.put("foo.bar.baz", "1");
81+
map.put("foo.bar[0]", "1");
82+
map.put("faf.bar[0]", "1");
83+
PropertySource<?> propertySource = new OriginTrackedMapPropertySource("test", map, true);
84+
SpringConfigurationPropertySource source = SpringConfigurationPropertySource.from(propertySource);
85+
IterableConfigurationPropertySource filtered = (IterableConfigurationPropertySource) source
86+
.filter(this::noBrackets);
87+
assertThat(Extractors.byName("filteredNames").apply(filtered)).isNotNull();
88+
assertThat(filtered.containsDescendantOf(ConfigurationPropertyName.of("foo")))
89+
.isEqualTo(ConfigurationPropertyState.PRESENT);
90+
assertThat(filtered.containsDescendantOf(ConfigurationPropertyName.of("faf")))
91+
.isEqualTo(ConfigurationPropertyState.ABSENT);
92+
}
93+
94+
@Override
95+
protected ConfigurationPropertySource convertSource(MockConfigurationPropertySource source) {
96+
return source;
97+
}
98+
5899
private boolean noBrackets(ConfigurationPropertyName name) {
59100
return !name.toString().contains("[");
60101
}

0 commit comments

Comments
 (0)