<font size=2 face="sans-serif">Thank you! <br>
<br>
Best regards<br>
Benjamin</font>
<br>
<br>
<br>
<br><font size=1 color=#5f5f5f face="sans-serif">From:
</font><font size=1 face="sans-serif">Matej Novotny <manovotn@redhat.com></font>
<br><font size=1 color=#5f5f5f face="sans-serif">To:
</font><font size=1 face="sans-serif">Benjamin Confino <BENJAMIC@uk.ibm.com></font>
<br><font size=1 color=#5f5f5f face="sans-serif">Cc:
</font><font size=1 face="sans-serif">Takayuki T Ishii <EBB0F3L@jp.ibm.com>,
weld-dev@lists.jboss.org</font>
<br><font size=1 color=#5f5f5f face="sans-serif">Date:
</font><font size=1 face="sans-serif">04/02/2020 10:32</font>
<br><font size=1 color=#5f5f5f face="sans-serif">Subject:
</font><font size=1 face="sans-serif">[EXTERNAL] Re:
[weld-dev] Question about conversations scope initilization
obeserver</font>
<br>
<hr noshade>
<br>
<br>
<br><tt><font size=2>There should be no functional difference between the
code in Weld 2.4 and 3.x in regards to the conversation @Init events (apart
from the code being in different modules).<br>
So your change should work just fine. Our CI passed too and I've merged
it.<br>
<br>
Matej<br>
<br>
----- Original Message -----<br>
> From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> To: "Matej Novotny" <manovotn@redhat.com><br>
> Cc: "Takayuki T Ishii" <EBB0F3L@jp.ibm.com>, weld-dev@lists.jboss.org<br>
> Sent: Monday, February 3, 2020 4:56:14 PM<br>
> Subject: RE: [weld-dev] Question about conversations scope initilization
obeserver<br>
> <br>
> Hello Matej,<br>
> <br>
> Thank you for the fix, I tested it and it worked perfectly. I also
created<br>
> a pull request to backport it to 2.4:<br>
> </font></tt><a href="https://github.com/weld/core/pull/1964"><tt><font size=2>https://github.com/weld/core/pull/1964</font></tt></a><tt><font size=2>
which has also worked locally for<br>
> me.<br>
> <br>
> My client is currently on CDI-1.2/Weld-2.4 so even if you don't want
to<br>
> patch 2.4 at this point I may need to give them a one-of patch until
they<br>
> are ready to move up to CDI-2.0; so if there's any fatal flaw in that<br>
> backport please let me know.<br>
> <br>
> Best regards<br>
> Benjamin<br>
> <br>
> <br>
> <br>
> From: Matej Novotny <manovotn@redhat.com><br>
> To: Benjamin Confino <BENJAMIC@uk.ibm.com><br>
> Cc: Takayuki T Ishii <EBB0F3L@jp.ibm.com>, weld-dev@lists.jboss.org<br>
> Date: 31/01/2020 17:09<br>
> Subject: [EXTERNAL] Re: [weld-dev] Question
about conversations<br>
> scope initilization obeserver<br>
> <br>
> <br>
> <br>
> Hello,<br>
> <br>
> so after some tedious debugging (and fair amount of time figuring
out how<br>
> lazy conversation work in this case) I managed to get to the state
you<br>
> were seeing.<br>
> I think this is a bug - Weld doesn't fire @Initialized event in case
where<br>
> user attempts to restore non-existing conversation.<br>
> We correctly associate the request with new conversation before the<br>
> exception is thrown (which is what spec requires and tests) but we
don't<br>
> fire the event.<br>
> <br>
> Issue is here -<br>
> </font></tt><a href="https://issues.redhat.com/browse/WELD-2611"><tt><font size=2>https://issues.redhat.com/browse/WELD-2611</font></tt></a><tt><font size=2><br>
> <br>
> And I've proposed a fix here -<br>
> </font></tt><a href="https://github.com/weld/core/pull/1962"><tt><font size=2>https://github.com/weld/core/pull/1962</font></tt></a><tt><font size=2><br>
> <br>
> <br>
> If you could try that and tell me if it works, that would be great.<br>
> Although I did use the same reproducer, so hopefully it'll work ;-)<br>
> <br>
> Regards and have a nice weekend!<br>
> Matej<br>
> <br>
> ----- Original Message -----<br>
> > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> > To: "Matej Novotny" <manovotn@redhat.com><br>
> > Cc: "Takayuki T Ishii" <EBB0F3L@jp.ibm.com>,
weld-dev@lists.jboss.org<br>
> > Sent: Friday, January 31, 2020 1:20:50 AM<br>
> > Subject: RE: [weld-dev] Question about conversations scope initilization<br>
> obeserver<br>
> > <br>
> > Hello Matej<br>
> > <br>
> > After testing adding a call to `bean.getMsg` in the catch block,
the<br>
> > behaviour is unchanged. I did some further digging and here's
what I<br>
> > found:<br>
> > <br>
> > On a fresh start of the sever I ping the url with a nonsense
cid.<br>
> > <br>
> > ConversationBean will call conversation.begin() in the try block.
This<br>
> > triggers a codepath that leads to<br>
> > LazyHttpConversationContextImpl.checkContextInitialized() line
124, that<br>
> > line will throw an exception. We go back out to ConversationBean
where<br>
> the<br>
> > exception is caught. Then when the catch block calls<br>
> conversation.begin()<br>
> > it will once again reach<br>
> > LazyHttpConversationContextImpl.checkContextInitialized() but
this time<br>
> > the if statement on line 121 returns true and so we never call<br>
> > initialize(). In both cases it is the same<br>
> > LazyHttpConversationContextImpl.<br>
> > <br>
> > I also put a breakpoint in the observer and pinged the URL without<br>
> > manually specificing a cid. From inside the observer method I
can see<br>
> that<br>
> > LazyHttpConversationContextImpl.checkContextInitialized() line
128 is on<br>
> > the stack.<br>
> > <br>
> > <br>
> > So to summarise. When I call the url with a nonsense cdi: The
try block<br>
> > reaches checkContextInitialized and gets an exception on line
121. Then<br>
> > the catch block reaches checkContextInitialized and does nothing
because<br>
> > isInitialized() returns true. Thus neither attempt reaches line
128 and<br>
> > the observer method is never fied.<br>
> > <br>
> > This feels like a bug to me, not just because the observer isn't
fired<br>
> but<br>
> > also because if the initilization method had an exception half
way<br>
> though<br>
> > is it left in a good state? I don't know enough about these weld<br>
> internals<br>
> > to check.<br>
> > <br>
> > It occurs to me that one possible fix is to swap line 89 with
line 90 so<br>
> > that the exception takes place before initilized is set to true.
Of<br>
> course<br>
> > that assumes that running initilized twice won't cause worse
problems.<br>
> > <br>
> > What do you think? Is this a bug?<br>
> > <br>
> > <br>
> > <br>
> > From: Matej Novotny <manovotn@redhat.com><br>
> > To: Benjamin Confino <BENJAMIC@uk.ibm.com><br>
> > Cc: Takayuki T Ishii <EBB0F3L@jp.ibm.com>,
weld-dev@lists.jboss.org<br>
> > Date: 28/01/2020 15:03<br>
> > Subject: [EXTERNAL] Re: [weld-dev]
Question about conversations<br>
> > scope initilization obeserver<br>
> > <br>
> > <br>
> > <br>
> > Hello,<br>
> > <br>
> > I think I know what is the "problem" here.<br>
> > Weld uses lazy conversation init - that means we don't activate
context<br>
> > until you try and access a conversation scoped bean.<br>
> > <br>
> > Now, in your example, the ConversationBean tries to begin() a<br>
> > conversation, then calls the bean (which initializes the context
and<br>
> > notifies the observer).<br>
> > However, in the situation where you try and pass in a non-existing<br>
> > conversation, the invocation to conversation.begin()[1] will
blow up<br>
> with<br>
> > NonExistingConversationException and<br>
> > you will jump right into the catch block[2] where you begin a<br>
> conversation<br>
> > with given ID, but you no longer invoke the bean, hence the context<br>
> won't<br>
> > get activated.<br>
> > Try adding the `bean.getMsg()` call to the catch block and see
if that<br>
> > helps.<br>
> > <br>
> > Note that CDI spec sets no requirements on how/when to activate
the<br>
> > conversation context, so the lazy behaviour is compliant with
spec (and<br>
> > this is also why you saw no such test in TCKs).<br>
> > <br>
> > Regards<br>
> > Matej<br>
> > <br>
> > <br>
> _________________________________________________________________________________<br>
> > [1]<br>
> > <br>
> </font></tt><a href="https://gist.github.com/manovotn/b9e9fde25ab77b5e481d5b34edf02b0c#file-conversationbean-java-L4"><tt><font size=2>https://gist.github.com/manovotn/b9e9fde25ab77b5e481d5b34edf02b0c#file-conversationbean-java-L4</font></tt></a><tt><font size=2><br>
> <br>
> > <br>
> > [2]<br>
> > <br>
> </font></tt><a href="https://gist.github.com/manovotn/b9e9fde25ab77b5e481d5b34edf02b0c#file-conversationbean-java-L10-L16"><tt><font size=2>https://gist.github.com/manovotn/b9e9fde25ab77b5e481d5b34edf02b0c#file-conversationbean-java-L10-L16</font></tt></a><tt><font size=2><br>
> <br>
> > <br>
> > <br>
> > <br>
> > <br>
> > ----- Original Message -----<br>
> > > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> > > To: "Matej Novotny" <manovotn@redhat.com><br>
> > > Cc: "Takayuki T Ishii" <EBB0F3L@jp.ibm.com>,
weld-dev@lists.jboss.org<br>
> > > Sent: Tuesday, January 28, 2020 12:06:14 PM<br>
> > > Subject: RE: [weld-dev] Question about conversations scope<br>
> initilization<br>
> > obeserver<br>
> > > <br>
> > > Hello<br>
> > > <br>
> > > Thanks for the link. I had a look but I couldn't find any
TCK tests<br>
> > > checking to see if an observer method will catch the new<br>
> > > ConversationContext being created for the "new transient<br>
> conversation".<br>
> > To<br>
> > > check if a new conversation was activated I created an entirely
fresh<br>
> > > server and ran the test application on it, the behaviour
was the same,<br>
> > the<br>
> > > first url I pinged on this new server ended with "cid="
and the<br>
> observer<br>
> > > didn't . Normally I've just been restarting the old server
but<br>
> > restarting<br>
> > > frequently.<br>
> > > <br>
> > > I've attached the recreate you requested. it consists of
the three<br>
> > classes<br>
> > > attached to my previous email as well as a minimal html
page. To run<br>
> it<br>
> > > load it onto your server and ping<br>
> > > <br>
> > <br>
> </font></tt><a href="http://localhost:9080/ConversationContextTest/index.xhtml"><tt><font size=2>http://localhost:9080/ConversationContextTest/index.xhtml</font></tt></a><tt><font size=2><br>
> <br>
> > to see the<br>
> > > observer fire, and<br>
> > > <br>
> > <br>
> </font></tt><a href="http://localhost:9080/ConversationContextTest/index.xhtml?cid=99999"><tt><font size=2>http://localhost:9080/ConversationContextTest/index.xhtml?cid=99999</font></tt></a><tt><font size=2><br>
> <br>
> > to see<br>
> > > the observer fail to fire.<br>
> > > <br>
> > > Best regards<br>
> > > Benjamin<br>
> > > <br>
> > > <br>
> > > <br>
> > > From: Matej Novotny <manovotn@redhat.com><br>
> > > To: Benjamin Confino <BENJAMIC@uk.ibm.com><br>
> > > Cc: weld-dev@lists.jboss.org, Takayuki T Ishii<br>
> <EBB0F3L@jp.ibm.com><br>
> > > Date: 27/01/2020 11:39<br>
> > > Subject: [EXTERNAL] Re: [weld-dev]
Question about conversations<br>
> > > scope initilization obeserver<br>
> > > <br>
> > > <br>
> > > <br>
> > > Hello,<br>
> > > <br>
> > > I'd start by pointing you to CDI TCK as that's a good starting
point<br>
> to<br>
> > > see what's covered.<br>
> > > For your question, that would be this test -<br>
> > > <br>
> > <br>
> </font></tt><a href="https://github.com/eclipse-ee4j/cdi-tck/blob/master/impl/src/main/java/org/jboss/cdi/tck/tests/context/conversation/ClientConversationContextTest.java#L344-L349"><tt><font size=2>https://github.com/eclipse-ee4j/cdi-tck/blob/master/impl/src/main/java/org/jboss/cdi/tck/tests/context/conversation/ClientConversationContextTest.java#L344-L349</font></tt></a><tt><font size=2><br>
> <br>
> > <br>
> > > <br>
> > > And possibly few more in the same test class.<br>
> > > <br>
> > > As for the linked classes - your `ConversationBean` is listening
for<br>
> > > @Initialized event. Can you verify that a new conversation
was<br>
> activated<br>
> > > for your request instead of verifying that context was activated?<br>
> > > E.g. check IDs or something along those lines? I suppose
that will<br>
> hold<br>
> > > true and in that case it works just as spec requires it
to.<br>
> > > From the top of my head I don't really know how we activate/deactivate<br>
> > > ConversationContext, I'd need to dig that up, but looking
at CDI spec,<br>
> > it<br>
> > > doesn't mandate that it is activated every time again and
it could<br>
> > already<br>
> > > be active for given request.<br>
> > > Plus from just the classes you linked, I cannot know if
you test this<br>
> > with<br>
> > > no existing conversation or maybe with some long running
one before<br>
> you<br>
> > > try to send a request for non-existing one...and so on.<br>
> > > So if the above doesn't is not enough to answer your question,
then<br>
> > we're<br>
> > > going to need a complete reproducer so that we both talk
about the<br>
> same<br>
> > > scenario :)<br>
> > > <br>
> > > Matej<br>
> > > <br>
> > > ----- Original Message -----<br>
> > > > From: "Benjamin Confino" <BENJAMIC@uk.ibm.com><br>
> > > > To: weld-dev@lists.jboss.org<br>
> > > > Cc: "Takayuki T Ishii" <EBB0F3L@jp.ibm.com><br>
> > > > Sent: Monday, January 27, 2020 11:42:14 AM<br>
> > > > Subject: [weld-dev] Question about conversations scope
initilization<br>
> > > obeserver<br>
> > > > <br>
> > > > Hello<br>
> > > > <br>
> > > > I have a customer who's sent me a sample application,
I have<br>
> attached<br>
> > > the<br>
> > > > source to it below.<br>
> > > > <br>
> > > > When the customer visits index.xhtml they see the following
output:<br>
> > > > <br>
> > > > Conversation initialized.<br>
> > > > Conversation begun. </font></tt><a href="cid:1"><tt><font size=2>cid:1</font></tt></a><tt><font size=2>
, timeout:3600000<br>
> > > > Conversation destroyed. </font></tt><a href="cid:1"><tt><font size=2>cid:1</font></tt></a><tt><font size=2><br>
> > > > <br>
> > > > However when they append "?cdi=" or a non-existnant
identifier like<br>
> > > > "?cdi=10000" to the url they do not see "Conversation
initialized."<br>
> > > > <br>
> > > > The CDI spec says that: If the propagated conversation
cannot be<br>
> > > restored,<br>
> > > > the container must associate the request with a new
transient<br>
> > > conversation<br>
> > > > and throw an exception of type<br>
> > > > javax.enterprise.context.NonexistentConversationException.<br>
> > > > <br>
> > > > I'm wondering if this should apply here? Or would it
only apply if<br>
> the<br>
> > > cid<br>
> > > > pointed to an existing conversation that could not
be restored? And<br>
> is<br>
> > > there<br>
> > > > anything in the spec that covers this specific situation?<br>
> > > > <br>
> > > > Unless stated otherwise above:<br>
> > > > IBM United Kingdom Limited - Registered in England
and Wales with<br>
> > number<br>
> > > > 741598.<br>
> > > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire<br>
> PO6<br>
> > > 3AU<br>
> > > > <br>
> > > > _______________________________________________<br>
> > > > weld-dev mailing list<br>
> > > > weld-dev@lists.jboss.org<br>
> > > > <br>
> > > <br>
> > <br>
> </font></tt><a href="https://lists.jboss.org/mailman/listinfo/weld-dev"><tt><font size=2>https://lists.jboss.org/mailman/listinfo/weld-dev</font></tt></a><tt><font size=2><br>
> <br>
> > <br>
> > > <br>
> > > <br>
> > > <br>
> > > <br>
> > > <br>
> > > Unless stated otherwise above:<br>
> > > IBM United Kingdom Limited - Registered in England and Wales
with<br>
> number<br>
> > > 741598.<br>
> > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire PO6<br>
> > 3AU<br>
> > > <br>
> > <br>
> > <br>
> > <br>
> > <br>
> > Unless stated otherwise above:<br>
> > IBM United Kingdom Limited - Registered in England and Wales
with number<br>
> > 741598.<br>
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6<br>
> 3AU<br>
> > <br>
> <br>
> <br>
> <br>
> <br>
> Unless stated otherwise above:<br>
> IBM United Kingdom Limited - Registered in England and Wales with
number<br>
> 741598.<br>
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU<br>
> <br>
<br>
</font></tt>
<br>
<br><font size=2 face="sans-serif"><br>
Unless stated otherwise above:<br>
IBM United Kingdom Limited - Registered in England and Wales with number
741598. <br>
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
3AU<br>
</font>