Revise RepeatableContainers API to better guide developers

Historically, the Spring Framework first had support for repeatable
annotations based on convention and later added explicit support for
Java 8's @⁠Repeatable facility. Consequently, the support for both
types of repeatable annotations has grown a bit intertwined over the
years. However, modern Java applications typically make use of
@⁠Repeatable, and convention-based repeatable annotations have become
more of a niche.

The RepeatableContainers API supports both types of repeatable
annotations with @⁠Repeatable support being the default. However,
RepeatableContainers.of() makes it very easy to enable support for
convention-based repeatable annotations while accidentally disabling
support for @⁠Repeatable, which can lead to subtle bugs – for example,
if convention-based annotations are combined with @⁠Repeatable
annotations. In addition, it is not readily clear how to combine
@⁠Repeatable support with convention-based repeatable annotations.

In light of the above, this commit revises the RepeatableContainers API
to better guide developers to use @⁠Repeatable support for almost all
use cases while still supporting convention-based repeatable
annotations for special use cases.

Specifically:

- RepeatableContainers.of() is now deprecated in favor of the new
  RepeatableContainers.explicitRepeatable() method.

- RepeatableContainers.and() is now deprecated in favor of the new
  RepeatableContainers.plus() method which declares the repeatable and
  container arguments in the same order as the rest of Spring
  Framework's repeated annotation APIs.

For example, instead of the following confusing mixture of
repeatable/container and container/repeatable:

RepeatableContainers.of(A.class, A.Container.class)
    .and(B.Container.class, B.class)

Developers are now be able to use:

RepeatableContainers.explicitRepeatable(A.class, A.Container.class)
    .plus(B.class, B.Container.class)

This commit also overhauls the Javadoc for RepeatableContainers and
explicitly points out that the following is the recommended approach to
support convention-based repeatable annotations while retaining support
for @⁠Repeatable.

RepeatableContainers.standardRepeatables()
    .plus(MyRepeatable1.class, MyContainer1.class)
    .plus(MyRepeatable2.class, MyContainer2.class)

See gh-20279
Closes gh-34637
This commit is contained in:
Sam Brannen
2025-03-21 17:42:16 +01:00
parent 7d5b3892c4
commit 274a689a10
7 changed files with 181 additions and 75 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@@ -274,7 +274,7 @@ class MergedAnnotationsRepeatableAnnotationTests {
private <A extends Annotation> Set<A> getAnnotations(Class<? extends Annotation> container,
Class<A> repeatable, SearchStrategy searchStrategy, AnnotatedElement element, AnnotationFilter annotationFilter) {
RepeatableContainers containers = RepeatableContainers.of(repeatable, container);
RepeatableContainers containers = RepeatableContainers.explicitRepeatable(repeatable, container);
MergedAnnotations annotations = MergedAnnotations.from(element, searchStrategy, containers, annotationFilter);
return annotations.stream(repeatable).collect(MergedAnnotationCollectors.toAnnotationSet());
}

View File

@@ -136,7 +136,7 @@ class MergedAnnotationsTests {
@Test
void searchFromClassWithCustomRepeatableContainers() {
assertThat(MergedAnnotations.from(HierarchyClass.class).stream(TestConfiguration.class)).isEmpty();
RepeatableContainers containers = RepeatableContainers.of(TestConfiguration.class, Hierarchy.class);
RepeatableContainers containers = RepeatableContainers.explicitRepeatable(TestConfiguration.class, Hierarchy.class);
MergedAnnotations annotations = MergedAnnotations.search(SearchStrategy.DIRECT)
.withRepeatableContainers(containers)
@@ -1364,7 +1364,7 @@ class MergedAnnotationsTests {
@SuppressWarnings("deprecation")
void streamRepeatableDeclaredOnClassWithAttributeAliases() {
assertThat(MergedAnnotations.from(HierarchyClass.class).stream(TestConfiguration.class)).isEmpty();
RepeatableContainers containers = RepeatableContainers.of(TestConfiguration.class, Hierarchy.class);
RepeatableContainers containers = RepeatableContainers.explicitRepeatable(TestConfiguration.class, Hierarchy.class);
MergedAnnotations annotations = MergedAnnotations.from(HierarchyClass.class,
SearchStrategy.DIRECT, containers, AnnotationFilter.NONE);
assertThat(annotations.stream(TestConfiguration.class)
@@ -1440,7 +1440,7 @@ class MergedAnnotationsTests {
private void testExplicitRepeatables(SearchStrategy searchStrategy, Class<?> element, String[] expected) {
MergedAnnotations annotations = MergedAnnotations.from(element, searchStrategy,
RepeatableContainers.of(MyRepeatable.class, MyRepeatableContainer.class));
RepeatableContainers.explicitRepeatable(MyRepeatable.class, MyRepeatableContainer.class));
Stream<String> values = annotations.stream(MyRepeatable.class)
.filter(MergedAnnotationPredicates.firstRunOf(MergedAnnotation::getAggregateIndex))
.map(annotation -> annotation.getString("value"));

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@@ -109,7 +109,7 @@ class NestedRepeatableAnnotationsTests {
@Test
void streamRepeatableAnnotationsWithExplicitRepeatables_MergedAnnotationsApi() {
RepeatableContainers repeatableContainers =
RepeatableContainers.of(A.class, A.Container.class).and(B.Container.class, B.class);
RepeatableContainers.explicitRepeatable(A.class, A.Container.class).plus(B.class, B.Container.class);
Set<A> annotations = MergedAnnotations.from(method, SearchStrategy.TYPE_HIERARCHY, repeatableContainers)
.stream(A.class).collect(MergedAnnotationCollectors.toAnnotationSet());
// Merged, so we expect to find @A twice with values coming from @B(5) and @B(10).
@@ -127,8 +127,8 @@ class NestedRepeatableAnnotationsTests {
void findMergedRepeatableAnnotationsWithExplicitContainer_AnnotatedElementUtils() {
Set<A> annotations = AnnotatedElementUtils.findMergedRepeatableAnnotations(method, A.class, A.Container.class);
// When findMergedRepeatableAnnotations(...) is invoked with an explicit container
// type, it uses RepeatableContainers.of(...) which limits the repeatable annotation
// support to a single container type.
// type, it uses RepeatableContainers.explicitRepeatable(...) which limits the
// repeatable annotation support to a single container type.
//
// In this test case, we are therefore limiting the support to @A.Container, which
// means that @B.Container is unsupported and effectively ignored as a repeatable
@@ -149,8 +149,8 @@ class NestedRepeatableAnnotationsTests {
void getMergedRepeatableAnnotationsWithExplicitContainer_AnnotatedElementUtils() {
Set<A> annotations = AnnotatedElementUtils.getMergedRepeatableAnnotations(method, A.class, A.Container.class);
// When getMergedRepeatableAnnotations(...) is invoked with an explicit container
// type, it uses RepeatableContainers.of(...) which limits the repeatable annotation
// support to a single container type.
// type, it uses RepeatableContainers.explicitRepeatable(...) which limits the
// repeatable annotation support to a single container type.
//
// In this test case, we are therefore limiting the support to @A.Container, which
// means that @B.Container is unsupported and effectively ignored as a repeatable

View File

@@ -87,7 +87,7 @@ class RepeatableContainersTests {
@Test
void ofExplicitWhenNonRepeatableReturnsNull() {
Object[] values = findRepeatedAnnotationValues(
RepeatableContainers.of(ExplicitRepeatable.class, ExplicitContainer.class),
RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, ExplicitContainer.class),
NonRepeatableTestCase.class, NonRepeatable.class);
assertThat(values).isNull();
}
@@ -95,7 +95,7 @@ class RepeatableContainersTests {
@Test
void ofExplicitWhenStandardRepeatableContainerReturnsNull() {
Object[] values = findRepeatedAnnotationValues(
RepeatableContainers.of(ExplicitRepeatable.class, ExplicitContainer.class),
RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, ExplicitContainer.class),
StandardRepeatablesTestCase.class, StandardContainer.class);
assertThat(values).isNull();
}
@@ -103,14 +103,14 @@ class RepeatableContainersTests {
@Test
void ofExplicitWhenContainerReturnsRepeats() {
Object[] values = findRepeatedAnnotationValues(
RepeatableContainers.of(ExplicitRepeatable.class, ExplicitContainer.class),
RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, ExplicitContainer.class),
ExplicitRepeatablesTestCase.class, ExplicitContainer.class);
assertThat(values).containsExactly("a", "b");
}
@Test
void ofExplicitWhenContainerIsNullDeducesContainer() {
Object[] values = findRepeatedAnnotationValues(RepeatableContainers.of(StandardRepeatable.class, null),
Object[] values = findRepeatedAnnotationValues(RepeatableContainers.explicitRepeatable(StandardRepeatable.class, null),
StandardRepeatablesTestCase.class, StandardContainer.class);
assertThat(values).containsExactly("a", "b");
}
@@ -118,7 +118,7 @@ class RepeatableContainersTests {
@Test
void ofExplicitWhenHasNoValueThrowsException() {
assertThatExceptionOfType(AnnotationConfigurationException.class)
.isThrownBy(() -> RepeatableContainers.of(ExplicitRepeatable.class, InvalidNoValue.class))
.isThrownBy(() -> RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, InvalidNoValue.class))
.withMessageContaining("Invalid declaration of container type [%s] for repeatable annotation [%s]",
InvalidNoValue.class.getName(), ExplicitRepeatable.class.getName());
}
@@ -126,7 +126,7 @@ class RepeatableContainersTests {
@Test
void ofExplicitWhenValueIsNotArrayThrowsException() {
assertThatExceptionOfType(AnnotationConfigurationException.class)
.isThrownBy(() -> RepeatableContainers.of(ExplicitRepeatable.class, InvalidNotArray.class))
.isThrownBy(() -> RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, InvalidNotArray.class))
.withMessage("Container type [%s] must declare a 'value' attribute for an array of type [%s]",
InvalidNotArray.class.getName(), ExplicitRepeatable.class.getName());
}
@@ -134,7 +134,7 @@ class RepeatableContainersTests {
@Test
void ofExplicitWhenValueIsArrayOfWrongTypeThrowsException() {
assertThatExceptionOfType(AnnotationConfigurationException.class)
.isThrownBy(() -> RepeatableContainers.of(ExplicitRepeatable.class, InvalidWrongArrayType.class))
.isThrownBy(() -> RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, InvalidWrongArrayType.class))
.withMessage("Container type [%s] must declare a 'value' attribute for an array of type [%s]",
InvalidWrongArrayType.class.getName(), ExplicitRepeatable.class.getName());
}
@@ -142,14 +142,14 @@ class RepeatableContainersTests {
@Test
void ofExplicitWhenAnnotationIsNullThrowsException() {
assertThatIllegalArgumentException()
.isThrownBy(() -> RepeatableContainers.of(null, null))
.isThrownBy(() -> RepeatableContainers.explicitRepeatable(null, null))
.withMessage("Repeatable must not be null");
}
@Test
void ofExplicitWhenContainerIsNullAndNotRepeatableThrowsException() {
assertThatIllegalArgumentException()
.isThrownBy(() -> RepeatableContainers.of(ExplicitRepeatable.class, null))
.isThrownBy(() -> RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, null))
.withMessage("Annotation type must be a repeatable annotation: failed to resolve container type for %s",
ExplicitRepeatable.class.getName());
}
@@ -159,7 +159,7 @@ class RepeatableContainersTests {
@Test
void standardAndExplicitReturnsRepeats() {
RepeatableContainers repeatableContainers = RepeatableContainers.standardRepeatables()
.and(ExplicitContainer.class, ExplicitRepeatable.class);
.plus(ExplicitRepeatable.class, ExplicitContainer.class);
assertThat(findRepeatedAnnotationValues(repeatableContainers, StandardRepeatablesTestCase.class, StandardContainer.class))
.containsExactly("a", "b");
assertThat(findRepeatedAnnotationValues(repeatableContainers, ExplicitRepeatablesTestCase.class, ExplicitContainer.class))
@@ -175,10 +175,10 @@ class RepeatableContainersTests {
@Test
void equalsAndHashcode() {
RepeatableContainers c1 = RepeatableContainers.of(ExplicitRepeatable.class, ExplicitContainer.class);
RepeatableContainers c2 = RepeatableContainers.of(ExplicitRepeatable.class, ExplicitContainer.class);
RepeatableContainers c1 = RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, ExplicitContainer.class);
RepeatableContainers c2 = RepeatableContainers.explicitRepeatable(ExplicitRepeatable.class, ExplicitContainer.class);
RepeatableContainers c3 = RepeatableContainers.standardRepeatables();
RepeatableContainers c4 = RepeatableContainers.standardRepeatables().and(ExplicitContainer.class, ExplicitRepeatable.class);
RepeatableContainers c4 = RepeatableContainers.standardRepeatables().plus(ExplicitRepeatable.class, ExplicitContainer.class);
assertThat(c1).hasSameHashCodeAs(c2);
assertThat(c1).isEqualTo(c1).isEqualTo(c2);
assertThat(c1).isNotEqualTo(c3).isNotEqualTo(c4);