mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-09-17 08:24:13 +00:00
Avoid unnecessarily synthesizing meta-annotations with attributes
In commit622fc3edf7, I introduced a check in TypeMappedAnnotation#isSynthesizable() intended to force synthesis when an attribute value needs to be resolved from a different level of a multi-level annotation hierarchy whose root annotation does not redeclare the target attribute itself. That check tested if `resolvedMirrors.length > 0` for a meta-annotation; however, resolvedMirrors is always sized according to the number of attributes declared by the mapped annotation type, regardless of whether any of those attributes actually participate in mirroring or an @AliasFor override. As a result, the check effectively synthesized any meta-annotation that declares at least one attribute, which reintroduced the unnecessary-synthesis behavior that commitd6768ccc18had fixed, merely narrowed to meta-annotations with attributes. This commit replaces that overly broad check with a precise one in AnnotationTypeMapping#computeSynthesizableFlag(), which now also considers whether any attribute's value must be resolved from a different annotation in the meta-annotation hierarchy (tracked via annotationValueSource). This correctly identifies the original multi-level hierarchy scenario without over-matching on ordinary meta-annotations that have nothing to merge or override. See gh-28704 See gh-28716 Closes gh-37135
This commit is contained in:
+11
@@ -275,6 +275,17 @@ final class AnnotationTypeMapping {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Does an attribute value need to be resolved from a different annotation
|
||||
// in the meta-annotation hierarchy? This can happen when an attribute is
|
||||
// overridden via @AliasFor further up a multi-level annotation hierarchy
|
||||
// whose root annotation does not redeclare the target attribute itself,
|
||||
// in which case the override cannot be tracked via aliasMappings/aliasedBy.
|
||||
for (int i = 0; i < this.annotationValueSource.length; i++) {
|
||||
if (this.annotationValueSource[i] != null && this.annotationValueSource[i] != this) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// Has nested annotations or arrays of annotations that are synthesizable?
|
||||
if (getAttributes().hasNestedAnnotation()) {
|
||||
AttributeMethods attributeMethods = getAttributes();
|
||||
|
||||
@@ -513,6 +513,9 @@ public interface MergedAnnotation<A extends Annotation> {
|
||||
* it has not already been synthesized and one of the following is true.
|
||||
* <ul>
|
||||
* <li>The annotation declares attributes annotated with {@link AliasFor @AliasFor}.</li>
|
||||
* <li>The annotation is used as a meta-annotation, and one of its attributes
|
||||
* is overridden via an {@link AliasFor @AliasFor} declaration in a composed
|
||||
* annotation somewhere in the annotation hierarchy.</li>
|
||||
* <li>The annotation declares attributes that are annotations or arrays of
|
||||
* annotations that are themselves synthesizable.</li>
|
||||
* </ul>
|
||||
|
||||
+1
-7
@@ -354,8 +354,7 @@ final class TypeMappedAnnotation<A extends Annotation> extends AbstractMergedAnn
|
||||
|
||||
/**
|
||||
* Determine if the supplied annotation has not already been synthesized
|
||||
* <strong>and</strong> whether the mapped annotation is a composed annotation
|
||||
* that needs to have its attributes merged or the mapped annotation is
|
||||
* <strong>and</strong> whether the mapped annotation is
|
||||
* {@linkplain AnnotationTypeMapping#isSynthesizable() synthesizable} in general.
|
||||
* @param annotation the annotation to check
|
||||
* @since 5.3.22
|
||||
@@ -365,11 +364,6 @@ final class TypeMappedAnnotation<A extends Annotation> extends AbstractMergedAnn
|
||||
if (AnnotationUtils.isSynthesizedAnnotation(annotation)) {
|
||||
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();
|
||||
}
|
||||
|
||||
+34
@@ -1565,6 +1565,22 @@ class MergedAnnotationsTests {
|
||||
assertNotSynthesized(enableGlobalAuthentication);
|
||||
}
|
||||
|
||||
/**
|
||||
* A meta-annotation that declares an attribute should not be synthesized
|
||||
* merely because it is meta-present rather than directly present, as long
|
||||
* as none of its attributes are mirrored or overridden anywhere in the
|
||||
* annotation hierarchy.
|
||||
*/
|
||||
@Test
|
||||
void synthesizeShouldNotSynthesizeMetaAnnotationsWithNonOverriddenAttributes() {
|
||||
MergedAnnotations mergedAnnotations = MergedAnnotations.from(ComponentWithPlainAttributeMetaAnnotation.class);
|
||||
|
||||
PlainAttributeMetaAnnotation plainAttributeMetaAnnotation =
|
||||
mergedAnnotations.get(PlainAttributeMetaAnnotation.class).synthesize();
|
||||
assertThat(plainAttributeMetaAnnotation.value()).isEqualTo("enigma");
|
||||
assertNotSynthesized(plainAttributeMetaAnnotation);
|
||||
}
|
||||
|
||||
/**
|
||||
* If an attempt is made to synthesize an annotation from an annotation instance
|
||||
* that has already been synthesized, the original synthesized annotation should
|
||||
@@ -3278,6 +3294,24 @@ class MergedAnnotationsTests {
|
||||
static class SecurityConfig {
|
||||
}
|
||||
|
||||
/**
|
||||
* Meta-annotation that declares an attribute which is never mirrored or
|
||||
* overridden anywhere in the annotation hierarchy.
|
||||
*/
|
||||
@Retention(RUNTIME)
|
||||
@interface PlainAttributeMetaAnnotation {
|
||||
String value() default "enigma";
|
||||
}
|
||||
|
||||
@PlainAttributeMetaAnnotation
|
||||
@Retention(RUNTIME)
|
||||
@interface ComposedPlainAttributeAnnotation {
|
||||
}
|
||||
|
||||
@ComposedPlainAttributeAnnotation
|
||||
static class ComponentWithPlainAttributeMetaAnnotation {
|
||||
}
|
||||
|
||||
@Retention(RetentionPolicy.RUNTIME)
|
||||
@Target({ ElementType.FIELD, ElementType.ANNOTATION_TYPE })
|
||||
@interface RootAnnotation {
|
||||
|
||||
Reference in New Issue
Block a user