After rereading Leonardo's response, he mentioned

"The solution proposed in the patch cause a problem when two webapps uses
faces flow, because one app could find the flows of the other one (variable
for FlowBuilderCDIExtension)."

I will test this when I have a chance.

On Wed, Dec 16, 2015 at 5:24 PM, Hank Ibell <[email protected]> wrote:

> I did another test with multiple web modules in an EAR with slightly
> different flows that have the same flow id. I was able to successfully
> enter the correct flow of each web module despite the flow ids being the
> same and with the extension being shared.
>
> Does anyone know if there is another case in mind that could break
> something?
>
> Regards,
> Hank Ibell
>
> On Wed, Dec 16, 2015 at 4:31 PM Thomas Andraschko <
> [email protected]> wrote:
>
>> Oh, i see.
>> The behavior is different in a EAR...
>> That could really break something.
>>
>> 2015-12-16 22:26 GMT+01:00 Thomas Andraschko <[email protected]
>> >:
>>
>>> The CDI spec clearly states:
>>> The container instantiates a single instance of each extension at the
>>> beginning of the application initialization process and maintains a
>>> reference to it until the application shuts down.
>>>
>>> So +1 for applying your patch.
>>> In DeltaSpike we have many Extensions which would behave weird if the
>>> Extensions instances would be shared across applications.
>>>
>>> 2015-12-16 22:12 GMT+01:00 Hank Ibell <[email protected]>:
>>>
>>>> Hello,
>>>>
>>>> I tested MyFaces on WildFly and WebSphere Liberty, and it looks like
>>>> FlowBuilderCDIExtension is not shared between different web applications
>>>> that are not in the same EAR.
>>>>
>>>> Here is what I see from WildFly:
>>>> ######################################
>>>> WildFly: Deployment of an EAR with the two web modules:
>>>> FlowBuilderCDIExtension is shared between the two web modules
>>>>
>>>> 11:40:36,826 INFO  [stdout] (MSC service thread 1-5) HWIBELL:
>>>> FlowBuilderCDIExtension: *Extension
>>>> org.apache.myfaces.flow.cdi.FlowBuilderCDIExtension@cb0c841b*:
>>>> 11:40:36,827 INFO  [stdout] (MSC service thread 1-5) HWIBELL:
>>>> FlowBuilderCDIExtension: Found flow Producer for Producer Method [Flow]
>>>> with qualifiers [@FlowDefinition @Any] declared as [[BackedAnnotatedMethod]
>>>> @Produces @FlowDefinition public
>>>> internal.FlowFactory.defineTestFlow(@FlowBuilderParameter FlowBuilder)]
>>>> declared on Managed Bean [class internal.FlowFactory] with qualifiers [@Any
>>>> @Default]
>>>> 11:40:36,884 INFO  [stdout] (MSC service thread 1-5) HWIBELL:
>>>> FlowBuilderCDIExtension: *Extension
>>>> org.apache.myfaces.flow.cdi.FlowBuilderCDIExtension@cb0c841b*:
>>>> 11:40:36,884 INFO  [stdout] (MSC service thread 1-5) HWIBELL:
>>>> FlowBuilderCDIExtension: Found flow Producer for Producer Method [Flow]
>>>> with qualifiers [@FlowDefinition @Any] declared as [[BackedAnnotatedMethod]
>>>> @Produces @FlowDefinition public
>>>> internal.FlowFactory.defineTestFlow(@FlowBuilderParameter FlowBuilder)]
>>>> declared on Managed Bean [class internal.FlowFactory] with qualifiers [@Any
>>>> @Default]
>>>> ...
>>>>
>>>> ######################################
>>>> WildFly: Deployment of two separate web modules:
>>>> FlowBuilderCDIExtension is different for each web application
>>>>
>>>> 2015-12-16 13:56:16,328 INFO  [stdout] (MSC service thread 1-4)
>>>> HWIBELL: FlowBuilderCDIExtension: *Extension
>>>> org.apache.myfaces.flow.cdi.FlowBuilderCDIExtension@fee8cb50*:
>>>> 2015-12-16 13:56:16,331 INFO  [stdout] (MSC service thread 1-4)
>>>> HWIBELL: FlowBuilderCDIExtension: Found flow Producer for Producer Method
>>>> [Flow] with qualifiers [@FlowDefinition @Any] declared as
>>>> [[BackedAnnotatedMethod] @Produces @FlowDefinition public
>>>> internal1.FlowFactory.defineTestFlow(@FlowBuilderParameter FlowBuilder)]
>>>> declared on Managed Bean [class internal1.FlowFactory] with qualifiers
>>>> [@Any @Default]
>>>> 2015-12-16 13:56:16,430 INFO  [stdout] (MSC service thread 1-3)
>>>> HWIBELL: FlowBuilderCDIExtension: *Extension
>>>> org.apache.myfaces.flow.cdi.FlowBuilderCDIExtension@988bd32f*:
>>>> 2015-12-16 13:56:16,430 INFO  [stdout] (MSC service thread 1-3)
>>>> HWIBELL: FlowBuilderCDIExtension: Found flow Producer for Producer Method
>>>> [Flow] with qualifiers [@FlowDefinition @Any] declared as
>>>> [[BackedAnnotatedMethod] @Produces @FlowDefinition public
>>>> internal2.FlowFactory.defineTestFlow(@FlowBuilderParameter FlowBuilder)]
>>>> declared on Managed Bean [class internal2.FlowFactory] with qualifiers
>>>> [@Any @Default]
>>>> ######################################
>>>>
>>>> The results from WebSphere Liberty are the same. Does anyone the best
>>>> place to verify the lifetime of a CDI Extension instance?
>>>>
>>>>
>>>>
>>>> On Wed, Dec 16, 2015 at 10:52 AM, Leonardo Uribe <[email protected]>
>>>> wrote:
>>>>
>>>>> I'm not sure how the extension instance is created and its lifetime.
>>>>> That's not documented, so we need to check that.
>>>>> On Dec 16, 2015 4:03 AM, "Thomas Andraschko" <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Are you sure Leo?
>>>>>> FlowBuilderCDIExtension should exist per WebApp.
>>>>>>
>>>>>> 2015-12-15 22:39 GMT+01:00 Leonardo Uribe <[email protected]>:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> I remember the current solution works in a case where myfaces jars
>>>>>>> are shared by different web applications. Suppose a TomEE environment. 
>>>>>>> The
>>>>>>> solution proposed in the patch cause a problem when two webapps uses 
>>>>>>> faces
>>>>>>> flow, because one app could find the flows of the other one (variable 
>>>>>>> for
>>>>>>> FlowBuilderCDIExtension).
>>>>>>>
>>>>>>> I don't have idea if the solution proposed works in that case, so we
>>>>>>> need to check that before apply it.
>>>>>>>
>>>>>>> regards,
>>>>>>>
>>>>>>> Leonardo Uribe
>>>>>>>
>>>>>>>
>>>>>>> 2015-12-15 14:58 GMT-05:00 Hank Ibell <[email protected]>:
>>>>>>>
>>>>>>>> Hello Thomas,
>>>>>>>>
>>>>>>>> I did think injecting the FlowBuilderCDIExtension would work -- I
>>>>>>>> was quite surprised when it did. Also, after looking at the code 
>>>>>>>> again, I
>>>>>>>> agree that the lists should be ArrayList instead. Thank you for the 
>>>>>>>> quick
>>>>>>>> review and suggestions!
>>>>>>>>
>>>>>>>> The new patch has been attached to this email and to the JIRA.
>>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Hank Ibell
>>>>>>>>
>>>>>>>> On Mon, Dec 14, 2015 at 4:09 PM, Thomas Andraschko <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> i did a small review:
>>>>>>>>>
>>>>>>>>> 1) Why you don't use @Inject for the FlowBuilderCDIExtension in
>>>>>>>>> the FlowBuilderFactoryBean?
>>>>>>>>> 2) Why do you use CopyOnWriteArrayList? ArrayList should be fine
>>>>>>>>> as the both lists are AppScoped and should only be used on startup.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2015-12-14 21:53 GMT+01:00 Hank Ibell <[email protected]>:
>>>>>>>>>
>>>>>>>>>> Hello Thomas,
>>>>>>>>>>
>>>>>>>>>> Thank you for the information. :) I will wait for Leo's review
>>>>>>>>>> then.
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Hank Ibell
>>>>>>>>>>
>>>>>>>>>> On Mon, Dec 14, 2015 at 3:20 PM, Thomas Andraschko <
>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> first of all: thanks for the patch.
>>>>>>>>>>>
>>>>>>>>>>> As the last release-vote just passed last week, please have a
>>>>>>>>>>> little patience.
>>>>>>>>>>>
>>>>>>>>>>> AFAIR the flows-feature was the developed by Leo, so it would be
>>>>>>>>>>> the best if he could review it.
>>>>>>>>>>> Otherwise i will check it.
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Thomas
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2015-12-14 3:46 GMT+01:00 Hank Ibell <[email protected]>:
>>>>>>>>>>>
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> It has been about a week since MYFACES-4022 [link
>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MYFACES-4022>] has been
>>>>>>>>>>>> opened and a potential patch has been submitted. There has been no 
>>>>>>>>>>>> feedback
>>>>>>>>>>>> on the issue however.
>>>>>>>>>>>>
>>>>>>>>>>>> Is there anything else that is needed so that we can resolve
>>>>>>>>>>>> this issue as soon as possible?
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Hank Ibell
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>>

Reply via email to