Skip to content

Commit 4483f41

Browse files
philwebbphilwebb
committed
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
1 parent c19bed1 commit 4483f41

File tree

6 files changed

+139
-69
lines changed

6 files changed

+139
-69
lines changed

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

Lines changed: 93 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@
1616

1717
package org.springframework.boot.context.properties.bind.validation;
1818

19-
import java.util.Arrays;
2019
import java.util.Deque;
20+
import java.util.LinkedHashMap;
2121
import java.util.LinkedHashSet;
2222
import java.util.LinkedList;
23+
import java.util.Map;
2324
import java.util.Set;
2425
import java.util.stream.Collectors;
2526

@@ -29,8 +30,8 @@
2930
import org.springframework.boot.context.properties.bind.Bindable;
3031
import org.springframework.boot.context.properties.source.ConfigurationProperty;
3132
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
32-
import org.springframework.validation.BeanPropertyBindingResult;
33-
import org.springframework.validation.BindingResult;
33+
import org.springframework.core.ResolvableType;
34+
import org.springframework.validation.AbstractBindingResult;
3435
import org.springframework.validation.Validator;
3536

3637
/**
@@ -44,6 +45,10 @@ public class ValidationBindHandler extends AbstractBindHandler {
4445

4546
private final Validator[] validators;
4647

48+
private final Map<ConfigurationPropertyName, ResolvableType> boundTypes = new LinkedHashMap<>();
49+
50+
private final Map<ConfigurationPropertyName, Object> boundResults = new LinkedHashMap<>();
51+
4752
private final Set<ConfigurationProperty> boundProperties = new LinkedHashSet<>();
4853

4954
private final Deque<BindValidationException> exceptions = new LinkedList<>();
@@ -57,8 +62,15 @@ public ValidationBindHandler(BindHandler parent, Validator... validators) {
5762
this.validators = validators;
5863
}
5964

65+
@Override
66+
public <T> Bindable<T> onStart(ConfigurationPropertyName name, Bindable<T> target, BindContext context) {
67+
this.boundTypes.put(name, target.getType());
68+
return super.onStart(name, target, context);
69+
}
70+
6071
@Override
6172
public Object onSuccess(ConfigurationPropertyName name, Bindable<?> target, BindContext context, Object result) {
73+
this.boundResults.put(name, result);
6274
if (context.getConfigurationProperty() != null) {
6375
this.boundProperties.add(context.getConfigurationProperty());
6476
}
@@ -70,12 +82,20 @@ public Object onFailure(ConfigurationPropertyName name, Bindable<?> target, Bind
7082
throws Exception {
7183
Object result = super.onFailure(name, target, context, error);
7284
if (result != null) {
73-
this.exceptions.clear();
85+
clear();
86+
this.boundResults.put(name, result);
7487
}
7588
validate(name, target, context, result);
7689
return result;
7790
}
7891

92+
private void clear() {
93+
this.boundTypes.clear();
94+
this.boundResults.clear();
95+
this.boundProperties.clear();
96+
this.exceptions.clear();
97+
}
98+
7999
@Override
80100
public void onFinish(ConfigurationPropertyName name, Bindable<?> target, BindContext context, Object result)
81101
throws Exception {
@@ -105,20 +125,78 @@ private Object getValidationTarget(Bindable<?> target, BindContext context, Obje
105125
}
106126

107127
private void validateAndPush(ConfigurationPropertyName name, Object target, Class<?> type) {
108-
BindingResult errors = new BeanPropertyBindingResult(target, name.toString());
109-
Arrays.stream(this.validators).filter((validator) -> validator.supports(type))
110-
.forEach((validator) -> validator.validate(target, errors));
111-
if (errors.hasErrors()) {
112-
this.exceptions.push(getBindValidationException(name, errors));
128+
ValidationResult result = null;
129+
for (Validator validator : this.validators) {
130+
if (validator.supports(type)) {
131+
result = (result != null) ? result : new ValidationResult(name, target);
132+
validator.validate(target, result);
133+
}
134+
}
135+
if (result != null && result.hasErrors()) {
136+
this.exceptions.push(new BindValidationException(result.getValidationErrors()));
113137
}
114138
}
115139

116-
private BindValidationException getBindValidationException(ConfigurationPropertyName name, BindingResult errors) {
117-
Set<ConfigurationProperty> boundProperties = this.boundProperties.stream()
118-
.filter((property) -> name.isAncestorOf(property.getName()))
119-
.collect(Collectors.toCollection(LinkedHashSet::new));
120-
ValidationErrors validationErrors = new ValidationErrors(name, boundProperties, errors.getAllErrors());
121-
return new BindValidationException(validationErrors);
140+
/**
141+
* {@link AbstractBindingResult} implementation backed by the bound properties.
142+
*/
143+
private class ValidationResult extends AbstractBindingResult {
144+
145+
private final ConfigurationPropertyName name;
146+
147+
private Object target;
148+
149+
protected ValidationResult(ConfigurationPropertyName name, Object target) {
150+
super(null);
151+
this.name = name;
152+
this.target = target;
153+
}
154+
155+
@Override
156+
public String getObjectName() {
157+
return this.name.toString();
158+
}
159+
160+
@Override
161+
public Object getTarget() {
162+
return this.target;
163+
}
164+
165+
@Override
166+
public Class<?> getFieldType(String field) {
167+
try {
168+
ResolvableType type = ValidationBindHandler.this.boundTypes.get(getName(field));
169+
Class<?> resolved = (type != null) ? type.resolve() : null;
170+
if (resolved != null) {
171+
return resolved;
172+
}
173+
}
174+
catch (Exception ex) {
175+
}
176+
return super.getFieldType(field);
177+
}
178+
179+
@Override
180+
protected Object getActualFieldValue(String field) {
181+
try {
182+
return ValidationBindHandler.this.boundResults.get(getName(field));
183+
}
184+
catch (Exception ex) {
185+
}
186+
return null;
187+
}
188+
189+
private ConfigurationPropertyName getName(String field) {
190+
return this.name.append(field);
191+
}
192+
193+
ValidationErrors getValidationErrors() {
194+
Set<ConfigurationProperty> boundProperties = ValidationBindHandler.this.boundProperties.stream()
195+
.filter((property) -> this.name.isAncestorOf(property.getName()))
196+
.collect(Collectors.toCollection(LinkedHashSet::new));
197+
return new ValidationErrors(this.name, boundProperties, getAllErrors());
198+
}
199+
122200
}
123201

