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

Programmatic lookup improvements #521

Closed
mkouba opened this issue Sep 10, 2021 · 6 comments · Fixed by #534
Closed

Programmatic lookup improvements #521

mkouba opened this issue Sep 10, 2021 · 6 comments · Fixed by #534
Assignees
Labels
spec-feature An issue requesting an addition specification

Comments

@mkouba
Copy link
Contributor

mkouba commented Sep 10, 2021

Both the reference implementation (Weld) and ArC have an enhanced version of javax.enterprise.inject.Instance that provide features that might be worth adding to the spec.

Enhancement Description
Instance#getHandler() (name TBC) Allows the client to obtain a "contextual reference handler" which not only holds the contextual reference but also makes it possible to inspect the metadata (i.e. Bean<?>) of the relevant bean and to destroy the underlying contextual instance.
Instance#handlers() (name TBC) Returns an Iterable of handlers described above.
select() method that accepts java.lang.reflect.Type This allows for generic selection of instances which can be handy while dealing with third party beans through extensions. However, it's not type-safe.
Caching the result of Instance#get() See https://quarkus.io/guides/cdi-reference#caching-the-result-of-programmatic-lookup

See also Weld docs about WeldInstance - Enhanced version of jakarta.enterprise.inject.Instance.

@manovotn
Copy link
Contributor

I am especially +1 for getHandler and handlers() as that allows you to iterate over bean metadata without creating bean instances.

The latter two I'd personally keep outside of spec, but if people want it, we can add it as well. I just haven't seen such demand and/or usage of those two.

@Ladicek
Copy link
Contributor

Ladicek commented Sep 13, 2021

OK so the Handler (I think Quarkus calls it Handle? That's a little nicer IMHO) allows:

  • creating the bean instance (contextual reference, if you must :-) );
  • obtaining the bean metadata (Bean);
  • destroying the bean instance.

Out of these 3 operations, 2 of them are already supported by Instance: you can create the bean instance by Instance.get and Instance.iterator, and you can destroy the bean instance by Instance.destroy.

I see you can get bean metadata from BeanManager, but creating the bean instance is ugly in such case. So yea, after looking at the current API, I think adding something like Handle is nice, as it lets you avoid the nasty low-level API on BeanManager and let you do everything through Instance.

I guess it would have been possible to design Instance differently to allow metadata access, but since that wasn't done and we certainly don't want to do breaking changes on that part of the API, something like a Handle seems necessary. So +1 from me.

@manovotn
Copy link
Contributor

Out of these 3 operations, 2 of them are already supported by Instance: you can create the bean instance by Instance.get and Instance.iterator, and you can destroy the bean instance by Instance.destroy.

The main point of Handle is really the ability to browse beans of certain type before creating their instances. The fact that you can perform get and destroy from there is just a convenience.

I guess it would have been possible to design Instance differently to allow metadata access,

Yea, but like you say, that's breaking. especially because current spec wording requires that iteration over Instance works with contextual references. This is captured in Instance javadoc.

@mkouba
Copy link
Contributor Author

mkouba commented Sep 13, 2021

FTR the Weld variant also includes the getPriorityComparator() method that can be used to sort handlers by priority in descending order.

OK so the Handler (I think Quarkus calls it Handle? That's a little nicer IMHO) allows:

The Quarkus variant returns InstanceHandle which is also used in other parts of the API. I also think that Handle is a bit nicer ;-).

@Ladicek
Copy link
Contributor

Ladicek commented Sep 13, 2021

The main point of Handle is really the ability to browse beans of certain type before creating their instances. The fact that you can perform get and destroy from there is just a convenience.

I wouldn't call it "just a convenience", because if you getHandles() and filter based on Bean metadata and then want to create an instance, going back to the Instance would require you to 1. remember position, 2. iterate again and rely on iteration order, which is clunky and generally a bad idea. So I'd consider instantiating and destroying essential.

In any case, I just wanted to clarify for myself, and thought perhaps that would be useful for someone else too :-)

@manovotn manovotn added the spec-feature An issue requesting an addition specification label Sep 13, 2021
@manovotn
Copy link
Contributor

FTR this issue was discussed during CDI meeting yesterday.
General consensus was that the Handler feature should be added into specification (e.g. Instance#handlers() and Instance.getHandler()) while the remaining two features got mixed reactions and the decision was to be keep track of them but don't add them to the specification just yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec-feature An issue requesting an addition specification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants