eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
162 stars 110 forks source link

CDI bean attribute `beanClass` is not used correctly in CDI extension and producers #5157

Closed manovotn closed 1 year ago

manovotn commented 2 years ago

Describe the bug

Mojarra registers a lot of synthetic CDI beans via a CDI extension and for all of them it sets a beanClass attribute. Setting this is valid operation but the values used don't make much sense. Currently, the extension uses Object as a basis and each producer then sets their own class - here is an example.

Now, CDI spec is not very helpful in telling users what beanClass stands for and its javadoc only says what it is in case of class-based beans and producer methods. In reality, the beanClass attribute is used (by Weld, the reference/compatible impl) to determine where to instantiate a proxy for any of these beans. This, based on environment, means by which class loader or in which Java module. For this reason, Weld defaults beanClass of all synthetic beans to the extension class that registered them.

EDIT: _After some more digging in the CDI spec, i should correct myself in that it is clearer what beanClass should stand for in case of synth beans. There is a section that mentions this and points out the importance of bean class in determining inter-module injection. In other words, this controls if the bean is available for injection in certain module or not._

This was recently noticed in WildFly, where synth beans from Mojarra were registered under a different module.

Expected behavior

Synth beans should have a beanClass attribute of the CDI extension that registers them or at least the class of the producer bean that it uses.

Just for the sake of clarity I will also mention that beanClass does not anyhow affect bean types and the resolution of the bean.

BalusC commented 1 year ago

Merged into 4.0.1

struberg commented 3 months ago

Folks, there is no evidence in the CDI spec to support this claim. Especially the section 3.5 indicates the contrary:

"3.5. Bean constructors When the container instantiates a bean class, it calls the bean constructor. The bean constructor is a default-access, public, protected or private constructor of the bean class."

This means that the 'bean class' is rather the class of the returned type of a custom bean. In OpenWebBeans we use Bean#getBeanClass() to detect the type of the proxy which should be used. Weld tries to guess which proxy type to use from the list of classes returned in Bean#getTypes(). But this is just a best guess and in edge cases is not deterministic. This patch breaks OpenWebBeans and is not backed by the CDI spec. So I ask to revert to the old handling. Thanks!

arjantijms commented 3 months ago

@manovotn WDYT?

manovotn commented 3 months ago

@arjantijms I disagree but let's not discuss it in several threads simultaneously; I will leave a comment in the new issue.