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

Arc - implement CDI inheritance rules properly #41

Closed
mkouba opened this issue Oct 2, 2018 · 14 comments · Fixed by #19759
Closed

Arc - implement CDI inheritance rules properly #41

mkouba opened this issue Oct 2, 2018 · 14 comments · Fixed by #19759
Labels
area/arc Issue related to ARC (dependency injection)
Milestone

Comments

@mkouba
Copy link
Contributor

mkouba commented Oct 2, 2018

See also http://docs.jboss.org/cdi/spec/2.0/cdi-spec.html#inheritance.

@mkouba
Copy link
Contributor Author

mkouba commented Oct 2, 2018

Follows up #38

@mkouba mkouba self-assigned this Oct 2, 2018
@cescoffier
Copy link
Member

@mkouba isn't this done?

@mkouba
Copy link
Contributor Author

mkouba commented Sep 26, 2019

I'm not sure. @manovotn could you pls verify the relevant functionalities?

@manovotn
Copy link
Contributor

@cescoffier @mkouba
Well, Arc deliberately doesn't implement specialization so there's that.
Other than that, in type-level inheritance, we seem to be missing qualifier and stereotype inheritance while scope and interceptor bindings work.
Member-level inheritance seems to work fully.
I've written these tests to try that out - https://github.com/manovotn/quarkus/tree/inheritanceRules

@manovotn
Copy link
Contributor

FTR, CDI spec recommends that qualifiers should not declare @Inherited:

qualifier types should not be declared @inherited,

so that's definitely of minor priority.

As for stereotypes, those would be nice to have feature, but probably not mandatory right away.
It would require deep recursive search through annotations because stereotypes can declare another stereotypes...

@Ladicek
Copy link
Contributor

Ladicek commented Dec 1, 2020

I can confirm that stereotype inheritance doesn't work, as well as transitive stereotypes (stereotype declared on another stereotype). I added some @Disabled tests for these in a mostly-unrelated PR (#13553).

@dufoli
Copy link
Contributor

dufoli commented Aug 29, 2021

so I have start to work on it but I am stuck because jandex seems to not return annotation of superclass got from classInfo.
I am maybe thinking to change completly the code and init all BeanInfo then enriched with qualifer/stereotype inherited
wdyt ? Or there is a solution to
@mkouba
Btw, I copy past the test from @manovotn but I will try to do my own later and add other test.

@cescoffier
Copy link
Member

CC @Ladicek - is it something doable?

@mkouba
Copy link
Contributor Author

mkouba commented Aug 30, 2021

I am stuck because jandex seems to not return annotation of superclass got from classInfo.

That's intentional and well-documented. You need to walk the class hiearchy instead.

@Ladicek
Copy link
Contributor

Ladicek commented Aug 30, 2021

@mkouba Considering recent developments in CDI Lite and extensions, I think the annotation store in ArC should probably always include all annotations of a class (including inherited ones). This is a little more complicated for scope annotations, and I didn't really think it through yet, so this is more like a heads up than a definitive claim :-)

@mkouba
Copy link
Contributor Author

mkouba commented Aug 30, 2021

I think the annotation store in ArC should probably always include all annotations of a class (including inherited ones)

I assume that you mean the annotations declared on superclasses and annotated with @Inherited?

@Ladicek
Copy link
Contributor

Ladicek commented Aug 30, 2021

Yes.

@manovotn
Copy link
Contributor

@Ladicek might be worth an issue with some details so we keep track of it?

@Ladicek
Copy link
Contributor

Ladicek commented Aug 30, 2021

At some point, sure. I need to think more about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants