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 does not find merged repeatable annotations on other repeatable annotations #20279

Closed
spring-projects-issues opened this issue Jul 1, 2017 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 1, 2017

Liam Bryan opened SPR-15723 and commented

Meta annotations are not detected when present on Repeatable annotations.

Provided zip contains a quick example highlighting this, but for a contained example here (@Retention and @Target annotations omitted):

@Repeatable(A.List.class)
@interface A {

    int value() default 1;
    
    @interface List {
        A[] value();
    }

}
@Repeatable(B.List.class)
@A
@interface B {

    @AliasFor(annotation = A.class, attribute = "value")
    int value();
    
    @interface List {
        B[] value();
    }

}

None of the provided methods in AnnotationUtils or AnnotatedElementUtils can locate the meta-@A annotations for an element with repeated @B annotations.


Affects: 4.3.8

Attachments:

@spring-projects-issues spring-projects-issues added type: bug A general bug status: waiting-for-triage An issue we've not yet triaged or decided on in: core Issues in core modules (aop, beans, core, context, expression) and removed type: bug A general bug labels Jan 11, 2019
@sbrannen
Copy link
Member

Thanks for raising the issue, and I apologize that we took so long to triage it.

I was able to reproduce this against master with the following all-in-one test class.

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.lang.annotation.ElementType;
import java.lang.annotation.Repeatable;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Method;
import java.util.List;
import java.util.Set;

import org.junit.jupiter.api.Test;
import org.springframework.core.annotation.AliasFor;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.core.annotation.AnnotationUtils;

@SuppressWarnings("unused")
class AnnotationRetrievalTest {

	@Test
	void findAnnotationsSingle() throws Exception {
		Method singleAnnotatedMethod = getClass().getDeclaredMethod("singleAnnotatedMethod");

		// Passes.
		performTest(singleAnnotatedMethod, 1);
	}

	@Test
	void findAnnotationsMulti() throws Exception {
		Method multiAnnotatedMethod = getClass().getDeclaredMethod("multiAnnotatedMethod");

		// Fails (for all 3 sub-assertions).
		performTest(multiAnnotatedMethod, 2);
	}

	private void performTest(Method method, int expectedAnnotationCount) {
		Set<A> fromFindMergedRepeatable = AnnotatedElementUtils.findMergedRepeatableAnnotations(method, A.class);
		Set<A> fromFindMergedRepeatableWithContainer = AnnotatedElementUtils.findMergedRepeatableAnnotations(method,
				A.class, A.Container.class);
		Set<A> fromGetRepeatable = AnnotationUtils.getRepeatableAnnotations(method, A.class);
		List<A> fromJUnitFindRepeatable = org.junit.platform.commons.util.AnnotationUtils
				.findRepeatableAnnotations(method, A.class);

		assertAll(() -> assertEquals(expectedAnnotationCount, fromFindMergedRepeatable.size()),
				() -> assertEquals(expectedAnnotationCount, fromFindMergedRepeatableWithContainer.size()),
				() -> assertEquals(expectedAnnotationCount, fromGetRepeatable.size()),
				() -> assertEquals(expectedAnnotationCount, fromJUnitFindRepeatable.size()));
	}

	@Retention(RetentionPolicy.RUNTIME)
	@Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE })
	@Repeatable(A.Container.class)
	public @interface A {

		int value() default 0;

		@Retention(RetentionPolicy.RUNTIME)
		@Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE })
		@interface Container {
			A[] value();
		}
	}

	@Retention(RetentionPolicy.RUNTIME)
	@Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE })
	@Repeatable(B.Container.class)
	@A
	public @interface B {

		@AliasFor(annotation = A.class)
		int value();

		@Retention(RetentionPolicy.RUNTIME)
		@Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE })
		@interface Container {
			B[] value();
		}
	}

	@B(5)
	void singleAnnotatedMethod() {
	}

	@B(5)
	@B(10)
	void multiAnnotatedMethod() {
	}

}

Interestingly, the supplied example.zip was testing org.junit.platform.commons.util.AnnotationUtils.findRepeatableAnnotations from JUnit 5 instead of org.springframework.core.annotation.AnnotationUtils.getRepeatableAnnotations from Spring. In any case, the error is the same. Neither Spring nor JUnit 5 find the repeatable annotation for the "multi" scenario.

@philwebb and @jhoeller, do you think we should try to add support for this scenario?

@sbrannen sbrannen added for: team-attention and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 18, 2020
@sbrannen
Copy link
Member

sbrannen commented Aug 18, 2020

Actually, after having put further thought into it, the expectation for AnnotationUtils.getRepeatableAnnotations (as well as for JUnit's similar method) is invalid.

getRepeatableAnnotations does not merge annotation attributes. Thus, the algorithm may encounter @A twice, but each encounter is seen as the same instance. The @A annotation is therefore only found once. Consequently, the expectation should be 1 instead of 2 for the non-merging search algorithms.

@sbrannen sbrannen added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 18, 2021
@rstoyanchev rstoyanchev self-assigned this Oct 11, 2022
@sbrannen sbrannen assigned sbrannen and unassigned rstoyanchev Oct 11, 2022
@sbrannen sbrannen added type: bug A general bug and removed for: team-attention status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 11, 2022
@sbrannen sbrannen added this to the 5.3.24 milestone Oct 11, 2022
@sbrannen sbrannen changed the title Repeated meta-annotations not locatable on repeated annotation. [SPR-15723] AnnotatedElementUtils.findMergedRepeatableAnnotations() does not support nested repeatable annotation types Oct 11, 2022
@sbrannen
Copy link
Member

Reopening to address the same issue in AnnotatedElementUtils.getMergedRepeatableAnnotations().

@sbrannen sbrannen reopened this Oct 11, 2022
@sbrannen sbrannen changed the title AnnotatedElementUtils.findMergedRepeatableAnnotations() does not support nested repeatable annotation types AnnotatedElementUtils does not find merged repeatable annotations on other repeatable annotations Oct 11, 2022
@sbrannen
Copy link
Member

It turns out that we already had support for finding repeatable annotations used as meta-annotations on other repeatable annotations.

This is possible via the MergedAnnotations API.

However, the way that RepeatableContainers were previously configured in AnnotatedElementUtils for the getMergedRepeatableAnnotations() and findMergedRepeatableAnnotations() methods effectively limited the annotation search in a way that it only supported one type of repeatable annotation.

I fixed this in 828f74f and 9876701.

See the commit messages as well as the associated tests for details.

sbrannen added a commit to sbrannen/spring-framework that referenced this issue Oct 11, 2022
sbrannen added a commit that referenced this issue Dec 13, 2022
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 gh-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.

See gh-29685
Closes gh-29686
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Dec 13, 2022
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
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) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants