-
Notifications
You must be signed in to change notification settings - Fork 77
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
Method invokers #639
Method invokers #639
Conversation
b90941d
to
624abfd
Compare
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 |
The |
Thanks a lot! |
BTW, if anyone is interested, we can discuss this proposal on the CDI call next week. |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like good progress, just a few suggestions
* @return this builder | ||
* @throws IllegalStateException if this method is called more than once | ||
*/ | ||
InvokerBuilder<T> setReturnValueTransformer(Class<?> clazz, String methodName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- 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 thestatic
method would just call that instance method.) - 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 extendsSerializable
, 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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- 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 asCompletionStage
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. - 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.
- 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I see there is InvokerBuilder<InvokerInfo> createInvoker(MethodInfo method);
so it is probably ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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 (
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:
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! :-)
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 I'm certainly not married to "invokable methods", but "managed methods" doesn't cut it in my opinion. |
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? |
You can obtain an
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". |
I think that "managed" is (1) overused in the Jakarta ecosystem and (2) does not imply what this API is used for. |
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 :)
I am definitely not a fan of "managed methods", so -1. I don't really see an issue with current naming... but then again, naming is hard :) |
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:
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. |
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. |
Do I understand properly that a so-called "invokable method" always belongs to a managed bean? |
Re: @Azquelt's excellent comment above: Would something like |
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. |
A few more points made on the call yesterday.
|
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 This commit does not (yet) remove the |
I promised to add a comment listing the use cases I found in Quarkus when I was prototyping this feature. Here goes:
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 |
api/src/main/java/jakarta/enterprise/inject/build/compatible/spi/SyntheticObserverBuilder.java
Outdated
Show resolved
Hide resolved
api/src/main/java/jakarta/enterprise/inject/build/compatible/spi/SyntheticObserverBuilder.java
Outdated
Show resolved
Hide resolved
* When this annotation is present on a class or interface, it marks all non-private | ||
* methods (except constructors) declared in this class as invokable. Note that this | ||
* does not apply to methods declared in superclasses or superinterfaces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :)
* Note that multiple managed beans may inherit an invokable method from a common | ||
* supertype. In that case, each bean conceptually has its own invokable method | ||
* and for example an invoker obtained for one bean cannot be used to invoke | ||
* the method on the other bean. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
836bb87
to
a7c4ba0
Compare
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 |
@Ladicek there is a javadoc build error
|
Sorry, my bad. Fixed. |
a7c4ba0
to
19996e8
Compare
b7a8a5d
to
407f6ef
Compare
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. |
407f6ef
to
96342a2
Compare
* TODO specify what happens when a transformer/wrapper declares a parameter of a primitive type | ||
* but the actual value passed to the invoker is `null` (the transformer should get a zero value?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect NullPointerException
in this case, as you would get if you tried to unbox a null
.
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 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 |
Good question. Arguably 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 |
That depends on whether we want basically any method to be invokable. 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 |
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. |
96342a2
to
621d774
Compare
I removed the |
I also marked this PR as ready for review. There's still a lot of open questions (notably: should we use the |
|
||
/** | ||
* Returns a new {@link InvokerBuilder} for given method. The builder eventually produces an invoker | ||
* for the given method. | ||
* <p> | ||
* The {@code method} must belongs to the bean being registered, otherwise an exception is thrown. | ||
* | ||
* @param method method of the bean being registered, must not be {@code null} | ||
* @return the invoker builder, never {@code null} | ||
* @since 4.1 | ||
*/ | ||
public InvokerBuilder<Invoker<X, ?>> createInvoker(AnnotatedMethod<? super X> method); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be on
ProcessBean
instead ofProcessManagedBean
?
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expanded the comment per above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually specified in Invoker
, but I added a note to BeanInfo
as well.
api/src/main/java/jakarta/enterprise/inject/spi/ProcessManagedBean.java
Outdated
Show resolved
Hide resolved
621d774
to
7ab970a
Compare
_`Invoker`_ is an indirect way of invoking a bean method. It is a simple functional interface: ```java 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.
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
7ab970a
to
ff9d0e3
Compare
* <h2>Input lookups</h2> | ||
* | ||
* For the target instance and for each argument, it is possible to specify that the value | ||
* passed to {@code Invoker.invoke()} should be ignored and a value should be looked up | ||
* from the CDI container instead. | ||
* <p> | ||
* For the target instance, a CDI lookup is performed with the required type equal to the bean | ||
* class of the bean to which the target method belongs, and required qualifiers equal to the set | ||
* of all qualifier annotations present on the bean class of the bean to which the target method | ||
* belongs. When the target method is {@code static}, the target instance lookup is skipped. | ||
* <p> | ||
* For an argument, a CDI lookup is performed with the required type equal to the type of | ||
* the corresponding parameter of the target method, and required qualifiers equal to the set | ||
* of all qualifier annotations present on the corresponding parameter of the target method. | ||
* <p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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 simplefunctional interface:
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:
private
;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 observerof the
ProcessManagedBean
type. TheInvokerBuilder
in this caseproduces
Invoker
objects directly.Build compatible extensions may obtain an
InvokerBuilder
during@Registration
from theBeanInfo
object. TheInvokerBuilder
inthis case produces
InvokerInfo
objects.InvokerInfo
is an opaquetoken that can only be used to materialize an
Invoker
in syntheticcomponents.
Initial proposal for method invokers: invoker builder features
This commit introduces actual interesting features to the
InvokerBuilder
: