Skip to content

Commit 9bbe3aa

Browse files
k-sethrstoyanchev
authored andcommitted
Expand support for adapting container type violations
See gh-31530
1 parent f16122d commit 9bbe3aa

File tree

3 files changed

+92
-36
lines changed

3 files changed

+92
-36
lines changed

spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java

+17-13
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ private MethodValidationResult adaptViolations(
302302
Function<Integer, Object> argumentFunction) {
303303

304304
Map<MethodParameter, ValueResultBuilder> parameterViolations = new LinkedHashMap<>();
305-
Map<Path.Node, BeanResultBuilder> cascadedViolations = new LinkedHashMap<>();
305+
Map<CascadedViolationsKey, BeanResultBuilder> cascadedViolations = new LinkedHashMap<>();
306306

307307
for (ConstraintViolation<Object> violation : violations) {
308308
Iterator<Path.Node> itr = violation.getPropertyPath().iterator();
@@ -329,7 +329,8 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) {
329329
}
330330
else {
331331
cascadedViolations
332-
.computeIfAbsent(node, n -> new BeanResultBuilder(parameter, argument, itr.next()))
332+
.computeIfAbsent(new CascadedViolationsKey(node, violation.getLeafBean()),
333+
n -> new BeanResultBuilder(parameter, argument, itr.next(), violation.getLeafBean()))
333334
.addViolation(violation);
334335
}
335336
break;
@@ -338,7 +339,7 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) {
338339

339340
List<ParameterValidationResult> validatonResultList = new ArrayList<>();
340341
parameterViolations.forEach((parameter, builder) -> validatonResultList.add(builder.build()));
341-
cascadedViolations.forEach((node, builder) -> validatonResultList.add(builder.build()));
342+
cascadedViolations.forEach((violationsKey, builder) -> validatonResultList.add(builder.build()));
342343
validatonResultList.sort(resultComparator);
343344

344345
return MethodValidationResult.create(target, method, validatonResultList);
@@ -372,6 +373,14 @@ private BindingResult createBindingResult(MethodParameter parameter, @Nullable O
372373
return result;
373374
}
374375

376+
/**
377+
* A unique key for the cascaded violations map. Individually, the node and leaf bean may not be unique for all
378+
* collection types ({@link Set} will have the same node and {@link List} may have the same leaf), but together
379+
* they should represent a distinct pairing.
380+
* @param node the path of the violation
381+
* @param leafBean the validated object
382+
*/
383+
record CascadedViolationsKey(Path.Node node, Object leafBean) { }
375384

376385
/**
377386
* Strategy to resolve the name of an {@code @Valid} method parameter to
@@ -446,25 +455,20 @@ private final class BeanResultBuilder {
446455

447456
private final Set<ConstraintViolation<Object>> violations = new LinkedHashSet<>();
448457

449-
public BeanResultBuilder(MethodParameter parameter, @Nullable Object argument, Path.Node node) {
458+
public BeanResultBuilder(MethodParameter parameter, @Nullable Object argument, Path.Node node, @Nullable Object leafBean) {
450459
this.parameter = parameter;
451460

452461
this.containerIndex = node.getIndex();
453462
this.containerKey = node.getKey();
454-
if (argument instanceof List<?> list && this.containerIndex != null) {
455-
this.container = list;
456-
argument = list.get(this.containerIndex);
457-
}
458-
else if (argument instanceof Map<?, ?> map && this.containerKey != null) {
459-
this.container = map;
460-
argument = map.get(this.containerKey);
463+
if (argument != null && !argument.equals(leafBean)) {
464+
this.container = argument;
461465
}
462466
else {
463467
this.container = null;
464468
}
465469

466-
this.argument = argument;
467-
this.errors = createBindingResult(parameter, argument);
470+
this.argument = leafBean;
471+
this.errors = createBindingResult(parameter, leafBean);
468472
}
469473

470474
public void addViolation(ConstraintViolation<Object> violation) {

spring-context/src/main/java/org/springframework/validation/method/ParameterErrors.java

+16-15
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,10 @@
3232
* {@link Errors#getAllErrors()}, but this subclass provides access to the same
3333
* as {@link FieldError}s.
3434
*
35-
* <p>When the method parameter is a {@link List} or {@link java.util.Map},
36-
* a separate {@link ParameterErrors} is created for each list or map value for
37-
* which there are validation errors. In such cases, the {@link #getContainer()}
38-
* method returns the list or map, while {@link #getContainerIndex()}
39-
* and {@link #getContainerKey()} return the value index or key.
35+
* <p>When the method parameter is a multi-element container like {@link List} or
36+
* {@link java.util.Map}, a separate {@link ParameterErrors} is created for each
37+
* value for which there are validation errors. Otherwise, only a single
38+
* {@link ParameterErrors} will be created.
4039
*
4140
* @author Rossen Stoyanchev
4241
* @since 6.1
@@ -71,30 +70,32 @@ public ParameterErrors(
7170

7271

7372
/**
74-
* When {@code @Valid} is declared on a {@link List} or {@link java.util.Map}
75-
* method parameter, this method returns the list or map that contained the
76-
* validated object {@link #getArgument() argument}, while
77-
* {@link #getContainerIndex()} and {@link #getContainerKey()} returns the
78-
* respective index or key.
73+
* When {@code @Valid} is declared on a container type method parameter such as
74+
* {@link java.util.Collection}, {@link java.util.Optional} or {@link java.util.Map},
75+
* this method returns the parent that contained the validated object
76+
* {@link #getArgument() argument}, while {@link #getContainerIndex()} and
77+
* {@link #getContainerKey()} returns the respective index or key if the parameter's
78+
* datatype supports such access.
7979
*/
8080
@Nullable
8181
public Object getContainer() {
8282
return this.container;
8383
}
8484

8585
/**
86-
* When {@code @Valid} is declared on a {@link List}, this method returns
87-
* the index under which the validated object {@link #getArgument() argument}
88-
* is stored in the list {@link #getContainer() container}.
86+
* When {@code @Valid} is declared on an indexed type, such as {@link List},
87+
* this method returns the index under which the validated object
88+
* {@link #getArgument() argument} is stored in the list
89+
* {@link #getContainer() container}.
8990
*/
9091
@Nullable
9192
public Integer getContainerIndex() {
9293
return this.containerIndex;
9394
}
9495

9596
/**
96-
* When {@code @Valid} is declared on a {@link java.util.Map}, this method
97-
* returns the key under which the validated object {@link #getArgument()
97+
* When {@code @Valid} is declared on a keyed typed, such as {@link java.util.Map},
98+
* this method returns the key under which the validated object {@link #getArgument()
9899
* argument} is stored in the map {@link #getContainer()}.
99100
*/
100101
@Nullable

spring-context/src/test/java/org/springframework/validation/beanvalidation/MethodValidationAdapterTests.java

+59-8
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717
package org.springframework.validation.beanvalidation;
1818

1919
import java.lang.reflect.Method;
20+
import java.util.Collection;
2021
import java.util.List;
2122
import java.util.Locale;
23+
import java.util.Set;
2224
import java.util.function.Consumer;
2325

2426
import jakarta.validation.Valid;
2527
import jakarta.validation.constraints.Max;
2628
import jakarta.validation.constraints.Min;
29+
import jakarta.validation.constraints.NotBlank;
2730
import jakarta.validation.constraints.Size;
2831
import org.junit.jupiter.api.AfterEach;
2932
import org.junit.jupiter.api.BeforeEach;
@@ -47,9 +50,9 @@
4750
*/
4851
public class MethodValidationAdapterTests {
4952

50-
private static final Person faustino1234 = new Person("Faustino1234");
53+
private static final Person faustino1234 = new Person("Faustino1234", List.of("Working on Spring"));
5154

52-
private static final Person cayetana6789 = new Person("Cayetana6789");
55+
private static final Person cayetana6789 = new Person("Cayetana6789", List.of(" "));
5356

5457

5558
private final MethodValidationAdapter validationAdapter = new MethodValidationAdapter();
@@ -88,7 +91,13 @@ void validateArguments() {
8891
codes [Size.guardian.name,Size.name,Size.java.lang.String,Size]; \
8992
arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \
9093
codes [guardian.name,name]; arguments []; default message [name],10,1]; \
91-
default message [size must be between 1 and 10]"""));
94+
default message [size must be between 1 and 10]""", """
95+
Field error in object 'guardian' on field 'hobbies[0]': rejected value [ ]; \
96+
codes [NotBlank.guardian.hobbies[0],NotBlank.guardian.hobbies,NotBlank.hobbies[0],\
97+
NotBlank.hobbies,NotBlank.java.lang.String,NotBlank]; arguments \
98+
[org.springframework.context.support.DefaultMessageSourceResolvable: codes \
99+
[guardian.hobbies[0],hobbies[0]]; arguments []; default message [hobbies[0]]]; \
100+
default message [must not be blank]"""));
92101

93102
assertValueResult(ex.getValueResults().get(0), 2, 3, List.of("""
94103
org.springframework.context.support.DefaultMessageSourceResolvable: \
@@ -106,7 +115,7 @@ void validateArgumentWithCustomObjectName() {
106115

107116
this.validationAdapter.setObjectNameResolver((param, value) -> "studentToAdd");
108117

109-
testArgs(target, method, new Object[] {faustino1234, new Person("Joe"), 1}, ex -> {
118+
testArgs(target, method, new Object[] {faustino1234, new Person("Joe", List.of()), 1}, ex -> {
110119

111120
assertThat(ex.getAllValidationResults()).hasSize(1);
112121

@@ -178,7 +187,49 @@ void validateListArgument() {
178187
codes [Size.people.name,Size.name,Size.java.lang.String,Size]; \
179188
arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \
180189
codes [people.name,name]; arguments []; default message [name],10,1]; \
181-
default message [size must be between 1 and 10]"""));
190+
default message [size must be between 1 and 10]""", """
191+
Field error in object 'people' on field 'hobbies[0]': rejected value [ ]; \
192+
codes [NotBlank.people.hobbies[0],NotBlank.people.hobbies,NotBlank.hobbies[0],\
193+
NotBlank.hobbies,NotBlank.java.lang.String,NotBlank]; arguments \
194+
[org.springframework.context.support.DefaultMessageSourceResolvable: codes \
195+
[people.hobbies[0],hobbies[0]]; arguments []; default message [hobbies[0]]]; \
196+
default message [must not be blank]"""));
197+
});
198+
}
199+
200+
@Test
201+
void validateSetArgument() {
202+
MyService target = new MyService();
203+
Method method = getMethod(target, "addPeople");
204+
205+
testArgs(target, method, new Object[] {Set.of(faustino1234, cayetana6789)}, ex -> {
206+
207+
assertThat(ex.getAllValidationResults()).hasSize(2);
208+
209+
int paramIndex = 0;
210+
String objectName = "people";
211+
List<ParameterErrors> results = ex.getBeanResults();
212+
213+
assertThat(results).satisfiesExactlyInAnyOrder(
214+
result -> assertBeanResult(result, paramIndex, objectName, faustino1234, List.of("""
215+
Field error in object 'people' on field 'name': rejected value [Faustino1234]; \
216+
codes [Size.people.name,Size.name,Size.java.lang.String,Size]; \
217+
arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \
218+
codes [people.name,name]; arguments []; default message [name],10,1]; \
219+
default message [size must be between 1 and 10]""")),
220+
result -> assertBeanResult(result, paramIndex, objectName, cayetana6789, List.of("""
221+
Field error in object 'people' on field 'name': rejected value [Cayetana6789]; \
222+
codes [Size.people.name,Size.name,Size.java.lang.String,Size]; \
223+
arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \
224+
codes [people.name,name]; arguments []; default message [name],10,1]; \
225+
default message [size must be between 1 and 10]""", """
226+
Field error in object 'people' on field 'hobbies[0]': rejected value [ ]; \
227+
codes [NotBlank.people.hobbies[0],NotBlank.people.hobbies,NotBlank.hobbies[0],\
228+
NotBlank.hobbies,NotBlank.java.lang.String,NotBlank]; arguments \
229+
[org.springframework.context.support.DefaultMessageSourceResolvable: codes \
230+
[people.hobbies[0],hobbies[0]]; arguments []; default message [hobbies[0]]]; \
231+
default message [must not be blank]"""))
232+
);
182233
});
183234
}
184235

@@ -191,7 +242,7 @@ private void testReturnValue(Object target, Method method, @Nullable Object valu
191242
}
192243

193244
private static void assertBeanResult(
194-
ParameterErrors errors, int parameterIndex, String objectName, Object argument,
245+
ParameterErrors errors, int parameterIndex, String objectName, @Nullable Object argument,
195246
List<String> fieldErrors) {
196247

197248
assertThat(errors.getMethodParameter().getParameterIndex()).isEqualTo(parameterIndex);
@@ -234,14 +285,14 @@ public Person getPerson() {
234285
throw new UnsupportedOperationException();
235286
}
236287

237-
public void addPeople(@Valid List<Person> people) {
288+
public void addPeople(@Valid Collection<Person> people) {
238289
}
239290

240291
}
241292

242293

243294
@SuppressWarnings("unused")
244-
private record Person(@Size(min = 1, max = 10) String name) {
295+
private record Person(@Size(min = 1, max = 10) String name, List<@NotBlank String> hobbies) {
245296
}
246297

247298
}

0 commit comments

Comments
 (0)