Improve method validation for container elements

This change moves container element properties from ParameterErrors
to base class ParameterValidationResult, and makes that support
independent of whether violations are nested within a container
element bean or through constraints on container elements, e.g.
`List<@NotBlank String>`.

Closes gh-31887
This commit is contained in:
rstoyanchev 2024-01-05 16:32:14 +00:00
parent e0d6b69195
commit 8552e149b5
5 changed files with 172 additions and 114 deletions

View File

@ -301,8 +301,8 @@ public class MethodValidationAdapter implements MethodValidator {
Function<Integer, MethodParameter> parameterFunction, Function<Integer, MethodParameter> parameterFunction,
Function<Integer, Object> argumentFunction) { Function<Integer, Object> argumentFunction) {
Map<MethodParameter, ParamResultBuilder> paramViolations = new LinkedHashMap<>(); Map<Path.Node, ParamValidationResultBuilder> paramViolations = new LinkedHashMap<>();
Map<Path.Node, BeanResultBuilder> beanViolations = new LinkedHashMap<>(); Map<Path.Node, ParamErrorsBuilder> nestedViolations = new LinkedHashMap<>();
for (ConstraintViolation<Object> violation : violations) { for (ConstraintViolation<Object> violation : violations) {
Iterator<Path.Node> itr = violation.getPropertyPath().iterator(); Iterator<Path.Node> itr = violation.getPropertyPath().iterator();
@ -322,59 +322,62 @@ public class MethodValidationAdapter implements MethodValidator {
} }
Object arg = argumentFunction.apply(parameter.getParameterIndex()); Object arg = argumentFunction.apply(parameter.getParameterIndex());
if (!itr.hasNext()) {
paramViolations
.computeIfAbsent(parameter, p -> new ParamResultBuilder(target, parameter, arg))
.addViolation(violation);
}
else {
// If the arg is a container, we need to element, but the only way to extract it
// is to check for and use a container index or key on the next node:
// https://github.com/jakartaee/validation/issues/194 // https://github.com/jakartaee/validation/issues/194
// If the argument is a container of elements, we need the element, but
// the only option is to see if the next part of the property path has
// a container index/key for its parent and use it.
Path.Node paramNode = node; Path.Node parameterNode = node;
if (itr.hasNext()) {
node = itr.next(); node = itr.next();
}
Object bean; Object value;
Object container; Object container;
Integer containerIndex = node.getIndex(); Integer index = node.getIndex();
Object containerKey = node.getKey(); Object key = node.getKey();
if (containerIndex != null && arg instanceof List<?> list) { if (index != null && arg instanceof List<?> list) {
bean = list.get(containerIndex); value = list.get(index);
container = list; container = list;
} }
else if (containerIndex != null && arg instanceof Object[] array) { else if (index != null && arg instanceof Object[] array) {
bean = array[containerIndex]; value = array[index];
container = array; container = array;
} }
else if (containerKey != null && arg instanceof Map<?, ?> map) { else if (key != null && arg instanceof Map<?, ?> map) {
bean = map.get(containerKey); value = map.get(key);
container = map; container = map;
} }
else if (arg instanceof Optional<?> optional) { else if (arg instanceof Optional<?> optional) {
bean = optional.orElse(null); value = optional.orElse(null);
container = optional; container = optional;
} }
else { else {
Assert.state(!node.isInIterable(), "No way to unwrap Iterable without index"); Assert.state(!node.isInIterable(), "No way to unwrap Iterable without index");
bean = arg; value = arg;
container = null; container = null;
} }
beanViolations if (node.getKind().equals(ElementKind.PROPERTY)) {
.computeIfAbsent(paramNode, k -> nestedViolations
new BeanResultBuilder(parameter, bean, container, containerIndex, containerKey)) .computeIfAbsent(parameterNode, k ->
new ParamErrorsBuilder(parameter, value, container, index, key))
.addViolation(violation); .addViolation(violation);
} }
else {
paramViolations
.computeIfAbsent(parameterNode, p ->
new ParamValidationResultBuilder(target, parameter, value, container, index, key))
.addViolation(violation);
}
break; break;
} }
} }
List<ParameterValidationResult> resultList = new ArrayList<>(); List<ParameterValidationResult> resultList = new ArrayList<>();
paramViolations.forEach((param, builder) -> resultList.add(builder.build())); paramViolations.forEach((param, builder) -> resultList.add(builder.build()));
beanViolations.forEach((key, builder) -> resultList.add(builder.build())); nestedViolations.forEach((key, builder) -> resultList.add(builder.build()));
resultList.sort(resultComparator); resultList.sort(resultComparator);
return MethodValidationResult.create(target, method, resultList); return MethodValidationResult.create(target, method, resultList);
@ -430,21 +433,35 @@ public class MethodValidationAdapter implements MethodValidator {
* Builds a validation result for a value method parameter with constraints * Builds a validation result for a value method parameter with constraints
* declared directly on it. * declared directly on it.
*/ */
private final class ParamResultBuilder { private final class ParamValidationResultBuilder {
private final Object target; private final Object target;
private final MethodParameter parameter; private final MethodParameter parameter;
@Nullable @Nullable
private final Object argument; private final Object value;
@Nullable
private final Object container;
@Nullable
private final Integer containerIndex;
@Nullable
private final Object containerKey;
private final List<MessageSourceResolvable> resolvableErrors = new ArrayList<>(); private final List<MessageSourceResolvable> resolvableErrors = new ArrayList<>();
public ParamResultBuilder(Object target, MethodParameter parameter, @Nullable Object argument) { public ParamValidationResultBuilder(
Object target, MethodParameter parameter, @Nullable Object value, @Nullable Object container,
@Nullable Integer containerIndex, @Nullable Object containerKey) {
this.target = target; this.target = target;
this.parameter = parameter; this.parameter = parameter;
this.argument = argument; this.value = value;
this.container = container;
this.containerIndex = containerIndex;
this.containerKey = containerKey;
} }
public void addViolation(ConstraintViolation<Object> violation) { public void addViolation(ConstraintViolation<Object> violation) {
@ -452,7 +469,9 @@ public class MethodValidationAdapter implements MethodValidator {
} }
public ParameterValidationResult build() { public ParameterValidationResult build() {
return new ParameterValidationResult(this.parameter, this.argument, this.resolvableErrors); return new ParameterValidationResult(
this.parameter, this.value, this.resolvableErrors, this.container,
this.containerIndex, this.containerKey);
} }
} }
@ -462,7 +481,7 @@ public class MethodValidationAdapter implements MethodValidator {
* Builds a validation result for an {@link jakarta.validation.Valid @Valid} * Builds a validation result for an {@link jakarta.validation.Valid @Valid}
* annotated bean method parameter with cascaded constraints. * annotated bean method parameter with cascaded constraints.
*/ */
private final class BeanResultBuilder { private final class ParamErrorsBuilder {
private final MethodParameter parameter; private final MethodParameter parameter;
@ -482,7 +501,7 @@ public class MethodValidationAdapter implements MethodValidator {
private final Set<ConstraintViolation<Object>> violations = new LinkedHashSet<>(); private final Set<ConstraintViolation<Object>> violations = new LinkedHashSet<>();
public BeanResultBuilder( public ParamErrorsBuilder(
MethodParameter param, @Nullable Object bean, @Nullable Object container, MethodParameter param, @Nullable Object bean, @Nullable Object container,
@Nullable Integer containerIndex, @Nullable Object containerKey) { @Nullable Integer containerIndex, @Nullable Object containerKey) {

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2023 the original author or authors. * Copyright 2002-2024 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -32,12 +32,6 @@ import org.springframework.validation.ObjectError;
* {@link Errors#getAllErrors()}, but this subclass provides access to the same * {@link Errors#getAllErrors()}, but this subclass provides access to the same
* as {@link FieldError}s. * as {@link FieldError}s.
* *
* <p>When the method parameter is a container such as a {@link List}, array,
* or {@link java.util.Map}, then a separate {@link ParameterErrors} is created
* for each element that has errors. In that case, the properties
* {@link #getContainer() container}, {@link #getContainerIndex() containerIndex},
* and {@link #getContainerKey() containerKey} provide additional context.
*
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @since 6.1 * @since 6.1
*/ */
@ -45,15 +39,6 @@ public class ParameterErrors extends ParameterValidationResult implements Errors
private final Errors errors; private final Errors errors;
@Nullable
private final Object container;
@Nullable
private final Integer containerIndex;
@Nullable
private final Object containerKey;
/** /**
* Create a {@code ParameterErrors}. * Create a {@code ParameterErrors}.
@ -62,45 +47,8 @@ public class ParameterErrors extends ParameterValidationResult implements Errors
MethodParameter parameter, @Nullable Object argument, Errors errors, MethodParameter parameter, @Nullable Object argument, Errors errors,
@Nullable Object container, @Nullable Integer index, @Nullable Object key) { @Nullable Object container, @Nullable Integer index, @Nullable Object key) {
super(parameter, argument, errors.getAllErrors()); super(parameter, argument, errors.getAllErrors(), container, index, key);
this.errors = errors; this.errors = errors;
this.container = container;
this.containerIndex = index;
this.containerKey = key;
}
/**
* When {@code @Valid} is declared on a container of elements such as
* {@link java.util.Collection}, {@link java.util.Map},
* {@link java.util.Optional}, and others, this method returns the container
* of the validated {@link #getArgument() argument}, while
* {@link #getContainerIndex()} and {@link #getContainerKey()} provide
* information about the index or key if applicable.
*/
@Nullable
public Object getContainer() {
return this.container;
}
/**
* When {@code @Valid} is declared on an indexed container of elements such as
* {@link List} or array, this method returns the index of the validated
* {@link #getArgument() argument}.
*/
@Nullable
public Integer getContainerIndex() {
return this.containerIndex;
}
/**
* When {@code @Valid} is declared on a container of elements referenced by
* key such as {@link java.util.Map}, this method returns the key of the
* validated {@link #getArgument() argument}.
*/
@Nullable
public Object getContainerKey() {
return this.containerKey;
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2023 the original author or authors. * Copyright 2002-2024 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -35,6 +35,12 @@ import org.springframework.util.ObjectUtils;
* {@link ParameterErrors}. * {@link ParameterErrors}.
* </ul> * </ul>
* *
* <p>When the method parameter is a container such as a {@link List}, array,
* or {@link java.util.Map}, then a separate {@link ParameterValidationResult}
* is created for each element with errors. In that case, the properties
* {@link #getContainer() container}, {@link #getContainerIndex() containerIndex},
* and {@link #getContainerKey() containerKey} provide additional context.
*
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @since 6.1 * @since 6.1
*/ */
@ -47,18 +53,43 @@ public class ParameterValidationResult {
private final List<MessageSourceResolvable> resolvableErrors; private final List<MessageSourceResolvable> resolvableErrors;
@Nullable
private final Object container;
@Nullable
private final Integer containerIndex;
@Nullable
private final Object containerKey;
/** /**
* Create a {@code ParameterValidationResult}. * Create a {@code ParameterValidationResult}.
*/ */
public ParameterValidationResult( public ParameterValidationResult(
MethodParameter param, @Nullable Object arg, Collection<? extends MessageSourceResolvable> errors) { MethodParameter param, @Nullable Object arg, Collection<? extends MessageSourceResolvable> errors,
@Nullable Object container, @Nullable Integer index, @Nullable Object key) {
Assert.notNull(param, "MethodParameter is required"); Assert.notNull(param, "MethodParameter is required");
Assert.notEmpty(errors, "`resolvableErrors` must not be empty"); Assert.notEmpty(errors, "`resolvableErrors` must not be empty");
this.methodParameter = param; this.methodParameter = param;
this.argument = arg; this.argument = arg;
this.resolvableErrors = List.copyOf(errors); this.resolvableErrors = List.copyOf(errors);
this.container = container;
this.containerIndex = index;
this.containerKey = key;
}
/**
* Create a {@code ParameterValidationResult}.
* @deprecated in favor of
* {@link ParameterValidationResult#ParameterValidationResult(MethodParameter, Object, Collection, Object, Integer, Object)}
*/
@Deprecated(since = "6.1.3", forRemoval = true)
public ParameterValidationResult(
MethodParameter param, @Nullable Object arg, Collection<? extends MessageSourceResolvable> errors) {
this(param, arg, errors, null, null, null);
} }
@ -100,6 +131,39 @@ public class ParameterValidationResult {
return this.resolvableErrors; return this.resolvableErrors;
} }
/**
* When {@code @Valid} is declared on a container of elements such as
* {@link java.util.Collection}, {@link java.util.Map},
* {@link java.util.Optional}, and others, this method returns the container
* of the validated {@link #getArgument() argument}, while
* {@link #getContainerIndex()} and {@link #getContainerKey()} provide
* information about the index or key if applicable.
*/
@Nullable
public Object getContainer() {
return this.container;
}
/**
* When {@code @Valid} is declared on an indexed container of elements such as
* {@link List} or array, this method returns the index of the validated
* {@link #getArgument() argument}.
*/
@Nullable
public Integer getContainerIndex() {
return this.containerIndex;
}
/**
* When {@code @Valid} is declared on a container of elements referenced by
* key such as {@link java.util.Map}, this method returns the key of the
* validated {@link #getArgument() argument}.
*/
@Nullable
public Object getContainerKey() {
return this.containerKey;
}
@Override @Override
public boolean equals(@Nullable Object other) { public boolean equals(@Nullable Object other) {
@ -111,7 +175,9 @@ public class ParameterValidationResult {
} }
ParameterValidationResult otherResult = (ParameterValidationResult) other; ParameterValidationResult otherResult = (ParameterValidationResult) other;
return (getMethodParameter().equals(otherResult.getMethodParameter()) && return (getMethodParameter().equals(otherResult.getMethodParameter()) &&
ObjectUtils.nullSafeEquals(getArgument(), otherResult.getArgument())); ObjectUtils.nullSafeEquals(getArgument(), otherResult.getArgument()) &&
ObjectUtils.nullSafeEquals(getContainerIndex(), otherResult.getContainerIndex()) &&
ObjectUtils.nullSafeEquals(getContainerKey(), otherResult.getContainerKey()));
} }
@Override @Override
@ -119,14 +185,18 @@ public class ParameterValidationResult {
int hashCode = super.hashCode(); int hashCode = super.hashCode();
hashCode = 29 * hashCode + getMethodParameter().hashCode(); hashCode = 29 * hashCode + getMethodParameter().hashCode();
hashCode = 29 * hashCode + ObjectUtils.nullSafeHashCode(getArgument()); hashCode = 29 * hashCode + ObjectUtils.nullSafeHashCode(getArgument());
hashCode = 29 * hashCode + ObjectUtils.nullSafeHashCode(getContainerIndex());
hashCode = 29 * hashCode + ObjectUtils.nullSafeHashCode(getContainerKey());
return hashCode; return hashCode;
} }
@Override @Override
public String toString() { public String toString() {
return "Validation results for method parameter '" + this.methodParameter + return getClass().getSimpleName() + " for " + this.methodParameter +
"': argument [" + ObjectUtils.nullSafeConciseToString(this.argument) + "]; " + ", argument value '" + ObjectUtils.nullSafeConciseToString(this.argument) + "'," +
getResolvableErrors(); (this.containerIndex != null ? "containerIndex[" + this.containerIndex + "]," : "") +
(this.containerKey != null ? "containerKey['" + this.containerKey + "']," : "") +
" errors: " + getResolvableErrors();
} }
} }

View File

@ -161,7 +161,7 @@ class MethodValidationAdapterTests {
} }
@Test @Test
void validateListArgument() { void validateBeanListArgument() {
MyService target = new MyService(); MyService target = new MyService();
Method method = getMethod(target, "addPeople"); Method method = getMethod(target, "addPeople");
@ -195,6 +195,24 @@ class MethodValidationAdapterTests {
}); });
} }
@Test
void validateValueListArgument() {
MyService target = new MyService();
Method method = getMethod(target, "addHobbies");
testArgs(target, method, new Object[] {List.of(" ")}, ex -> {
assertThat(ex.getAllValidationResults()).hasSize(1);
assertValueResult(ex.getValueResults().get(0), 0, " ", List.of("""
org.springframework.context.support.DefaultMessageSourceResolvable: \
codes [NotBlank.myService#addHobbies.hobbies,NotBlank.hobbies,NotBlank.java.util.List,NotBlank]; \
arguments [org.springframework.context.support.DefaultMessageSourceResolvable: \
codes [myService#addHobbies.hobbies,hobbies]; \
arguments []; default message [hobbies]]; default message [must not be blank]"""));
});
}
private void testArgs(Object target, Method method, Object[] args, Consumer<MethodValidationResult> consumer) { private void testArgs(Object target, Method method, Object[] args, Consumer<MethodValidationResult> consumer) {
consumer.accept(this.validationAdapter.validateArguments(target, method, null, args, new Class<?>[0])); consumer.accept(this.validationAdapter.validateArguments(target, method, null, args, new Class<?>[0]));
} }
@ -250,6 +268,9 @@ class MethodValidationAdapterTests {
public void addPeople(@Valid List<Person> people) { public void addPeople(@Valid List<Person> people) {
} }
public void addHobbies(List<@NotBlank String> hobbies) {
}
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2023 the original author or authors. * Copyright 2002-2024 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -128,7 +128,7 @@ public class HandlerMethodValidationExceptionTests {
} }
else { else {
MessageSourceResolvable error = new DefaultMessageSourceResolvable("Size"); MessageSourceResolvable error = new DefaultMessageSourceResolvable("Size");
return new ParameterValidationResult(param, "123", List.of(error)); return new ParameterValidationResult(param, "123", List.of(error), null, null, null);
} }
}) })
.toList()); .toList());