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

BeanContainer#getInjectableReference should probably be on BeanManager instead #568

Closed
manovotn opened this issue Dec 1, 2021 · 2 comments · Fixed by #574
Closed

BeanContainer#getInjectableReference should probably be on BeanManager instead #568

manovotn opened this issue Dec 1, 2021 · 2 comments · Fixed by #574
Assignees
Labels
Lite Related to CDI Lite

Comments

@manovotn
Copy link
Contributor

manovotn commented Dec 1, 2021

BeanContainer API currently houses getInjectableReference(InjectionPoint ij, CreationalContext<?> ctx) which was originally on BeanManager.

After some discussion with @Ladicek we arrived at the conclusion that this doesn't make much sense.
In Lite, you don't have access (via BeanContainer) to programmatic creation of InjectionPoint because that's based off the former metadata model (AnnotatedType and friend). You cannot store them from extensions either so your means of getting hold of them are very limited. Possibly only to creating a class directly implementing an IP which, again, requires usage of the Annotated* metamodel.

Therefore, we should move this method back to BeanManager.
Note that recently we changed the wording so that implementations in Lite are allowed to support BeanManager in some limited form as well meaning that if this functionality turns out to be needed, it can still be implemented without breaking spec compatibility.

@Ladicek
Copy link
Contributor

Ladicek commented Dec 1, 2021

Technically, we don't "allow Lite implementations to support BeanManager in some limited form". We require Lite implementations to provide an implementation of BeanManager when they're asked for it, but invoking any method that doesn't come from BeanContainer is non-portable.

@Ladicek Ladicek added the Lite Related to CDI Lite label Dec 1, 2021
@manovotn
Copy link
Contributor Author

manovotn commented Dec 1, 2021

Technically, we don't "allow Lite implementations to support BeanManager in some limited form".

True, I was to lazy to search for exact wording in the spec :)

@manovotn manovotn self-assigned this Dec 7, 2021
Ladicek added a commit to Ladicek/cdi-spec that referenced this issue May 31, 2023
… Full

We previously [1] moved the `getInjectableReference()` from the `BeanContainer`
interface to the `BeanManager` [2], but did not update the specification text
accordingly. Thus, the method is specified in the CDI Lite part of the spec,
but it in fact belongs to CDI Full. This commit moves the spec text around
to fix that.

Additionally, this commit removes the declaration of `resolveObserverMethods()`
from `BeanManager`, because that just overrides the same declaration that is
present on `BeanContainer`.

Finally, this commit moves the declaration of `getInjectableReference()` from
the end of the `BeanManager` source code to the beginning, for symmetry with
the `BeanContainer` declaration and to restore the previous layout.

[1] jakartaee@520bad1
[2] jakartaee#568
Ladicek added a commit that referenced this issue May 31, 2023
… Full

We previously [1] moved the `getInjectableReference()` from the `BeanContainer`
interface to the `BeanManager` [2], but did not update the specification text
accordingly. Thus, the method is specified in the CDI Lite part of the spec,
but it in fact belongs to CDI Full. This commit moves the spec text around
to fix that.

Additionally, this commit removes the declaration of `resolveObserverMethods()`
from `BeanManager`, because that just overrides the same declaration that is
present on `BeanContainer`.

Finally, this commit moves the declaration of `getInjectableReference()` from
the end of the `BeanManager` source code to the beginning, for symmetry with
the `BeanContainer` declaration and to restore the previous layout.

[1] 520bad1
[2] #568
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lite Related to CDI Lite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants