Skip to content

Support repeatable annotations as composed annotations [SPR-13973] #18545

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

Closed
spring-projects-issues opened this issue Feb 23, 2016 · 11 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Feb 23, 2016

Phil Webb opened SPR-13973 and commented

Given the following annotations:

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public @interface MockBeans {

	MockBean[] value();
}
@Target({ ElementType.TYPE, ElementType.FIELD })
@Retention(RetentionPolicy.RUNTIME)
@Repeatable(MockBeans.class)
public @interface MockBean {

	@AliasFor("classToMock")
	Class<?> value() default Void.class;

	@AliasFor("value")
	Class<?> classToMock() default Void.class;
}
@Target({ ElementType.TYPE, ElementType.FIELD })
@Retention(RetentionPolicy.RUNTIME)
@MockBean
public @interface MyMockBean {

	@AliasFor(annotation = MockBean.class)
	Class<?> value();
}

And the following class:

@MyMockBean(ExampleService.class)
static class MetaMockBean {
}

Calling AnnotationUtils.getRepeatableAnnotations(MetaMockBean.class,MockBean.class, MockBeans.class) will return a synthesized annotation. However, calling value() on the result returns Void.class rather than the expected ExampleService.class.

Is this expected behavior?


Affects: 4.2.4

Issue Links:

Referenced from: commits 4836d06, 46e0484, 2ed3382, b0c6357, c6f6e19, 3597879, d572b02, 2535469, 4fa11e3, c6b1f38, 4742aa0, b6d9fd3

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Yes, I do believe this is the expected behavior.

Basically, none of the methods in AnnotationUtils support merging of annotation attributes within an annotation hierarchy.

Only AnnotatedElementUtils supports merging of attributes (i.e., composed annotations).

Regarding the (original) subject of this issue, getRepeatableAnnotations does actually support @AliasFor, however only within a single annotation.

In other words, we do not currently have any search algorithms that support merging of annotation attributes within an annotation hierarchy for repeatable annotations. So... if that's something you'd like to have support for, we could create another issue or repurpose this one.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 23, 2016

Juergen Hoeller commented

Sam Brannen, I was about to create such an issue myself, actually... since it seems to be the root cause of #18376. Definitely something to sort out for 4.3, I guess as a new variant of repeatable annotation lookup on AnnotatedElementUtils? Let's repurpose this issue for it.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Our main remaining use of getRepeatableAnnotations is for @JmsListener, @Scheduled and JMX ManagedNotification / ManagedOperationParameter lookup, as well as Sql lookup in the test context framework, all for very similar purposes. So requirement-wise, we just need a variant of the search algorithm that works for those.

That's probably the biggest gap that we have in our annotation composability story at this point, so worth some priority.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 25, 2016

Sam Brannen commented

Note that this issue is related to #18054 and #17490.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Juergen Hoeller, yes, this new functionality would have to reside in AnnotatedElementUtils, keeping in line with the documentation for AnnotationUtils:

Meta-annotation Support

... For support for meta-annotations with attribute overrides in composed annotations, use AnnotatedElementUtils instead.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Completed as described in GitHub commit 2535469:

Support repeatable annotations as composed annotations

Prior to this commit, AnnotationUtils supported searching for
repeatable annotations even if the repeatable annotation was declared
on a custom stereotype annotation. However, there was no support for
merging of attributes in composed repeatable annotations. In other
words, it was not possible for a custom annotation to override
attributes in a repeatable annotation.

This commit addresses this by introducing
findMergedRepeatableAnnotations() methods in AnnotatedElementUtils.
These new methods provide full support for explicit annotation
attribute overrides configured via @AliasFor (as well as
convention-based overrides) with "find semantics".

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Phil Webb, please give this a try in the next 4.3 snapshot and let me know if it meets your needs.

Cheers!

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Reopening in order to introduce getMergedRepeatableAnnotations(..) methods in AnnotatedElementUtils that search using get semantics.

This is needed, for example, in SqlScriptsTestExecutionListener where find semantics for repeatable annotations are not applicable.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Added support for "get" search semantics in 46e0484

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Juergen Hoeller,

Based on your comments above, I went ahead and migrated the lookups for @JmsListener, @Scheduled, @ManagedNotification, @ManagedOperationParameter, and @Sql to use the new AnnotatedElementUtils.getMergedRepeatableAnnotations() method.

I have also added tests for all of these, except for @ManagedOperationParameter.

When I started writing a test for @ManagedOperationParameter I realized that it can only be declared on methods and therefore cannot be used to create a composed annotation to begin with, which leads to the following questions:

  1. Do we want to allow @ManagedOperationParameter (and all related JMX annotations) to be used as meta-annotations?
  2. If not, should we revert to the old algorithm in AnnotationUtils for @ManagedOperationParameter lookups?

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Team Decisions

  1. Do we want to allow @ManagedOperationParameter (and all related JMX annotations) to be used as meta-annotations?
  2. If not, should we revert to the old algorithm in AnnotationUtils for @ManagedOperationParameter lookups?

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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants