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

@Enhancement(withAnnotations = ...) restricts usability of non-annotated classes added via @Discovery #564

Closed
Ladicek opened this issue Nov 25, 2021 · 3 comments · Fixed by #566
Labels
Lite Related to CDI Lite lite-extension-api Issues related to CDI Lite extension API proposal

Comments

@Ladicek
Copy link
Contributor

Ladicek commented Nov 25, 2021

We (actually @manovotn) just found that the @Enhancement annotation's withAnnotations member restricts how non-annotated classes may be used. The way withAnnotations is currently specified, it expects that all classes processed via @Enhancement have some annotation (preferrably a bean defining annotation, but withAnnotations = Annotation.class means any annotation is enough). That constraint exists for annotation processors.

That seems entirely reasonable for classes that are subject to normal bean discovery, because CDI Lite only has annotated discovery. However, for classes that are added to bean discovery through @Discovery, maybe that is too strict? What if I want to add a non-annotated class through @Discovery and then give it a bean defining annotation in @Enhancement?

@graemerocher do you think that would be possible in an annotation processor?

If so, I can see several options:

  • add yet another marker annotation similar to Enhancement.BeanDefiningAnnotations, something like Enhancement.NoneRequired (this is still constrained to discovered classes, as currently defined, no change on that front);
  • make withAnnotations = {} mean "any classes, even classes without annotations" (this is still constrained to discovered classes, as currently defined, no change on that front);
  • some other way to express this?

We may also want to change the default. Currently, withAnnotations defaults to BeanDefiningAnnotations.class, which makes sense in my opinion, but diverges from what Portable Extensions do (there, there's no annotation constraint by default).

If not, then ... well ... maybe we could make ScannedClasses.add() return a ClassConfig to be able to add annotations in the @Discovery phase, before @Enhancement? (I'd prefer not, but if it would be the only way, then it's better than nothing.)

If not even that, then I don't know and we have a problem :-)

@Ladicek Ladicek added lite-extension-api Issues related to CDI Lite extension API proposal Lite Related to CDI Lite labels Nov 25, 2021
@manovotn
Copy link
Contributor

However, for classes that are added to bean discovery through @discovery, maybe that is too strict?

I agree, it is perfectly fine to register a class without annotation (or possibly with annotation that is not bean defining; be it qualifier or an annotation that isn't CDI-related at all). In fact, it is pretty common case of turning 3rd party classes into beans.

... and then give it a bean defining annotation in @enhancement?

Here I'd like to point out that such a class doesn't even need to have any @Enhancement linked to it. It is a valid bean even without annotation because CDI will automatically give it @Dependent scope.

We may also want to change the default. Currently, withAnnotations defaults to BeanDefiningAnnotations.class, which makes sense in my opinion, but diverges from what Portable Extensions do (there, there's no annotation constraint by default).

BeanDefiningAnnotations.class makes sense for all normally discovered classes in Lite. It doesn't make much sense if someone uses Lite extension in CDI Full (which is allowed). If no-annotation default policy wouldn't work for MN, then we could go with Annotation.class to broaden it. It will still differ but might be closer to what portable extension do.

Basically, at the very least, we need to be able to add non-annotated classes in @Discovery and have them processed in @Enhancement.

@graemerocher
Copy link
Contributor

it is fine to add a type without an annotation using bean discovery because typically those beans get associated with some other annotation. I am fine with lifting this limitation for beans added during discovery.

@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 26, 2021

Thanks! I'll put a proposal together and submit a PR for discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lite Related to CDI Lite lite-extension-api Issues related to CDI Lite extension API proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants