[weld-dev] AfterBeanDiscovery and observer resolver cache
Christian Sadilek
csadilek at redhat.com
Wed Mar 19 17:18:49 EDT 2014
Thanks, Martin!
So, using @Observes (with an EventMetadata parameter) instead of a custom ObserverMethod is the way to go then for our CDI extension. Jozef's idea of having a subinterface with the overloaded notify method to avoid breaking backward compatibility would have been a nice solution though :).
Cheers,
Christian
On 2014-03-19, at 4:50 PM, Martin Kouba <mkouba at redhat.com> wrote:
> Hi Christian,
>
> in CDI 1.1 you can use the EventMetadata interface [1]. But it's not
> possible to inspect metadata in a custom observer notify() method. At
> some point, there was a notify(T event, Set<Annotation> qualifiers)
> method, but it was removed because of backwards compatibility [2].
>
> M
>
> [1]
> http://docs.jboss.org/cdi/spec/1.1/cdi-spec.html#event_metadata
> [2]
> https://issues.jboss.org/browse/CDI-281
>
>
> Dne 19.3.2014 19:46, Christian Sadilek napsal(a):
>> Hi guys,
>>
>> I have to follow up on this because our proposed solution using a single ObserverMethod for @Any and Object.class won't work. The problem is that the notify() method will only provide the event object but not its actual qualifiers. So, I won't be able to dispatch to the corresponding observers since I don't know which qualifiers the event actually has.
>>
>> So, taking away the ability to register observer methods on ABD after the bootstrap process will cause a critical problem for us that is not resolvable.
>>
>> Is it possible to enhance or overload notify() in ObserverMethod to contain the actual set of qualifiers as parameter (I think providing this metadata would be good from an API perspective anyway)?
>>
>> If that's not possible, can we rethink the options I listed below for clearing the observer caches?
>>
>> Can you think of other solutions?
>>
>> Thanks,
>> Christian
>>
>> On 2014-03-06, at 10:11 AM, Martin Kouba <mkouba at redhat.com> wrote:
>>
>>> Dne 6.3.2014 15:51, Christian Sadilek napsal(a):
>>>> Hi guys,
>>>>
>>>> Fair enough. This is the outcome I expected. I do think, however, that
>>>> the docs for ABD need some clarification here and ideally the ABD
>>>> instance should be "deactivated" once the bootstrap process finished so
>>>> it can throw an exception when e.g. addObserverMethod is invoked too
>>>> late in the game (better than silently having no effect).
>>>
>>> +1
>>> I've created https://issues.jboss.org/browse/CDI-425 to address this issue.
>>>
>>>>
>>>> Thanks for your answers!
>>>>
>>>> Cheers,
>>>> Christian
>>>>
>>>> On 2014-03-06, at 5:24 AM, Jozef Hartinger <jharting at redhat.com
>>>> <mailto:jharting at redhat.com>> wrote:
>>>>
>>>>> Yes, I don't think Errai should depend on this behavior as it is not
>>>>> even in the "undefined" category but rather in "implicitly not
>>>>> allowed" or "not the intention of the spec" category.
>>>>>
>>>>> Even from the Weld point of view we use two levels of caching and
>>>>> while clearing the main one is easy, dealing with the second layer
>>>>> would require adding further complexity just to support this corner case.
>>>>>
>>>>> Therefore, I would suggest to use the other approach you proposed
>>>>> where you register a general observer method and dispatch to the
>>>>> dynamically added observers within there. It is always hard to guess
>>>>> performance implications without doing measurements but as long as the
>>>>> general observer method implementation is efficient, I would not worry
>>>>> about that one additional method invocation much.
>>>>>
>>>>> Jozef
>>>>>
>>>>> On 03/06/2014 10:04 AM, Mark Struberg wrote:
>>>>>> Hi Christian!
>>>>>>
>>>>>> While I find it nice that this works with OWB I also have to agree
>>>>>> that this is not a guaranteed behaviour by the spec intention.
>>>>>> What you could do in to hack around this issue is to have an
>>>>>> @Observes at Any Object method which delivers the events with your own
>>>>>> dynamic rules.
>>>>>> But be prepared that this might slow down your app a bit.
>>>>>>
>>>>>> LieGrue,
>>>>>> strub
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wednesday, 5 March 2014, 17:04, Christian Sadilek
>>>>>> <csadilek at redhat.com> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Yes, I expected this to not be an officially supported use case.
>>>>>> So, I guess it's just a matter of improving the API
>>>>>> design/documentation.
>>>>>>
>>>>>> However, dynamically registering an observer method would really
>>>>>> be useful in the case of Errai where we set up an event bridge
>>>>>> between the server and the client and potentially discover new
>>>>>> observers at runtime.
>>>>>>
>>>>>> We could work around this by registering an observer method that
>>>>>> observes all events and then handle the dispatching internally
>>>>>> but that seems less efficient. Right now this works because
>>>>>> OpenWebBeans doesn't cache the observers and allows invocations
>>>>>> to AfterBeanDiscovery.addObserverMethod at runtime and because
>>>>>> Weld has the functionality to clear the observer cache (although
>>>>>> that's not public API).
>>>>>>
>>>>>> My questions is: Is there a good reason not to support this going
>>>>>> forward? Can we add alternative functionality to add observer
>>>>>> methods at runtime (not using ABD)?
>>>>>>
>>>>>> Cheers,
>>>>>> Christian
>>>>>>
>>>>>> On 2014-03-05, at 4:37 AM, Martin Kouba <mkouba at redhat.com
>>>>>> <mailto:mkouba at redhat.com>> wrote:
>>>>>>
>>>>>>> FYI I've informed CDI EG and it will be discussed on the next
>>>>>> meeting
>>>>>>> whether to clarify this already in CDI 1.2 MR...
>>>>>>>
>>>>>>> M
>>>>>>>
>>>>>>> Dne 5.3.2014 10:19, Jozef Hartinger napsal(a):
>>>>>>>> Agreed. It is definitely not the intention of the
>>>>>> specification to allow
>>>>>>>> beans/observers/contexts to be added at runtime and
>>>>>> applications should
>>>>>>>> not have any expectations of what these methods do when
>>>>>> invoked outside
>>>>>>>> of the AfterBeanDiscovery observer.
>>>>>>>>
>>>>>>>> We'll add stricter validation to Weld to disallow this.
>>>>>>>>
>>>>>>>> On 03/05/2014 08:53 AM, Martin Kouba wrote:
>>>>>>>>> Hi Christian,this
>>>>>>>>>
>>>>>>>>> actually invoking any container lifecycle event method after the
>>>>>>>>> container initialization finished should have no effect. ABD
>>>>>> event
>>>>>>>>> reference can escape but it does not mean you can invoke
>>>>>> ABD.addBean()
>>>>>>>>> after ADV is fired.
>>>>>>>>>
>>>>>>>>> The spec wording is not very explicit here:
>>>>>>>>> "During the application initialization process, the container
>>>>>> fires a
>>>>>>>>> series of events, allowing portable extensions to integrate with
>>>>>>>>> the container initialization process defined in Section 12.2."
>>>>>>>>>
>>>>>>>>> Maybe we should file a new spec issue to clarify that such
>>>>>> invocations
>>>>>>>>> should result in IllegalStateException...
>>>>>>>>>
>>>>>>>>> Martin
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Dne 4.3.2014 17:42, Christian Sadilek napsal(a):
>>>>>>>>>> Hi Jozef,
>>>>>>>>>>
>>>>>>>>>> I think clearing the cache at the end of the Weld bootstrap
>>>>>> process is not enough to solve that particular problem since a
>>>>>> CDI extension can hold on to the ABD reference and invoke
>>>>>> addObserverMethod later (multiple times) which causes the same
>>>>>> problem I described below. There's no indication to the caller of
>>>>>> addObserverMethod that it's in fact too late to call that method.
>>>>>>>>>>
>>>>>>>>>> Since an ABD event reference can always escape (can be used
>>>>>> outside the method that observes it) it seems this issue should
>>>>>> be resolved (although it admittedly is an edge case).
>>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>> Christian
>>>>>>>>>>
>>>>>>>>>> On 2014-03-04, at 11:29 AM, Jozef Hartinger
>>>>>> <jharting at redhat.com <mailto:jharting at redhat.com>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Christian,
>>>>>>>>>>>
>>>>>>>>>>> this sounds like a bug. All the resolution caches should be
>>>>>> cleared at the very end of Weld's bootstrap sequence (after ABD
>>>>>> observers are called). (see
>>>>>> https://github.com/weld/core/blob/master/impl/src/main/java/org/jboss/weld/bootstrap/WeldStartup.java#L415)
>>>>>>>>>>>
>>>>>>>>>>> Jozef
>>>>>>>>>>>
>>>>>>>>>>> On 03/04/2014 04:36 PM, Christian Sadilek wrote:
>>>>>>>>>>>> Hi everyone,
>>>>>>>>>>>>
>>>>>>>>>>>> CDI extensions can observe the AfterBeanDiscovery event to
>>>>>> register observer methods (addObserverMethod). However, when an
>>>>>> event is first fired, the observers for that event are resolved
>>>>>> and then cached (in TypeSafeResolver). All future calls to
>>>>>> addObserverMethod for an already fired event with corresponding
>>>>>> qualifiers will have no effect because the observer result is
>>>>>> read from cache and not recomputed.
>>>>>>>>>>>>
>>>>>>>>>>>>> From an API perspective that's unfortunate because
>>>>>> addObserverMethod will only work until an event (with
>>>>>> corresponding qualifiers) is fired and there is no indication to
>>>>>> the caller of that method that it didn't have any effect when
>>>>>> invoked after that.
>>>>>>>>>>>>
>>>>>>>>>>>> Possible solutions:
>>>>>>>>>>>>
>>>>>>>>>>>> - Provide some public API to clear/recompute that part the
>>>>>> observer cache. Maybe that exists? I couldn't find it which is
>>>>>> why I am using the private API and Reflection :(. Also let
>>>>>> AfterBeanDiscovery.addObserverMethod fail in that case with the
>>>>>> advice to reset the cache.
>>>>>>>>>>>>
>>>>>>>>>>>> - Recompute the corresponding part of the cache when
>>>>>> addObserverMethod is called (seems preferable).
>>>>>>>>>>>>
>>>>>>>>>>>> OpenWebBeans doesn't have this issue as their
>>>>>> NotificationManager will simply add the new ObserverMethod to a
>>>>>> ConcurrentHashMap that is also accessed when an event is fired.
>>>>>>>>>>>>
>>>>>>>>>>>> What do you think? Can this already be done or is there
>>>>>> another solution?
>>>>>>>>>>>>
>>>>>>>>>>>> Cheers,
>>>>>>>>>>>> Christian
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> weld-dev mailing list
>>>>>>>>>>>> weld-dev at lists.jboss.org <mailto:weld-dev at lists.jboss.org>
>>>>>>>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> weld-dev mailing list
>>>>>>>>>> weld-dev at lists.jboss.org <mailto:weld-dev at lists.jboss.org>
>>>>>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> weld-dev mailing list
>>>>>> weld-dev at lists.jboss.org <mailto:weld-dev at lists.jboss.org>
>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> weld-dev mailing list
>>>>>> weld-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev
>>>>>
>>>>
>>
>
> --
> Martin Kouba
> Software Engineer
> Red Hat, Czech Republic
More information about the weld-dev
mailing list