Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AnnotatedElementUtils.getMergedRepeatableAnnotations does not return meta annotations on repeatedly-annotated class #26188

Closed
mischkes opened this issue Dec 1, 2020 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue

Comments

@mischkes
Copy link

mischkes commented Dec 1, 2020

  • We use a meta-annotation @As400Sql for the annotation @Sql. It is repeatable.
  • We annotate DoubleAnnotatedClass.class with two such annotations.
  • AnnotatedElementUtils.getMergedRepeatableAnnotations() does then not retreive the @Sql annotations.
  • When using an intermediate annotation (@TwoSql) it works as expected.

Observed in spring-core 5.2.8.RELEASE via spring-boot 2.3.3.RELEASE

public class As400SqlTest {

  @Target({ElementType.TYPE, ElementType.METHOD})
  @Retention(RetentionPolicy.RUNTIME)
  @Repeatable(As400SqlGroup.class)
  @Sql(config = @SqlConfig(dataSource = "as400DataSource", transactionManager =
      "as400TransactionManager"))
  public @interface As400Sql {

    @AliasFor(attribute = "scripts", annotation = Sql.class)
    String[] value() default {};
  }

  @Target({ElementType.TYPE, ElementType.METHOD})
  @Retention(RetentionPolicy.RUNTIME)
  public @interface As400SqlGroup {

    As400Sql[] value();
  }



  @As400Sql("script1")
  public static class AnnotatedClass {

  }

  @Test
  void getMergedRepeatableAnnotations_shouldReturnSql_forClassWithAs400SqlAnnotation() {
    Set<Sql> actual = AnnotatedElementUtils
        .getMergedRepeatableAnnotations(AnnotatedClass.class, Sql.class, SqlGroup.class);

    assertThat(actual).hasSize(1);
    Sql actual0 = actual.iterator().next();
    assertThat(actual0.scripts()).containsExactly("script1");
    assertThat(actual0.config().dataSource()).isEqualTo("as400DataSource");
  }


  @As400Sql("script1")
  @As400Sql("script2")
  public static class DoubleAnnotatedClass {

  }

  @Disabled("getMergedRepeatableAnnotations returns 0 annotations - why?")
  @Test
  void getMergedRepeatableAnnotations_shouldReturnTwoSql_forDoubleAnnotatedClass() {
    Set<Sql> actual = AnnotatedElementUtils
        .getMergedRepeatableAnnotations(DoubleAnnotatedClass.class, Sql.class, SqlGroup.class);

    assertThat(actual).hasSize(2);
    Iterator<Sql> actualElements = actual.iterator();
    assertThat(actualElements.next().config().dataSource()).isEqualTo("as400DataSource");
    assertThat(actualElements.next().config().dataSource()).isEqualTo("as400DataSource");
  }


  @As400Sql("script1")
  @As400Sql("script2")
  @Retention(RetentionPolicy.RUNTIME)
  public @interface TwoSql {

  }

  @TwoSql
  public static class MetaDoubleAnnotatedClass {

  }

  @Test
  void getMergedRepeatableAnnotations_shouldReturnTwoSql_forMetaDoubleAnnotatedClass() {
    Set<Sql> actual = AnnotatedElementUtils
        .getMergedRepeatableAnnotations(MetaDoubleAnnotatedClass.class, Sql.class, SqlGroup.class);

    assertThat(actual).hasSize(2);
    Iterator<Sql> actualElements = actual.iterator();
    assertThat(actualElements.next().config().dataSource()).isEqualTo("as400DataSource");
    assertThat(actualElements.next().config().dataSource()).isEqualTo("as400DataSource");
  }
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 1, 2020
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 10, 2021
@schaa0
Copy link

schaa0 commented Sep 26, 2022

Additional information about this issue:

Method overloads of AnnotatedElementUtils.getMergedRepeatableAnnotations() use two different types of RepeatableContainers:

  • ExplicitRepeatableContainers
  • StandardRepeatableContainers

ExplicitRepeatableContainers is used to extract repeatable annotations from the annotated element based on the provided container type. Since the provided container type is SqlGroup but multiple As400Sql annotations are composed into As400SqlGroup, the types don't match and the As400Sql annotations on DoubleAnnotatedClass are skipped.
This happens in the failing test case getMergedRepeatableAnnotations_shouldReturnTwoSql_forDoubleAnnotatedClass.

Once the annotations are determined from the annotated element, further processing of the annotation hierarchy
will use StandardRepeatableContainers which extracts repeatable annotations from any container type which are found as meta annotation.
That's why test case getMergedRepeatableAnnotations_shouldReturnTwoSql_forMetaDoubleAnnotatedClass passes.

Using StandardRepeatableContainers for both steps gives the expected result, but requires modification in AnnotatedElementUtils:

private static MergedAnnotations getRepeatableAnnotations(AnnotatedElement element, @Nullable Class<? extends Annotation> containerType, Class<? extends Annotation> annotationType) {
    // Instead of: RepeatableContainers.of(annotationType, containerType);
    RepeatableContainers repeatableContainers = RepeatableContainers.standardRepeatables();
    return MergedAnnotations.from(element, SearchStrategy.INHERITED_ANNOTATIONS, repeatableContainers);
}

As an alternative, it's possible to define a separate utility method since the required API components are public. The following is an example using the get semantics.

class AnnotatedElementUtilsExt {

    static <A extends Annotation> Set<A> getAllMergedRepeatableAnnotations(AnnotatedElement element, Class<A> annotationType) {
        assertIsRepeatable(annotationType);
        return MergedAnnotations.from(element, MergedAnnotations.SearchStrategy.INHERITED_ANNOTATIONS, RepeatableContainers.standardRepeatables())
                .stream(annotationType)
                .sorted(highAggregateIndexesFirst())
                .collect(MergedAnnotationCollectors.toAnnotationSet());
    }

    private static <A extends Annotation> void assertIsRepeatable(Class<A> annotationType) {
        boolean isRepeatable = annotationType.isAnnotationPresent(Repeatable.class);
        Assert.isTrue(isRepeatable, () -> "Annotation type must be a repeatable annotation: " +
                "failed to resolve container type for " + annotationType.getName());
    }

    private static <A extends Annotation> Comparator<MergedAnnotation<A>> highAggregateIndexesFirst() {
        return Comparator.<MergedAnnotation<A>> comparingInt(MergedAnnotation::getAggregateIndex).reversed();
    }
}

@sbrannen sbrannen self-assigned this Dec 14, 2023
@sbrannen
Copy link
Member

Hi @mischkes,

Sorry for taking such a long time to get to this issue.

The good news is that this has been resolved in the interim in conjunction with #20279.

Starting with Spring Framework 5.3.24, to get getMergedRepeatableAnnotations_shouldReturnTwoSql_forDoubleAnnotatedClass() to pass, you need to invoke getMergedRepeatableAnnotations() without specifying the containerType as follows.

Set<Sql> actual = 
        AnnotatedElementUtils.getMergedRepeatableAnnotations(DoubleAnnotatedClass.class, Sql.class);

I am therefore closing this as a:

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Dec 14, 2023
@sbrannen sbrannen added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

5 participants