Skip to content

Commit

Permalink
Include repeatable annotation container in MergedAnnotations results
Browse files Browse the repository at this point in the history
A bug has existed in Spring's MergedAnnotations support since it was
introduced in Spring Framework 5.2. Specifically, if the
MergedAnnotations API is used to search for annotations with "standard
repeatable annotation" support enabled (which is the default), it's
possible to search for a repeatable annotation but not for the
repeatable annotation's container annotation.

The reason is that MergedAnnotationFinder.process(Object, int, Object,
Annotation) does not process the container annotation and instead only
processes the "contained" annotations, which prevents a container
annotation from being included in search results.

In #29685, we fixed a bug that prevented the MergedAnnotations support
from recognizing an annotation as a container if the container
annotation declares attributes other than the required `value`
attribute. As a consequence of that bug fix, since Spring Framework
5.3.25, the MergedAnnotations infrastructure considers such an
annotation a container, and due to the aforementioned bug the container
is no longer processed, which results in a regression in behavior for
annotation searches for such a container annotation.

This commit addresses the original bug as well as the regression by
processing container annotations in addition to the contained
repeatable annotations.

See gh-29685
Closes gh-32731

(cherry picked from commit 4baad16)
  • Loading branch information
sbrannen committed May 3, 2024
1 parent 924b684 commit f5baa32
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2024 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.
Expand Down Expand Up @@ -410,7 +410,10 @@ private MergedAnnotation<A> process(

Annotation[] repeatedAnnotations = repeatableContainers.findRepeatedAnnotations(annotation);
if (repeatedAnnotations != null) {
return doWithAnnotations(type, aggregateIndex, source, repeatedAnnotations);
MergedAnnotation<A> result = doWithAnnotations(type, aggregateIndex, source, repeatedAnnotations);
if (result != null) {
return result;
}
}
AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(
annotation.annotationType(), repeatableContainers, annotationFilter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.AnnotatedElement;
import java.util.Arrays;
import java.util.Set;
import java.util.stream.Stream;

Expand Down Expand Up @@ -175,7 +176,7 @@ void typeHierarchyWhenOnClassReturnsAnnotations() {
}

@Test
void typeHierarchyWhenWhenOnSuperclassReturnsAnnotations() {
void typeHierarchyWhenOnSuperclassReturnsAnnotations() {
Set<PeteRepeat> annotations = getAnnotations(null, PeteRepeat.class,
SearchStrategy.TYPE_HIERARCHY, SubRepeatableClass.class);
assertThat(annotations.stream().map(PeteRepeat::value)).containsExactly("A", "B",
Expand Down Expand Up @@ -240,6 +241,44 @@ void typeHierarchyAnnotationsWithLocalComposedAnnotationWhoseRepeatableMetaAnnot
assertThat(annotationTypes).containsExactly(WithRepeatedMetaAnnotations.class, Noninherited.class, Noninherited.class);
}

@Test // gh-32731
void searchFindsRepeatableContainerAnnotationAndRepeatedAnnotations() {
Class<?> clazz = StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class;

// NO RepeatableContainers
MergedAnnotations mergedAnnotations = MergedAnnotations.from(clazz, TYPE_HIERARCHY, RepeatableContainers.none());
ContainerWithMultipleAttributes container = mergedAnnotations
.get(ContainerWithMultipleAttributes.class)
.synthesize(MergedAnnotation::isPresent).orElse(null);
assertThat(container).as("container").isNotNull();
assertThat(container.name()).isEqualTo("enigma");
RepeatableWithContainerWithMultipleAttributes[] repeatedAnnotations = container.value();
assertThat(Arrays.stream(repeatedAnnotations).map(RepeatableWithContainerWithMultipleAttributes::value))
.containsExactly("A", "B");
Set<RepeatableWithContainerWithMultipleAttributes> set =
mergedAnnotations.stream(RepeatableWithContainerWithMultipleAttributes.class)
.collect(MergedAnnotationCollectors.toAnnotationSet());
// Only finds the locally declared repeated annotation.
assertThat(set.stream().map(RepeatableWithContainerWithMultipleAttributes::value))
.containsExactly("C");

// Standard RepeatableContainers
mergedAnnotations = MergedAnnotations.from(clazz, TYPE_HIERARCHY, RepeatableContainers.standardRepeatables());
container = mergedAnnotations
.get(ContainerWithMultipleAttributes.class)
.synthesize(MergedAnnotation::isPresent).orElse(null);
assertThat(container).as("container").isNotNull();
assertThat(container.name()).isEqualTo("enigma");
repeatedAnnotations = container.value();
assertThat(Arrays.stream(repeatedAnnotations).map(RepeatableWithContainerWithMultipleAttributes::value))
.containsExactly("A", "B");
set = mergedAnnotations.stream(RepeatableWithContainerWithMultipleAttributes.class)
.collect(MergedAnnotationCollectors.toAnnotationSet());
// Finds the locally declared repeated annotation plus the 2 in the container.
assertThat(set.stream().map(RepeatableWithContainerWithMultipleAttributes::value))
.containsExactly("A", "B", "C");
}

private <A extends Annotation> Set<A> getAnnotations(Class<? extends Annotation> container,
Class<A> repeatable, SearchStrategy searchStrategy, AnnotatedElement element) {

Expand Down Expand Up @@ -449,4 +488,27 @@ static class SubNoninheritedRepeatableClass extends NoninheritedRepeatableClass
static class WithRepeatedMetaAnnotationsClass {
}

@Retention(RetentionPolicy.RUNTIME)
@interface ContainerWithMultipleAttributes {

RepeatableWithContainerWithMultipleAttributes[] value();

String name() default "";
}

@Retention(RetentionPolicy.RUNTIME)
@Repeatable(ContainerWithMultipleAttributes.class)
@interface RepeatableWithContainerWithMultipleAttributes {

String value() default "";
}

@ContainerWithMultipleAttributes(name = "enigma", value = {
@RepeatableWithContainerWithMultipleAttributes("A"),
@RepeatableWithContainerWithMultipleAttributes("B")
})
@RepeatableWithContainerWithMultipleAttributes("C")
static class StandardRepeatablesWithContainerWithMultipleAttributesTestCase {
}

}

0 comments on commit f5baa32

Please sign in to comment.