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
>>>>
>>>
>>>
>>
>
Index: impl/src/main/java/org/apache/myfaces/flow/cdi/DefaultCDIFacesFlowProvider.java
===================================================================
--- impl/src/main/java/org/apache/myfaces/flow/cdi/DefaultCDIFacesFlowProvider.java	(revision 1720218)
+++ impl/src/main/java/org/apache/myfaces/flow/cdi/DefaultCDIFacesFlowProvider.java	(working copy)
@@ -19,10 +19,10 @@
 package org.apache.myfaces.flow.cdi;
 
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import javax.enterprise.inject.Instance;
 import javax.enterprise.inject.spi.BeanManager;
 import javax.faces.context.FacesContext;
 import javax.faces.flow.Flow;
@@ -51,9 +51,9 @@
             FlowBuilderFactoryBean bean = CDIUtils.lookup(
                 beanManager, FlowBuilderFactoryBean.class);
 
-            Instance<Flow> instance = bean.getFlowDefinitions();
+            List<Flow> flows = bean.getFlowDefinitions();
             
-            return instance.iterator();
+            return flows.iterator();
         }
         else
         {
Index: impl/src/main/java/org/apache/myfaces/flow/cdi/FlowBuilderCDIExtension.java
===================================================================
--- impl/src/main/java/org/apache/myfaces/flow/cdi/FlowBuilderCDIExtension.java	(revision 1720218)
+++ impl/src/main/java/org/apache/myfaces/flow/cdi/FlowBuilderCDIExtension.java	(working copy)
@@ -18,32 +18,56 @@
  */
 package org.apache.myfaces.flow.cdi;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import javax.enterprise.event.Observes;
 import javax.enterprise.inject.spi.AnnotatedType;
 import javax.enterprise.inject.spi.BeanManager;
 import javax.enterprise.inject.spi.BeforeBeanDiscovery;
 import javax.enterprise.inject.spi.Extension;
+import javax.enterprise.inject.spi.ProcessProducer;
+import javax.enterprise.inject.spi.Producer;
+import javax.faces.flow.Flow;
+import javax.faces.flow.builder.FlowDefinition;
 
 /**
  * This extension is responsible of scan flow definitions through CDI. For example:
- * 
+ *
  * <code>
  * @Produces @FlowDefinition
  * public Flow defineFlow(@FlowBuilderParameter FlowBuilder flowBuilder) {...}
  * </code>
- * 
+ *
  * @author Leonardo Uribe
  */
 public class FlowBuilderCDIExtension implements Extension
 {
-    
+    private List<Producer<Flow>> flowProducers = new ArrayList<Producer<Flow>>();
+
+    public List<Producer<Flow>> getFlowProducers()
+    {
+        return flowProducers;
+    }
+
     void beforeBeanDiscovery(
         @Observes final BeforeBeanDiscovery event, BeanManager beanManager)
     {
         // Register FlowBuilderFactoryBean as a bean with CDI annotations, so the system
         // can take it into account, and use it later when necessary.
-        AnnotatedType flowDiscoveryHelper = beanManager.createAnnotatedType(FlowBuilderFactoryBean.class);
+        AnnotatedType<FlowBuilderFactoryBean> flowDiscoveryHelper =
+                        beanManager.createAnnotatedType(FlowBuilderFactoryBean.class);
         event.addAnnotatedType(flowDiscoveryHelper);
     }
 
+    /**
+     * Stores any producer method that is annotated with @FlowDefinition.
+     */
+    <T> void findFlowDefinition(@Observes ProcessProducer<T, Flow> processProducer)
+    {
+        if (processProducer.getAnnotatedMember().isAnnotationPresent(FlowDefinition.class))
+        {
+            flowProducers.add(processProducer.getProducer());
+        }
+    }
 }
Index: impl/src/main/java/org/apache/myfaces/flow/cdi/FlowBuilderFactoryBean.java
===================================================================
--- impl/src/main/java/org/apache/myfaces/flow/cdi/FlowBuilderFactoryBean.java	(revision 1720218)
+++ impl/src/main/java/org/apache/myfaces/flow/cdi/FlowBuilderFactoryBean.java	(working copy)
@@ -18,10 +18,15 @@
  */
 package org.apache.myfaces.flow.cdi;
 
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
 import javax.enterprise.context.ApplicationScoped;
-import javax.enterprise.inject.Any;
-import javax.enterprise.inject.Instance;
 import javax.enterprise.inject.Produces;
+import javax.enterprise.inject.spi.BeanManager;
+import javax.enterprise.inject.spi.Producer;
+import javax.faces.context.FacesContext;
 import javax.faces.flow.Flow;
 import javax.faces.flow.builder.FlowBuilder;
 import javax.faces.flow.builder.FlowBuilderParameter;
@@ -28,6 +33,7 @@
 import javax.inject.Inject;
 import javax.inject.Named;
 
+import org.apache.myfaces.cdi.util.CDIUtils;
 import org.apache.myfaces.flow.builder.FlowBuilderImpl;
 
 /**
@@ -41,22 +47,16 @@
 {
     public static final String FLOW_BUILDER_FACTORY_BEAN_NAME = 
         "oam_FLOW_BUILDER_FACTORY_BEAN_NAME";
-    
-    /**
-     * In this point two things are important:
-     * 
-     * 1. Initialize flows in a lazy way (triggered by JSF),
-     * 2. Get multiple Flow instances.
-     * 
-     */
-    @Inject 
-    @Any
-    private Instance<Flow> flowDefinitions;
 
+    private List<Flow> flowDefinitions = new ArrayList<Flow>();
+
+    @Inject
+    private FlowBuilderCDIExtension flowBuilderExtension;
+
     public FlowBuilderFactoryBean()
     {
     }
-    
+
     @Produces
     @FlowBuilderParameter
     public FlowBuilder createFlowBuilderInstance()
@@ -63,16 +63,25 @@
     {
         return new FlowBuilderImpl();
     }
-    
+
     /**
      * @return the flowDefinitions
      */
-    public Instance<Flow> getFlowDefinitions()
+    public List<Flow> getFlowDefinitions()
     {
-        // Pass the @FlowDefinition qualifier, so only all producer methods involving
-        // this annotation will be taken into account.
-        return flowDefinitions.select(
-                new FlowDefinitionQualifier());
+        BeanManager beanManager = CDIUtils.getBeanManager(FacesContext.getCurrentInstance().getExternalContext());
+
+        Iterator<Producer<Flow>> it = flowBuilderExtension.getFlowProducers().iterator();
+
+        if (it != null)
+        {
+            while (it.hasNext())
+            {
+                Flow flow = it.next().produce(beanManager.<Flow>createCreationalContext(null));
+                flowDefinitions.add(flow);
+            }
+        }
+
+        return flowDefinitions;
     }
-    
 }
Index: impl/src/main/java/org/apache/myfaces/flow/cdi/FlowDefinitionQualifier.java
===================================================================
--- impl/src/main/java/org/apache/myfaces/flow/cdi/FlowDefinitionQualifier.java	(revision 1720218)
+++ impl/src/main/java/org/apache/myfaces/flow/cdi/FlowDefinitionQualifier.java	(working copy)
@@ -1,38 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.myfaces.flow.cdi;
-
-import javax.enterprise.util.AnnotationLiteral;
-import javax.faces.flow.builder.FlowDefinition;
-
-/**
- * This class follows the hack proposed in section 4.10
- * "Obtaining a contextual instance by programmatic lookup", to specify
- * a qualifier dynamically. 
- * 
- * See:
- * http://docs.jboss.org/weld/reference/latest/en-US/html/injection.html
- *
- * @author Leonardo Uribe
- */
-public class FlowDefinitionQualifier extends AnnotationLiteral<FlowDefinition> 
-    implements FlowDefinition
-{
-    
-}

Reply via email to