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

Add "addBeanDefiningAnnotation" method #827

Open
arjantijms opened this issue Sep 6, 2024 · 7 comments
Open

Add "addBeanDefiningAnnotation" method #827

arjantijms opened this issue Sep 6, 2024 · 7 comments

Comments

@arjantijms
Copy link

A library (possibly another spec in Jakarta EE) can currently add custom bean defining annotations using MetaAnnotations.addStereotype. This is however quite ugly and just uses the side-effect that stereo types are bean defining.

Implementations like Weld have an explicit method for this. See
https://docs.jboss.org/weld/reference/latest/en-US/html/environments.html#_extending_bean_defining_annotations

I would like to propose adding a method so libraries can add their own custom bean defining annotations, e.g. using MetaAnnotations.addBeanDefiningAnnotation.

@Ladicek
Copy link
Contributor

Ladicek commented Sep 6, 2024

I honestly don't think this is necessary, it would be just an alias for addStereotype():

default ClassConfig addBeanDefiningAnnotation(Class<? extends Annotation> annotation) {
    // stereotypes are bean defining annotations
    return addStereotype(annotation);
}

I'm also fairly sure no such API exists in Portable Extensions (looking at BeforeBeanDiscovery).

@manovotn
Copy link
Contributor

manovotn commented Sep 6, 2024

I honestly don't think this is necessary, it would be just an alias for addStereotype()

That's a fair point, I agree it doesn't seem very helpful.

Implementations like Weld have an explicit method for this. See
https://docs.jboss.org/weld/reference/latest/en-US/html/environments.html#_extending_bean_defining_annotations

Note that Weld has this for SE environment only.
I recall we tried to do this in EE as well but there were issues with it.

What I mean is that there is a subtle distinction between:

  1. Adding a bean defining annotation based on which you determine if an archive is a bean archive
  2. Adding a bean defining annotation into an archive you already know to be a bean archive

Namely in WFLY, there were/are use cases where not all deployments are CDI-enabled allowing WFLY to skip Weld bootstrap altogether. But for that, you need to be able to scan (search Jandex index) the deployment for known CDI bean defining annotations before you ask Weld to process CDI extensions.
Hence any programmatically added bean defining annotation discovered during CDI container bootstrap would not be taken into account as that would simply happen too late.

A few links relevant links that I was able to dig up:

@arjantijms
Copy link
Author

That's a fair point, I agree it doesn't seem very helpful.

Why not? addStereotype() doesn't seem to be well know for essentially being a "trick" to make an annotation bean defining. In Jakarta EE we have made changes before for even just URL vs Url for clarity.

At the least it should maybe be added or explained, and officially supported that addStereotype() is THE method for libraries that need to add their own bean defining annotations.

@manovotn
Copy link
Contributor

manovotn commented Sep 6, 2024

Personally, I don't see general lack of knowledge as a justification to duplicate methods in API. If users (perhaps rather extension/framework authors) do not know addStereotype(), they might not know newly added method either. Soon you start digging for how bean defining annotations work, you should find out about stereotypes pretty fast.
It is just my take on things though, if we have others voting for new method, we could do that.

That being said, I agree documenting it is a good idea 👍

@arjantijms
Copy link
Author

addStereotype(), they might not know newly added method either

I don't know. If a method is called sellBook(), would you also expect it to startEngine()?

As a developer you look for X, but then you need to invoke a method Y. I mean, conceptually creating a stereotype is something different from adding a bean defining annotation is it not?

Otherwise, why did you choose the name Weld.addBeanDefiningAnnotations(Class<? extends Annotation>…​ annotations) and not call it Weld.addStereoType(Class<? extends Annotation>…​ annotations)?

@arjantijms
Copy link
Author

Btw, the stereo type description also doesn't say that stereo types are specifically or primarily for making random annotations bean defining:

"
In many systems, use of architectural patterns produces a set of recurring bean roles. A stereotype allows a framework developer to identify such a role and declare some common metadata for beans with that role in a central place.

A stereotype encapsulates any combination of:

a default scope, and

a set of interceptor bindings.

A stereotype may also specify that:

all beans with the stereotype have defaulted bean names, or that

all beans with the stereotype are alternatives.

A bean may declare zero, one or multiple stereotypes. Stereotype annotations may be applied to a bean class or producer method or field.
"

@Azquelt
Copy link
Contributor

Azquelt commented Sep 10, 2024

Yeah, I agree that we should state that a stereotype is a bean defining annotation in that description. It's sort of implied by the fact you can set a default scope, but it would be helpful to spell it out.

However, I don't think that using a stereotype to add a bean defining annotation is a hack, based on that description:

In many systems, use of architectural patterns produces a set of recurring bean roles. A stereotype allows a framework developer to identify such a role and declare some common metadata for beans with that role in a central place.

What reason does a framework have for adding a bean defining annotation if not to add a scope or to identify a special role for certain beans? (There may be some, but those seem like the two obvious reasons.)

What wasn't really obvious to me when I first read this description was:

  1. applying a stereotype to a class is sufficient to make it a bean
  2. the stereotype annotation would be used by the framework to do more than just add default metadata to the bean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants