diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java index 637c5c1e118..c12881ddaf1 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java @@ -202,7 +202,7 @@ public class AnnotatedMethod { } for (int i = 0; i < paramTypes.length; i++) { if (paramTypes[i] != - ResolvableType.forMethodParameter(candidate, i, this.method.getDeclaringClass()).resolve()) { + ResolvableType.forMethodParameter(candidate, i, this.method.getDeclaringClass()).toClass()) { return false; } } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java index ba3617141f2..c2c901e2481 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java @@ -374,7 +374,7 @@ abstract class AnnotationsScanner { } for (int i = 0; i < rootParameterTypes.length; i++) { Class resolvedParameterType = ResolvableType.forMethodParameter( - candidateMethod, i, sourceDeclaringClass).resolve(); + candidateMethod, i, sourceDeclaringClass).toClass(); if (rootParameterTypes[i] != resolvedParameterType) { return false; } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedMethodTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedMethodTests.java new file mode 100644 index 00000000000..1118239d614 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedMethodTests.java @@ -0,0 +1,116 @@ +/* + * Copyright 2002-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.annotation; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.core.MethodParameter; +import org.springframework.util.ClassUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link AnnotatedMethod}. + * + * @author Sam Brannen + * @since 6.2.11 + */ +class AnnotatedMethodTests { + + @Test + void shouldFindAnnotationOnMethodInGenericAbstractSuperclass() { + Method processTwo = getMethod("processTwo", String.class); + + AnnotatedMethod annotatedMethod = new AnnotatedMethod(processTwo); + + assertThat(annotatedMethod.hasMethodAnnotation(Handler.class)).isTrue(); + } + + @Test + void shouldFindAnnotationOnMethodInGenericInterface() { + Method processOneAndTwo = getMethod("processOneAndTwo", Long.class, Object.class); + + AnnotatedMethod annotatedMethod = new AnnotatedMethod(processOneAndTwo); + + assertThat(annotatedMethod.hasMethodAnnotation(Handler.class)).isTrue(); + } + + @Test + void shouldFindAnnotationOnMethodParameterInGenericAbstractSuperclass() { + Method processTwo = getMethod("processTwo", String.class); + + AnnotatedMethod annotatedMethod = new AnnotatedMethod(processTwo); + MethodParameter[] methodParameters = annotatedMethod.getMethodParameters(); + + assertThat(methodParameters).hasSize(1); + assertThat(methodParameters[0].hasParameterAnnotation(Param.class)).isTrue(); + } + + @Test + void shouldFindAnnotationOnMethodParameterInGenericInterface() { + Method processOneAndTwo = getMethod("processOneAndTwo", Long.class, Object.class); + + AnnotatedMethod annotatedMethod = new AnnotatedMethod(processOneAndTwo); + MethodParameter[] methodParameters = annotatedMethod.getMethodParameters(); + + assertThat(methodParameters).hasSize(2); + assertThat(methodParameters[0].hasParameterAnnotation(Param.class)).isFalse(); + assertThat(methodParameters[1].hasParameterAnnotation(Param.class)).isTrue(); + } + + + private static Method getMethod(String name, Class...parameterTypes) { + return ClassUtils.getMethod(GenericInterfaceImpl.class, name, parameterTypes); + } + + + @Retention(RetentionPolicy.RUNTIME) + @interface Handler { + } + + @Retention(RetentionPolicy.RUNTIME) + @interface Param { + } + + interface GenericInterface { + + @Handler + void processOneAndTwo(A value1, @Param B value2); + } + + abstract static class GenericAbstractSuperclass implements GenericInterface { + + @Override + public void processOneAndTwo(Long value1, C value2) { + } + + @Handler + public abstract void processTwo(@Param C value); + } + + static class GenericInterfaceImpl extends GenericAbstractSuperclass { + + @Override + public void processTwo(String value) { + } + } + +} diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java index 473cbdd7a68..51231e151ba 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java @@ -946,6 +946,15 @@ class MergedAnnotationsTests { Order.class).getDistance()).isEqualTo(0); } + @Test + void getFromMethodWithUnresolvedGenericsInGenericTypeHierarchy() { + // The following method is GenericAbstractSuperclass.processOneAndTwo(java.lang.Long, C), + // where 'C' is an unresolved generic, for which ResolvableType.resolve() returns null. + Method method = ClassUtils.getMethod(GenericInterfaceImpl.class, "processOneAndTwo", Long.class, Object.class); + assertThat(MergedAnnotations.from(method, SearchStrategy.TYPE_HIERARCHY) + .get(Transactional.class).isDirectlyPresent()).isTrue(); + } + @Test void getFromMethodWithInterfaceOnSuper() throws Exception { Method method = SubOfImplementsInterfaceWithAnnotatedMethod.class.getMethod("foo"); @@ -3014,6 +3023,26 @@ class MergedAnnotationsTests { } } + interface GenericInterface { + + @Transactional + void processOneAndTwo(A value1, B value2); + } + + abstract static class GenericAbstractSuperclass implements GenericInterface { + + @Override + public void processOneAndTwo(Long value1, C value2) { + } + } + + static class GenericInterfaceImpl extends GenericAbstractSuperclass { + // The compiler does not require us to declare a concrete + // processOneAndTwo(Long, String) method, and we intentionally + // do not declare one here. + } + + @Retention(RetentionPolicy.RUNTIME) @Inherited @interface MyRepeatableContainer { 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 b114133bc16..291ba4925bd 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 @@ -35,32 +35,46 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link HandlerMethod}. * * @author Rossen Stoyanchev + * @author Sam Brannen */ class HandlerMethodTests { @Test - void shouldValidateArgsWithConstraintsDirectlyOnClass() { + void shouldValidateArgsWithConstraintsDirectlyInClass() { Object target = new MyClass(); testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons", "addPeople", "addNames"), true); testValidateArgs(target, List.of("addPerson", "getPerson", "getIntValue", "addPersonNotValidated"), false); } @Test - void shouldValidateArgsWithConstraintsOnInterface() { + void shouldValidateArgsWithConstraintsInInterface() { Object target = new MyInterfaceImpl(); testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons", "addPeople"), true); testValidateArgs(target, List.of("addPerson", "addPersonNotValidated", "getPerson", "getIntValue"), false); } @Test - void shouldValidateReturnValueWithConstraintsDirectlyOnClass() { + void shouldValidateArgsWithConstraintsInGenericAbstractSuperclass() { + Object target = new GenericInterfaceImpl(); + shouldValidateArguments(getHandlerMethod(target, "processTwo", String.class), true); + } + + @Test + void shouldValidateArgsWithConstraintsInGenericInterface() { + Object target = new GenericInterfaceImpl(); + shouldValidateArguments(getHandlerMethod(target, "processOne", Long.class), false); + shouldValidateArguments(getHandlerMethod(target, "processOneAndTwo", Long.class, Object.class), true); + } + + @Test + void shouldValidateReturnValueWithConstraintsDirectlyInClass() { Object target = new MyClass(); testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true); testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false); } @Test - void shouldValidateReturnValueWithConstraintsOnInterface() { + void shouldValidateReturnValueWithConstraintsInInterface() { Object target = new MyInterfaceImpl(); testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true); testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false); @@ -97,9 +111,19 @@ class HandlerMethodTests { assertThat(hm3.getResolvedFromHandlerMethod()).isSameAs(hm1); } + + private static void shouldValidateArguments(HandlerMethod handlerMethod, boolean expected) { + if (expected) { + assertThat(handlerMethod.shouldValidateArguments()).as(handlerMethod.getMethod().getName()).isTrue(); + } + else { + assertThat(handlerMethod.shouldValidateArguments()).as(handlerMethod.getMethod().getName()).isFalse(); + } + } + private static void testValidateArgs(Object target, List methodNames, boolean expected) { for (String methodName : methodNames) { - assertThat(getHandlerMethod(target, methodName).shouldValidateArguments()).isEqualTo(expected); + shouldValidateArguments(getHandlerMethod(target, methodName), expected); } } @@ -110,7 +134,11 @@ class HandlerMethodTests { } private static HandlerMethod getHandlerMethod(Object target, String methodName) { - Method method = ClassUtils.getMethod(target.getClass(), methodName, (Class[]) null); + return getHandlerMethod(target, methodName, (Class[]) null); + } + + private static HandlerMethod getHandlerMethod(Object target, String methodName, Class... parameterTypes) { + Method method = ClassUtils.getMethod(target.getClass(), methodName, parameterTypes); return new HandlerMethod(target, method).createWithValidateFlags(); } @@ -236,4 +264,32 @@ class HandlerMethodTests { } } + + interface GenericInterface { + + void processOne(@Valid A value1); + + void processOneAndTwo(A value1, @Max(42) B value2); + } + + abstract static class GenericAbstractSuperclass implements GenericInterface { + + @Override + public void processOne(Long value1) { + } + + @Override + public void processOneAndTwo(Long value1, C value2) { + } + + public abstract void processTwo(@Max(42) C value); + } + + static class GenericInterfaceImpl extends GenericAbstractSuperclass { + + @Override + public void processTwo(String value) { + } + } + }