Skip to content

Commit 6d710d4

Browse files
committed
Find annotation on overridden method in type hierarchy with unresolved generics
Prior to this commit, the MergedAnnotations support (specifically AnnotationsScanner) and AnnotatedMethod did not find annotations on overridden methods in type hierarchies with unresolved generics. The reason for this is that ResolvableType.resolve() returns null for such an unresolved type, which prevents the search algorithms from considering such methods as override candidates. For example, given the following type hierarchy, the compiler does not generate a method corresponding to processOneAndTwo(Long, String) for GenericInterfaceImpl. Nonetheless, one would expect an invocation of processOneAndTwo(Long, String) to be @⁠Transactional since it is effectively an invocation of processOneAndTwo(Long, C) in GenericAbstractSuperclass, which overrides/implements processOneAndTwo(A, B) in GenericInterface, which is annotated with @⁠Transactional. However, the MergedAnnotations infrastructure currently does not determine that processOneAndTwo(Long, C) is @⁠Transactional since it is not able to determine that processOneAndTwo(Long, C) overrides processOneAndTwo(A, B) because of the unresolved generic C. interface GenericInterface<A, B> { @⁠Transactional void processOneAndTwo(A value1, B value2); } abstract class GenericAbstractSuperclass<C> implements GenericInterface<Long, C> { @⁠Override public void processOneAndTwo(Long value1, C value2) { } } static GenericInterfaceImpl extends GenericAbstractSuperclass<String> { } To address such issues, this commit changes the logic in AnnotationsScanner.hasSameGenericTypeParameters() and AnnotatedMethod.isOverrideFor() so that they use ResolvableType.toClass() instead of ResolvableType.resolve(). The former returns Object.class for an unresolved generic which in turn allows the search algorithms to properly detect method overrides in such type hierarchies. Closes gh-35342
1 parent ed28390 commit 6d710d4

File tree

5 files changed

+209
-8
lines changed

5 files changed

+209
-8
lines changed

spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ private boolean isOverrideFor(Method candidate) {
204204
}
205205
for (int i = 0; i < paramTypes.length; i++) {
206206
if (paramTypes[i] !=
207-
ResolvableType.forMethodParameter(candidate, i, this.method.getDeclaringClass()).resolve()) {
207+
ResolvableType.forMethodParameter(candidate, i, this.method.getDeclaringClass()).toClass()) {
208208
return false;
209209
}
210210
}

spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ private static boolean hasSameGenericTypeParameters(
379379
}
380380
for (int i = 0; i < rootParameterTypes.length; i++) {
381381
Class<?> resolvedParameterType = ResolvableType.forMethodParameter(
382-
candidateMethod, i, sourceDeclaringClass).resolve();
382+
candidateMethod, i, sourceDeclaringClass).toClass();
383383
if (rootParameterTypes[i] != resolvedParameterType) {
384384
return false;
385385
}
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* Copyright 2002-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.core.annotation;
18+
19+
import java.lang.annotation.Retention;
20+
import java.lang.annotation.RetentionPolicy;
21+
import java.lang.reflect.Method;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
import org.springframework.core.MethodParameter;
26+
import org.springframework.util.ClassUtils;
27+
28+
import static org.assertj.core.api.Assertions.assertThat;
29+
30+
/**
31+
* Tests for {@link AnnotatedMethod}.
32+
*
33+
* @author Sam Brannen
34+
* @since 6.2.11
35+
*/
36+
class AnnotatedMethodTests {
37+
38+
@Test
39+
void shouldFindAnnotationOnMethodInGenericAbstractSuperclass() {
40+
Method processTwo = getMethod("processTwo", String.class);
41+
42+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(processTwo);
43+
44+
assertThat(annotatedMethod.hasMethodAnnotation(Handler.class)).isTrue();
45+
}
46+
47+
@Test
48+
void shouldFindAnnotationOnMethodInGenericInterface() {
49+
Method processOneAndTwo = getMethod("processOneAndTwo", Long.class, Object.class);
50+
51+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(processOneAndTwo);
52+
53+
assertThat(annotatedMethod.hasMethodAnnotation(Handler.class)).isTrue();
54+
}
55+
56+
@Test
57+
void shouldFindAnnotationOnMethodParameterInGenericAbstractSuperclass() {
58+
Method processTwo = getMethod("processTwo", String.class);
59+
60+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(processTwo);
61+
MethodParameter[] methodParameters = annotatedMethod.getMethodParameters();
62+
63+
assertThat(methodParameters).hasSize(1);
64+
assertThat(methodParameters[0].hasParameterAnnotation(Param.class)).isTrue();
65+
}
66+
67+
@Test
68+
void shouldFindAnnotationOnMethodParameterInGenericInterface() {
69+
Method processOneAndTwo = getMethod("processOneAndTwo", Long.class, Object.class);
70+
71+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(processOneAndTwo);
72+
MethodParameter[] methodParameters = annotatedMethod.getMethodParameters();
73+
74+
assertThat(methodParameters).hasSize(2);
75+
assertThat(methodParameters[0].hasParameterAnnotation(Param.class)).isFalse();
76+
assertThat(methodParameters[1].hasParameterAnnotation(Param.class)).isTrue();
77+
}
78+
79+
80+
private static Method getMethod(String name, Class<?>...parameterTypes) {
81+
return ClassUtils.getMethod(GenericInterfaceImpl.class, name, parameterTypes);
82+
}
83+
84+
85+
@Retention(RetentionPolicy.RUNTIME)
86+
@interface Handler {
87+
}
88+
89+
@Retention(RetentionPolicy.RUNTIME)
90+
@interface Param {
91+
}
92+
93+
interface GenericInterface<A, B> {
94+
95+
@Handler
96+
void processOneAndTwo(A value1, @Param B value2);
97+
}
98+
99+
abstract static class GenericAbstractSuperclass<C> implements GenericInterface<Long, C> {
100+
101+
@Override
102+
public void processOneAndTwo(Long value1, C value2) {
103+
}
104+
105+
@Handler
106+
public abstract void processTwo(@Param C value);
107+
}
108+
109+
static class GenericInterfaceImpl extends GenericAbstractSuperclass<String> {
110+
111+
@Override
112+
public void processTwo(String value) {
113+
}
114+
}
115+
116+
}

spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,15 @@ void getFromMethodWithGenericSuperclass() throws Exception {
945945
Order.class).getDistance()).isEqualTo(0);
946946
}
947947

948+
@Test
949+
void getFromMethodWithUnresolvedGenericsInGenericTypeHierarchy() {
950+
// The following method is GenericAbstractSuperclass.processOneAndTwo(java.lang.Long, C),
951+
// where 'C' is an unresolved generic, for which ResolvableType.resolve() returns null.
952+
Method method = ClassUtils.getMethod(GenericInterfaceImpl.class, "processOneAndTwo", Long.class, Object.class);
953+
assertThat(MergedAnnotations.from(method, SearchStrategy.TYPE_HIERARCHY)
954+
.get(Transactional.class).isDirectlyPresent()).isTrue();
955+
}
956+
948957
@Test
949958
void getFromMethodWithInterfaceOnSuper() throws Exception {
950959
Method method = SubOfImplementsInterfaceWithAnnotatedMethod.class.getMethod("foo");
@@ -3032,6 +3041,26 @@ public void foo(String t) {
30323041
}
30333042
}
30343043

3044+
interface GenericInterface<A, B> {
3045+
3046+
@Transactional
3047+
void processOneAndTwo(A value1, B value2);
3048+
}
3049+
3050+
abstract static class GenericAbstractSuperclass<C> implements GenericInterface<Long, C> {
3051+
3052+
@Override
3053+
public void processOneAndTwo(Long value1, C value2) {
3054+
}
3055+
}
3056+
3057+
static class GenericInterfaceImpl extends GenericAbstractSuperclass<String> {
3058+
// The compiler does not require us to declare a concrete
3059+
// processOneAndTwo(Long, String) method, and we intentionally
3060+
// do not declare one here.
3061+
}
3062+
3063+
30353064
@Retention(RetentionPolicy.RUNTIME)
30363065
@Inherited
30373066
@interface MyRepeatableContainer {

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

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,32 +35,46 @@
3535
* Tests for {@link HandlerMethod}.
3636
*
3737
* @author Rossen Stoyanchev
38+
* @author Sam Brannen
3839
*/
3940
class HandlerMethodTests {
4041

4142
@Test
42-
void shouldValidateArgsWithConstraintsDirectlyOnClass() {
43+
void shouldValidateArgsWithConstraintsDirectlyInClass() {
4344
Object target = new MyClass();
4445
testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons", "addPeople", "addNames"), true);
4546
testValidateArgs(target, List.of("addPerson", "getPerson", "getIntValue", "addPersonNotValidated"), false);
4647
}
4748

4849
@Test
49-
void shouldValidateArgsWithConstraintsOnInterface() {
50+
void shouldValidateArgsWithConstraintsInInterface() {
5051
Object target = new MyInterfaceImpl();
5152
testValidateArgs(target, List.of("addIntValue", "addPersonAndIntValue", "addPersons", "addPeople"), true);
5253
testValidateArgs(target, List.of("addPerson", "addPersonNotValidated", "getPerson", "getIntValue"), false);
5354
}
5455

5556
@Test
56-
void shouldValidateReturnValueWithConstraintsDirectlyOnClass() {
57+
void shouldValidateArgsWithConstraintsInGenericAbstractSuperclass() {
58+
Object target = new GenericInterfaceImpl();
59+
shouldValidateArguments(getHandlerMethod(target, "processTwo", String.class), true);
60+
}
61+
62+
@Test
63+
void shouldValidateArgsWithConstraintsInGenericInterface() {
64+
Object target = new GenericInterfaceImpl();
65+
shouldValidateArguments(getHandlerMethod(target, "processOne", Long.class), false);
66+
shouldValidateArguments(getHandlerMethod(target, "processOneAndTwo", Long.class, Object.class), true);
67+
}
68+
69+
@Test
70+
void shouldValidateReturnValueWithConstraintsDirectlyInClass() {
5771
Object target = new MyClass();
5872
testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true);
5973
testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false);
6074
}
6175

6276
@Test
63-
void shouldValidateReturnValueWithConstraintsOnInterface() {
77+
void shouldValidateReturnValueWithConstraintsInInterface() {
6478
Object target = new MyInterfaceImpl();
6579
testValidateReturnValue(target, List.of("getPerson", "getIntValue"), true);
6680
testValidateReturnValue(target, List.of("addPerson", "addIntValue", "addPersonNotValidated"), false);
@@ -97,9 +111,19 @@ void resolvedFromHandlerMethod() {
97111
assertThat(hm3.getResolvedFromHandlerMethod()).isSameAs(hm1);
98112
}
99113

114+
115+
private static void shouldValidateArguments(HandlerMethod handlerMethod, boolean expected) {
116+
if (expected) {
117+
assertThat(handlerMethod.shouldValidateArguments()).as(handlerMethod.getMethod().getName()).isTrue();
118+
}
119+
else {
120+
assertThat(handlerMethod.shouldValidateArguments()).as(handlerMethod.getMethod().getName()).isFalse();
121+
}
122+
}
123+
100124
private static void testValidateArgs(Object target, List<String> methodNames, boolean expected) {
101125
for (String methodName : methodNames) {
102-
assertThat(getHandlerMethod(target, methodName).shouldValidateArguments()).isEqualTo(expected);
126+
shouldValidateArguments(getHandlerMethod(target, methodName), expected);
103127
}
104128
}
105129

@@ -110,7 +134,11 @@ private static void testValidateReturnValue(Object target, List<String> methodNa
110134
}
111135

112136
private static HandlerMethod getHandlerMethod(Object target, String methodName) {
113-
Method method = ClassUtils.getMethod(target.getClass(), methodName, (Class<?>[]) null);
137+
return getHandlerMethod(target, methodName, (Class<?>[]) null);
138+
}
139+
140+
private static HandlerMethod getHandlerMethod(Object target, String methodName, Class<?>... parameterTypes) {
141+
Method method = ClassUtils.getMethod(target.getClass(), methodName, parameterTypes);
114142
return new HandlerMethod(target, method).createWithValidateFlags();
115143
}
116144

@@ -236,4 +264,32 @@ public Person getPerson() {
236264
}
237265
}
238266

267+
268+
interface GenericInterface<A, B> {
269+
270+
void processOne(@Valid A value1);
271+
272+
void processOneAndTwo(A value1, @Max(42) B value2);
273+
}
274+
275+
abstract static class GenericAbstractSuperclass<C> implements GenericInterface<Long, C> {
276+
277+
@Override
278+
public void processOne(Long value1) {
279+
}
280+
281+
@Override
282+
public void processOneAndTwo(Long value1, C value2) {
283+
}
284+
285+
public abstract void processTwo(@Max(42) C value);
286+
}
287+
288+
static class GenericInterfaceImpl extends GenericAbstractSuperclass<String> {
289+
290+
@Override
291+
public void processTwo(String value) {
292+
}
293+
}
294+
239295
}

0 commit comments

Comments
 (0)