diff --git a/spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java b/spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java index 2863b0d33707..a472531bfbba 100644 --- a/spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java +++ b/spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java @@ -302,7 +302,7 @@ private MethodValidationResult adaptViolations( Function argumentFunction) { Map parameterViolations = new LinkedHashMap<>(); - Map cascadedViolations = new LinkedHashMap<>(); + Map cascadedViolations = new LinkedHashMap<>(); for (ConstraintViolation violation : violations) { Iterator itr = violation.getPropertyPath().iterator(); @@ -329,7 +329,8 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) { } else { cascadedViolations - .computeIfAbsent(node, n -> new BeanResultBuilder(parameter, argument, itr.next())) + .computeIfAbsent(new CascadedViolationsKey(node, violation.getLeafBean()), + n -> new BeanResultBuilder(parameter, argument, itr.next(), violation.getLeafBean())) .addViolation(violation); } break; @@ -338,7 +339,7 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) { List validatonResultList = new ArrayList<>(); parameterViolations.forEach((parameter, builder) -> validatonResultList.add(builder.build())); - cascadedViolations.forEach((node, builder) -> validatonResultList.add(builder.build())); + cascadedViolations.forEach((violationsKey, builder) -> validatonResultList.add(builder.build())); validatonResultList.sort(resultComparator); return MethodValidationResult.create(target, method, validatonResultList); @@ -372,6 +373,14 @@ private BindingResult createBindingResult(MethodParameter parameter, @Nullable O return result; } + /** + * A unique key for the cascaded violations map. Individually, the node and leaf bean may not be unique for all + * collection types ({@link Set} will have the same node and {@link List} may have the same leaf), but together + * they should represent a distinct pairing. + * @param node the path of the violation + * @param leafBean the validated object + */ + record CascadedViolationsKey(Path.Node node, Object leafBean) { } /** * Strategy to resolve the name of an {@code @Valid} method parameter to @@ -446,25 +455,20 @@ private final class BeanResultBuilder { private final Set> violations = new LinkedHashSet<>(); - public BeanResultBuilder(MethodParameter parameter, @Nullable Object argument, Path.Node node) { + public BeanResultBuilder(MethodParameter parameter, @Nullable Object argument, Path.Node node, @Nullable Object leafBean) { this.parameter = parameter; this.containerIndex = node.getIndex(); this.containerKey = node.getKey(); - if (argument instanceof List list && this.containerIndex != null) { - this.container = list; - argument = list.get(this.containerIndex); - } - else if (argument instanceof Map map && this.containerKey != null) { - this.container = map; - argument = map.get(this.containerKey); + if (argument != null && !argument.equals(leafBean)) { + this.container = argument; } else { this.container = null; } - this.argument = argument; - this.errors = createBindingResult(parameter, argument); + this.argument = leafBean; + this.errors = createBindingResult(parameter, leafBean); } public void addViolation(ConstraintViolation violation) { diff --git a/spring-context/src/main/java/org/springframework/validation/method/ParameterErrors.java b/spring-context/src/main/java/org/springframework/validation/method/ParameterErrors.java index 58c54713bf04..3f317cdaf89e 100644 --- a/spring-context/src/main/java/org/springframework/validation/method/ParameterErrors.java +++ b/spring-context/src/main/java/org/springframework/validation/method/ParameterErrors.java @@ -32,11 +32,10 @@ * {@link Errors#getAllErrors()}, but this subclass provides access to the same * as {@link FieldError}s. * - *

When the method parameter is a {@link List} or {@link java.util.Map}, - * a separate {@link ParameterErrors} is created for each list or map value for - * which there are validation errors. In such cases, the {@link #getContainer()} - * method returns the list or map, while {@link #getContainerIndex()} - * and {@link #getContainerKey()} return the value index or key. + *

When the method parameter is a multi-element container like {@link List} or + * {@link java.util.Map}, a separate {@link ParameterErrors} is created for each + * value for which there are validation errors. Otherwise, only a single + * {@link ParameterErrors} will be created. * * @author Rossen Stoyanchev * @since 6.1 @@ -71,11 +70,12 @@ public ParameterErrors( /** - * When {@code @Valid} is declared on a {@link List} or {@link java.util.Map} - * method parameter, this method returns the list or map that contained the - * validated object {@link #getArgument() argument}, while - * {@link #getContainerIndex()} and {@link #getContainerKey()} returns the - * respective index or key. + * When {@code @Valid} is declared on a container type method parameter such as + * {@link java.util.Collection}, {@link java.util.Optional} or {@link java.util.Map}, + * this method returns the parent that contained the validated object + * {@link #getArgument() argument}, while {@link #getContainerIndex()} and + * {@link #getContainerKey()} returns the respective index or key if the parameter's + * datatype supports such access. */ @Nullable public Object getContainer() { @@ -83,9 +83,10 @@ public Object getContainer() { } /** - * When {@code @Valid} is declared on a {@link List}, this method returns - * the index under which the validated object {@link #getArgument() argument} - * is stored in the list {@link #getContainer() container}. + * When {@code @Valid} is declared on an indexed type, such as {@link List}, + * this method returns the index under which the validated object + * {@link #getArgument() argument} is stored in the list + * {@link #getContainer() container}. */ @Nullable public Integer getContainerIndex() { @@ -93,8 +94,8 @@ public Integer getContainerIndex() { } /** - * When {@code @Valid} is declared on a {@link java.util.Map}, this method - * returns the key under which the validated object {@link #getArgument() + * When {@code @Valid} is declared on a keyed typed, such as {@link java.util.Map}, + * this method returns the key under which the validated object {@link #getArgument() * argument} is stored in the map {@link #getContainer()}. */ @Nullable diff --git a/spring-context/src/test/java/org/springframework/validation/beanvalidation/MethodValidationAdapterTests.java b/spring-context/src/test/java/org/springframework/validation/beanvalidation/MethodValidationAdapterTests.java index 9714afbce837..7f367e2d13b7 100644 --- a/spring-context/src/test/java/org/springframework/validation/beanvalidation/MethodValidationAdapterTests.java +++ b/spring-context/src/test/java/org/springframework/validation/beanvalidation/MethodValidationAdapterTests.java @@ -17,13 +17,16 @@ package org.springframework.validation.beanvalidation; import java.lang.reflect.Method; +import java.util.Collection; import java.util.List; import java.util.Locale; +import java.util.Set; import java.util.function.Consumer; import jakarta.validation.Valid; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.Size; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -47,9 +50,9 @@ */ public class MethodValidationAdapterTests { - private static final Person faustino1234 = new Person("Faustino1234"); + private static final Person faustino1234 = new Person("Faustino1234", List.of("Working on Spring")); - private static final Person cayetana6789 = new Person("Cayetana6789"); + private static final Person cayetana6789 = new Person("Cayetana6789", List.of(" ")); private final MethodValidationAdapter validationAdapter = new MethodValidationAdapter(); @@ -88,7 +91,13 @@ void validateArguments() { codes [Size.guardian.name,Size.name,Size.java.lang.String,Size]; \ arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \ codes [guardian.name,name]; arguments []; default message [name],10,1]; \ - default message [size must be between 1 and 10]""")); + default message [size must be between 1 and 10]""", """ + Field error in object 'guardian' on field 'hobbies[0]': rejected value [ ]; \ + codes [NotBlank.guardian.hobbies[0],NotBlank.guardian.hobbies,NotBlank.hobbies[0],\ + NotBlank.hobbies,NotBlank.java.lang.String,NotBlank]; arguments \ + [org.springframework.context.support.DefaultMessageSourceResolvable: codes \ + [guardian.hobbies[0],hobbies[0]]; arguments []; default message [hobbies[0]]]; \ + default message [must not be blank]""")); assertValueResult(ex.getValueResults().get(0), 2, 3, List.of(""" org.springframework.context.support.DefaultMessageSourceResolvable: \ @@ -106,7 +115,7 @@ void validateArgumentWithCustomObjectName() { this.validationAdapter.setObjectNameResolver((param, value) -> "studentToAdd"); - testArgs(target, method, new Object[] {faustino1234, new Person("Joe"), 1}, ex -> { + testArgs(target, method, new Object[] {faustino1234, new Person("Joe", List.of()), 1}, ex -> { assertThat(ex.getAllValidationResults()).hasSize(1); @@ -178,7 +187,49 @@ void validateListArgument() { codes [Size.people.name,Size.name,Size.java.lang.String,Size]; \ arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \ codes [people.name,name]; arguments []; default message [name],10,1]; \ - default message [size must be between 1 and 10]""")); + default message [size must be between 1 and 10]""", """ + Field error in object 'people' on field 'hobbies[0]': rejected value [ ]; \ + codes [NotBlank.people.hobbies[0],NotBlank.people.hobbies,NotBlank.hobbies[0],\ + NotBlank.hobbies,NotBlank.java.lang.String,NotBlank]; arguments \ + [org.springframework.context.support.DefaultMessageSourceResolvable: codes \ + [people.hobbies[0],hobbies[0]]; arguments []; default message [hobbies[0]]]; \ + default message [must not be blank]""")); + }); + } + + @Test + void validateSetArgument() { + MyService target = new MyService(); + Method method = getMethod(target, "addPeople"); + + testArgs(target, method, new Object[] {Set.of(faustino1234, cayetana6789)}, ex -> { + + assertThat(ex.getAllValidationResults()).hasSize(2); + + int paramIndex = 0; + String objectName = "people"; + List results = ex.getBeanResults(); + + assertThat(results).satisfiesExactlyInAnyOrder( + result -> assertBeanResult(result, paramIndex, objectName, faustino1234, List.of(""" + Field error in object 'people' on field 'name': rejected value [Faustino1234]; \ + codes [Size.people.name,Size.name,Size.java.lang.String,Size]; \ + arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \ + codes [people.name,name]; arguments []; default message [name],10,1]; \ + default message [size must be between 1 and 10]""")), + result -> assertBeanResult(result, paramIndex, objectName, cayetana6789, List.of(""" + Field error in object 'people' on field 'name': rejected value [Cayetana6789]; \ + codes [Size.people.name,Size.name,Size.java.lang.String,Size]; \ + arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \ + codes [people.name,name]; arguments []; default message [name],10,1]; \ + default message [size must be between 1 and 10]""", """ + Field error in object 'people' on field 'hobbies[0]': rejected value [ ]; \ + codes [NotBlank.people.hobbies[0],NotBlank.people.hobbies,NotBlank.hobbies[0],\ + NotBlank.hobbies,NotBlank.java.lang.String,NotBlank]; arguments \ + [org.springframework.context.support.DefaultMessageSourceResolvable: codes \ + [people.hobbies[0],hobbies[0]]; arguments []; default message [hobbies[0]]]; \ + default message [must not be blank]""")) + ); }); } @@ -191,7 +242,7 @@ private void testReturnValue(Object target, Method method, @Nullable Object valu } private static void assertBeanResult( - ParameterErrors errors, int parameterIndex, String objectName, Object argument, + ParameterErrors errors, int parameterIndex, String objectName, @Nullable Object argument, List fieldErrors) { assertThat(errors.getMethodParameter().getParameterIndex()).isEqualTo(parameterIndex); @@ -234,14 +285,14 @@ public Person getPerson() { throw new UnsupportedOperationException(); } - public void addPeople(@Valid List people) { + public void addPeople(@Valid Collection people) { } } @SuppressWarnings("unused") - private record Person(@Size(min = 1, max = 10) String name) { + private record Person(@Size(min = 1, max = 10) String name, List<@NotBlank String> hobbies) { } }