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

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
This commit is contained in:
Phillip Webb 2019-07-03 21:55:52 -07:00
parent c19bed15d2
commit 4483f41791
6 changed files with 139 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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.validation.BindingResult;
import org.springframework.core.ResolvableType;
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());
Arrays.stream(this.validators).filter((validator) -> validator.supports(type))
.forEach((validator) -> validator.validate(target, errors));
if (errors.hasErrors()) {
this.exceptions.push(getBindValidationException(name, errors));
ValidationResult result = null;
for (Validator validator : this.validators) {
if (validator.supports(type)) {
result = (result != null) ? result : new ValidationResult(name, target);
validator.validate(target, result);
}
}
if (result != null && result.hasErrors()) {
this.exceptions.push(new BindValidationException(result.getValidationErrors()));
}
}
private BindValidationException getBindValidationException(ConfigurationPropertyName name, BindingResult errors) {
Set<ConfigurationProperty> boundProperties = this.boundProperties.stream()
.filter((property) -> name.isAncestorOf(property.getName()))
.collect(Collectors.toCollection(LinkedHashSet::new));
ValidationErrors validationErrors = new ValidationErrors(name, boundProperties, errors.getAllErrors());
return new BindValidationException(validationErrors);
/**
* {@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();
}
@Override
public Object getTarget() {
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));
return new ValidationErrors(this.name, boundProperties, getAllErrors());
}
}
}

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

@ -179,17 +179,16 @@ public final class ConfigurationPropertyName implements Comparable<Configuration
}
/**
* Create a new {@link ConfigurationPropertyName} by appending the given element
* value.
* @param elementValue the single element value to append
* Create a new {@link ConfigurationPropertyName} by appending the given elements.
* @param elements the elements 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) {
if (elementValue == null) {
public ConfigurationPropertyName append(String elements) {
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,
() -> "Element value '" + additional.getSource() + "' must be a single item");
ElementType[] type = new ElementType[this.size + 1];
int size = this.size + additional.size;
ElementType[] type = new ElementType[size];
System.arraycopy(this.type, 0, type, 0, this.size);
type[this.size] = additional.type[0];
CharSequence[] resolved = newResolved(this.size + 1);
resolved[this.size] = additional.get(0);
return new Elements(this.source, this.size + 1, this.start, this.end, type, resolved);
System.arraycopy(additional.type, 0, type, this.size, additional.size);
CharSequence[] resolved = newResolved(size);
for (int i = 0; i < additional.size; i++) {
resolved[this.size + i] = additional.get(i);
}
return new Elements(this.source, size, this.start, this.end, type, resolved);
}
Elements chop(int size) {

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

@ -1626,9 +1626,7 @@ class ConfigurationPropertiesTests {
@EnableConfigurationProperties
@ConfigurationProperties
public static class ValidatorProperties implements Validator {
// Needs to be public due to validator (see gh-17394)
static class ValidatorProperties implements Validator {
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 {
// Needs to be public due to validator (see gh-17394)
static class WithCustomValidatorProperties {
private String foo;
public String getFoo() {
String getFoo() {
return this.foo;
}
public void setFoo(String foo) {
void setFoo(String foo) {
this.foo = foo;
}

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

@ -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 {
// Needs to be public due to validator (see gh-17394)
static class ExampleValidatedWithNestedBean {
@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 {
// Needs to be public due to validator (see gh-17394)
static class ExampleNested {
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;
}

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

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

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

@ -139,9 +139,7 @@ class BindValidationFailureAnalyzerTests {
@ConfigurationProperties("test.foo")
@Validated
public static class FieldValidationFailureProperties {
// Needs to be public due to validator (see gh-17394)
static class FieldValidationFailureProperties {
@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;
}