Method invokers

[Ladicek]

This PR has 2 commits with detailed commit messages. I'm copying the commit messages here for easier reference.

This PR doesn't include changes to the specification text, but the API javadocs are intentionally a specification-level material. There are several TODOs and everything is up for debate anyway, but I still wanted to provide something that is pretty well defined.

Initial proposal for method invokers: introduce core concepts

Invoker is an indirect way of invoking a bean method. It is a simple
functional interface:

interface Invoker<T, R> {
    R invoke(T instance, Object[] arguments);
}

Invokers are built using an InvokerBuilder. They must be thread-safe,
so frameworks can build an invoker for each relevant method once and then
reuse it to invoke that method whenever necessary.

An invoker may be built for a method that:

1. is not private;
2. is not a constructor;
3. is declared on a bean class of a managed bean (class-based bean),
   with the exception of interceptors and decorators, or inherited
   from its supertypes.

If an invocation of a method is intercepted, an equivalent invocation
through an invoker is also intercepted.

Portable extensions may obtain an InvokerBuilder in an observer
of the ProcessManagedBean type. The InvokerBuilder in this case
produces Invoker objects directly.

Build compatible extensions may obtain an InvokerBuilder during
@Registration from the BeanInfo object. The InvokerBuilder in
this case produces InvokerInfo objects. InvokerInfo is an opaque
token that can only be used to materialize an Invoker in synthetic
components.

Initial proposal for method invokers: invoker builder features

This commit introduces actual interesting features to the InvokerBuilder:

• automatic lookup of the target instance and arguments
• transformation of the target instance and arguments before invocation
• transformation of the return value and thrown exception after invocation
• wrapping the invoker into a custom piece of code for maximum flexibility

[tomas-langer]

Is there a way to find instances of parameters using CDI? E.g. if we consider that each parameter is an injection point, could we delegete the whole invocation to CDI?

Something as:

invokable.invoke(myBeanInstance); // all arguments are bound from current CDI context

or

invokable.invocationBuilder()
  .injectParam(0)  // injection point
  .param(1, myEntity) // explicit argument value
  .injectParam(2) // injection point

In such a case, we could discover injection points in extensions, and correctly prepare producers for them (such as for @QueryParam etc.) rather than analyzing each method and creating all parameters by hand.

[Ladicek]

The InvokerBuilder allows this using setInstanceLookup() and setArgumentLookup(int).

[tomas-langer]

The InvokerBuilder allows this using setInstanceLookup() and setArgumentLookup(int).

Thanks a lot!

[Ladicek]

BTW, if anyone is interested, we can discuss this proposal on the CDI call next week.

[ljnelson]

The name seems problematic (a method can be invoked due to its very nature) and possibly misspelled. Perhaps instead: if a managed bean is a bean you don't create but the container creates for you, and is a bean for which you don't supply dependencies, but for which instead the container supplies dependencies, then so, perhaps, a managed method is a method you don't invoke but the container invokes for you, and is a method for which you don't supply arguments, but for which instead the container supplies arguments? Just a thought.

[graemerocher]

looks like good progress, just a few suggestions

[graemerocher]

These seem too stringly typed and dynamic, can we not use method handles and provide generics for the return type? Why add a requirement for reflection here?

[Ladicek]

It is rather stringly typed, but all alternatives I could think of are as much stringly typed if not more. The idea here is to type check what we can during deployment (which would be during application build for build-time implementations) and let the JVM to sort out the rest. To allow transformers to convert values from one type to another, which seems like a hard requirement to me, we can only type check the output type for input transformers (as the output of an input transformer is passed to the invokable method and hence its type must match) and the input type for output transformers (because the input of an output transformer is obtained from the invokable method and hence its type must match).

Or did you mean the syntax of referring to the transformer method, the tuple (Class, String)? I considered 2 alternatives:

1. Require transformers to implement some interface. Not nice, because it requires extra allocations and virtual dispatch. I wanted transformers to be static methods mainly to eliminate overhead, but it's also nice in that it tells users that transformers should be stateless. (They can also be instance methods, but that is [or should be] by construction restricted to simple scenarios where the static method would just call that instance method.)
2. Use method references (as in, SomeClass::transformer). This would require defining functional interfaces for all possible signatures of transformers and provide overloads that accept these functional interfaces. It would also require some way to find the method during deployment (that is, during application build for build-time implementations). Apparently there's a trick to do that if the functional interface extends Serializable, which seems strange to me. Finally, we might also want to detect (and forbid?) situation where the caller doesn't pass us a method reference, but an actual lambda.

I'd really love to use method references, but I couldn't figure out how to implement it in a reasonable way. So I settled with just a pair (Class, String), which is basically what I'd need from the method reference, except explicit (and unfortunately not checked during compilation).

Finally, the idea is that all invokers are built during deployment -- there's no way to obtain an InvokerBuilder during application runtime. Therefore, there's no requirement for reflection here -- build-time implementations are supposed to find the transformer method using whatever language model they use (e.g. Jandex in case of Quarkus) and build the invoker using whatever mechanism they prefer (e.g. bytecode generation in case of Quarkus).

[graemerocher]

ok I am also wondering though why all of this transformation and decoration is necessary? Surely this change to the spec should be purely focused on executable methods and the transformation / decoration side should be in the interceptors spec?

[Ladicek]

You're right that the InvokerBuilder partially overlaps with interceptors, except it can be much cleaner and more efficient. The reason why I'm adding it here is that most frameworks that want to invoke application methods do need some kind of transformations. Here's a few concrete examples:

1. Frameworks often want to let users use multiple signatures of the methods the framework will invoke, but also want to handle them uniformly internally.
   1.a) For example, a framework wants to let users write both blocking and non-blocking methods, and internally treat them all as non-blocking. This requires a seam somewhere, and a return value transformer (and probably also an exception transformer) is a perfect fit.
   1.b) As another example, a framework might want to support multiple non-blocking return types (such as CompletionStage from the JDK and some Rx-style type). A return value transformer abstracts that away.
   1.c) As yet another example, a framework wants to let users write methods that accept multiple different representations of the same thing, e.g. some kind of a "message" that includes headers and payload, or just the payload in case the user isn't interested in the headers or anything more advanced. Again, this requires a seam somewhere, and an argument transformer is a perfect fit.
2. Frameworks often want to invoke methods on beans and use beans as arguments, without having to do lookups (and possible disposals) on their own. The target instance and argument lookup is a perfect fit.
3. Sometimes, frameworks have to wrap the entire invocation into some piece of code that establishes some form of context. A good example here is support for Kotlin suspending functions, where the framework has to set up a coroutine scope and call the suspending function in that scope. An invoker wrapper lets you do this nicely, and it composes well with the other kinds of transformations.

As you can imagine, I have built a prototype for this and all the examples above are real use cases I found.

I like to compare this to Java method handles -- there's also a slew of transformation functions (called combinators) that you can apply to method handles to create a seam between the caller and the ultimate callee. This is the exact same idea.

[manovotn]

the transformation / decoration side should be in the interceptors spec?

That was also my initial question when I saw Ladislav's prototype.
However, given the use cases above, I tend to think this is better approach. The idea is that invokeable methods are a feature used by extensions (or integrators, if you will) and as such the integrator might want to do their own specific tweak in how the method is invoked and which parameters are looked up and so on. This would be hard to achieve via interception as multiple extensions trying to handle it differently might easily clash.
Furthermore, unlike interceptors, in this case there is always at most one such transformation per registered invoker (or per extension/integrator that wants to tweak it specifically) - it also doesn't need explicit enablement and doesn't deal with priority meaning it isn't subject to interference from ordering. Last but not least, you can still apply other interceptors onto the method independently of what each extension decides to perform with the invocation.

[manovotn]

This is a user facing API - how is user going to get ClassInfo object for their arbitrary Foo class? That class doesn't even need to be a bean, so you cannot get that from another BCE event either.
And as far as I can tell, we don't have a way to create that in the lang model API either.

If we wanted to go without Class, we'd probably need to go full String mode which is pretty ugly...

[Ladicek]

So the Class parameter does not mean reflection needs to be used. It is a possible implementation, but not the only one. The API is designed so that it can be implemented purely at build time. I actually have a fairly complete implementation of this API in ArC, where all invokers created through the InvokerBuilder are generated during build. In such implementation, the Class object is only used to obtain the binary name of the class, nothing else. I'd be happy to present how my implementation works, and I'd of course love to hear if this API contains any obstacles for your implementation.

I'll experiment with using method references, but I fear that it isn't exactly easy to extract the necessary information from them during build time, as a method reference is indistinguishable from a lambda expression.

[Ladicek]

Actually thinking about it, I see one possible problem for your implementation (based on what I know -- if you could confirm, that would be great). The specification currently assumes that an implementation can inspect the class so that it can:

• find the named method, and use its signature for code generation,
• do the limited type checking the specification prescribes.

I didn't notice any issue with that, because our implementation has full access to the deployment. That isn't exactly the case for your implementation, if I remember correctly.

Now, indeed one way to solve it would be to use reflection to inspect the class. That would be reflection during build time, not at runtime -- but this might still be problematic. I'm no expert here, so please correct me if I'm wrong.

I assume if we could pull off some trick with method references, that would fix the issue for you, as the method reference would come with the full signature and you wouldn't have to search for the method on the class. Is that right?

[graemerocher]

so I see there is InvokerBuilder<InvokerInfo> createInvoker(MethodInfo method); so it is probably ok.

[Ladicek]

Yes, the InvokerBuilder is supposed to be used at deployment time, which for build-time implementations means during build. Which means that the Build Compatible Extensions entrypoint for creating an InvokerBuilder accepts a MethodInfo and the InvokerBuilder itself produces InvokerInfo.

(The entrypoints for creating an InvokerBuilder are provisionally placed. A better place for them likely exists. Their nature however should not change. In Portable Extensions, the InvokerBuilder produces Invoker objects directly, while in Build Compatible Extensions, it produces opaque tokens that only turn into Invoker objects at runtime.)

I don't know what's the best way to refer to transformer methods. As I mention in another comment on this PR, using method references is possible, but only using a pretty gross hack. Using the (Class, String) tuple works fairly well as far as I can tell, but I see how it isn't very appealing.

[Ladicek]

The name seems problematic (a method can be invoked due to its very nature)

I used the same argument before, when we were discussing the idea of "executable methods". At the same time, it seems reasonable to refer to the core property of this new feature in its name. A somewhat more correct/descriptive name would be "indirectly-invokable methods", but that's a really ugly name. I went with "invokable" because the core types use this word (Invokable, Invoker) and the JDK already contains colliding classes for the verbs "call" and "execute" (Callable, Executable, Executor).

and possibly misspelled.

Interesting. I'm not a native speaker and "invocable" looks terribly alien to me :-) Google knows both terms and "invocable" seems more often used (cca 60K results for invokable and 200K results for invocable). There are some other APIs that use "invocable", too:

• https://en.cppreference.com/w/cpp/concepts/invocable
• https://docs.oracle.com/javase/7/docs/api/javax/script/Invocable.html
• https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_annotation_InvocableMethod.htm

I can be persuaded to rename, though I also think public disobedience is in order here. If something can be invoked, it should be invokable! :-)

Perhaps instead: if a managed bean is a bean you don't create but the container creates for you, and is a bean for which you don't supply dependencies, but for which instead the container supplies dependencies, then so, perhaps, a managed method is a method you don't invoke but the container invokes for you, and is a method for which you don't supply arguments, but for which instead the container supplies arguments? Just a thought.

Managed bean specifically in CDI is what I call a class-based bean. There are other kinds of beans, producer methods and producer fields, that are created by the container and for which dependencies are provided by the container (though the ultimate constructor call lies in user code). Also, for invokable methods, the caller must supply the target instance and the arguments (unless they build a special kind of invoker with InvokerBuilder that looks up the target instance and all arguments from CDI). So I don't think the analogy holds.

I'm certainly not married to "invokable methods", but "managed methods" doesn't cut it in my opinion.

[ljnelson]

What is a non-invocable method? How is "invocable method" (or "callable method" or "executable method" or "runnable method" or…) different from "three-sided triangle"? Can you say what the property is that distinguishes these methods from other methods? It seems to me the property is that they are managed, like other managed things in Jakarta EE, in CDI, JPA, Servlet and elsewhere, where managed means, loosely, "the container does most or all of it for you". Isn't that the property you're going after? If not that, then what?

[Ladicek]

Can you say what the property is that distinguishes these methods from other methods?

You can obtain an Invoker for them and indirectly invoke them through the invoker. So they are "indirectly-invokable through a CDI-provided mechanism", or "invokable" for short. The very first paragraph of @Invokable javadoc says that: https://github.com/jakartaee/cdi/pull/639/files#diff-caa2a3ec23acc164cb3dee736b270531344e7daf84def4acbc8fb210fa7c18ce

It seems to me the property is that they are managed, like other managed things in Jakarta EE, in CDI, JPA, Servlet and elsewhere, where managed means, loosely, "the container does most or all of it for you". Isn't that the property you're going after? If not that, then what?

So if there's anything "managed", it's the invoker. I'd be fine with calling the feature "managed invokers", but that doesn't help with figuring out a name for an annotation to mark methods as "subject to creating managed invokers".

[mkouba]

I think that "managed" is (1) overused in the Jakarta ecosystem and (2) does not imply what this API is used for. @Invokable is IMO ok and if the javadoc is clear then +1.

[manovotn]

and possibly misspelled.

I am not a native speaker but "invokable" versus "invocable" both seem valid at least according to dictionaries and some Google-fu. I'd therefore say that it is probably a matter of localized preferences depending on where you live :)

managed method is a method you don't invoke but the container invokes for you, and is a method for which you don't supply arguments, but for which instead the container supplies arguments? Just a thought.

I am definitely not a fan of "managed methods", so -1.
The word managed is overused already and not too clear either. Even in your own definition, you say that the container supplies the arguments, but that's actually optional. The container does perform the invocation, but the extension/integrator is who controls args, target instance, return value transformation and so on. So in a way, the extension manages these a lot more than the container does (as opposed to the "managed beans" case).

I don't really see an issue with current naming... but then again, naming is hard :)
Either way, I think we should first see if we can agree on the use cases for these methods and see if the API meets expectations and based on that it should be easier to derive a name capturing the intent more closely.

[Azquelt]

I don't like "invokable methods" as a name. It's confusing because all methods are invokable by definition.

However, I think what we're doing here is allowing the user to create "method invokers", which can invoke methods in a special way and can eliminate a lot of complex boilerplate code for frameworks.

I would be happy to talk about "method invokers", although in general conversation I'd probably always refer to them as "CDI method invokers" since the concept is fairly generic, but I'd prefer that the spec and Javadoc never use the phrase "invokable method" and instead talk about "creating an invoker for a method", or a method being "the target for an invoker".

I think the parts of the API that I think would need to change are:

• @Invokable annotation, which could perhaps be @InvokerTarget
• InvokableMethodInfo which could be InvokerTargetInfo or InvokerTargetMethodInfo
• InvokableMethod which could be InvokerTarget or InvokerTargetMethod

As a side-note, "invoke" is a weird word, I would use "invocation" as the noun so it's not clear whether another derived word should use a 'c' or a 'k'. However, "invoker" and "invokable" seem more natural to me, though as noted above I'd like to remove "invokable" altogether.

[Ladicek]

Note to self: @Azquelt had a good point on the call today, the javadoc should (after the spec material moves to the spec itself) also contain an explanation of when is this feature useful and when not. The current design is heavily geared towards frameworks that require calling application code; application code is not expected to use this feature at all.

[ljnelson]

Do I understand properly that a so-called "invokable method" always belongs to a managed bean?

[ljnelson]

Re: @Azquelt's excellent comment above: Would something like @InvocationTarget be better, which calls to mind other usages like InvocationTargetException, suggesting that perhaps this is a preferred term in the overall Java ecosystem for such a thing?

[Ladicek]

Do I understand properly that a so-called "invokable method" always belongs to a managed bean?

Correct, but note that I use the term managed bean in the very specific CDI sense: https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#managed_beans That is, for example, producer beans are not managed beans.

[Ladicek]

A few more points made on the call yesterday.

1. Currently, the InvokableMethod interface allows obtaining a direct invoker, which always exists, and also all previously registered invokers, if the caller has the registration key. The registration key is a Class, which is supposed to allow for some level of privacy, but that can easily be circumvented using Class.forName. The more I think about it, the more I believe we should get rid of InvokableMethod and the concept of direct invoker and the concept of registration key, always requiring framework integrators to build the invokers they need during deployment and stash them somewhere for usage at application runtime. That would probably simplify the API quite a bit, and streamline the expected usage path.

2. While the current API only allows building invokers during deployment time, CDI Full could allow building invokers at application runtime. Not sure if that's useful, but it's an option we could add.

[Ladicek]

Just added one commit to simplify the invoker registration process.

This commit removes the ability to obtain an invoker without previous registration. It also removes the concept of registration key: whenever an invoker is registered, the extension must remember it for later use.

After that, the InvokableMethod[Info] interfaces are useless, hence this commit removes them. Access to invoker registration is through the extension API: ProcessManagedBean and BeanInfo. Both of these places are provisional; better API for invoker registration likely exists.

This commit does not (yet) remove the MethodMetadata and ParameterMetadata interfaces, because there is one more possible use case for them: the invoker transformers may want to access the called method, including transformed annotations. The API definition currently does not allow that, but it's something that should be explored.

[Ladicek]

I promised to add a comment listing the use cases I found in Quarkus when I was prototyping this feature. Here goes:

• the JAX-RS implementation, of course
• scheduler (framework invokes some application method on a schedule, something like cron)
• a declarative API for the Vert.x event bus (something like CDI events, except the event bus is not provided by CDI)

There's at least 1 more that I don't recall at the moment, perhaps our implementation of MicroProfile Reactive Messaging.

In all these cases, Quarkus already has an interface that is very close (or identical) to the Invoker interface added here. It gives the framework a way to invoke an application method. I found that all these cases also support some kind of transformations, which is generalized here as the InvokerBuilder. (And the InvokerBuilder looks so weird because it allows the implementation to generate invokers during application build. Runtime implementations, such as Weld, should be able to implement this using Java MethodHandles and method handle combinators, but I don't have a prototype for that yet.)

[manovotn]

Is there a specific reason why we don't want these to apply to methods declared in superclasses? I.e. why not have it inherited?

[manovotn]

Furthermore, do we actually need to support anything beyond public methods?
Technically, those methods are supposed to be invokable by anyone - and any user can write an extension to be able to do that which sounds like a case for public method.

Now imagine a bean some.other.lib.Foo which has a package-private method ping(). Furthermore imagine a user bean my.app.Bar which has @Inject Foo foo. Now, from Bar, you normally cannot invoke foo.ping() - but if you created an invoker, you might be able to do just that. That sounds pretty wrong to me :)

[manovotn]

I don't think this isolation makes much sense.
You can have something like class Foo with method ping() and then you can have n implementations (FooImpl1, FooImpl2, ...) of this class that are beans.
Some of the impls you have invokers for might be disabled, specialized, vetoed, ....
You'd need to create and keep reference to a separate invoker for each variant of the bean - despite the fact that you don't care about what else impl variants bring to the table, in fact you might not even care which impl it is, you just want to get the default one that comes via @Inject Foo and execute the method on that one.
Determining which invoker you can use at runtime could become a hassle.

I'd rather have an invoker bound to a method and so long as you pass in a "compatible" bean which has such method, you invoke it there.

[Ladicek]

Good question, conceptually. I probably had more reasons for it, which I don't remember exactly right now, but one is fairly obvious: if I want an invoker to be able to automatically lookup the target instance (InvokerBuilder.setInstanceLookup()), then the invoker must be bound not only to the method, but also to the bean.

[manovotn]

Fair enough but you get the invoker from a ProcessManagedBean where you can see bean types (and qualifiers). You can use those for lookup regardless of what the resulting bean is - actually that's exactly what will happen anyway if you have an alternative in place; you'll end up invoking it on different impl instance.

On the topic of instance lookup, assuming the rules weren't this strict, I was thinking if the method could be something like InvokerBuilder.setInstanceLookup(TypeLiteral<T> t, Annotation... a) allowing you to specify how to perform the lookup while still leaving dependent instance handling to CDI. Obviously this brings the issue of assignability but that's the same as with many other parameters user can wrongly pass in.

[Ladicek]

Good point about alternatives -- I don't think the existing spec text (in the javadoc) mentions it, but I think we should only discover invokable methods on enabled beans, or something like that.

[manovotn]

Well, ProcessBean (or managed bean in this case) is fired for enabled beans only, so that's automatically true, at least via portable extensions. However IIRC, that doesn't solve the alternative issue fully as you can have multiple enabled alternatives (meaning alternatives selected for given archive) for Foo, each with a different priority.

[Ladicek]

I have just force-pushed an refreshed proposal (and updated the description of this PR to match the new commits). The API surfrace should be much smaller, I removed most of the unnecessary things and streamlined the access to invokers. The concept of InvokerBuilder is now necessary, so I left the InvokerBuilder in the 1st commit virtually empty, and the interesting features (that might be contentious) are in the 2nd commit.

[manovotn]

@Ladicek there is a javadoc build error

Error:  Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.3.0:jar (attach-javadocs) on project jakarta.enterprise.cdi-api: MavenReportException: Error while generating Javadoc: 
Error:  Exit code: 1 - /home/runner/work/cdi/cdi/api/src/main/java/jakarta/enterprise/inject/build/compatible/spi/InvokerInfo.java:17: error: reference not found
Error:   * Opaque token that stands in for an invoker registered using {@link BeanInfo#registerInvoker(MethodInfo)}.

[Ladicek]

Sorry, my bad. Fixed.

[manovotn]

Just a note WRT to the recent discussion on the call around exception transformer arbitrary return values versus what method handles allow. I have managed to implement a workaround in Weld that can support the API as proposed here - the crux of it is to apply a return value transformation to the exception transformer and then "hide" the actual transformer ret. value within an exception. It's not the prettiest thing, but it can be done conditionally and still using just method handles allowing me to construct one final handle and then invoke it repeatedly without too much overhead.
In case you're curious how it works, see the third commit in the Weld PR.

[Azquelt]

I'd expect NullPointerException in this case, as you would get if you tried to unbox a null.

[Azquelt]

I know this has been answered before, but I can't remember the answer and couldn't find it written down anywhere.

Why do we need the @Invokable annotation?

Why can't I create an invoker for any method? Invokers are always created by extensions, which are running at startup. In addition, we already have the capability to get information on any method via @Enhancement with ClassInfo or MethodInfo so I'd have thought that whatever information is needed to create an invoker must already be available to support @Enhancement?

[Ladicek]

Good question. Arguably @Invokable is not as meaningful now as it might have been before, because one must always create an invoker explicitly. Previously, all invokable methods got a direct invoker automatically, for free, but that's no longer the case.

The annotation has never been intended to be used in applications directly, it should be mainly a meta annotation for other annotations that constitute invokable markers, but the only argument for the existence of invokable markers today is that they explicitly designate methods for which an invoker may be built. That is, it's some sort of documentation. I agree that's a fairly weak argument.

I'll sleep on it, but at the moment, I think removing the @Invokable annotation would be harmless and I'd be in favor.

[manovotn]

The annotation has never been intended to be used in applications directly, it should be mainly a meta annotation for other annotations that constitute invokable markers, but the only argument for the existence of invokable markers today is that they explicitly designate methods for which an invoker may be built. That is, it's some sort of documentation. I agree that's a fairly weak argument.

That depends on whether we want basically any method to be invokable.
Note that we currently disallow private methods and constructors but everything else is allowed. You could therefore create invokable method off of any package private method that you normally cannot invoke.
Granted, you could do that now if the method is marked @Invokable but that's still different from opening up all of them by default.

The only other thing I can think of is annotation processor approach perhaps needing the annotation? But then again, if they can meet the requirements for @Enhancement, then I don't suppose this is required (at least not as long as we talk managed class bean's methods).

[Ladicek]

Good point about annotation processing-based implementations. I'm no expert, but I think they should be fine -- invokable methods only exist on managed beans, which already must have a bean defining annotation (in CDI Lite), so we should be fine. I'd be glad to be proven wrong (or right) though.

[Ladicek]

I removed the @Invokable annotation and renamed the entire feature to method invokers. This should resolve one of the greatest pain points of the proposal: the correct spelling of "invokable" / "invocable", because that word doesn't appear in the proposal anymore :-)

[Ladicek]

I also marked this PR as ready for review. There's still a lot of open questions (notably: should we use the (Class, String) pair to identify transformers/wrappers or should we require users to implement an interface, and what exactly are the assignability rules?), but I believe the current state is good enough for inclusion in an alpha release.

[Azquelt]

Should this be on ProcessBean instead of ProcessManagedBean?

I can't think of a reason you shouldn't be able to create an invoker for a method for a producer method, a producer field, a session bean or a synthetic bean.

Possibly you shouldn't be able to create one for an interceptor or decorator.

[manovotn]

Should this be on ProcessBean instead of ProcessManagedBean?

I don't think so; the idea was to support methods declared on class-based beans.
@Ladicek I am pretty sure we discussed this but I don't recall what were the reasons for this exactly?

I can't think of a reason you shouldn't be able to create an invoker for a method for a producer method, a producer field, a session bean or a synthetic bean.

A producer method has to be declared on a CDI bean - therefore you can create the invoker for it there.
A producer field isn't a method at all; a poor fit for method invokers :)
A session bean will still work, because ProcessSessionBean is a subtype of ProcessManagedBean
A synthetic bean - I am not sure I follow here. Anything can be a synthetic bean and the logic of these closely follows that of producers. Do you mean the return type of those beans? Note that it can be (just like with producers) even a primitive type.

Possibly you shouldn't be able to create one for an interceptor or decorator.

Yes, that doesn't make much sense.

[Azquelt]

A producer method has to be declared on a CDI bean - therefore you can create the invoker for it there.
A producer field isn't a method at all; a poor fit for method invokers :)

I'm not suggesting creating an invoker for the producer field or method itself, but for methods on the bean types of the bean created by the producer method or field. (i.e. usually the return type of the producer method, or the type of the producer field).

E.g. if I have a producer method

@ApplicationScoped
public Connection getConnection() { /*...*/ }

then I should be able to create an invoker for Connection.prepareStatement(String)

[Azquelt]

Possibly we need to clarify what's meant by The {@code method} must belong to the bean being registered.

I assumed it meant you must be able to call the method on one of the bean types of the bean.

[Ladicek]

So I originally wanted this to only apply to managed beans, because you know the exact class of the bean and the full set of its methods. It's also the only thing that I have actual use cases for.

It would probably be possible to extend this to producers, where you at least know the supertype of the bean and only the methods declared on that supertype (or inherited from further supertypes) could be invoked.

For synthetic beans, there's no way to obtain anything about the bean implementation class, so that's a no-go.

[manovotn]

I'm not suggesting creating an invoker for the producer field or method itself, but for methods on the bean types of the bean created by the producer method or field. (i.e. usually the return type of the producer method, or the type of the producer field).

I see, thanks for clarification.
Currently, invokers are created (in Portable Extensions) from AnnotatedMethod - obtaining AM for arbitrary method of arbitrary type might be a little challenging whereas getting them from ProcessManagedBean is straightforward.

Possibly we need to clarify what's meant by The {@code method} must belong to the bean being registered.

I think this is in place because otherwise you could observe ProcessManagedBean<Foo> and within this OM try to register an invoker for an AnnotatedMethod from bean Bar.

[Ladicek]

I'll try to expand the word "belong", but @manovotn is right -- this is to prevent using ProcessManagedBean<Foo> to create an invoker for a method of a completely different class. How about:

The {@code method} must be declared on the bean class or inherited from a supertype
of the bean class of the bean being registered, otherwise an exception is thrown.

?

[Ladicek]

Expanded the comment per above.

[Azquelt]

That's much clearer 👍

If the intention is that you can only create an Invoker for a managed bean, then the documentation on BeanInfo should probably be updated to reflect that.

[Ladicek]

That's actually specified in Invoker, but I added a note to BeanInfo as well.

[Azquelt]

At some point, we probably need to be more specific about what we mean by a CDI lookup.

I guess at deployment we're performing typesafe resolution, and then when the method is invoked we're obtaining a contextual instance (or contextual reference?) of the Bean?

[Ladicek]

It would be a contextual reference. But I think I'd like to keep that specified in terms of programmatic lookup, because that shields us from these low-level concerns.

[Azquelt]

This looks good. I'm happy for this to be merged with the remaining TODOs, so that we can get an alpha release out.

I still have some reservations about doing argument, return type and exception transformations, but we can extend those in future releases if it turns out that users want additional functions and it's always possible for users to ignore our methods and use another mechanism (e.g. method handles) if they need more advanced transformations.

[manovotn]

Going to merge this and issue an Alpha release.

Following that, I would like to release Weld version with the prototype so that we can (hopefully) get some feedback.
There are still some TODOs here that we should create a tracking issue for (or keep existing one open) as well as create tracking issue for TCKs.