From 33c149077ad04904c340603ff37ce9a261e3d55a Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 20 Dec 2023 18:34:10 +0000 Subject: [PATCH] Check constraints on container elements We now check for constraint annotations on elements of a container to decide whether to apply method validation. Closes gh-31870 --- .../web/method/HandlerMethod.java | 43 +++++++++++++++---- .../web/method/HandlerMethodTests.java | 28 ++++++------ 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java index 285a597c82..8fd02b012f 100644 --- a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java @@ -17,8 +17,13 @@ package org.springframework.web.method; import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedParameterizedType; +import java.lang.reflect.AnnotatedType; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.StringJoiner; import java.util.function.Predicate; @@ -396,17 +401,19 @@ public class HandlerMethod extends AnnotatedMethod { public static boolean checkArguments(Class beanType, MethodParameter[] parameters) { if (AnnotationUtils.findAnnotation(beanType, Validated.class) == null) { - for (MethodParameter parameter : parameters) { - MergedAnnotations merged = MergedAnnotations.from(parameter.getParameterAnnotations()); + for (MethodParameter param : parameters) { + MergedAnnotations merged = MergedAnnotations.from(param.getParameterAnnotations()); if (merged.stream().anyMatch(CONSTRAINT_PREDICATE)) { return true; } - else { - Class type = parameter.getParameterType(); - if (merged.stream().anyMatch(VALID_PREDICATE) && - (Collection.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type))) { - return true; - } + Class type = param.getParameterType(); + if (merged.stream().anyMatch(VALID_PREDICATE) && + (Collection.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type))) { + return true; + } + merged = MergedAnnotations.from(getContainerElementAnnotations(param)); + if (merged.stream().anyMatch(CONSTRAINT_PREDICATE)) { + return true; } } } @@ -420,6 +427,26 @@ public class HandlerMethod extends AnnotatedMethod { } return false; } + + /** + * There may be constraints on elements of a container (list, map). + */ + private static Annotation[] getContainerElementAnnotations(MethodParameter param) { + List result = null; + int i = param.getParameterIndex(); + Method method = param.getMethod(); + if (method != null && method.getAnnotatedParameterTypes()[i] instanceof AnnotatedParameterizedType apt) { + for (AnnotatedType type : apt.getAnnotatedActualTypeArguments()) { + for (Annotation annot : type.getAnnotations()) { + result = (result != null ? result : new ArrayList<>()); + result.add(annot); + } + } + } + result = (result != null ? result : Collections.emptyList()); + return result.toArray(new Annotation[0]); + } + } } diff --git a/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java b/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java index 14ef967ae3..f8e2f21c66 100644 --- a/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java @@ -21,6 +21,7 @@ import java.util.List; import jakarta.validation.Valid; import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.Size; import org.junit.jupiter.api.Test; @@ -38,45 +39,45 @@ public class HandlerMethodTests { @Test void shouldValidateArgsWithConstraintsDirectlyOnClass() { Object target = new MyClass(); - testShouldValidateArguments(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons"), true); - testShouldValidateArguments(target, List.of("addPerson", "getPerson", "getIntValue", "addPersonNotValidated"), false); + testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons", "addNames"), true); + testValidateArgs(target, List.of("addPerson", "getPerson", "getIntValue", "addPersonNotValidated"), false); } @Test void shouldValidateArgsWithConstraintsOnInterface() { Object target = new MyInterfaceImpl(); - testShouldValidateArguments(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons"), true); - testShouldValidateArguments(target, List.of("addPerson", "addPersonNotValidated", "getPerson", "getIntValue"), false); + testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons"), true); + testValidateArgs(target, List.of("addPerson", "addPersonNotValidated", "getPerson", "getIntValue"), false); } @Test void shouldValidateReturnValueWithConstraintsDirectlyOnClass() { Object target = new MyClass(); - testShouldValidateReturnValue(target, List.of("getPerson", "getIntValue"), true); - testShouldValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false); + testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true); + testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false); } @Test void shouldValidateReturnValueWithConstraintsOnInterface() { Object target = new MyInterfaceImpl(); - testShouldValidateReturnValue(target, List.of("getPerson", "getIntValue"), true); - testShouldValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false); + testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true); + testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false); } @Test void classLevelValidatedAnnotation() { Object target = new MyValidatedClass(); - testShouldValidateArguments(target, List.of("addPerson"), false); - testShouldValidateReturnValue(target, List.of("getPerson"), false); + testValidateArgs(target, List.of("addPerson"), false); + testValidateReturnValue(target, List.of("getPerson"), false); } - private static void testShouldValidateArguments(Object target, List methodNames, boolean expected) { + private static void testValidateArgs(Object target, List methodNames, boolean expected) { for (String methodName : methodNames) { assertThat(getHandlerMethod(target, methodName).shouldValidateArguments()).isEqualTo(expected); } } - private static void testShouldValidateReturnValue(Object target, List methodNames, boolean expected) { + private static void testValidateReturnValue(Object target, List methodNames, boolean expected) { for (String methodName : methodNames) { assertThat(getHandlerMethod(target, methodName).shouldValidateReturnValue()).isEqualTo(expected); } @@ -113,6 +114,9 @@ public class HandlerMethodTests { public void addPersons(@Valid List persons) { } + public void addNames(List<@NotEmpty String> names) { + } + public void addPersonNotValidated(Person person) { }