root
/
spring-boot
mirror of https://github.com/spring-projects/spring-boot.git
1
0
Fork 0
Code Issues Actions 12 Packages Projects Releases Wiki Activity

Restore IndexedElementsBinder knownIndexedChildren checking logic 

Restore knownIndexedChildren logic in IndexedElementsBinder to that of
Spring Boot 3.4, effectively reverting commit 93113a415f.

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
This commit is contained in:
Phillip Webb 2025-06-17 10:04:01 -07:00
parent 34a8f3773d
commit 8f14dca164
6 changed files with 176 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java
View File

@ -18,10 +18,11 @@ package org.springframework.boot.context.properties.bind;
import java.lang.annotation.Annotation;
import java.util.Collection;
import java.util.List;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.springframework.boot.context.properties.bind.Binder.Context;
import org.springframework.boot.context.properties.source.ConfigurationProperty;
@ -30,8 +31,6 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyN
import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
import org.springframework.boot.context.properties.source.IterableConfigurationPropertySource;
import org.springframework.core.ResolvableType;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
/**
* Base class for {@link AggregateBinder AggregateBinders} that read a sequential run of
@ -106,23 +105,52 @@ abstract class IndexedElementsBinder<T> extends AggregateBinder<T> {
private void bindIndexed(ConfigurationPropertySource source, ConfigurationPropertyName root,
AggregateElementBinder elementBinder, IndexedCollectionSupplier collection, ResolvableType elementType) {
int firstUnboundIndex = 0;
boolean hasBindingGap = false;
Set<String> knownIndexedChildren = Collections.emptySet();
if (source instanceof IterableConfigurationPropertySource iterableSource) {
source = iterableSource.filter(root::isAncestorOf);
knownIndexedChildren = getKnownIndexedChildren(iterableSource, root);
}
for (int i = 0; i < Integer.MAX_VALUE; i++) {
ConfigurationPropertyName name = appendIndex(root, i);
Object value = elementBinder.bind(name, Bindable.of(elementType), source);
if (value != null) {
collection.get().add(value);
hasBindingGap = hasBindingGap || firstUnboundIndex > 0;
continue;
}
firstUnboundIndex = (firstUnboundIndex <= 0) ? i : firstUnboundIndex;
if (i - firstUnboundIndex > 10) {
if (value == null) {
break;
}
knownIndexedChildren.remove(name.getLastElement(Form.UNIFORM));
collection.get().add(value);
}
if (hasBindingGap) {
assertNoUnboundChildren(source, root, firstUnboundIndex);
if (source instanceof IterableConfigurationPropertySource iterableSource) {
assertNoUnboundChildren(knownIndexedChildren, iterableSource, root);
}
}
private Set<String> getKnownIndexedChildren(IterableConfigurationPropertySource filteredSource,
ConfigurationPropertyName root) {
Set<String> knownIndexedChildren = new HashSet<>();
for (ConfigurationPropertyName name : filteredSource) {
ConfigurationPropertyName choppedName = name.chop(root.getNumberOfElements() + 1);
if (choppedName.isLastElementIndexed()) {
knownIndexedChildren.add(choppedName.getLastElement(Form.UNIFORM));
}
}
return knownIndexedChildren;
}
private void assertNoUnboundChildren(Set<String> unboundIndexedChildren,
IterableConfigurationPropertySource filteredSource, ConfigurationPropertyName root) {
if (unboundIndexedChildren.isEmpty()) {
return;
}
Set<ConfigurationProperty> unboundProperties = new TreeSet<>();
for (ConfigurationPropertyName name : filteredSource) {
ConfigurationPropertyName choppedName = name.chop(root.getNumberOfElements() + 1);
if (choppedName.isLastElementIndexed()
&& unboundIndexedChildren.contains(choppedName.getLastElement(Form.UNIFORM))) {
unboundProperties.add(filteredSource.getConfigurationProperty(name));
}
}
if (!unboundProperties.isEmpty()) {
throw new UnboundConfigurationPropertiesException(unboundProperties);
}
}
@ -130,43 +158,6 @@ abstract class IndexedElementsBinder<T> extends AggregateBinder<T> {
return root.append((i < INDEXES.length) ? INDEXES[i] : "[" + i + "]");
}
private void assertNoUnboundChildren(ConfigurationPropertySource source, ConfigurationPropertyName root,
int firstUnboundIndex) {
MultiValueMap<String, ConfigurationPropertyName> knownIndexedChildren = getKnownIndexedChildren(source, root);
for (int i = 0; i < firstUnboundIndex; i++) {
ConfigurationPropertyName name = appendIndex(root, i);
knownIndexedChildren.remove(name.getLastElement(Form.UNIFORM));
}
assertNoUnboundChildren(source, knownIndexedChildren);
}
private MultiValueMap<String, ConfigurationPropertyName> getKnownIndexedChildren(ConfigurationPropertySource source,
ConfigurationPropertyName root) {
MultiValueMap<String, ConfigurationPropertyName> children = new LinkedMultiValueMap<>();
if (!(source instanceof IterableConfigurationPropertySource iterableSource)) {
return children;
}
for (ConfigurationPropertyName name : iterableSource.filter(root::isAncestorOf)) {
ConfigurationPropertyName choppedName = name.chop(root.getNumberOfElements() + 1);
if (choppedName.isLastElementIndexed()) {
String key = choppedName.getLastElement(Form.UNIFORM);
children.add(key, name);
}
}
return children;
}
private void assertNoUnboundChildren(ConfigurationPropertySource source,
MultiValueMap<String, ConfigurationPropertyName> children) {
if (!children.isEmpty()) {
throw new UnboundConfigurationPropertiesException(children.values()
.stream()
.flatMap(List::stream)
.map(source::getConfigurationProperty)
.collect(Collectors.toCollection(TreeSet::new)));
}
}
private <C> C convert(Object value, ResolvableType type, Annotation... annotations) {
value = getContext().getPlaceholdersResolver().resolvePlaceholders(value);
return getContext().getConverter().convert(value, type, annotations);

23
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyState.java
View File

@ -66,4 +66,27 @@ public enum ConfigurationPropertyState {
return ABSENT;
}
/**
* Search the given iterable using a predicate to determine if content is
* {@link #PRESENT} or {@link #ABSENT}.
* @param <T> the data type
* @param source the source iterable to search
* @param startInclusive the first index to cover
* @param endExclusive index immediately past the last index to cover
* @param predicate the predicate used to test for presence
* @return {@link #PRESENT} if the iterable contains a matching item, otherwise
* {@link #ABSENT}.
*/
static <T> ConfigurationPropertyState search(T[] source, int startInclusive, int endExclusive,
Predicate<T> predicate) {
Assert.notNull(source, "'source' must not be null");
Assert.notNull(predicate, "'predicate' must not be null");
for (int i = startInclusive; i < endExclusive; i++) {
if (predicate.test(source[i])) {
return PRESENT;
}
}
return ABSENT;
}
}

35
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/FilteredIterableConfigurationPropertiesSource.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -16,6 +16,7 @@
package org.springframework.boot.context.properties.source;
import java.util.Arrays;
import java.util.function.Predicate;
import java.util.stream.Stream;
@ -28,13 +29,41 @@ import java.util.stream.Stream;
class FilteredIterableConfigurationPropertiesSource extends FilteredConfigurationPropertiesSource
implements IterableConfigurationPropertySource {
private ConfigurationPropertyName[] filteredNames;
private int numerOfFilteredNames;
FilteredIterableConfigurationPropertiesSource(IterableConfigurationPropertySource source,
Predicate<ConfigurationPropertyName> filter) {
super(source, filter);
ConfigurationPropertyName[] filterableNames = getFilterableNames(source);
if (filterableNames != null) {
this.filteredNames = new ConfigurationPropertyName[filterableNames.length];
this.numerOfFilteredNames = 0;
for (ConfigurationPropertyName name : filterableNames) {
if (filter.test(name)) {
this.filteredNames[this.numerOfFilteredNames++] = name;
}
}
}
}
private ConfigurationPropertyName[] getFilterableNames(IterableConfigurationPropertySource source) {
if (source instanceof SpringIterableConfigurationPropertySource springPropertySource
&& springPropertySource.isImmutablePropertySource()) {
return springPropertySource.getConfigurationPropertyNames();
}
if (source instanceof FilteredIterableConfigurationPropertiesSource filteredSource) {
return filteredSource.filteredNames;
}
return null;
}
@Override
public Stream<ConfigurationPropertyName> stream() {
if (this.filteredNames != null) {
return Arrays.stream(this.filteredNames, 0, this.numerOfFilteredNames);
}
return getSource().stream().filter(getFilter());
}
@ -45,6 +74,10 @@ class FilteredIterableConfigurationPropertiesSource extends FilteredConfiguratio
@Override
public ConfigurationPropertyState containsDescendantOf(ConfigurationPropertyName name) {
if (this.filteredNames != null) {
return ConfigurationPropertyState.search(this.filteredNames, 0, this.numerOfFilteredNames,
name::isAncestorOf);
}
return ConfigurationPropertyState.search(this, name::isAncestorOf);
}

4
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java
View File

@ -169,7 +169,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
return false;
}
private ConfigurationPropertyName[] getConfigurationPropertyNames() {
ConfigurationPropertyName[] getConfigurationPropertyNames() {
if (!isImmutablePropertySource()) {
return getCache().getConfigurationPropertyNames(getPropertySource().getPropertyNames());
}
@ -197,7 +197,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
return cache;
}
private boolean isImmutablePropertySource() {
boolean isImmutablePropertySource() {
EnumerablePropertySource<?> source = getPropertySource();
if (source instanceof OriginLookup<?> originLookup) {
return originLookup.isImmutable();

26
spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -123,6 +123,26 @@ class CollectionBinderTests {
});
}
@Test
void bindToCollectionWhenNonKnownIndexedChildNotBoundThrowsException() {
// gh-45994
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("foo[0].first", "Spring");
source.put("foo[0].last", "Boot");
source.put("foo[1].missing", "bad");
this.sources.add(source);
assertThatExceptionOfType(BindException.class)
.isThrownBy(() -> this.binder.bind("foo", Bindable.listOf(Name.class)))
.satisfies((ex) -> {
Set<ConfigurationProperty> unbound = ((UnboundConfigurationPropertiesException) ex.getCause())
.getUnboundProperties();
assertThat(unbound).hasSize(1);
ConfigurationProperty property = unbound.iterator().next();
assertThat(property.getName()).hasToString("foo[1].missing");
assertThat(property.getValue()).isEqualTo("bad");
});
}
@Test
void bindToNonScalarCollectionWhenNonSequentialShouldThrowException() {
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
@ -562,4 +582,8 @@ class CollectionBinderTests {
}
record Name(String first, String last) {
}
}

57
spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/FilteredIterableConfigurationPropertiesSourceTests.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -16,8 +16,15 @@
package org.springframework.boot.context.properties.source;
import java.util.LinkedHashMap;
import java.util.Map;
import org.assertj.core.extractor.Extractors;
import org.junit.jupiter.api.Test;
import org.springframework.boot.env.OriginTrackedMapPropertySource;
import org.springframework.core.env.PropertySource;
import static org.assertj.core.api.Assertions.assertThat;
/**
@ -29,7 +36,7 @@ import static org.assertj.core.api.Assertions.assertThat;
class FilteredIterableConfigurationPropertiesSourceTests extends FilteredConfigurationPropertiesSourceTests {
@Test
void iteratorShouldFilterNames() {
void iteratorFiltersNames() {
MockConfigurationPropertySource source = (MockConfigurationPropertySource) createTestSource();
IterableConfigurationPropertySource filtered = source.filter(this::noBrackets);
assertThat(filtered.iterator()).toIterable()
@ -37,13 +44,8 @@ class FilteredIterableConfigurationPropertiesSourceTests extends FilteredConfigu
.containsExactly("a", "b", "c");
}
@Override
protected ConfigurationPropertySource convertSource(MockConfigurationPropertySource source) {
return source;
}
@Test
void containsDescendantOfShouldUseContents() {
void containsDescendantOfUsesContents() {
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("foo.bar.baz", "1");
source.put("foo.bar[0]", "1");
@ -55,6 +57,45 @@ class FilteredIterableConfigurationPropertiesSourceTests extends FilteredConfigu
.isEqualTo(ConfigurationPropertyState.ABSENT);
}
@Test
void iteratorWhenSpringPropertySourceFiltersNames() {
IterableConfigurationPropertySource testSource = (IterableConfigurationPropertySource) createTestSource();
Map<String, Object> map = new LinkedHashMap<>();
for (ConfigurationPropertyName name : testSource) {
map.put(name.toString(), testSource.getConfigurationProperty(name).getValue());
}
PropertySource<?> propertySource = new OriginTrackedMapPropertySource("test", map, true);
SpringConfigurationPropertySource source = SpringConfigurationPropertySource.from(propertySource);
IterableConfigurationPropertySource filtered = (IterableConfigurationPropertySource) source
.filter(this::noBrackets);
assertThat(Extractors.byName("filteredNames").apply(filtered)).isNotNull();
assertThat(filtered.iterator()).toIterable()
.extracting(ConfigurationPropertyName::toString)
.containsExactly("a", "b", "c");
}
@Test
void containsDescendantOfWhenSpringPropertySourceUsesContents() {
Map<String, Object> map = new LinkedHashMap<>();
map.put("foo.bar.baz", "1");
map.put("foo.bar[0]", "1");
map.put("faf.bar[0]", "1");
PropertySource<?> propertySource = new OriginTrackedMapPropertySource("test", map, true);
SpringConfigurationPropertySource source = SpringConfigurationPropertySource.from(propertySource);
IterableConfigurationPropertySource filtered = (IterableConfigurationPropertySource) source
.filter(this::noBrackets);
assertThat(Extractors.byName("filteredNames").apply(filtered)).isNotNull();
assertThat(filtered.containsDescendantOf(ConfigurationPropertyName.of("foo")))
.isEqualTo(ConfigurationPropertyState.PRESENT);
assertThat(filtered.containsDescendantOf(ConfigurationPropertyName.of("faf")))
.isEqualTo(ConfigurationPropertyState.ABSENT);
}
@Override
protected ConfigurationPropertySource convertSource(MockConfigurationPropertySource source) {
return source;
}
private boolean noBrackets(ConfigurationPropertyName name) {
return !name.toString().contains("[");
}