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/wel...) 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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/weld-dev

Hi Christian, 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(a)redhat.com&gt; 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/wel...) >> >> 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(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/weld-dev > > _______________________________________________ > weld-dev mailing list > weld-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/weld-dev >

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(a)redhat.com&gt; 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/wel...) >>>> >>>> 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(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>> >>> _______________________________________________ >>> weld-dev mailing list >>> weld-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/weld-dev >>> >

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(a)redhat.com&gt; 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(a)redhat.com&gt; 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/wel...) >>>>> >>>>> 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(a)lists.jboss.org >>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>> >>>> _______________________________________________ >>>> weld-dev mailing list >>>> weld-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>> >> _______________________________________________ weld-dev mailing list weld-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-dev

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@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(a)redhat.com&gt; 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(a)redhat.com&gt; 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(a)redhat.com&gt; 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/wel...) > >>>>> > >>>>> 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(a)lists.jboss.org > >>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev > >>>> > >>>> _______________________________________________ > >>>> weld-dev mailing list > >>>> weld-dev(a)lists.jboss.org > >>>> https://lists.jboss.org/mailman/listinfo/weld-dev > >>>> > >> > > > _______________________________________________ > weld-dev mailing list > weld-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/weld-dev > > > > > _______________________________________________ > weld-dev mailing list > weld-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/weld-dev

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(a)redhat.com > <mailto:jharting@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@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(a)redhat.com&gt; 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(a)redhat.com >>> <mailto:mkouba@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(a)redhat.com <mailto:jharting@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/wel...) >>>>>>>> >>>>>>>> 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(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>>>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>>>>> >>>>>>> _______________________________________________ >>>>>>> weld-dev mailing list >>>>>>> weld-dev(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>>>>> >>>>> >>> >>> >>> _______________________________________________ >>> weld-dev mailing list >>> weld-dev(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>> https://lists.jboss.org/mailman/listinfo/weld-dev >>> >>> >>> >>> >>> _______________________________________________ >>> weld-dev mailing list >>> weld-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/weld-dev >> >

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(a)redhat.com&gt; 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(a)redhat.com >>> <mailto:jharting@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@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(a)redhat.com&gt; 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(a)redhat.com >>>>> <mailto:mkouba@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(a)redhat.com <mailto:jharting@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/wel...) >>>>>>>>>> >>>>>>>>>> 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(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>>>>>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> weld-dev mailing list >>>>>>>>> weld-dev(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>>>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>>>>>>> >>>>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> weld-dev mailing list >>>>> weld-dev(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>>> >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> weld-dev mailing list >>>>> weld-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>> >>> > -- Martin Kouba Software Engineer Red Hat, Czech Republic