Skip to content

Commit

Permalink
Support repeatable annotation containers with multiple attributes
Browse files Browse the repository at this point in the history
Prior to this commit, there was a bug in the implementation of
StandardRepeatableContainers.computeRepeatedAnnotationsMethod() which
has existed since Spring Framework 5.2 (when
StandardRepeatableContainers was introduced). Specifically,
StandardRepeatableContainers ignored any repeatable container
annotation if it declared attributes other than `value()`. However,
Java permits any number of attributes in a repeatable container
annotation.

In addition, the changes made in conjunction with spring-projectsgh-20279 made the bug
in StandardRepeatableContainers apparent when using the
getMergedRepeatableAnnotations() or findMergedRepeatableAnnotations()
method in AnnotatedElementUtils, resulting in regressions for the
behavior of those two methods.

This commit fixes the regressions and bug by altering the logic in
StandardRepeatableContainers.computeRepeatedAnnotationsMethod() so that
it explicitly looks for the `value()` method and ignores any other
methods declared in a repeatable container annotation candidate.

Closes spring-projectsgh-29685
  • Loading branch information
sbrannen authored and mdeinum committed Jun 29, 2023
1 parent 8a4264a commit 4c24bfd
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 2 deletions.
Expand Up @@ -166,8 +166,8 @@ private static Method getRepeatedAnnotationsMethod(Class<? extends Annotation> a

private static Object computeRepeatedAnnotationsMethod(Class<? extends Annotation> annotationType) {
AttributeMethods methods = AttributeMethods.forAnnotationType(annotationType);
if (methods.hasOnlyValueAttribute()) {
Method method = methods.get(0);
Method method = methods.get(MergedAnnotation.VALUE);
if (method != null) {
Class<?> returnType = method.getReturnType();
if (returnType.isArray()) {
Class<?> componentType = returnType.getComponentType();
Expand Down
Expand Up @@ -20,6 +20,7 @@
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Inherited;
import java.lang.annotation.Repeatable;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
Expand Down Expand Up @@ -77,6 +78,7 @@
* @see AnnotationUtilsTests
* @see MultipleComposedAnnotationsOnSingleAnnotatedElementTests
* @see ComposedRepeatableAnnotationsTests
* @see NestedRepeatableAnnotationsTests
*/
class AnnotatedElementUtilsTests {

Expand Down Expand Up @@ -908,6 +910,31 @@ void getMergedAnnotationOnThreeDeepMetaWithValue() {
assertThat(annotation.value()).containsExactly("FromValueAttributeMeta");
}

/**
* @since 5.3.25
*/
@Test // gh-29685
void getMergedRepeatableAnnotationsWithContainerWithMultipleAttributes() {
Set<StandardRepeatableWithContainerWithMultipleAttributes> repeatableAnnotations =
AnnotatedElementUtils.getMergedRepeatableAnnotations(
StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class,
StandardRepeatableWithContainerWithMultipleAttributes.class);
assertThat(repeatableAnnotations).map(StandardRepeatableWithContainerWithMultipleAttributes::value)
.containsExactly("a", "b");
}

/**
* @since 5.3.25
*/
@Test // gh-29685
void findMergedRepeatableAnnotationsWithContainerWithMultipleAttributes() {
Set<StandardRepeatableWithContainerWithMultipleAttributes> repeatableAnnotations =
AnnotatedElementUtils.findMergedRepeatableAnnotations(
StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class,
StandardRepeatableWithContainerWithMultipleAttributes.class);
assertThat(repeatableAnnotations).map(StandardRepeatableWithContainerWithMultipleAttributes::value)
.containsExactly("a", "b");
}

// -------------------------------------------------------------------------

Expand Down Expand Up @@ -1567,4 +1594,24 @@ class ForAnnotationsClass {
static class ValueAttributeMetaMetaClass {
}

@Retention(RetentionPolicy.RUNTIME)
@interface StandardContainerWithMultipleAttributes {

StandardRepeatableWithContainerWithMultipleAttributes[] value();

String name() default "";
}

@Retention(RetentionPolicy.RUNTIME)
@Repeatable(StandardContainerWithMultipleAttributes.class)
@interface StandardRepeatableWithContainerWithMultipleAttributes {

String value() default "";
}

@StandardRepeatableWithContainerWithMultipleAttributes("a")
@StandardRepeatableWithContainerWithMultipleAttributes("b")
static class StandardRepeatablesWithContainerWithMultipleAttributesTestCase {
}

}
Expand Up @@ -67,6 +67,15 @@ void standardRepeatablesWhenContainerReturnsRepeats() {
StandardRepeatablesTestCase.class, StandardContainer.class);
assertThat(values).containsExactly("a", "b");
}

@Test
void standardRepeatablesWithContainerWithMultipleAttributes() {
Object[] values = findRepeatedAnnotationValues(RepeatableContainers.standardRepeatables(),
StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class,
StandardContainerWithMultipleAttributes.class);
assertThat(values).containsExactly("a", "b");
}

}

@Nested
Expand Down Expand Up @@ -247,6 +256,26 @@ static class SingleStandardRepeatableTestCase {
static class StandardRepeatablesTestCase {
}

@Retention(RetentionPolicy.RUNTIME)
@interface StandardContainerWithMultipleAttributes {

StandardRepeatableWithContainerWithMultipleAttributes[] value();

String name() default "";
}

@Retention(RetentionPolicy.RUNTIME)
@Repeatable(StandardContainerWithMultipleAttributes.class)
@interface StandardRepeatableWithContainerWithMultipleAttributes {

String value() default "";
}

@StandardRepeatableWithContainerWithMultipleAttributes("a")
@StandardRepeatableWithContainerWithMultipleAttributes("b")
static class StandardRepeatablesWithContainerWithMultipleAttributesTestCase {
}

@ExplicitContainer({ @ExplicitRepeatable("a"), @ExplicitRepeatable("b") })
static class ExplicitRepeatablesTestCase {
}
Expand Down

0 comments on commit 4c24bfd

Please sign in to comment.