From 1e742aae34e4d32a703720fa8326c6c439efb498 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 (cherry picked from commit 75da9c3c474374289cc128b714126d9d644b69a9) --- .../core/annotation/AnnotationsScanner.java | 7 ++-- .../annotation/AnnotationsScannerTests.java | 35 ++++++++++++++++++- .../annotation/MergedAnnotationsTests.java | 12 +++++++ 3 files changed, 49 insertions(+), 5 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 61fea0685226..e6a8626e05ad 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -336,11 +336,10 @@ private static Method[] getBaseTypeMethods(C context, Class baseType) { 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 b0be3a216b57..3fa06ca8e780 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -357,6 +357,15 @@ void typeHierarchyStrategyOnMethodWhenHasInterfaceScansInterfaces() { 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 @@ public void method() { } } + 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 340b0031a210..f73654666c91 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.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 @@ void getWithTypeHierarchyInheritedFromInterfaceMethod() throws Exception { 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");