[cdi-dev] New builders API
Antonin Stefanutti
antonin at stefanutti.fr
Wed May 4 08:08:38 EDT 2016
> On 04 May 2016, at 13:01, Martin Kouba <mkouba at redhat.com> wrote:
>
>
> Dne 4.5.2016 v 11:57 Antonin Stefanutti napsal(a):
>> Hi Martin,
>>
>>> On 04 May 2016, at 10:43, Martin Kouba <mkouba at redhat.com> wrote:
>>>
>>> Dne 4.5.2016 v 10:26 Antonin Stefanutti napsal(a):
>>>> Hi Matej,
>>>>
>>>> Please find my answer inline.
>>>>
>>>> Let me add an additional point:
>>>>
>>>> It is not possible to add @Dependent programmatic bean whose production depends on the injection point.
>>>>
>>>> There is the BeanConfigurator<U> produceWith(Function<Instance<Object>, U> callback) method which is a smart way to be able to produce bean instances depending on other bean instances (like producer methods with injection points as parameters), but it cannot depend on the injection point for @Dependent beans.
>>>>
>>>> So similarly to:
>>>>
>>>> ObserverMethodConfigurator<T> notifyWith(BiConsumer<T, EventMetadata> callback);
>>>>
>>>> where EventMetadata is kind of the equivalent of InjectionPoint for event
>>>>
>>>> we could have:
>>>>
>>>> <U extends T> BeanConfigurator<U> produceWith(Function<InjectionPoint, U> callback);
>>>>
>>>> and:
>>>>
>>>> <U extends T> BeanConfigurator<U> produceWith(BiFunction<Instance<Object>, InjectionPoint, U> callback);
>>>
>>> Sound useful. On the other hand, it's a very specific use case - more or less simulating a producer method returning a dependent bean instance. For other scopes the IP would have to be null/absent. Also this is not possible with the current API either (at least not in a portable way - no IP available during Contextual.create()).
>>
>> I’d say it is the equivalent of:
>>
>> <U extends T> BeanConfigurator<U> produceWith(Function<Instance<Object>, U> callback);
>>
>> For @Dependent bean that rely on the InjectionPoint metadata.
>>
>> The point is that it is very convoluted to customise that kind of bean (producer method that depends on the injection point) in the AfterBeanDiscovery event (given the information is not available at the moment ProcessBeanAttributes is fired). Plus I don’t know of any way to add a programmatic bean that rely on the injection point metadata to create instances.
>>
>> In Camel CDI, here is the way it is done for every Camel primitives beans (like Endpoint, ProducerTemplate, ...):
>>
>> You have a producer method bean, like:
>>
>> @Produces
>> @Typed(MockEndpoint.class)
>> // Alternative is dynamically added in CdiCamelExtension
>> private static MockEndpoint mockEndpointFromMember(InjectionPoint ip, @Any Instance<CamelContext> instance, CdiCamelExtension extension) {
>> String uri = "mock:" + ip.getMember().getName();
>> return selectContext(ip, instance, extension).getEndpoint(uri, MockEndpoint.class);
>> }
>>
>> That bean needs to be customised as extra qualifiers need to be added in AfterBeanDiscovery. So the producer method is first vetoed and a new bean is added that way:
>>
>> private Bean<?> camelProducerBean(BeanManager manager, AnnotatedMethod<? super CdiCamelFactory> am, Set<Annotation> qualifiers) {
>> return manager.createBean(
>> new BeanDecorator<>(manager.createBeanAttributes(am), qualifiers),
>> CdiCamelFactory.class,
>> manager.getProducerFactory(am,
>> (Bean<CdiCamelFactory>) manager.resolve(manager.getBeans(CdiCamelFactory.class))));
>> }
>>
>> With that API, It could be done with:
>>
>> abd.addBean()
>> .addType(MockEndpoint.class)
>> .produceWith((instance, ip) -> selectContext(ip, instance, extension)
>> .getEndpoint("mock:" + ip.getMember().getName(), MockEndpoint.class));
>
> Yes, this looks nice. My point is that users might be confused if they get null/absent IP.
I’d rather have the container raise a DefinitionException in that case.
> Also I'm not sure the "configurator API" should be more powerful than SPI (i.e. Contextual.create()). It's true we allow to simulate producer methods with produceWith() but that's nothing you couldn't do within extension observer methods.
>
> Maybe we should address this on the SPI level first?
Indeed, the gap at the SPI level should ideally be addressed as well.
>>>> Antonin
>>>>
>>>>> On 03 May 2016, at 16:40, Matej Novotny <manovotn at redhat.com> wrote:
>>>>>
>>>>> Hello
>>>>>
>>>>>> - There is a read(AnnotatedType<U> type) API: equivalent API for
>>>>>> AnnotatedField and AnnotatedMethod would be useful
>>>>>
>>>>> Not sure I understand the use case here. You start with AnnotatedTypeConfigurator<T> and you can call, say, methods().
>>>>> That will give you a Set<AnnotatedMethodConfigurator<? super T>>. So the type of of MethodConfigurator is based on the type of the former configurator.
>>>>> Could you support your thoughts by a code snippet showing the usage?
>>>>
>>>> AfterBeanDiscovery.read(AnnotatedType<U> type) is useful to initialise a BeanConfigurator from an annotated type corresponding to a managed bean. In my use case, I’d need to be able to initialise a BeanConfigurator from an annotated method corresponding to a producer method bean. I can imagine that useful for annotated field as well.
>>>>
>>>>>> - Having the ability to customise the toString method by providing a
>>>>>> Supplier<String> would be useful
>>>>>
>>>>> This might be a good idea. Where exactly do you need this method? BeanConfigurator or even elsewhere?
>>>>
>>>> Yes BeanConfigurator. Ideally there should have a toString(Supplier<String>) and a toString(Function<Bean<T>>) method as it my case I rely on the bean information.
>>>>
>>>>>> It’d be useful to be able to reuse some InjectionTarget that are used for
>>>>>> discovered beans as well directly instead of having to call instantiate the
>>>>>> injection target and call produceWith and destroyWith separately
>>>>>
>>>>> Not sure I follow you here - produceWith is a way to provide a producer method for your bean (assuming we are talking about Beanconfigurator.produceWith).
>>>>> That merely tells CDI how to instantiate the bean if you are to inject it somewhere. You do not really need to handle InjectionTarget here. Same goes for destroyWith.
>>>>> I might be missing something though.
>>>>
>>>> In Camel CDI, there is a mix of discovered beans and programmatic beans to manage Camel context beans. They both should use the same InjectionTarget / Producer. So as the discovered Camel context beans have their InjectionTarget and Producer customised https://github.com/astefanutti/camel-cdi/blob/ea8b111d285ccb0cd18e08f45c9205ed852b2814/impl/src/main/java/org/apache/camel/cdi/CdiCamelExtension.java#L139-L145, the programmatic beans configured with addBean should reuse the came InjectionTarget / Producer:
>>>> https://github.com/astefanutti/camel-cdi/blob/ea8b111d285ccb0cd18e08f45c9205ed852b2814/impl/src/main/java/org/apache/camel/cdi/CamelContextProducer.java#L46
>>>> https://github.com/astefanutti/camel-cdi/blob/ea8b111d285ccb0cd18e08f45c9205ed852b2814/impl/src/main/java/org/apache/camel/cdi/CamelContextOsgiProducer.java
>>>>
>>>> So instead of having to do something like:
>>>> Producer customProducer = new CustomProducer();
>>>> abd.addBean().createWith(customProducer::produce).disposeWith(customProducer::dispose);
>>>>
>>>> I’d like to write:
>>>> abd.addBean().producer(customProducer);
>>>>
>>>>>> - I found a bit cumbersome to work with addObserverMethod: as the type
>>>>>> parameter is not driven by the observedType, generics hiccups come into play
>>>>>> with the notifyWith API. In my example, I had to rely on an extra method to
>>>>>> have the type parameter provided:
>>>>>
>>>>> As configurators are not reusable, your approach is imho perfectly right. If you were to add several observers, you need to call AfterBeanDiscovery.addObserver() several times anyway.
>>>>> It should be also possible to achieve the same things you have there without additional method (just within the for cycle). So I do not really see the 'generic hiccup' you are talking about.
>>>>
>>>> The point is that the following code doesn’t compile:
>>>>
>>>> cdiEventEndpoints.values().stream().forEach(endpoint -> abd.addObserverMethod()
>>>> .beanClass(CdiEventComponent.class)
>>>> .observedType(endpoint.getType())
>>>> .qualifiers(endpoint.getQualifiers())
>>>> .notifyWith(endpoint::notify);
>>>>
>>>> I need to write:
>>>>
>>>> cdiEventEndpoints.values().stream().forEach(endpoint -> addCdiEventObserver(abd, endpoint));
>>>>
>>>> private <T> void addCdiEventObserver(AfterBeanDiscovery abd, CdiEventEndpoint<T> endpoint) {
>>>> abd.<T>addObserverMethod()
>>>> .beanClass(CdiEventComponent.class)
>>>> .observedType(endpoint.getType())
>>>> .qualifiers(endpoint.getQualifiers())
>>>> .notifyWith(endpoint::notify);
>>>> }
>>>>
>>>> The code is available here:
>>>>
>>>> https://github.com/astefanutti/camel-cdi/blob/cdi-2.0/impl/src/main/java/org/apache/camel/cdi/CdiCamelExtension.java#L263-L271
>>>>
>>>> It’s similar to the issue with the example presented in Weld 3.0.0.Alpha16:
>>>>
>>>> void afterBeanDiscovery(@Observes AfterBeanDiscovery event) {
>>>> // Add a new synthetic observer method - no need to use the fluent API
>>>> ObserverMethodConfigurator configurator = event.addObserverMethod();
>>>> configurator.observedType(Foo.class);
>>>> configurator.reception(Reception.ALWAYS);
>>>> configurator.transactionPhase(TransactionPhase.IN_PROGRESS);
>>>> configurator.notifyWith((foo) -> System.out.println("Foo observed: " + foo));
>>>> }
>>>>
>>>> The problem is that the type captured in the lambda expression for “foo” is Object which is rather useless in most cases, while it should be Foo.
>>>>
>>>>> Btw please make sure to attend the CDI mtg in the evening. You can voice your needs/ideas there :)
>>>>>
>>>>> Matej
>>>>>
>>>>> ----- Original Message -----
>>>>>> From: "Antonin Stefanutti" <antonin at stefanutti.fr>
>>>>>> To: cdi-dev at lists.jboss.org
>>>>>> Sent: Tuesday, May 3, 2016 3:48:39 PM
>>>>>> Subject: Re: [cdi-dev] New builders API
>>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>> Not sure what’s the best form to share my feedback, so here it is :)
>>>>>>
>>>>>> I’ve tested CDI 2.0.Alpha4 / Weld 3.0.0.Alpha3 and the new "builder API" on
>>>>>> the Camel CDI extension and here are the points I’ve had gathered:
>>>>>>
>>>>>> - There is a read(AnnotatedType<U> type) API: equivalent API for
>>>>>> AnnotatedField and AnnotatedMethod would be useful
>>>>>> - Having the ability to customise the toString method by providing a
>>>>>> Supplier<String> would be useful
>>>>>> - There are API (createWith, produceWith, ...) that are basically the
>>>>>> decomposition of what an injection target is: in the Camel CDI use case,
>>>>>> It’d be useful to be able to reuse some InjectionTarget that are used for
>>>>>> discovered beans as well directly instead of having to call instantiate the
>>>>>> injection target and call produceWith and destroyWith separately
>>>>>> - I found a bit cumbersome to work with addObserverMethod: as the type
>>>>>> parameter is not driven by the observedType, generics hiccups come into play
>>>>>> with the notifyWith API. In my example, I had to rely on an extra method to
>>>>>> have the type parameter provided:
>>>>>> https://github.com/astefanutti/camel-cdi/blob/310ac823376d086c2c3a9a1b2dabda4be93d04f0/impl/src/main/java/org/apache/camel/cdi/CdiCamelExtension.java#L263-L271
>>>>>>
>>>>>> Unfortunately, I haven’t been able to get rid of the boilerplate code (like
>>>>>> SyntheticBean [1], SyntheticBeanAttributes [2], SyntheticInjectionTarget
>>>>>> [3]) that the new builder API would have permitted to eliminate. Indeed,
>>>>>> while the "Metadata configuration API" probably solves the majority /
>>>>>> simplest use cases, it does not address the one in Camel CDI (as already
>>>>>> illustrated by John and I from different angles).
>>>>>>
>>>>>> IMO, it’d be worth investigating on an unified Builder and configurator API
>>>>>> bringing the best of both worlds.
>>>>>>
>>>>>> The rewrite of Camel CDI on CDI 2.0 is available here:
>>>>>> https://github.com/astefanutti/camel-cdi/commits/cdi-2.0.
>>>>>>
>>>>>> [1]:
>>>>>> https://github.com/astefanutti/camel-cdi/blob/310ac823376d086c2c3a9a1b2dabda4be93d04f0/impl/src/main/java/org/apache/camel/cdi/SyntheticBean.java#L34
>>>>>> [2]:
>>>>>> https://github.com/astefanutti/camel-cdi/blob/310ac823376d086c2c3a9a1b2dabda4be93d04f0/impl/src/main/java/org/apache/camel/cdi/SyntheticBeanAttributes.java#L31
>>>>>> [3]:
>>>>>> https://github.com/astefanutti/camel-cdi/blob/310ac823376d086c2c3a9a1b2dabda4be93d04f0/impl/src/main/java/org/apache/camel/cdi/SyntheticInjectionTarget.java#L29
>>>>>>
>>>>>> Antonin
>>>>>>
>>>>>>> On 03 May 2016, at 15:14, Antoine Sabot-Durand <antoine at sabot-durand.net>
>>>>>>> wrote:
>>>>>>>
>>>>>>> Thanks for all the feedback on the API.
>>>>>>>
>>>>>>> Before going into detail I wanted to put the accent on the Weld 3.0.Alpha16
>>>>>>> release post[1] which give a good explanation regarding this API. Its the
>>>>>>> best starting point to discover this new API IMO. Thanks Martin, Tomas and
>>>>>>> Matej for this release.
>>>>>>>
>>>>>>> @John. The debate Configurator vs Builder is still open. At the moment we
>>>>>>> focused on configurator since they are easier to limit to a given
>>>>>>> container lifecycle, but having builders to reuse the configuration could
>>>>>>> make sense as well. AnnotatedType is a special case since it could make
>>>>>>> sense to use it at runtime (to ease creation of an InjectionTarget for
>>>>>>> instance, so perhaps it will require a Builder.
>>>>>>> If we keep the API in this spirit for the release, it should be better to
>>>>>>> talk about Configurator API vs Builder API to avoid confusion for missing
>>>>>>> build() methods ;)
>>>>>>>
>>>>>>> @Antonin the enhancement you propose is very interesting but as Martin said
>>>>>>> we could imagine a solution to load an existing bean after decision on the
>>>>>>> API (a mute(Bean) method in ABD perhaps).
>>>>>>>
>>>>>>> For me the goal is to decide if we are on the right path and avoid decision
>>>>>>> that will prevent future enhancement. I think having this API perfect and
>>>>>>> complete in one PR is almost impossible. So we should validate something
>>>>>>> to move on missing features or enhancements.
>>>>>>>
>>>>>>> Antoine
>>>>>>>
>>>>>>> [1] http://weld.cdi-spec.org/news/2016/04/28/weld-300Alpha16/
>>>>>>>
>>>>>>> Le jeu. 28 avr. 2016 à 10:43, Martin Kouba <mkouba at redhat.com> a écrit :
>>>>>>> Hi all,
>>>>>>>
>>>>>>> we've just released Weld 3.0.0.Alpha16. So anyone can start playing with
>>>>>>> the API, discover possibilities and find potential issues:
>>>>>>>
>>>>>>> http://weld.cdi-spec.org/news/2016/04/28/weld-300Alpha16/
>>>>>>>
>>>>>>> Any feedback is appreciated!
>>>>>>>
>>>>>>> Martin
>>>>>>>
>>>>>>> Dne 25.4.2016 v 09:30 Martin Kouba napsal(a):
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> me and Matej, we've already tried to explain some points in cdi/pull/287
>>>>>>>> discussion. Anyway, we're going to release Weld 3.0.0.Alpha16 (base on
>>>>>>>> 2.0.Alpha4) later this week so that everyone can start playing with the
>>>>>>>> new API. And we'd like to prepare some simple examples to help people
>>>>>>>> get started as well.
>>>>>>>>
>>>>>>>> Martin
>>>>>>>>
>>>>>>>>
>>>>>>>> Dne 23.4.2016 v 12:56 John D. Ament napsal(a):
>>>>>>>>> Hey guys
>>>>>>>>>
>>>>>>>>> Based on the last f2f I was in, I took an action item to look at how
>>>>>>>>> applications can leverage the new builder methods/classes from this PR:
>>>>>>>>> https://github.com/cdi-spec/cdi/pull/287
>>>>>>>>>
>>>>>>>>> To do this, I took some existing OSS CDI extensions and converted parts
>>>>>>>>> to use the new APIs instead of the old ones.
>>>>>>>>>
>>>>>>>>> The results were iffy to be honest. Here's some of the key issues I
>>>>>>>>> noticed:
>>>>>>>>>
>>>>>>>>> - AfterBeanDiscovery#addBean - vs AfterBeanDiscovery.addBean(Bean<?>
>>>>>>>>> bean)
>>>>>>>>> In the latter, it's clearer to a developer which attributes are required
>>>>>>>>> vs optional. Builders typically use sensible defaults. Maybe that was
>>>>>>>>> the intention here, but I couldn't really get that sense when converting
>>>>>>>>> over. It also wasn't clear what to do when done. I suspect I just
>>>>>>>>> leave it, but without some kind of closing "build()" or "done()" method,
>>>>>>>>> it becomes ambiguous.
>>>>>>>>>
>>>>>>>>> - Annotated*Configurator
>>>>>>>>> TBH, I have no idea what I was configuring in this one at the first
>>>>>>>>> pass. I started with a method. I wanted to replace the method's
>>>>>>>>> annotations. It seemed like I could set that up using the configurator,
>>>>>>>>> but I ended up having to do setAnnotated at the end anyways, so I'm not
>>>>>>>>> sure what the configurator bought me.
>>>>>>>>>
>>>>>>>>> The one nice thing I saw was the simpler to use lambda functions. Being
>>>>>>>>> able to stream through things like annotated method made the code much
>>>>>>>>> cleaner.
>>>>>>>>>
>>>>>>>>> For the open source code, I'll try to get some gists together that show
>>>>>>>>> the changes. Maybe there's something I'm missing, so wouldn't mind a
>>>>>>>>> second set of eyes on the changes to see.
>>>>>>>>>
>>>>>>>>> John
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> cdi-dev mailing list
>>>>>>>>> cdi-dev at lists.jboss.org
>>>>>>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>>>>>>>
>>>>>>>>> Note that for all code provided on this list, the provider licenses the
>>>>>>>>> code under the Apache License, Version 2
>>>>>>>>> (http://www.apache.org/licenses/LICENSE-2.0.html). For all other ideas
>>>>>>>>> provided on this list, the provider waives all patent and other
>>>>>>>>> intellectual property rights inherent in such information.
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Martin Kouba
>>>>>>> Software Engineer
>>>>>>> Red Hat, Czech Republic
>>>>>>> _______________________________________________
>>>>>>> cdi-dev mailing list
>>>>>>> cdi-dev at lists.jboss.org
>>>>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>>>>>
>>>>>>> Note that for all code provided on this list, the provider licenses the
>>>>>>> code under the Apache License, Version 2
>>>>>>> (http://www.apache.org/licenses/LICENSE-2.0.html). For all other ideas
>>>>>>> provided on this list, the provider waives all patent and other
>>>>>>> intellectual property rights inherent in such information.
>>>>>>> _______________________________________________
>>>>>>> cdi-dev mailing list
>>>>>>> cdi-dev at lists.jboss.org
>>>>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>>>>>
>>>>>>> Note that for all code provided on this list, the provider licenses the
>>>>>>> code under the Apache License, Version 2
>>>>>>> (http://www.apache.org/licenses/LICENSE-2.0.html). For all other ideas
>>>>>>> provided on this list, the provider waives all patent and other
>>>>>>> intellectual property rights inherent in such information.
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cdi-dev mailing list
>>>>>> cdi-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>>>>
>>>>>> Note that for all code provided on this list, the provider licenses the code
>>>>>> under the Apache License, Version 2
>>>>>> (http://www.apache.org/licenses/LICENSE-2.0.html). For all other ideas
>>>>>> provided on this list, the provider waives all patent and other intellectual
>>>>>> property rights inherent in such information.
>>>>
>>>>
>>>> _______________________________________________
>>>> cdi-dev mailing list
>>>> cdi-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>>
>>>> Note that for all code provided on this list, the provider licenses the code under the Apache License, Version 2 (http://www.apache.org/licenses/LICENSE-2.0.html). For all other ideas provided on this list, the provider waives all patent and other intellectual property rights inherent in such information.
>>>>
>>>
>>> --
>>> Martin Kouba
>>> Software Engineer
>>> Red Hat, Czech Republic
>>
>
> --
> Martin Kouba
> Software Engineer
> Red Hat, Czech Republic
More information about the cdi-dev
mailing list