From 75da9c3c474374289cc128b714126d9d644b69a9 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 12 Dec 2023 18:28:48 +0100 Subject: [PATCH] Scan annotations on method in interface hierarchy only once Prior to this commit, the AnnotationsScanner used in the MergedAnnotations infrastructure found duplicate annotations on methods within multi-level interface hierarchies. This commit addresses this issue by scanning methods at a given level in the interface hierarchy using ReflectionUtils#getDeclaredMethods instead of Class#getMethods, since the latter includes public methods declared in super-interfaces which will anyway be scanned when processing super-interfaces recursively. Closes gh-31803 --- .../core/annotation/AnnotationsScanner.java | 5 ++- .../annotation/AnnotationsScannerTests.java | 33 +++++++++++++++++++ .../annotation/MergedAnnotationsTests.java | 12 +++++++ 3 files changed, 47 insertions(+), 3 deletions(-) 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 a98c005f378..c3f44cc897f 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 @@ -334,11 +334,10 @@ abstract class AnnotationsScanner { Method[] methods = baseTypeMethodsCache.get(baseType); if (methods == null) { - boolean isInterface = baseType.isInterface(); - methods = isInterface ? baseType.getMethods() : ReflectionUtils.getDeclaredMethods(baseType); + methods = ReflectionUtils.getDeclaredMethods(baseType); int cleared = 0; for (int i = 0; i < methods.length; i++) { - if ((!isInterface && Modifier.isPrivate(methods[i].getModifiers())) || + if (Modifier.isPrivate(methods[i].getModifiers()) || hasPlainJavaAnnotationsOnly(methods[i]) || getDeclaredAnnotations(methods[i], false).length == 0) { methods[i] = null; diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java index 66cc1f015b1..6400a880c33 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java @@ -357,6 +357,15 @@ class AnnotationsScannerTests { Method source = methodFrom(WithSingleInterface.class); assertThat(scan(source, SearchStrategy.TYPE_HIERARCHY)).containsExactly( "0:TestAnnotation1", "1:TestAnnotation2", "1:TestInheritedAnnotation2"); + + source = methodFrom(Hello1Impl.class); + assertThat(scan(source, SearchStrategy.TYPE_HIERARCHY)).containsExactly("1:TestAnnotation1"); + } + + @Test // gh-31803 + void typeHierarchyStrategyOnMethodWhenHasInterfaceHierarchyScansInterfacesOnlyOnce() { + Method source = methodFrom(Hello2Impl.class); + assertThat(scan(source, SearchStrategy.TYPE_HIERARCHY)).containsExactly("1:TestAnnotation1"); } @Test @@ -691,6 +700,30 @@ class AnnotationsScannerTests { } } + interface Hello1 { + + @TestAnnotation1 + void method(); + } + + interface Hello2 extends Hello1 { + } + + static class Hello1Impl implements Hello1 { + + @Override + public void method() { + } + } + + static class Hello2Impl implements Hello2 { + + @Override + public void method() { + } + } + + @TestAnnotation2 @TestInheritedAnnotation2 static class HierarchySuperclass extends HierarchySuperSuperclass { 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 f7d2389192f..0a2371fa4b1 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 @@ -40,6 +40,8 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.core.Ordered; +import org.springframework.core.annotation.AnnotationsScannerTests.Hello2Impl; +import org.springframework.core.annotation.AnnotationsScannerTests.TestAnnotation1; import org.springframework.core.annotation.MergedAnnotation.Adapt; import org.springframework.core.annotation.MergedAnnotations.Search; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; @@ -686,6 +688,16 @@ class MergedAnnotationsTests { assertThat(annotation.getAggregateIndex()).isEqualTo(1); } + @Test // gh-31803 + void streamWithTypeHierarchyInheritedFromSuperInterfaceMethod() throws Exception { + Method method = Hello2Impl.class.getMethod("method"); + long count = MergedAnnotations.search(SearchStrategy.TYPE_HIERARCHY) + .from(method) + .stream(TestAnnotation1.class) + .count(); + assertThat(count).isEqualTo(1); + } + @Test void getWithTypeHierarchyInheritedFromAbstractMethod() throws NoSuchMethodException { Method method = ConcreteClassWithInheritedAnnotation.class.getMethod("handle");