<font size=2 face="sans-serif">Hello Matej,<br>
<br>
Thank you for the fix, I tested it and it worked perfectly. I also created
a pull request to backport it to 2.4: </font><a href="https://github.com/weld/core/pull/1964"><font size=2 color=blue face="sans-serif">https://github.com/weld/core/pull/1964</font></a><font size=2 face="sans-serif">
which has also worked locally for me. <br>
<br>
My client is currently on CDI-1.2/Weld-2.4 so even if you don't want to
patch 2.4 at this point I may need to give them a one-of patch until they
are ready to move up to CDI-2.0; so if there's any fatal flaw in that backport
please let me know. <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">31/01/2020 17:09</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>Hello,<br>
<br>
so after some tedious debugging (and fair amount of time figuring out how
lazy conversation work in this case) I managed to get to the state you
were seeing.<br>
I think this is a bug - Weld doesn't fire @Initialized event in case where
user attempts to restore non-existing conversation.<br>
We correctly associate the request with new conversation before the exception
is thrown (which is what spec requires and tests) but we don't fire the
event.<br>
<br>
Issue is here - </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>
And I've proposed a fix here - </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>
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
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
the<br>
> exception is caught. Then when the catch block calls 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 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
but<br>
> also because if the initilization method had an exception half way
though<br>
> is it left in a good state? I don't know enough about these weld 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
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 with<br>
> NonExistingConversationException and<br>
> you will jump right into the catch block[2] where you begin a conversation<br>
> with given ID, but you no longer invoke the bean, hence the context
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>
> [1]<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>
> [2]<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>
> ----- 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 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
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 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 it<br>
> > load it onto your server and ping<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>
> to see the<br>
> > observer fire, and<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>
> 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
<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 to<br>
> > see what's covered.<br>
> > For your question, that would be this test -<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>
> > 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
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 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 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 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 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 the<br>
> > cid<br>
> > > pointed to an existing conversation that could not be restored?
And 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 PO6<br>
> > 3AU<br>
> > > <br>
> > > _______________________________________________<br>
> > > weld-dev mailing list<br>
> > > weld-dev@lists.jboss.org<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>
> > 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>