[weld-dev] Question about conversations scope initilization obeserver

Tue Feb 4 05:32:09 EST 2020

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).
So your change should work just fine. Our CI passed too and I've merged it.

Matej

----- Original Message -----
> From: "Benjamin Confino" <BENJAMIC at uk.ibm.com>
> To: "Matej Novotny" <manovotn at redhat.com>
> Cc: "Takayuki T Ishii" <EBB0F3L at jp.ibm.com>, weld-dev at lists.jboss.org
> Sent: Monday, February 3, 2020 4:56:14 PM
> Subject: RE: [weld-dev] Question about conversations scope initilization	obeserver
> 
> Hello Matej,
> 
> Thank you for the fix, I tested it and it worked perfectly. I also created
> a pull request to backport it to 2.4:
> https://github.com/weld/core/pull/1964 which has also worked locally for
> me.
> 
> 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.
> 
> Best regards
> Benjamin
> 
> 
> 
> From:   Matej Novotny <manovotn at redhat.com>
> To:     Benjamin Confino <BENJAMIC at uk.ibm.com>
> Cc:     Takayuki T Ishii <EBB0F3L at jp.ibm.com>, weld-dev at lists.jboss.org
> Date:   31/01/2020 17:09
> Subject:        [EXTERNAL] Re: [weld-dev] Question about conversations
> scope initilization     obeserver
> 
> 
> 
> Hello,
> 
> 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.
> I think this is a bug - Weld doesn't fire @Initialized event in case where
> user attempts to restore non-existing conversation.
> 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.
> 
> Issue is here -
> https://urldefense.proofpoint.com/v2/url?u=https-3A__issues.redhat.com_browse_WELD-2D2611&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=I2oNI10g3jJNWR8G4bWqGBIogKtn74fZP9erCJAhSKc&s=JCp1S9H4SIr2woz-CmeHoB0qwihK-ZLlcwdOpCBolWg&e=
> 
> And I've proposed a fix here -
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_weld_core_pull_1962&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=I2oNI10g3jJNWR8G4bWqGBIogKtn74fZP9erCJAhSKc&s=pRopCkOBojQyfJkGsttlD8g8DLGGQc7XkAdwnP_oQs8&e=
> 
> 
> If you could try that and tell me if it works, that would be great.
> Although I did use the same reproducer, so hopefully it'll work ;-)
> 
> Regards and have a nice weekend!
> Matej
> 
> ----- Original Message -----
> > From: "Benjamin Confino" <BENJAMIC at uk.ibm.com>
> > To: "Matej Novotny" <manovotn at redhat.com>
> > Cc: "Takayuki T Ishii" <EBB0F3L at jp.ibm.com>, weld-dev at lists.jboss.org
> > Sent: Friday, January 31, 2020 1:20:50 AM
> > Subject: RE: [weld-dev] Question about conversations scope initilization
>          obeserver
> > 
> > Hello Matej
> > 
> > After testing adding a call to `bean.getMsg` in the catch block, the
> > behaviour is unchanged. I did some further digging and here's what I
> > found:
> > 
> > On a fresh start of the sever I ping the url with a nonsense cid.
> > 
> > ConversationBean will call conversation.begin() in the try block. This
> > triggers a codepath that leads to
> > LazyHttpConversationContextImpl.checkContextInitialized() line 124, that
> > line will throw an exception. We go back out to ConversationBean where
> the
> > exception is caught. Then when the catch block calls
> conversation.begin()
> > it will once again reach
> > LazyHttpConversationContextImpl.checkContextInitialized() but this time
> > the if statement on line 121 returns true and so we never call
> > initialize(). In both cases it is the same
> > LazyHttpConversationContextImpl.
> > 
> > I also put a breakpoint in the observer and pinged the URL without
> > manually specificing a cid. From inside the observer method I can see
> that
> > LazyHttpConversationContextImpl.checkContextInitialized() line 128 is on
> > the stack.
> > 
> > 
> > So to summarise. When I call the url with a nonsense cdi: The try block
> > reaches checkContextInitialized and gets an exception on line 121. Then
> > the catch block reaches checkContextInitialized and does nothing because
> > isInitialized() returns true. Thus neither attempt reaches line 128 and
> > the observer method is never fied.
> > 
> > This feels like a bug to me, not just because the observer isn't fired
> but
> > also because if the initilization method had an exception half way
> though
> > is it left in a good state? I don't know enough about these weld
> internals
> > to check.
> > 
> > It occurs to me that one possible fix is to swap line 89 with line 90 so
> > that the exception takes place before initilized is set to true. Of
> course
> > that assumes that running initilized twice won't cause worse problems.
> > 
> > What do you think? Is this a bug?
> > 
> > 
> > 
> > From:   Matej Novotny <manovotn at redhat.com>
> > To:     Benjamin Confino <BENJAMIC at uk.ibm.com>
> > Cc:     Takayuki T Ishii <EBB0F3L at jp.ibm.com>, weld-dev at lists.jboss.org
> > Date:   28/01/2020 15:03
> > Subject:        [EXTERNAL] Re: [weld-dev] Question about conversations
> > scope initilization     obeserver
> > 
> > 
> > 
> > Hello,
> > 
> > I think I know what is the "problem" here.
> > Weld uses lazy conversation init - that means we don't activate context
> > until you try and access a conversation scoped bean.
> > 
> > Now, in your example, the ConversationBean tries to begin() a
> > conversation, then calls the bean (which initializes the context and
> > notifies the observer).
> > However, in the situation where you try and pass in a non-existing
> > conversation, the invocation to conversation.begin()[1] will blow up
> with
> > NonExistingConversationException and
> > you will jump right into the catch block[2] where you begin a
> conversation
> > with given ID, but you no longer invoke the bean, hence the context
> won't
> > get activated.
> > Try adding the `bean.getMsg()` call to the catch block and see if that
> > helps.
> > 
> > Note that CDI spec sets no requirements on how/when to activate the
> > conversation context, so the lazy behaviour is compliant with spec (and
> > this is also why you saw no such test in TCKs).
> > 
> > Regards
> > Matej
> > 
> > 
> _________________________________________________________________________________
> > [1]
> > 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__gist.github.com_manovotn_b9e9fde25ab77b5e481d5b34edf02b0c-23file-2Dconversationbean-2Djava-2DL4&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=uvdQf1sxR6VmxxDyV0JaOkEISRIJR3-N8dd2BXxazMQ&s=2TlH_k5wLmD8CYWrNh6BC2oYQwPs04gBfwJZ7yGgTjo&e=
> 
> > 
> > [2]
> > 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__gist.github.com_manovotn_b9e9fde25ab77b5e481d5b34edf02b0c-23file-2Dconversationbean-2Djava-2DL10-2DL16&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=uvdQf1sxR6VmxxDyV0JaOkEISRIJR3-N8dd2BXxazMQ&s=sjRD4J8rN0Vi9ZLoAC6sQ9x0WiJ96YjwVmefVelXceU&e=
> 
> > 
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Benjamin Confino" <BENJAMIC at uk.ibm.com>
> > > To: "Matej Novotny" <manovotn at redhat.com>
> > > Cc: "Takayuki T Ishii" <EBB0F3L at jp.ibm.com>, weld-dev at lists.jboss.org
> > > Sent: Tuesday, January 28, 2020 12:06:14 PM
> > > Subject: RE: [weld-dev] Question about conversations scope
> initilization
> >          obeserver
> > > 
> > > Hello
> > > 
> > > Thanks for the link. I had a look but I couldn't find any TCK tests
> > > checking to see if an observer method will catch the new
> > > ConversationContext being created for the "new transient
> conversation".
> > To
> > > check if a new conversation was activated I created an entirely fresh
> > > server and ran the test application on it, the behaviour was the same,
> > the
> > > first url I pinged on this new server ended with "cid=" and the
> observer
> > > didn't . Normally I've just been restarting the old server but
> > restarting
> > > frequently.
> > > 
> > > I've attached the recreate you requested. it consists of the three
> > classes
> > > attached to my previous email as well as a minimal html page. To run
> it
> > > load it onto your server and ping
> > > 
> > 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A9080_ConversationContextTest_index.xhtml&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=uvdQf1sxR6VmxxDyV0JaOkEISRIJR3-N8dd2BXxazMQ&s=OvJ-n1itv_FHZczFGtGTVn8L-4U7bv921PgvTBYisfA&e=
> 
> >  to see the
> > > observer fire, and
> > > 
> > 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A9080_ConversationContextTest_index.xhtml-3Fcid-3D99999&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=uvdQf1sxR6VmxxDyV0JaOkEISRIJR3-N8dd2BXxazMQ&s=IUIJSEejwSp_M36ux_W_THun4lDX62tuS5uT76Pq1N4&e=
> 
> >  to see
> > > the observer fail to fire.
> > > 
> > > Best regards
> > > Benjamin
> > > 
> > > 
> > > 
> > > From:   Matej Novotny <manovotn at redhat.com>
> > > To:     Benjamin Confino <BENJAMIC at uk.ibm.com>
> > > Cc:     weld-dev at lists.jboss.org, Takayuki T Ishii
> <EBB0F3L at jp.ibm.com>
> > > Date:   27/01/2020 11:39
> > > Subject:        [EXTERNAL] Re: [weld-dev] Question about conversations
> > > scope initilization     obeserver
> > > 
> > > 
> > > 
> > > Hello,
> > > 
> > > I'd start by pointing you to CDI TCK as that's a good starting point
> to
> > > see what's covered.
> > > For your question, that would be this test -
> > > 
> > 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_eclipse-2Dee4j_cdi-2Dtck_blob_master_impl_src_main_java_org_jboss_cdi_tck_tests_context_conversation_ClientConversationContextTest.java-23L344-2DL349&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=LuqfrtLLuJH86SKffsaBnDrxGJkBeoRqjz2EMS6K9mM&s=EO58wz7fGAjoo_qGMInGkEU8QMORj_W6IiAR-CM1WF4&e=
> 
> > 
> > > 
> > > And possibly few more in the same test class.
> > > 
> > > As for the linked classes - your `ConversationBean` is listening for
> > > @Initialized event. Can you verify that a new conversation was
> activated
> > > for your request instead of verifying that context was activated?
> > > E.g. check IDs or something along those lines? I suppose that will
> hold
> > > true and in that case it works just as spec requires it to.
> > > From the top of my head I don't really know how we activate/deactivate
> > > ConversationContext, I'd need to dig that up, but looking at CDI spec,
> > it
> > > doesn't mandate that it is activated every time again and it could
> > already
> > > be active for given request.
> > > Plus from just the classes you linked, I cannot know if you test this
> > with
> > > no existing conversation or maybe with some long running one before
> you
> > > try to send a request for non-existing one...and so on.
> > > So if the above doesn't is not enough to answer your question, then
> > we're
> > > going to need a complete reproducer so that we both talk about the
> same
> > > scenario :)
> > > 
> > > Matej
> > > 
> > > ----- Original Message -----
> > > > From: "Benjamin Confino" <BENJAMIC at uk.ibm.com>
> > > > To: weld-dev at lists.jboss.org
> > > > Cc: "Takayuki T Ishii" <EBB0F3L at jp.ibm.com>
> > > > Sent: Monday, January 27, 2020 11:42:14 AM
> > > > Subject: [weld-dev] Question about conversations scope initilization
> > > obeserver
> > > > 
> > > > Hello
> > > > 
> > > > I have a customer who's sent me a sample application, I have
> attached
> > > the
> > > > source to it below.
> > > > 
> > > > When the customer visits index.xhtml they see the following output:
> > > > 
> > > > Conversation initialized.
> > > > Conversation begun. cid:1 , timeout:3600000
> > > > Conversation destroyed. cid:1
> > > > 
> > > > However when they append "?cdi=" or a non-existnant identifier like
> > > > "?cdi=10000" to the url they do not see "Conversation initialized."
> > > > 
> > > > The CDI spec says that: If the propagated conversation cannot be
> > > restored,
> > > > the container must associate the request with a new transient
> > > conversation
> > > > and throw an exception of type
> > > > javax.enterprise.context.NonexistentConversationException.
> > > > 
> > > > I'm wondering if this should apply here? Or would it only apply if
> the
> > > cid
> > > > pointed to an existing conversation that could not be restored? And
> is
> > > there
> > > > anything in the spec that covers this specific situation?
> > > > 
> > > > Unless stated otherwise above:
> > > > IBM United Kingdom Limited - Registered in England and Wales with
> > number
> > > > 741598.
> > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
> PO6
> > > 3AU
> > > > 
> > > > _______________________________________________
> > > > weld-dev mailing list
> > > > weld-dev at lists.jboss.org
> > > > 
> > > 
> > 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.jboss.org_mailman_listinfo_weld-2Ddev&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=hl8XiFr1UrTSgyXVc4KO_i51sM3Gyhlu1KZ4TkyseEQ&m=LuqfrtLLuJH86SKffsaBnDrxGJkBeoRqjz2EMS6K9mM&s=ptwSTb9inzN42wgy9ctxUbuZO9ho6NYWekXQCURzWL4&e=
> 
> > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > Unless stated otherwise above:
> > > IBM United Kingdom Limited - Registered in England and Wales with
> number
> > > 741598.
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
> > 3AU
> > > 
> > 
> > 
> > 
> > 
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with number
> > 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
> 3AU
> > 
> 
> 
> 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> 