Skip to content

Improve method validation support for errors on elements within a container #31530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ private MethodValidationResult adaptViolations(
Function<Integer, Object> argumentFunction) {

Map<MethodParameter, ValueResultBuilder> parameterViolations = new LinkedHashMap<>();
Map<Path.Node, BeanResultBuilder> cascadedViolations = new LinkedHashMap<>();
Map<CascadedViolationsKey, BeanResultBuilder> cascadedViolations = new LinkedHashMap<>();

for (ConstraintViolation<Object> violation : violations) {
Iterator<Path.Node> itr = violation.getPropertyPath().iterator();
Expand All @@ -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;
Expand All @@ -338,7 +339,7 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) {

List<ParameterValidationResult> 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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -446,25 +455,20 @@ private final class BeanResultBuilder {

private final Set<ConstraintViolation<Object>> 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<Object> violation) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@
* {@link Errors#getAllErrors()}, but this subclass provides access to the same
* as {@link FieldError}s.
*
* <p>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.
* <p>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
Expand Down Expand Up @@ -71,30 +70,32 @@ 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() {
return this.container;
}

/**
* 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() {
return this.containerIndex;
}

/**
* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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: \
Expand All @@ -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);

Expand Down Expand Up @@ -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<ParameterErrors> 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]"""))
);
});
}

Expand All @@ -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<String> fieldErrors) {

assertThat(errors.getMethodParameter().getParameterIndex()).isEqualTo(parameterIndex);
Expand Down Expand Up @@ -234,14 +285,14 @@ public Person getPerson() {
throw new UnsupportedOperationException();
}

public void addPeople(@Valid List<Person> people) {
public void addPeople(@Valid Collection<Person> 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) {
}

}