From 622fc3edf7d5f3939204fd7ef8c38c7cfc614ad6 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 28 Jun 2022 14:22:53 +0200 Subject: [PATCH] Fix merged annotation attributes regression Commit d6768ccc18 introduced a regression in the support for merging annotation attributes in a multi-level annotation hierarchy. This commit addresses that issue by ensuring that a merged annotation is once again synthesized if a meta-annotation in the annotation hierarchy declares attributes that override attributes in the target annotation. Closes gh-28716 --- .../core/annotation/TypeMappedAnnotation.java | 25 +++++--- .../annotation/MergedAnnotationsTests.java | 58 ++++++++++++++++++- 2 files changed, 75 insertions(+), 8 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 af603213a3..0b18e30bf0 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 @@ -330,11 +330,11 @@ final class TypeMappedAnnotation extends AbstractMergedAnn @SuppressWarnings("unchecked") protected A createSynthesizedAnnotation() { // Check root annotation - if (isTargetAnnotation(this.rootAttributes) && isNotSynthesizable((Annotation) this.rootAttributes)) { + if (isTargetAnnotation(this.rootAttributes) && !isSynthesizable((Annotation) this.rootAttributes)) { return (A) this.rootAttributes; } // Check meta-annotation - else if (isTargetAnnotation(this.mapping.getAnnotation()) && isNotSynthesizable(this.mapping.getAnnotation())) { + else if (isTargetAnnotation(this.mapping.getAnnotation()) && !isSynthesizable(this.mapping.getAnnotation())) { return (A) this.mapping.getAnnotation(); } return SynthesizedMergedAnnotationInvocationHandler.createProxy(this, getType()); @@ -351,14 +351,25 @@ final class TypeMappedAnnotation extends AbstractMergedAnn } /** - * Determine if the supplied annotation has already been synthesized or if the - * mapped annotation is not {@linkplain AnnotationTypeMapping#isSynthesizable() - * synthesizable} in general. + * Determine if the supplied annotation has not already been synthesized + * and whether the mapped annotation is a composed annotation + * that needs to have its attributes merged or the mapped annotation is + * {@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()); + private boolean isSynthesizable(Annotation annotation) { + // Already synthesized? + if (annotation instanceof SynthesizedAnnotation) { + return false; + } + // Is this a mapped annotation for a composed annotation, and are there + // annotation attributes (mirrors) that need to be merged? + if (getDistance() > 0 && this.resolvedMirrors.length > 0) { + return true; + } + // Is the mapped annotation itself synthesizable? + return 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 71e30d8b5d..bd32cdbe0a 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 @@ -24,6 +24,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; @@ -1420,7 +1421,28 @@ class MergedAnnotationsTests { assertThat(generatedValue).isSameAs(synthesizedGeneratedValue); } - @Test + @Test // gh-28716 + void synthesizeWhenUsingMergedAnnotationsFromApi() { + Field directlyAnnotatedField = ReflectionUtils.findField(DomainType.class, "directlyAnnotated"); + MergedAnnotations mergedAnnotations = MergedAnnotations.from(directlyAnnotatedField); + RootAnnotation rootAnnotation = mergedAnnotations.get(RootAnnotation.class).synthesize(); + assertThat(rootAnnotation.flag()).isFalse(); + assertThat(rootAnnotation).isNotInstanceOf(SynthesizedAnnotation.class); + + Field metaAnnotatedField = ReflectionUtils.findField(DomainType.class, "metaAnnotated"); + mergedAnnotations = MergedAnnotations.from(metaAnnotatedField); + rootAnnotation = mergedAnnotations.get(RootAnnotation.class).synthesize(); + assertThat(rootAnnotation.flag()).isTrue(); + assertThat(rootAnnotation).isInstanceOf(SynthesizedAnnotation.class); + + Field metaMetaAnnotatedField = ReflectionUtils.findField(DomainType.class, "metaMetaAnnotated"); + mergedAnnotations = MergedAnnotations.from(metaMetaAnnotatedField); + rootAnnotation = mergedAnnotations.get(RootAnnotation.class).synthesize(); + assertThat(rootAnnotation.flag()).isTrue(); + assertThat(rootAnnotation).isInstanceOf(SynthesizedAnnotation.class); + } + + @Test // gh-28704 void synthesizeShouldNotSynthesizeNonsynthesizableAnnotationsWhenUsingMergedAnnotationsFromApi() { MergedAnnotations mergedAnnotations = MergedAnnotations.from(SecurityConfig.class); @@ -3127,6 +3149,40 @@ class MergedAnnotationsTests { static class SecurityConfig { } + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.FIELD, ElementType.ANNOTATION_TYPE }) + @interface RootAnnotation { + String value() default ""; + boolean flag() default false; + } + + @RootAnnotation + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.FIELD, ElementType.ANNOTATION_TYPE }) + @interface ComposedRootAnnotation { + + @AliasFor(annotation = RootAnnotation.class, attribute = "flag") + boolean enabled() default true; + } + + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.FIELD) + @ComposedRootAnnotation + @interface DoublyComposedRootAnnotation { + } + + class DomainType { + + @RootAnnotation + Object directlyAnnotated; + + @ComposedRootAnnotation + Object metaAnnotated; + + @DoublyComposedRootAnnotation + Object metaMetaAnnotated; + } + @Retention(RetentionPolicy.RUNTIME) @interface TestConfiguration {