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

CdiExtension incorrectly uses beanClass as means to determine implementation class of a bean #5159

Closed
manovotn opened this issue Oct 20, 2022 · 3 comments
Assignees
Labels

Comments

@manovotn
Copy link
Contributor

Describe the bug

In the current state of CdiExtension, it uses Bean#getBeanClass() to derive the impl class of a bean. See this code.
This is wrong because there can be three types of beans and this only works as expected with one of them:

  • Class-based beans (managed beans) - this works because beanClass is equal to the impl class of the bean
  • Producers - beanClass equals to the class of the bean declaring the producer which can be a completely different bean
  • Synthetic beans - spec doesn't say what exactly is beanClass in this case and users can set arbitrary values. Weld defaults to the class of the CDI extension that declared the bean. Nonetheless, user can pick any class here (as seen in CDI bean attribute beanClass is not used correctly in CDI extension and producers #5157)

I am far from being expert in Mojarra, so maybe this is fine and the bean will always be a managed bean; I'll let someone else decide that. I just wanted to point this out since I noticed it. It could be that this is perfectly fine.

I am also not sure what would be a good replacement because being able to derive the exact impl class (if that's the requirement here?) from a bean is beyond CDI extension capabilities simply because you can either (a) limit bean types and (b) with producers you can choose to instantiate one of multiple subclasses during runtime and CDI cannot know which.

@BalusC
Copy link
Contributor

BalusC commented May 20, 2023

@arjantijms can you take a look at it?

@struberg
Copy link
Member

struberg commented Jul 8, 2024

-1

See my comment in #5157 (comment)

There is nothing in the spec backing this claim, and it breaks OpenWebBeans and TomEE.

@BalusC
Copy link
Contributor

BalusC commented Sep 3, 2024

Fixed via #5457

@BalusC BalusC closed this as completed Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants