-
Notifications
You must be signed in to change notification settings - Fork 77
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
Allow @Enhancement to not restrict the types based on annotations #566
Allow @Enhancement to not restrict the types based on annotations #566
Conversation
In case an unannotated type is added through `@Discovery`, such type previously couldn't be transformed during `@Enhancement`. With this commit, it is possible to specify that the set of types considered for `@Enhancement` is not restricted based on annotations in any way. To do that, the `@Enhancement#withAnnotations` member should be set to an empty array.
Previously, the default value of `@Enhancement#withAnnotations` was the `BeanDefiningAnnotations.class` marker type. To align with Portable Extensions, which don't have any annotation restriction by default, this commit changes the default value to an empty array.
It should be noted somewhere that if a bean defining annotation is not specified then then enhancement applies to all types discovered via annotations or added via the discovery phase, but not every type in the application |
I think that directly follows from the |
Added a commit with clarifications of type discovery / bean discovery. |
The terms "type discovery" and "bean discovery" are sometimes used interchangeably, though they really shouldn't. This commit improves the usage of these terms in `@Discovery` and `@Enhancement`. In particular, the `@Enhancement` wording is changed to refer to "type discovery" instead of "bean discovery", because bean discovery happens _after_ `@Enhancement`.
3cf3043
to
560f832
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks @Ladicek!
* Marker annotation type that, for the purpose of {@link Enhancement#withAnnotations()}, | ||
* represents set of bean defining annotations after the {@link Discovery @Discovery} | ||
* phase is finished. That is, it includes custom normal scope annotations as well as | ||
* custom stereotypes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ladicek I have been thinking about this some more and I am not sure if BeanDefiningAnnotations
makes much sense with latest change (i.e. if it is justifiable to have that coded into spec).
Setting aside that I am not sure how well can we filter out classes with just custom stereotypes (which I will try to figure tomorrow), in what case would you want to filter for these classes? With annotated mode, this basically translates to "all classes". The only ones which are potentially omitted are those added by build compatible extensions that that don't have any annotation on them. That, to me, seems like a weird filter but maybe I am just not seeing the use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to indicate document that in annotated mode only classes with at least a bean definition annotation are processed. Having said that there is no technical reason for needing the annotation itself and maybe it is just documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to think about this for a while, but @manovotn is right. During type discovery, we discover classes with a BDA and classes added via extension in @Discovery
. Classes added via extension automatically get @Dependent
if they don't have a BDA already. So saying "only process classes that were discovered during type discovery and have a BDA" is basically equivalent to "only process classes that were discovered during type discovery".
I'll submit a PR removing this marker.
Fixes #564
I intentionally split this into 2 commits:
@Enhancement(withAnnotations = {})
mean "no restriction on annotations". That is, types without annotations added through@Discovery
may be subject to@Enhancement
. The default is stillBeanDefiningAnnotations.class
.That's because I'm pretty sure we'd have a consensus on item 1, but not necessarily on item 2.