Skip to content

Commit

Permalink
Improve method validation for container elements
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@rstoyanchev
rstoyanchev committed Jan 5, 2024
1 parent e0d6b69 commit 8552e14
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 112 deletions.
117 changes: 68 additions & 49 deletions .../src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ private MethodValidationResult adaptViolations(
Function<Integer, MethodParameter> parameterFunction,
Function<Integer, Object> argumentFunction) {

Map<MethodParameter, ParamResultBuilder> paramViolations = new LinkedHashMap<>();
Map<Path.Node, BeanResultBuilder> beanViolations = new LinkedHashMap<>();
Map<Path.Node, ParamValidationResultBuilder> paramViolations = new LinkedHashMap<>();
Map<Path.Node, ParamErrorsBuilder> nestedViolations = new LinkedHashMap<>();

for (ConstraintViolation<Object> violation : violations) {
Iterator<Path.Node> itr = violation.getPropertyPath().iterator();
Expand All @@ -322,59 +322,62 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) {
}

Object arg = argumentFunction.apply(parameter.getParameterIndex());
if (!itr.hasNext()) {
paramViolations
.computeIfAbsent(parameter, p -> new ParamResultBuilder(target, parameter, arg))
.addViolation(violation);
}
else {

// 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.
// 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

Path.Node paramNode = node;
Path.Node parameterNode = node;
if (itr.hasNext()) {
node = itr.next();
}

Object bean;
Object container;
Integer containerIndex = node.getIndex();
Object containerKey = node.getKey();
if (containerIndex != null && arg instanceof List<?> list) {
bean = list.get(containerIndex);
container = list;
}
else if (containerIndex != null && arg instanceof Object[] array) {
bean = array[containerIndex];
container = array;
}
else if (containerKey != null && arg instanceof Map<?, ?> map) {
bean = map.get(containerKey);
container = map;
}
else if (arg instanceof Optional<?> optional) {
bean = optional.orElse(null);
container = optional;
}
else {
Assert.state(!node.isInIterable(), "No way to unwrap Iterable without index");
bean = arg;
container = null;
}
Object value;
Object container;
Integer index = node.getIndex();
Object key = node.getKey();
if (index != null && arg instanceof List<?> list) {
value = list.get(index);
container = list;
}
else if (index != null && arg instanceof Object[] array) {
value = array[index];
container = array;
}
else if (key != null && arg instanceof Map<?, ?> map) {
value = map.get(key);
container = map;
}
else if (arg instanceof Optional<?> optional) {
value = optional.orElse(null);
container = optional;
}
else {
Assert.state(!node.isInIterable(), "No way to unwrap Iterable without index");
value = arg;
container = null;
}

beanViolations
.computeIfAbsent(paramNode, k ->
new BeanResultBuilder(parameter, bean, container, containerIndex, containerKey))
if (node.getKind().equals(ElementKind.PROPERTY)) {
nestedViolations
.computeIfAbsent(parameterNode, k ->
new ParamErrorsBuilder(parameter, value, container, index, key))
.addViolation(violation);
}
else {
paramViolations
.computeIfAbsent(parameterNode, p ->
new ParamValidationResultBuilder(target, parameter, value, container, index, key))
.addViolation(violation);
}

break;
}
}

List<ParameterValidationResult> resultList = new ArrayList<>();
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);

return MethodValidationResult.create(target, method, resultList);
Expand Down Expand Up @@ -430,29 +433,45 @@ public interface ObjectNameResolver {
* Builds a validation result for a value method parameter with constraints
* declared directly on it.
*/
private final class ParamResultBuilder {
private final class ParamValidationResultBuilder {

private final Object target;

private final MethodParameter parameter;

@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<>();

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.parameter = parameter;
this.argument = argument;
this.value = value;
this.container = container;
this.containerIndex = containerIndex;
this.containerKey = containerKey;
}

public void addViolation(ConstraintViolation<Object> violation) {
this.resolvableErrors.add(createMessageSourceResolvable(this.target, this.parameter, violation));
}

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);
}

}
Expand All @@ -462,7 +481,7 @@ public ParameterValidationResult build() {
* Builds a validation result for an {@link jakarta.validation.Valid @Valid}
* annotated bean method parameter with cascaded constraints.
*/
private final class BeanResultBuilder {
private final class ParamErrorsBuilder {

private final MethodParameter parameter;

Expand All @@ -482,7 +501,7 @@ private final class BeanResultBuilder {

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

public BeanResultBuilder(
public ParamErrorsBuilder(
MethodParameter param, @Nullable Object bean, @Nullable Object container,
@Nullable Integer containerIndex, @Nullable Object containerKey) {

Expand Down
56 changes: 2 additions & 54 deletions spring-context/src/main/java/org/springframework/validation/method/ParameterErrors.java
View file
Original file line number Diff line number Diff line change
@@ -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");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -32,28 +32,13 @@
* {@link Errors#getAllErrors()}, but this subclass provides access to the same
* 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
* @since 6.1
*/
public class ParameterErrors extends ParameterValidationResult implements Errors {

private final Errors errors;

@Nullable
private final Object container;

@Nullable
private final Integer containerIndex;

@Nullable
private final Object containerKey;


/**
* Create a {@code ParameterErrors}.
Expand All @@ -62,45 +47,8 @@ public ParameterErrors(
MethodParameter parameter, @Nullable Object argument, Errors errors,
@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.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;
}


Expand Down

0 comments on commit 8552e14

Please sign in to comment.