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 access to Assignability rules #498

Closed
JHahnHRO opened this issue Jun 7, 2021 · 9 comments · Fixed by #700
Closed

Programmatic access to Assignability rules #498

JHahnHRO opened this issue Jun 7, 2021 · 9 comments · Fixed by #700
Assignees
Milestone

Comments

@JHahnHRO
Copy link

JHahnHRO commented Jun 7, 2021

Some of the important algorithms outlined in pseudo-code in the spec are already available to developers of extensions, e.g. BeanManager#resolve, BeanManager#resolveDecorators and BeanManager#resolveObserverMethods. However, underlying those are other algorithms which are currently not made available to the programmer without depending on specific vendors, the type assignability rules. It is of course possible to implement the algorithms by hand when needed, the spec is clear enough to do that, but the rules are subtle enough that this is error prone and not completely trivial.

Specific use case that I have in mind: I'd like to add to the CDI container's event mechanism by also delivering events to consumers which can be registered and unregistered at runtime. In order to be consistent, I'd like the consumers to conform to the same strict type rules that observer methods have. Unfortunately, there is no quick and simple way to apply these rules except for diving into Weld's source code and stealing what I need hoping that I don't inadvertently break some subtle edge case in the process.

@manovotn
Copy link
Contributor

There is already several sets of runtime assignability rules in place - at least one for beans and another for events.
And with CDI Lite, implementations are likely to have to implement a different set of rules for bean assignability for build time (injection point validation phase) and then for runtime. This is because runtime is reflection based on Type whereas in build time (and hence even in build compatible extensions) there is an abstraction that tries really hard to avoid any reflection.

This is a situation I know I saw in Quarkus. And the implementations of assignability rules can then differ based on what metamodel the implementor runs on (if they use any abstraction; for example Quarkus uses Jandex).

In other words, I am not sure this is unified enough to warrant putting it into specification.
Means such as BeanManager#resolve() are there exactly to cover up the need to expose internals like this.

@manovotn
Copy link
Contributor

For the time being, I'd suggest grabbing resolution rules from an impl that fits your case (runtime/build time) as the feature you are implementing will be only in that space anyway. Those rules are pretty stable and don't change much so your code is highly likely to run fine even after updates. I don't think exposing those rules publicly makes a lot of sense plus I am not even sure it is doable due to aforementioned differences.

The only other thing I can think of would be exposing the algorithm via BeanContainer/BeanManger methods - i.e. a method that accepts two Type objects and returns true if assignable. That's certainly doable but there are different rules for events/observers, classic injection and delegate inject point if memory serves. Therefore, we should expose all three ways (either 3 methods, of one with enum param that picks the ruleset you want). Though I am not sure it is worth it since I cannot imagine use cases for it.
WDYT @Ladicek?

@Ladicek
Copy link
Contributor

Ladicek commented Jul 25, 2022

Personally, I wouldn't expose the assignability rules directly, as that's too low-level. What might be interesting is to expose the resolution algorithm in a more fine-grained manner. For example, BeanContainer currently exposes

<T> Set<ObserverMethod<? super T>> resolveObserverMethods(T event, Annotation... qualifiers);

which doesn't let you plug your own "observer method". We could add something like this:

<T> boolean isMatchingObserverMethod(T event, Annotation... qualifiers, ObserverMethod<? super T> observerMethod);

This method would apply the observer resolution rules to given observerMethod (probably not all of them, would require more thinking) and return whether the observerMethod should be notified of given event.

We could expose something similar for bean resolution, too.

@manovotn
Copy link
Contributor

Personally, I wouldn't expose the assignability rules directly, as that's too low-level.

For the scenario presented here, the annotation comparison is trivial, so it boils down to type assignability.
I agree that we could do that in some form with annotations as well however that also requires users to know how things like @Any and @All works but I assume whoever uses this will probably know.

This method would apply the observer resolution rules to given observerMethod

Passing around whole OM might be tricky and IMO it would be simpler to do that with type and qualifiers only.

Ok, let's keep this in the CDI.next tracker and see what we can come up with.

@manovotn manovotn changed the title Provide Assignability rules programmtically Programmtic access to Assignability rules Jul 25, 2022
@manovotn manovotn changed the title Programmtic access to Assignability rules Programmatic access to Assignability rules Jul 25, 2022
@Ladicek
Copy link
Contributor

Ladicek commented Jul 25, 2022

I used ObserverMethod in the proposal sketch above with a bit of hesitation, as indeed it contains way more information than necessary. If the method only accepted Type, Annotation[], Type, Annotation[], that would of course be simpler and more straightforward, but I always find this kind of methods a bit hard to follow.

A slight variation on the sketch above would be:

public interface ObserverMethodData { // bad name, anyone has a better one?
    Type type();
    Annotation[] qualifiers();
}

...
Set<ObserverMethodData> resolveObservers(Set<ObserverMethodData> candidates, Type eventType, Annotation... qualifiers);
...

Where resolveObservers would run through candidates and only return those that match given event type and qualifiers, according to the rules of observer resolution.

EDIT: or we could just use ObserverMethod instead of introducing a new interface, but guarantee that resolveObservers will only ever look at the event type and qualifiers.

@manovotn
Copy link
Contributor

@Ladicek I don't think this meets the requirements of the scenario @JHahnHRO drafted.

Specific use case that I have in mind: I'd like to add to the CDI container's event mechanism by also delivering events to consumers which can be registered and unregistered at runtime. In order to be consistent, I'd like the consumers to conform to the same strict type rules that observer methods have.

In such case, you won't necessarily have ObserverMethod instance for given observer which is why I wanted to go with just Type and annotations. OP doesn't ask for annotations either, but I think that would be handy anyway and in CDI it always goes together.

@Ladicek
Copy link
Contributor

Ladicek commented Jul 26, 2022

I believe the original use case still requires the knowledge of event types and perhaps qualifiers, and implementing the ObserverMethod interface where most methods throw UnsupportedOperationException is pretty straightforward :-) At least that's what I had in mind when sketching the API above.

@manovotn
Copy link
Contributor

Hmm, I see.
But I still think it might be easier to just skip that OM part. It is not that low level either since spec has all these rules laid out fairly plainly for everyone. Alright, either way would work I guess.

As a side note, if we chose to expose these rules, we should IMO do that for beans and events but not for delegate injection points. At least not initially as I cannot imagine anyone trying to extend decorator's delegate IP somehow so it sounds like an overkill for now.

@Ladicek
Copy link
Contributor

Ladicek commented Jul 26, 2022

Sure. We can either add an interface, as I mentioned above, or just accept types and annotations.

Agree about the delegate injection points, that can't be practically useful.

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