From d6768ccc1852f4720781d490e13529e0283d9d63 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 25 Jun 2022 19:04:21 +0200 Subject: [PATCH] Ensure meta-annotations are not unnecessarily synthesized Prior to this commit, meta-annotations were unnecessarily synthesized when attempting to synthesize a MergedAnnotation retrieved via the MergedAnnotations.from(AnnotatedElement, ...).get() API. This is a regression in our merged annotation support that was introduced when the MergedAnnotations API replaced our previous support. This commit fixes this by revising the logic in TypeMappedAnnotation's createSynthesizedAnnotation() method so that a meta-annotation is returned unmodified if it is not synthesizable. This commit also updates BootstrapUtilsTests, since @BootstrapWith should never have been synthesized, and Class#getCanonicalName() is only used in the toString() implementation of an annotation synthesized by Spring or normal annotations on Java 19+ (see https://bugs.openjdk.org/browse/JDK-8281462). Closes gh-28704 --- .../core/annotation/TypeMappedAnnotation.java | 34 ++++++++++++++----- .../annotation/MergedAnnotationsTests.java | 30 ++++++++++++++++ .../test/context/BootstrapUtilsTests.java | 4 +-- 3 files changed, 58 insertions(+), 10 deletions(-) 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 d38042b718..af603213a3 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -329,18 +329,36 @@ final class TypeMappedAnnotation extends AbstractMergedAnn @Override @SuppressWarnings("unchecked") protected A createSynthesizedAnnotation() { - if (getType().isInstance(this.rootAttributes) && !isSynthesizable()) { + // Check root annotation + if (isTargetAnnotation(this.rootAttributes) && isNotSynthesizable((Annotation) this.rootAttributes)) { return (A) this.rootAttributes; } + // Check meta-annotation + else if (isTargetAnnotation(this.mapping.getAnnotation()) && isNotSynthesizable(this.mapping.getAnnotation())) { + return (A) this.mapping.getAnnotation(); + } return SynthesizedMergedAnnotationInvocationHandler.createProxy(this, getType()); } - private boolean isSynthesizable() { - // Already synthesized? - if (this.rootAttributes instanceof SynthesizedAnnotation) { - return false; - } - return this.mapping.isSynthesizable(); + /** + * Determine if the supplied object is an annotation of the required + * {@linkplain #getType() type}. + * @param obj the object to check + * @since 5.3.22 + */ + private boolean isTargetAnnotation(@Nullable Object obj) { + return getType().isInstance(obj); + } + + /** + * Determine if the supplied annotation has already been synthesized or if the + * mapped annotation is not {@linkplain AnnotationTypeMapping#isSynthesizable() + * synthesizable} in general. + * @param annotation the annotation to check + * @since 5.3.22 + */ + private boolean isNotSynthesizable(Annotation annotation) { + return (annotation instanceof SynthesizedAnnotation || !this.mapping.isSynthesizable()); } @Override 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 02ec8f0980..71e30d8b5d 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 @@ -1420,6 +1420,17 @@ class MergedAnnotationsTests { assertThat(generatedValue).isSameAs(synthesizedGeneratedValue); } + @Test + void synthesizeShouldNotSynthesizeNonsynthesizableAnnotationsWhenUsingMergedAnnotationsFromApi() { + MergedAnnotations mergedAnnotations = MergedAnnotations.from(SecurityConfig.class); + + EnableWebSecurity enableWebSecurity = mergedAnnotations.get(EnableWebSecurity.class).synthesize(); + assertThat(enableWebSecurity).isNotInstanceOf(SynthesizedAnnotation.class); + + EnableGlobalAuthentication enableGlobalAuthentication = mergedAnnotations.get(EnableGlobalAuthentication.class).synthesize(); + assertThat(enableGlobalAuthentication).isNotInstanceOf(SynthesizedAnnotation.class); + } + /** * If an attempt is made to synthesize an annotation from an annotation instance * that has already been synthesized, the original synthesized annotation should @@ -3097,6 +3108,25 @@ class MergedAnnotationsTests { return 42L; } + /** + * Mimics org.springframework.security.config.annotation.authentication.configuration.EnableGlobalAuthentication + */ + @Retention(RUNTIME) + @interface EnableGlobalAuthentication { + } + + /** + * Mimics org.springframework.security.config.annotation.web.configuration.EnableWebSecurity + */ + @Retention(RUNTIME) + @EnableGlobalAuthentication + @interface EnableWebSecurity { + } + + @EnableWebSecurity + static class SecurityConfig { + } + @Retention(RetentionPolicy.RUNTIME) @interface TestConfiguration { diff --git a/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.java b/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.java index 3603532552..db3297c5fc 100644 --- a/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/BootstrapUtilsTests.java @@ -73,8 +73,8 @@ class BootstrapUtilsTests { assertThatIllegalStateException() .isThrownBy(() -> resolveTestContextBootstrapper(bootstrapContext)) .withMessageContaining("Configuration error: found multiple declarations of @BootstrapWith") - .withMessageContaining(FooBootstrapper.class.getCanonicalName()) - .withMessageContaining(BarBootstrapper.class.getCanonicalName()); + .withMessageContaining(FooBootstrapper.class.getSimpleName()) + .withMessageContaining(BarBootstrapper.class.getSimpleName()); } @Test