Skip to content

Commit 33c1490

Browse files
committed
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
1 parent e00a882 commit 33c1490

File tree

2 files changed

+51
-20
lines changed

2 files changed

+51
-20
lines changed

spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java

+35-8
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,13 @@
1717
package org.springframework.web.method;
1818

1919
import java.lang.annotation.Annotation;
20+
import java.lang.reflect.AnnotatedParameterizedType;
21+
import java.lang.reflect.AnnotatedType;
2022
import java.lang.reflect.Method;
23+
import java.util.ArrayList;
2124
import java.util.Collection;
25+
import java.util.Collections;
26+
import java.util.List;
2227
import java.util.Map;
2328
import java.util.StringJoiner;
2429
import java.util.function.Predicate;
@@ -396,17 +401,19 @@ private static class MethodValidationInitializer {
396401

397402
public static boolean checkArguments(Class<?> beanType, MethodParameter[] parameters) {
398403
if (AnnotationUtils.findAnnotation(beanType, Validated.class) == null) {
399-
for (MethodParameter parameter : parameters) {
400-
MergedAnnotations merged = MergedAnnotations.from(parameter.getParameterAnnotations());
404+
for (MethodParameter param : parameters) {
405+
MergedAnnotations merged = MergedAnnotations.from(param.getParameterAnnotations());
401406
if (merged.stream().anyMatch(CONSTRAINT_PREDICATE)) {
402407
return true;
403408
}
404-
else {
405-
Class<?> type = parameter.getParameterType();
406-
if (merged.stream().anyMatch(VALID_PREDICATE) &&
407-
(Collection.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type))) {
408-
return true;
409-
}
409+
Class<?> type = param.getParameterType();
410+
if (merged.stream().anyMatch(VALID_PREDICATE) &&
411+
(Collection.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type))) {
412+
return true;
413+
}
414+
merged = MergedAnnotations.from(getContainerElementAnnotations(param));
415+
if (merged.stream().anyMatch(CONSTRAINT_PREDICATE)) {
416+
return true;
410417
}
411418
}
412419
}
@@ -420,6 +427,26 @@ public static boolean checkReturnValue(Class<?> beanType, Method method) {
420427
}
421428
return false;
422429
}
430+
431+
/**
432+
* There may be constraints on elements of a container (list, map).
433+
*/
434+
private static Annotation[] getContainerElementAnnotations(MethodParameter param) {
435+
List<Annotation> result = null;
436+
int i = param.getParameterIndex();
437+
Method method = param.getMethod();
438+
if (method != null && method.getAnnotatedParameterTypes()[i] instanceof AnnotatedParameterizedType apt) {
439+
for (AnnotatedType type : apt.getAnnotatedActualTypeArguments()) {
440+
for (Annotation annot : type.getAnnotations()) {
441+
result = (result != null ? result : new ArrayList<>());
442+
result.add(annot);
443+
}
444+
}
445+
}
446+
result = (result != null ? result : Collections.emptyList());
447+
return result.toArray(new Annotation[0]);
448+
}
449+
423450
}
424451

425452
}

spring-web/src/test/java/org/springframework/web/method/HandlerMethodTests.java

+16-12
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import jakarta.validation.Valid;
2323
import jakarta.validation.constraints.Max;
24+
import jakarta.validation.constraints.NotEmpty;
2425
import jakarta.validation.constraints.Size;
2526
import org.junit.jupiter.api.Test;
2627

@@ -38,45 +39,45 @@ public class HandlerMethodTests {
3839
@Test
3940
void shouldValidateArgsWithConstraintsDirectlyOnClass() {
4041
Object target = new MyClass();
41-
testShouldValidateArguments(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons"), true);
42-
testShouldValidateArguments(target, List.of("addPerson", "getPerson", "getIntValue", "addPersonNotValidated"), false);
42+
testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons", "addNames"), true);
43+
testValidateArgs(target, List.of("addPerson", "getPerson", "getIntValue", "addPersonNotValidated"), false);
4344
}
4445

4546
@Test
4647
void shouldValidateArgsWithConstraintsOnInterface() {
4748
Object target = new MyInterfaceImpl();
48-
testShouldValidateArguments(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons"), true);
49-
testShouldValidateArguments(target, List.of("addPerson", "addPersonNotValidated", "getPerson", "getIntValue"), false);
49+
testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons"), true);
50+
testValidateArgs(target, List.of("addPerson", "addPersonNotValidated", "getPerson", "getIntValue"), false);
5051
}
5152

5253
@Test
5354
void shouldValidateReturnValueWithConstraintsDirectlyOnClass() {
5455
Object target = new MyClass();
55-
testShouldValidateReturnValue(target, List.of("getPerson", "getIntValue"), true);
56-
testShouldValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false);
56+
testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true);
57+
testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false);
5758
}
5859

5960
@Test
6061
void shouldValidateReturnValueWithConstraintsOnInterface() {
6162
Object target = new MyInterfaceImpl();
62-
testShouldValidateReturnValue(target, List.of("getPerson", "getIntValue"), true);
63-
testShouldValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false);
63+
testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true);
64+
testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false);
6465
}
6566

6667
@Test
6768
void classLevelValidatedAnnotation() {
6869
Object target = new MyValidatedClass();
69-
testShouldValidateArguments(target, List.of("addPerson"), false);
70-
testShouldValidateReturnValue(target, List.of("getPerson"), false);
70+
testValidateArgs(target, List.of("addPerson"), false);
71+
testValidateReturnValue(target, List.of("getPerson"), false);
7172
}
7273

73-
private static void testShouldValidateArguments(Object target, List<String> methodNames, boolean expected) {
74+
private static void testValidateArgs(Object target, List<String> methodNames, boolean expected) {
7475
for (String methodName : methodNames) {
7576
assertThat(getHandlerMethod(target, methodName).shouldValidateArguments()).isEqualTo(expected);
7677
}
7778
}
7879

79-
private static void testShouldValidateReturnValue(Object target, List<String> methodNames, boolean expected) {
80+
private static void testValidateReturnValue(Object target, List<String> methodNames, boolean expected) {
8081
for (String methodName : methodNames) {
8182
assertThat(getHandlerMethod(target, methodName).shouldValidateReturnValue()).isEqualTo(expected);
8283
}
@@ -113,6 +114,9 @@ public void addPersonAndIntValue(@Valid Person person, @Max(10) int value) {
113114
public void addPersons(@Valid List<Person> persons) {
114115
}
115116

117+
public void addNames(List<@NotEmpty String> names) {
118+
}
119+
116120
public void addPersonNotValidated(Person person) {
117121
}
118122

0 commit comments

Comments
 (0)