124202
}

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -179,17 +179,16 @@ public int getNumberOfElements() {
179179
}
180180

181181
/**
182-
* Create a new {@link ConfigurationPropertyName} by appending the given element
183-
* value.
184-
* @param elementValue the single element value to append
182+
* Create a new {@link ConfigurationPropertyName} by appending the given elements.
183+
* @param elements the elements to append
185184
* @return a new {@link ConfigurationPropertyName}
186-
* @throws InvalidConfigurationPropertyNameException if elementValue is not valid
185+
* @throws InvalidConfigurationPropertyNameException if the result is not valid
187186
*/
188-
public ConfigurationPropertyName append(String elementValue) {
189-
if (elementValue == null) {
187+
public ConfigurationPropertyName append(String elements) {
188+
if (elements == null) {
190189
return this;
191190
}
192-
Elements additionalElements = probablySingleElementOf(elementValue);
191+
Elements additionalElements = probablySingleElementOf(elements);
193192
return new ConfigurationPropertyName(this.elements.append(additionalElements));
194193
}
195194

@@ -660,14 +659,15 @@ private static class Elements {
660659
}
661660

662661
Elements append(Elements additional) {
663-
Assert.isTrue(additional.getSize() == 1,
664-
() -> "Element value '" + additional.getSource() + "' must be a single item");
665-
ElementType[] type = new ElementType[this.size + 1];
662+
int size = this.size + additional.size;
663+
ElementType[] type = new ElementType[size];
666664
System.arraycopy(this.type, 0, type, 0, this.size);
667-
type[this.size] = additional.type[0];
668-
CharSequence[] resolved = newResolved(this.size + 1);
669-
resolved[this.size] = additional.get(0);
670-
return new Elements(this.source, this.size + 1, this.start, this.end, type, resolved);
665+
System.arraycopy(additional.type, 0, type, this.size, additional.size);
666+
CharSequence[] resolved = newResolved(size);
667+
for (int i = 0; i < additional.size; i++) {
668+
resolved[this.size + i] = additional.get(i);
669+
}
670+
return new Elements(this.source, size, this.start, this.end, type, resolved);
671671
}
672672

673673
Elements chop(int size) {

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,9 +1626,7 @@ Map<String, BasicProperties> getProperties() {
16261626

16271627
@EnableConfigurationProperties
16281628
@ConfigurationProperties
1629-
public static class ValidatorProperties implements Validator {
1630-
1631-
// Needs to be public due to validator (see gh-17394)
1629+
static class ValidatorProperties implements Validator {
16321630

16331631
private String foo;
16341632

@@ -1642,11 +1640,11 @@ public void validate(Object target, Errors errors) {
16421640
ValidationUtils.rejectIfEmpty(errors, "foo", "TEST1");
16431641
}
16441642

1645-
public String getFoo() {
1643+
String getFoo() {
16461644
return this.foo;
16471645
}
16481646

1649-
public void setFoo(String foo) {
1647+
void setFoo(String foo) {
16501648
this.foo = foo;
16511649
}
16521650

@@ -1672,17 +1670,15 @@ void setFoo(String foo) {
16721670
}
16731671

16741672
@ConfigurationProperties(prefix = "custom")
1675-
public static class WithCustomValidatorProperties {
1676-
1677-
// Needs to be public due to validator (see gh-17394)
1673+
static class WithCustomValidatorProperties {
16781674

16791675
private String foo;
16801676

1681-
public String getFoo() {
1677+
String getFoo() {
16821678
return this.foo;
16831679
}
16841680

1685-
public void setFoo(String foo) {
1681+
void setFoo(String foo) {
16861682
this.foo = foo;
16871683
}
16881684

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,13 @@ void bindShouldFailWithAccessToBoundProperties() {
118118
}
119119

120120
@Test
121-
void bindShouldFailWithAccessToName() {
121+
void bindShouldFailWithAccessToNameAndValue() {
122122
this.sources.add(new MockConfigurationPropertySource("foo.nested.age", "4"));
123123
BindValidationException cause = bindAndExpectValidationError(() -> this.binder.bind(
124124
ConfigurationPropertyName.of("foo"), Bindable.of(ExampleValidatedWithNestedBean.class), this.handler));
125125
assertThat(cause.getValidationErrors().getName().toString()).isEqualTo("foo");
126126
assertThat(cause.getMessage()).contains("nested.age");
127+
assertThat(cause.getMessage()).contains("rejected value [4]");
127128
}
128129

129130
@Test
@@ -254,26 +255,22 @@ public void setYears(int years) {
254255
}
255256

256257
@Validated
257-
public static class ExampleValidatedWithNestedBean {
258-
259-
// Needs to be public due to validator (see gh-17394)
258+
static class ExampleValidatedWithNestedBean {
260259

261260
@Valid
262261
private ExampleNested nested = new ExampleNested();
263262

264-
public ExampleNested getNested() {
263+
ExampleNested getNested() {
265264
return this.nested;
266265
}
267266

268-
public void setNested(ExampleNested nested) {
267+
void setNested(ExampleNested nested) {
269268
this.nested = nested;
270269
}
271270

272271
}
273272

274-
public static class ExampleNested {
275-
276-
// Needs to be public due to validator (see gh-17394)
273+
static class ExampleNested {
277274

278275
private String name;
279276

@@ -283,27 +280,27 @@ public static class ExampleNested {
283280
@NotNull
284281
private String address;
285282

286-
public String getName() {
283+
String getName() {
287284
return this.name;
288285
}
289286

290-
public void setName(String name) {
287+
void setName(String name) {
291288
this.name = name;
292289
}
293290

294-
public int getAge() {
291+
int getAge() {
295292
return this.age;
296293
}
297294

298-
public void setAge(int age) {
295+
void setAge(int age) {
299296
this.age = age;
300297
}
301298

302-
public String getAddress() {
299+
String getAddress() {
303300
return this.address;
304301
}
305302

306-
public void setAddress(String address) {
303+
void setAddress(String address) {
307304
this.address = address;
308305
}
309306

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -401,9 +401,10 @@ void appendWhenElementNameIsNotValidShouldThrowException() {
401401
}
402402

403403
@Test
404-
void appendWhenElementNameMultiDotShouldThrowException() {
405-
assertThatIllegalArgumentException().isThrownBy(() -> ConfigurationPropertyName.of("foo").append("bar.baz"))
406-
.withMessageContaining("Element value 'bar.baz' must be a single item");
404+
void appendWhenElementNameMultiDotShouldAppend() {
405+
ConfigurationPropertyName name = ConfigurationPropertyName.of("foo").append("bar.baz");
406+
assertThat(name.toString()).isEqualTo("foo.bar.baz");
407+
assertThat(name.getNumberOfElements()).isEqualTo(3);
407408
}
408409

409410
@Test

0 commit comments

Comments
 (0)