From 0ec03a8dd6d65888e1176e25296b20ebf0e77755 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 2 Sep 2022 11:00:40 +0200 Subject: [PATCH] Stop implementing SynthesizedAnnotation in annotation proxies SynthesizedAnnotation was originally introduced as a convenience for easily detecting if an annotation had been synthesized by Spring via a simple `if (myAnnotation instanceof SynthesizedAnnotation)` check. However, the introduction of SynthesizedAnnotation in the JDK dynamic proxy for a synthesized annotation results in a separate proxy class for each annotation synthesized by Spring, and this causes issues with GraalVM native images since users and framework developers must always ensure that the additional proxy classes are registered. This commit completely removes the use of SynthesizedAnnotation in synthesized annotation proxies. This change allows the proxy class for an annotation to be reused for a synthesized annotation of the same annotation type. Consequently: - Extra proxy classes are not generated on the JVM or in a native image. - Extra proxy classes are not required to be registered for a native image. Closes gh-29041 --- .../aot/hint/support/RuntimeHintsUtils.java | 2 -- .../core/annotation/AnnotationUtils.java | 11 +++++++++- ...izedMergedAnnotationInvocationHandler.java | 16 +------------- .../core/annotation/TypeMappedAnnotation.java | 2 +- .../annotation/MergedAnnotationsTests.java | 21 ++++++++++++++++++- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/aot/hint/support/RuntimeHintsUtils.java b/spring-core/src/main/java/org/springframework/aot/hint/support/RuntimeHintsUtils.java index da88f675ba..6f687889a4 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/support/RuntimeHintsUtils.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/support/RuntimeHintsUtils.java @@ -49,7 +49,6 @@ public abstract class RuntimeHintsUtils { * at runtime. * @param hints the {@link RuntimeHints} instance to use * @param annotationType the annotation type - * @see SynthesizedAnnotation * @deprecated as annotation attributes are visible without additional hints */ @Deprecated @@ -70,7 +69,6 @@ public abstract class RuntimeHintsUtils { * that determines if the hints are required. * @param hints the {@link RuntimeHints} instance to use * @param annotationType the annotation type - * @see SynthesizedAnnotation */ public static void registerSynthesizedAnnotation(RuntimeHints hints, Class annotationType) { hints.proxies().registerJdkProxy(annotationType, SynthesizedAnnotation.class); diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java index f42873522f..53add8a193 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java @@ -22,6 +22,7 @@ import java.lang.reflect.Array; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.lang.reflect.Proxy; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -1288,7 +1289,15 @@ public abstract class AnnotationUtils { * @since 5.3.23 */ public static boolean isSynthesizedAnnotation(@Nullable Annotation annotation) { - return (annotation instanceof SynthesizedAnnotation); + try { + return ((annotation != null) && Proxy.isProxyClass(annotation.getClass()) && + (Proxy.getInvocationHandler(annotation) instanceof SynthesizedMergedAnnotationInvocationHandler)); + } + catch (SecurityException ex) { + // Security settings disallow reflective access to the InvocationHandler: + // assume the annotation has not been synthesized by Spring. + return false; + } } /** diff --git a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java index 992b1a07fa..c8eaf22e41 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java @@ -299,9 +299,8 @@ final class SynthesizedMergedAnnotationInvocationHandler i @SuppressWarnings("unchecked") static A createProxy(MergedAnnotation annotation, Class type) { ClassLoader classLoader = type.getClassLoader(); + Class[] interfaces = new Class[] {type}; InvocationHandler handler = new SynthesizedMergedAnnotationInvocationHandler<>(annotation, type); - Class[] interfaces = isVisible(classLoader, SynthesizedAnnotation.class) ? - new Class[] {type, SynthesizedAnnotation.class} : new Class[] {type}; return (A) Proxy.newProxyInstance(classLoader, interfaces, handler); } @@ -310,17 +309,4 @@ final class SynthesizedMergedAnnotationInvocationHandler i return (canonicalName != null ? canonicalName : clazz.getName()); } - - private static boolean isVisible(ClassLoader classLoader, Class interfaceClass) { - if (classLoader == interfaceClass.getClassLoader()) { - return true; - } - try { - return Class.forName(interfaceClass.getName(), false, classLoader) == interfaceClass; - } - catch (ClassNotFoundException ex) { - return false; - } - } - } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java index 1fcbb18883..c2ae3a5dc7 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java @@ -363,7 +363,7 @@ final class TypeMappedAnnotation extends AbstractMergedAnn * @since 5.3.22 */ private boolean isSynthesizable(Annotation annotation) { - if (annotation instanceof SynthesizedAnnotation) { + if (AnnotationUtils.isSynthesizedAnnotation(annotation)) { return false; } return isSynthesizable(); 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 1f9ee30680..d97e10050b 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 @@ -1511,6 +1511,26 @@ class MergedAnnotationsTests { assertThat(MergedAnnotation.from(component).isSynthesizable()).isFalse(); } + /** + * @since 6.0 + */ + @Test + void synthesizedAnnotationShouldReuseJdkProxyClass() throws Exception { + Method method = WebController.class.getMethod("handleMappedWithValueAttribute"); + + RequestMapping jdkRequestMapping = method.getAnnotation(RequestMapping.class); + assertThat(jdkRequestMapping).isNotNull(); + assertThat(jdkRequestMapping.value()).containsExactly("/test"); + assertThat(jdkRequestMapping.path()).containsExactly(""); + + RequestMapping synthesizedRequestMapping = MergedAnnotation.from(jdkRequestMapping).synthesize(); + assertSynthesized(synthesizedRequestMapping); + assertThat(synthesizedRequestMapping.value()).containsExactly("/test"); + assertThat(synthesizedRequestMapping.path()).containsExactly("/test"); + + assertThat(jdkRequestMapping.getClass()).isSameAs(synthesizedRequestMapping.getClass()); + } + @Test void synthesizeAlreadySynthesized() throws Exception { Method method = WebController.class.getMethod("handleMappedWithValueAttribute"); @@ -1585,7 +1605,6 @@ class MergedAnnotationsTests { mergedAnnotations.get(EnableGlobalAuthentication.class); assertThat(enableGlobalAuthenticationMergedAnnotation.isSynthesizable()).isFalse(); EnableGlobalAuthentication enableGlobalAuthentication = enableGlobalAuthenticationMergedAnnotation.synthesize(); - assertThat(enableGlobalAuthentication).isNotInstanceOf(SynthesizedAnnotation.class); assertNotSynthesized(enableGlobalAuthentication); }