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

Enable to lookup Events programmatically. #587

Open
rmannibucau opened this issue Jan 15, 2022 · 10 comments
Open

Enable to lookup Events programmatically. #587

rmannibucau opened this issue Jan 15, 2022 · 10 comments

Comments

@rmannibucau
Copy link

Currently there are 2 API to lookup events programmatically from the bean manager - I ignore the derivative like Instance:

  1. bm.getEvent().select(...)
  2. bm.getReference(<bean resolving a type matching Event))

I can envision 2 is not an option (even if it should be implementable) since there is 1 but 1 is not enabled because it only support Class or TypeLiteral which are two static types so Type should be added to event#select to enable this use case.

There is no blocker in main implementations AFAIK so can be a quick win for everyone.

@mkouba
Copy link
Contributor

mkouba commented Jan 17, 2022

The problem with a java.lang.reflect.Type subtype parameter is that it's not type-safe. I think there was a similar discussion for javax.enterprise.inject.Instance in JIRA and the result was a non-portable method in Weld API: https://github.com/weld/api/blob/master/weld/src/main/java/org/jboss/weld/inject/WeldInstance.java#L146-L161

@rmannibucau
Copy link
Author

@mkouba I can agree it is not type safe but getReference is not too so guess it is just adding some consistency and maybe documenting it is for advanced uses?

@manovotn
Copy link
Contributor

The problem with a java.lang.reflect.Type subtype parameter is that it's not type-safe. I think there was a similar discussion for javax.enterprise.inject.Instance in JIRA and the result was a non-portable method in Weld API: https://github.com/weld/api/blob/master/weld/src/main/java/org/jboss/weld/inject/WeldInstance.java#L146-L161

This also popped up during a CDI mtg when we discussed some Instance improvements and majority of people weren't sold on it. I think there wasn't a strong use case that would justify adding a non type-safe method.

I ignore the derivative like Instance:

Just a note that is we implement this, we should probably also allow to invoke Instance#select() based on Type as the two are pretty similar.

There is no blocker in main implementations AFAIK so can be a quick win for everyone.

Yes, Weld already allows this, even for Event, see this. You just need to be careful around wording, since select should always return a subtype, you can only query by Type from Event<Object>/Instance<Object> and anything else should throw an ISE.

@rmannibucau
Copy link
Author

Yes, openwebbeans also has internals methods to do that but it is not portable whcih is an issue.

Also agree the issue is wider but actually can be refined to a single case, let me explain:

Think the issue is actually TypeLiteral, it enforces - for the type safety constraint) to compute the Type whereas its should allow to pass a precomputed Type instance IMHO, it wouldn't reduce the type safety (generic would still be there at compile time) and solve this issue.

wdyt?

@Ladicek
Copy link
Contributor

Ladicek commented Jan 17, 2022

If we ever wanted to unify CDI's TypeLiteral and JAX-RS's GenericType, we'd have to solve that problem too, as GenericType can be created out of a plain Type.

It's not exactly pretty, but it might be one of those cases where we just bite the bullet.

@rmannibucau
Copy link
Author

well TypeLteral will stay for backward compatibility so it can be fixed. JAX-RS/GenericType does not have that pitfall since Type is already handled in the related API so, even if the unification is out of scope of this issue, it can mean we use Type since it would be bad to couple CDI with yet another dependency for that, even assuming TypeLiteral goes into a generic EE lib would add a lib for almost nothing so it is robably sane to:

  1. Enable TypeLiteral to get a Type,
  2. (optional) support Type in CDI API(s)
  3. Define how a generic event can be looked up (from getEvent() seems the intended way but others API like getReference/Instance are undefined right now and even not expected to work surprisingly

@mkouba
Copy link
Contributor

mkouba commented Jan 17, 2022

Enable TypeLiteral to get a Type

javax.enterprise.util.TypeLiteral.getType()?

@Ladicek
Copy link
Contributor

Ladicek commented Jan 17, 2022

I think that was meant in the opposite direction :-) A constructor accepting Type.

@rmannibucau
Copy link
Author

@mkouba TypeLiteral.of(Type) ;)

@manovotn
Copy link
Contributor

@rmannibucau @Ladicek this issue sounds like something we should consider for #622, WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants