Don't validate using BeanPropertyBindingResult

Update `ValidationBindHandler` so that a custom `AbstractBindingResult`
is used rather than `BeanPropertyBindingResult`. This allows us to
validate results, regardless of whether the actual bound instance has
public getters or setter.

Closes gh-17424

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandler.java

@@ -16,10 +16,11 @@
 
 package org.springframework.boot.context.properties.bind.validation;
 
-import java.util.Arrays;
 import java.util.Deque;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -29,8 +30,8 @@ import org.springframework.boot.context.properties.bind.BindHandler;
 import org.springframework.boot.context.properties.bind.Bindable;
 import org.springframework.boot.context.properties.source.ConfigurationProperty;
 import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
-import org.springframework.validation.BeanPropertyBindingResult;
+import org.springframework.core.ResolvableType;
-import org.springframework.validation.BindingResult;
+import org.springframework.validation.AbstractBindingResult;
 import org.springframework.validation.Validator;
 
 /**
@@ -44,6 +45,10 @@ public class ValidationBindHandler extends AbstractBindHandler {
 
 	private final Validator[] validators;
 
+	private final Map<ConfigurationPropertyName, ResolvableType> boundTypes = new LinkedHashMap<>();
+
+	private final Map<ConfigurationPropertyName, Object> boundResults = new LinkedHashMap<>();
+
 	private final Set<ConfigurationProperty> boundProperties = new LinkedHashSet<>();
 
 	private final Deque<BindValidationException> exceptions = new LinkedList<>();
@@ -57,8 +62,15 @@ public class ValidationBindHandler extends AbstractBindHandler {
 		this.validators = validators;
 	}
 
+	@Override
+	public <T> Bindable<T> onStart(ConfigurationPropertyName name, Bindable<T> target, BindContext context) {
+		this.boundTypes.put(name, target.getType());
+		return super.onStart(name, target, context);
+	}
+
 	@Override
 	public Object onSuccess(ConfigurationPropertyName name, Bindable<?> target, BindContext context, Object result) {
+		this.boundResults.put(name, result);
 		if (context.getConfigurationProperty() != null) {
 			this.boundProperties.add(context.getConfigurationProperty());
 		}
@@ -70,12 +82,20 @@ public class ValidationBindHandler extends AbstractBindHandler {
 			throws Exception {
 		Object result = super.onFailure(name, target, context, error);
 		if (result != null) {
-			this.exceptions.clear();
+			clear();
+			this.boundResults.put(name, result);
 		}
 		validate(name, target, context, result);
 		return result;
 	}
 
+	private void clear() {
+		this.boundTypes.clear();
+		this.boundResults.clear();
+		this.boundProperties.clear();
+		this.exceptions.clear();
+	}
+
 	@Override
 	public void onFinish(ConfigurationPropertyName name, Bindable<?> target, BindContext context, Object result)
 			throws Exception {
@@ -105,20 +125,78 @@ public class ValidationBindHandler extends AbstractBindHandler {
 	}
 
 	private void validateAndPush(ConfigurationPropertyName name, Object target, Class<?> type) {
-		BindingResult errors = new BeanPropertyBindingResult(target, name.toString());
+		ValidationResult result = null;
-		Arrays.stream(this.validators).filter((validator) -> validator.supports(type))
+		for (Validator validator : this.validators) {
-				.forEach((validator) -> validator.validate(target, errors));
+			if (validator.supports(type)) {
-		if (errors.hasErrors()) {
+				result = (result != null) ? result : new ValidationResult(name, target);
-			this.exceptions.push(getBindValidationException(name, errors));
+				validator.validate(target, result);
+			}
 		}
+		if (result != null && result.hasErrors()) {
+			this.exceptions.push(new BindValidationException(result.getValidationErrors()));
+		}
+	}
+
+	/**
+	 * {@link AbstractBindingResult} implementation backed by the bound properties.
+	 */
+	private class ValidationResult extends AbstractBindingResult {
+
+		private final ConfigurationPropertyName name;
+
+		private Object target;
+
+		protected ValidationResult(ConfigurationPropertyName name, Object target) {
+			super(null);
+			this.name = name;
+			this.target = target;
+		}
+
+		@Override
+		public String getObjectName() {
+			return this.name.toString();
 		}
 
-	private BindValidationException getBindValidationException(ConfigurationPropertyName name, BindingResult errors) {
+		@Override
-		Set<ConfigurationProperty> boundProperties = this.boundProperties.stream()
+		public Object getTarget() {
-				.filter((property) -> name.isAncestorOf(property.getName()))
+			return this.target;
+		}
+
+		@Override
+		public Class<?> getFieldType(String field) {
+			try {
+				ResolvableType type = ValidationBindHandler.this.boundTypes.get(getName(field));
+				Class<?> resolved = (type != null) ? type.resolve() : null;
+				if (resolved != null) {
+					return resolved;
+				}
+			}
+			catch (Exception ex) {
+			}
+			return super.getFieldType(field);
+		}
+
+		@Override
+		protected Object getActualFieldValue(String field) {
+			try {
+				return ValidationBindHandler.this.boundResults.get(getName(field));
+			}
+			catch (Exception ex) {
+			}
+			return null;
+		}
+
+		private ConfigurationPropertyName getName(String field) {
+			return this.name.append(field);
+		}
+
+		ValidationErrors getValidationErrors() {
+			Set<ConfigurationProperty> boundProperties = ValidationBindHandler.this.boundProperties.stream()
+					.filter((property) -> this.name.isAncestorOf(property.getName()))
 					.collect(Collectors.toCollection(LinkedHashSet::new));
-		ValidationErrors validationErrors = new ValidationErrors(name, boundProperties, errors.getAllErrors());
+			return new ValidationErrors(this.name, boundProperties, getAllErrors());
-		return new BindValidationException(validationErrors);
+		}
+
 	}
 
 }

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

@@ -179,17 +179,16 @@ public final class ConfigurationPropertyName implements Comparable<Configuration
 	}
 
 	/**
-	 * Create a new {@link ConfigurationPropertyName} by appending the given element
+	 * Create a new {@link ConfigurationPropertyName} by appending the given elements.
-	 * value.
+	 * @param elements the elements to append
-	 * @param elementValue the single element value to append
 	 * @return a new {@link ConfigurationPropertyName}
-	 * @throws InvalidConfigurationPropertyNameException if elementValue is not valid
+	 * @throws InvalidConfigurationPropertyNameException if the result is not valid
 	 */
-	public ConfigurationPropertyName append(String elementValue) {
+	public ConfigurationPropertyName append(String elements) {
-		if (elementValue == null) {
+		if (elements == null) {
 			return this;
 		}
-		Elements additionalElements = probablySingleElementOf(elementValue);
+		Elements additionalElements = probablySingleElementOf(elements);
 		return new ConfigurationPropertyName(this.elements.append(additionalElements));
 	}
 
@@ -660,14 +659,15 @@ public final class ConfigurationPropertyName implements Comparable<Configuration
 		}
 
 		Elements append(Elements additional) {
-			Assert.isTrue(additional.getSize() == 1,
+			int size = this.size + additional.size;
-					() -> "Element value '" + additional.getSource() + "' must be a single item");
+			ElementType[] type = new ElementType[size];
-			ElementType[] type = new ElementType[this.size + 1];
 			System.arraycopy(this.type, 0, type, 0, this.size);
-			type[this.size] = additional.type[0];
+			System.arraycopy(additional.type, 0, type, this.size, additional.size);
-			CharSequence[] resolved = newResolved(this.size + 1);
+			CharSequence[] resolved = newResolved(size);
-			resolved[this.size] = additional.get(0);
+			for (int i = 0; i < additional.size; i++) {
-			return new Elements(this.source, this.size + 1, this.start, this.end, type, resolved);
+				resolved[this.size + i] = additional.get(i);
+			}
+			return new Elements(this.source, size, this.start, this.end, type, resolved);
 		}
 
 		Elements chop(int size) {

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java

@@ -1626,9 +1626,7 @@ class ConfigurationPropertiesTests {
 
 	@EnableConfigurationProperties
 	@ConfigurationProperties
-	public static class ValidatorProperties implements Validator {
+	static class ValidatorProperties implements Validator {
-
-		// Needs to be public due to validator (see gh-17394)
 
 		private String foo;
 
@@ -1642,11 +1640,11 @@ class ConfigurationPropertiesTests {
 			ValidationUtils.rejectIfEmpty(errors, "foo", "TEST1");
 		}
 
-		public String getFoo() {
+		String getFoo() {
 			return this.foo;
 		}
 
-		public void setFoo(String foo) {
+		void setFoo(String foo) {
 			this.foo = foo;
 		}
 
@@ -1672,17 +1670,15 @@ class ConfigurationPropertiesTests {
 	}
 
 	@ConfigurationProperties(prefix = "custom")
-	public static class WithCustomValidatorProperties {
+	static class WithCustomValidatorProperties {
-
-		// Needs to be public due to validator (see gh-17394)
 
 		private String foo;
 
-		public String getFoo() {
+		String getFoo() {
 			return this.foo;
 		}
 
-		public void setFoo(String foo) {
+		void setFoo(String foo) {
 			this.foo = foo;
 		}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandlerTests.java

@@ -118,12 +118,13 @@ class ValidationBindHandlerTests {
 	}
 
 	@Test
-	void bindShouldFailWithAccessToName() {
+	void bindShouldFailWithAccessToNameAndValue() {
 		this.sources.add(new MockConfigurationPropertySource("foo.nested.age", "4"));
 		BindValidationException cause = bindAndExpectValidationError(() -> this.binder.bind(
 				ConfigurationPropertyName.of("foo"), Bindable.of(ExampleValidatedWithNestedBean.class), this.handler));
 		assertThat(cause.getValidationErrors().getName().toString()).isEqualTo("foo");
 		assertThat(cause.getMessage()).contains("nested.age");
+		assertThat(cause.getMessage()).contains("rejected value [4]");
 	}
 
 	@Test
@@ -254,26 +255,22 @@ class ValidationBindHandlerTests {
 	}
 
 	@Validated
-	public static class ExampleValidatedWithNestedBean {
+	static class ExampleValidatedWithNestedBean {
-
-		// Needs to be public due to validator (see gh-17394)
 
 		@Valid
 		private ExampleNested nested = new ExampleNested();
 
-		public ExampleNested getNested() {
+		ExampleNested getNested() {
 			return this.nested;
 		}
 
-		public void setNested(ExampleNested nested) {
+		void setNested(ExampleNested nested) {
 			this.nested = nested;
 		}
 
 	}
 
-	public static class ExampleNested {
+	static class ExampleNested {
-
-		// Needs to be public due to validator (see gh-17394)
 
 		private String name;
 
@@ -283,27 +280,27 @@ class ValidationBindHandlerTests {
 		@NotNull
 		private String address;
 
-		public String getName() {
+		String getName() {
 			return this.name;
 		}
 
-		public void setName(String name) {
+		void setName(String name) {
 			this.name = name;
 		}
 
-		public int getAge() {
+		int getAge() {
 			return this.age;
 		}
 
-		public void setAge(int age) {
+		void setAge(int age) {
 			this.age = age;
 		}
 
-		public String getAddress() {
+		String getAddress() {
 			return this.address;
 		}
 
-		public void setAddress(String address) {
+		void setAddress(String address) {
 			this.address = address;
 		}
 

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

@@ -401,9 +401,10 @@ class ConfigurationPropertyNameTests {
 	}
 
 	@Test
-	void appendWhenElementNameMultiDotShouldThrowException() {
+	void appendWhenElementNameMultiDotShouldAppend() {
-		assertThatIllegalArgumentException().isThrownBy(() -> ConfigurationPropertyName.of("foo").append("bar.baz"))
+		ConfigurationPropertyName name = ConfigurationPropertyName.of("foo").append("bar.baz");
-				.withMessageContaining("Element value 'bar.baz' must be a single item");
+		assertThat(name.toString()).isEqualTo("foo.bar.baz");
+		assertThat(name.getNumberOfElements()).isEqualTo(3);
 	}
 
 	@Test

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindValidationFailureAnalyzerTests.java

@@ -139,9 +139,7 @@ class BindValidationFailureAnalyzerTests {
 
 	@ConfigurationProperties("test.foo")
 	@Validated
-	public static class FieldValidationFailureProperties {
+	static class FieldValidationFailureProperties {
-
-		// Needs to be public due to validator (see gh-17394)
 
 		@NotNull
 		private String foo;
@@ -152,40 +150,40 @@ class BindValidationFailureAnalyzerTests {
 		@Valid
 		private FieldValidationFailureProperties.Nested nested = new FieldValidationFailureProperties.Nested();
 
-		public String getFoo() {
+		String getFoo() {
 			return this.foo;
 		}
 
-		public void setFoo(String foo) {
+		void setFoo(String foo) {
 			this.foo = foo;
 		}
 
-		public int getValue() {
+		int getValue() {
 			return this.value;
 		}
 
-		public void setValue(int value) {
+		void setValue(int value) {
 			this.value = value;
 		}
 
-		public FieldValidationFailureProperties.Nested getNested() {
+		FieldValidationFailureProperties.Nested getNested() {
 			return this.nested;
 		}
 
-		public void setNested(FieldValidationFailureProperties.Nested nested) {
+		void setNested(FieldValidationFailureProperties.Nested nested) {
 			this.nested = nested;
 		}
 
-		public static class Nested {
+		static class Nested {
 
 			@NotNull
 			private String bar;
 
-			public String getBar() {
+			String getBar() {
 				return this.bar;
 			}
 
-			public void setBar(String bar) {
+			void setBar(String bar) {
 				this.bar = bar;
 			}