You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by sk...@apache.org on 2008/07/10 17:03:49 UTC

svn commit: r675607 - in /myfaces/orchestra/trunk/flow/src: main/java/org/apache/myfaces/orchestra/flow/ main/java/org/apache/myfaces/orchestra/flow/config/ main/java/org/apache/myfaces/orchestra/flow/digest/ site/apt/

Author: skitching
Date: Thu Jul 10 08:03:48 2008
New Revision: 675607

URL: http://svn.apache.org/viewvc?rev=675607&view=rev
Log:
* stricter validation of flow.xml files
* rename onReturn to onCommit, and support just one rather than a list.
* allow an onCommit to return a navigation outcome
* simplify FlowHandler class
* simplify FlowViewHandler class

Added:
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowOnCommit.java   (with props)
Modified:
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowHandler.java
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowNavigationHandler.java
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowViewHandler.java
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowAccept.java
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowCall.java
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowConfig.java
    myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/digest/FlowDigester.java
    myfaces/orchestra/trunk/flow/src/site/apt/usage.apt

Modified: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowHandler.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowHandler.java?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowHandler.java (original)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowHandler.java Thu Jul 10 08:03:48 2008
@@ -24,7 +24,6 @@
 import java.util.List;
 import java.util.Map;
 
-import javax.faces.FacesException;
 import javax.faces.component.UIViewRoot;
 import javax.faces.context.ExternalContext;
 import javax.faces.context.FacesContext;
@@ -37,6 +36,7 @@
 import org.apache.myfaces.orchestra.flow.config.FlowAccept;
 import org.apache.myfaces.orchestra.flow.config.FlowCall;
 import org.apache.myfaces.orchestra.flow.config.FlowConfig;
+import org.apache.myfaces.orchestra.flow.config.FlowOnCommit;
 import org.apache.myfaces.orchestra.flow.config.FlowParamAccept;
 import org.apache.myfaces.orchestra.flow.config.FlowParamSend;
 import org.apache.myfaces.orchestra.flow.config.FlowReturnAccept;
@@ -68,9 +68,20 @@
     {
         public String viewId;
         public UIViewRoot viewRoot;
+        public String outcome;
         public FlowInfo flowInfo; // null unless a flowcall is being started
     }
 
+    /**
+     * Return the FlowCall object associated with the current conversation
+     * context (if any).
+     * <p>
+     * There will be one if the current context was created by some page
+     * doing a "call" to a flow.
+     * <p>
+     * Returns null if there is no FlowCall object in the current conversation
+     * context.
+     */
     static FlowCall getFlowCall(String viewId, String outcome)
     {
         if (viewId == null)
@@ -90,6 +101,14 @@
         return fc;
     }
 
+    /**
+     * Return the FlowAccept object associated with the current conversation
+     * context (if any).
+     * <p>
+     * There will be one if the current context was created by some page
+     * doing a "call" to a flow, AND the flow call initialisation has
+     * completed. 
+     */
     static FlowAccept getFlowAccept(FlowInfo flowInfo, String viewId)
     {
         if ((flowInfo != null) && (flowInfo.getFlowAccept() != null))
@@ -120,6 +139,10 @@
      * <li>determine whether this navigation triggers a new flow. If so, create
      * a child context and place a partially-initialised FlowInfo object in it.</li>
      * </ul> 
+     * <p>
+     * This is expected to be called from the FlowNavigationHandler after some
+     * postback has caused a navigation to occur. Note that in that case we do
+     * not yet know what viewId we are going <i>to</i>.
      */
     static void processCall(Selection sel, FacesContext facesContext, String oldViewId, String outcome)
     {
@@ -160,21 +183,41 @@
                 // all the output parameters
                 Map map = flowInfo.getFlowAccept().readReturnParams(facesContext);
 
-                // Now with the original context active, write them back.
+                // Activate the parent and discard the child context
                 cm.activateConversationContext(parent);
+                cm.removeAndInvalidateConversationContext(ctx);
+
+                // Now with the original context active, write parameters back.
                 flowInfo.getFlowCall().writeAcceptParams(facesContext, map);
 
-                // And discard the child. Note that we do this after activating
-                // the parent just for safety in case of races.
-                cm.removeAndInvalidateConversationContext(ctx);
+                // And execute any actions the caller wants to run.
+                // Hmm..but these cannot do anything to set the viewroot, because
+                // we stomp over that after returning from this method. Maybe this
+                // method should be responsible for setting up viewroot itself, and
+                // return a boolean to say it did it?
+                //
+                // And should we restore the saved view before running these callbacks,
+                // in case the invoked code tries to access it?
+                FlowOnCommit onCommit = flowInfo.getFlowCall().getOnCommit();
+                if (onCommit != null)
+                {
+                    Object actionOutcome = onCommit.execute(facesContext);
+                    if (actionOutcome != null)
+                    {
+                        // Note: sel.viewId is already set; that is important as the
+                        // navigation handler will use that when choosing the nav-case.
+                        sel.outcome = actionOutcome.toString();
+                    }
+                }
 
-                // Return the restored viewId with the parent context
+                // Return with sel holding the restored view and the parent context
                 // selected as the active one.
                 return;
             }
         }
 
-        // OK, we know we are not committing or canceling a flow. Are we entering one?
+        // OK, we know we are not committing or canceling a flow. Are we trying to
+        // enter one?
         FlowCall flowCall = getFlowCall(oldViewId, outcome);
         if (flowCall != null)
         {
@@ -201,7 +244,7 @@
      * Handle case where the user is in a flow, then navigates somehow to a page that is not
      * within the flow.
      */
-    static boolean isAbnormalFlowExit(Selection sel, FlowInfo flowInfo, String newViewId)
+    private static boolean isAbnormalFlowExit(FlowInfo flowInfo, String newViewId)
     {
         // Are we leaving a flow in an abnormal manner?
         if (flowInfo != null)
@@ -217,10 +260,6 @@
                 ConversationContext parent = ctx.getParent();
                 for(;;)
                 {
-                    cm.removeAndInvalidateConversationContext(ctx);
-                    ctx = parent;
-                    parent = ctx.getParent();
-                    
                     if (parent == null)
                     {
                         // After discarding all invalid flows, let normal navigation occur
@@ -228,16 +267,21 @@
                         break;
                     }
     
+                    cm.activateConversationContext(parent);
+                    cm.removeAndInvalidateConversationContext(ctx);
+                    ctx = parent;
+                    parent = ctx.getParent();
+                    
                     FlowInfo fi = getFlowInfo(ctx);
                     if ((fi != null) && (flowPath.startsWith(fi.getFlowPath())))
                     {
-                        sel.viewId = fi.getCallerViewId();
-                        sel.viewRoot = fi.getCallerViewRoot();
+                        // ok, the view the user wants to navigate to is within
+                        // the context just activated, so we now have things set
+                        // up as required; break out of loop.
                         break;
                     }
                 }
 
-                cm.activateConversationContext(ctx);
                 return true;
             }
         }
@@ -254,8 +298,11 @@
      * FlowInfo object will be in the context. We need to now fetch the called flow's specs,
      * check that the caller and called flows match in type and parameters, then finish
      * initialising the FlowInfo and import the passed params into the child context.
+     * <p>
+     * This is expected to be called from FlowViewHandler when a new view is being created
+     * (due either to a GET or POST from the user, or an internal forward due to navigation.
      */
-    static boolean isNewFlowEntry(Selection sel, FacesContext facesContext, FlowInfo flowInfo, String newViewId)
+    static boolean isNewFlowEntry(FacesContext facesContext, FlowInfo flowInfo, String newViewId)
     {
         if (flowInfo == null)
         {
@@ -297,89 +344,30 @@
     /**
      * Invoked to handle actions that require info about the called page, but not the caller.
      */
-    static Selection processAccept(FacesContext facesContext, String newViewId)
+    static void processAccept(FacesContext facesContext, String newViewId)
     {
-        Selection sel = new Selection();
-
         log.debug("processAccept: [" + newViewId + "]");
         FlowInfo flowInfo = getFlowInfo();
         
-        if (isNewFlowEntry(sel, facesContext, flowInfo, newViewId))
+        if (isNewFlowEntry(facesContext, flowInfo, newViewId))
         {
-            return sel;
+            return;
         }
 
-        if (isAbnormalFlowExit(sel, flowInfo, newViewId))
+        if (isAbnormalFlowExit(flowInfo, newViewId))
         {
-            return sel;
+            return;
         }
 
         FlowAccept flowAccept = getFlowAccept(flowInfo, newViewId);
-
         boolean isFlowEntryPoint = (flowAccept != null);
         boolean isInFlow = (flowInfo != null);
-        boolean isIntraFlowNav = isInFlow && newViewId.startsWith(flowInfo.getFlowPath());
-
-        if (isInFlow && !isIntraFlowNav)
-        {
-            // Internal error. The isAbnormalFlowExit method should always detect when a
-            // nav outside the current flow happens, and handle it.
-            throw new OrchestraException("Internal error: nav outside flow not handled correctly");
-        }
-
-        // isFlowEntryPoint | isInFlow ==> result
-        //
-        // Note that we don't need to test intraFlowNav below. When !isInFlow, it is always false.
-        // And when isInFlow, it is always true (see check above). So testing isInFlow is enough.
-        // 
-        // N N --> normal root navigation. Do nothing
-        // N Y --> normal internal flow nav; do nothing
-        // Y N --> missing caller declaration: report error
-        // Y Y --> normal internal flow nav; do nothing
-        //
-
-        if (!isFlowEntryPoint && !isInFlow)
-        {
-            log.debug("processAccept: Normal root nav: no action");
-            return sel;
-        }
-
-        if (!isFlowEntryPoint && isInFlow)
-        {
-            log.debug("processAccept: Normal internal nav: no action");
-            return sel;
-        }
 
         if (isFlowEntryPoint && !isInFlow)
         {
             log.debug("processAccept: Error: invocation of flow without caller declaration");
             throw new OrchestraException("Invocation of flow without caller declaration");
         }
-
-        if (isFlowEntryPoint && isInFlow)
-        {
-            // Ok to nav back to the flow entry point in this case; we don't treat that
-            // as starting a new flow *unless* the navigation explicitly made a call.
-            //
-            // This is a little inconsistent, but practical. We want to report errors when
-            // normal code tries to nav to the entry point of a flow without a call. It's
-            // just not sane to do that, as we have no input parameters and nowhere to
-            // return to when the flow ends. But flows are often just a couple of pages
-            // that users can navigate around multiple times. Very occasionally it is
-            // useful to make "nested" calls to a flow, so we support that by having the
-            // flow make an explicit "call" to itself. But that is not the case here.
-            //
-            // Note that if a flow tries to navigate to the entry point of a different
-            // flow (or any other place for that matter) then the caller handling code
-            // will trap that and discard the originating flow immediately, so we
-            // never need to deal with that case here.
-            log.debug("processAccept: Normal internal nav(2): no action");
-            return sel;
-        }
-
-        // Hmm..above should cover all cases. If this ever runs, then there
-        // is a missing section above.
-        throw new FacesException("Internal error: should never get here");
     }
 
     private static FlowConfig getFlowConfig(String viewId)

Modified: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowNavigationHandler.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowNavigationHandler.java?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowNavigationHandler.java (original)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowNavigationHandler.java Thu Jul 10 08:03:48 2008
@@ -68,6 +68,23 @@
         FlowHandler.Selection sel = new FlowHandler.Selection();
         FlowHandler.processCall(sel, facesContext, oldViewId, outcome);
 
+        if (sel.outcome != null)
+        {
+            // Doing custom navigation. We need to have the current view root set
+            // correctly so that the navigation handler selects the right nav-case.
+            //
+            // The UIViewRoot installed also needs a locale and renderKitId, as the
+            // NavHandler will call ViewHandler.createView, which will try to copy
+            // those values onto the new UIViewRoot it creates.
+            UIViewRoot dummyViewRoot = new UIViewRoot();
+            dummyViewRoot.setViewId(sel.viewId);
+            dummyViewRoot.setLocale(currViewRoot.getLocale());
+            dummyViewRoot.setRenderKitId(currViewRoot.getRenderKitId());
+            facesContext.setViewRoot(dummyViewRoot);
+            delegate.handleNavigation(facesContext, fromAction, sel.outcome);
+            return;
+        }
+
         if (sel.viewRoot != null)
         {
             // We are leaping back to a previous view, and have a saved version of

Modified: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowViewHandler.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowViewHandler.java?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowViewHandler.java (original)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/FlowViewHandler.java Thu Jul 10 08:03:48 2008
@@ -27,58 +27,75 @@
 import javax.faces.component.UIViewRoot;
 import javax.faces.context.FacesContext;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.myfaces.orchestra.lib.OrchestraException;
 
-// Warning: this class is fragile when later JSF specifications add
-// new methods. The method is then not properly delegated and instead
-// just falls back to the base class implementation.
-//
-// For upwards compatibility, we can subclass from ViewHandlerWrapper - but
-// that doesn't run on JSF1.1. Ecch. Methods added to this class in JSF1.2
-// which will therefore not be correctly delegated on a JSF1.2 instance are:
-//  * calculateCharacterEncoding
-//  * initView (which just calls calculateCharacterEncoding)
-// This class instead subclasses ViewHandler directly then adds implementations
-// of the missing JSF1.2 methods so that an exception does not occur when a
-// JSF1.2 environment tries to call them. This is of course not the correct
-// solution, as there will be problems if later JSF specification versions add
-// more methods. But it will work for JSF1.1 and JSF1.2.
+/**
+ * Custom ViewHandler that executes the necessary logic to handle
+ * entering and exiting a Flow.
+ * <p>
+ * Warning: this class is fragile when later JSF specifications add
+ * new methods. The method is then not properly delegated and instead
+ * just falls back to the base class implementation. For upwards
+ * compatibility, we could subclass from ViewHandlerWrapper - but that
+ * code would then fail to run on JSF1.1. Ecch. This class therefore
+ * subclasses ViewHandler directly then adds implementations of all
+ * methods added in JSF1.2, and uses reflection to invoke them on the
+ * underlying implementation. This is of course not a "real" solution
+ * as there will be problems if a later JSF specification adds more
+ * methods. But it does make this class work for both JSF1.1 and JSF1.2.
+ */
 public class FlowViewHandler extends ViewHandler
 {
-    private static final Log log = LogFactory.getLog(FlowViewHandler.class);
+    private static final Method calcCharEncMethod;
+    private static final Method initViewMethod;
 
-    ViewHandler delegate;
-    Method calcCharEncMethod;
-    Method initViewMethod;
+    private ViewHandler delegate;
 
-    public FlowViewHandler(ViewHandler delegate)
+    /**
+     * Static initialization block.
+     */
+    static
+    {
+        calcCharEncMethod = getMethodOpt(ViewHandler.class,
+                "calculateCharacterEncoding",
+                new Class[] {FacesContext.class});
+
+        initViewMethod = getMethodOpt(ViewHandler.class,
+                "initView",
+                new Class[] {FacesContext.class});
+    }
+
+    /**
+     * If the specified class has a method with the specified name and params, return
+     * it else return null.
+     */
+    private static Method getMethodOpt(Class clazz, String methodName, Class[] args)
     {
-        this.delegate = delegate;
-        
-        try
-        {
-            calcCharEncMethod = delegate.getClass().getMethod(
-                "calculateCharacterEncoding", new Class[] {FacesContext.class});
-        }
-        catch(NoSuchMethodException e)
-        {
-            calcCharEncMethod = null;
-        }
-        
         try
         {
-            initViewMethod = delegate.getClass().getMethod(
-                "initView", new Class[] {FacesContext.class});
+            return clazz.getMethod(methodName, args);
         }
         catch(NoSuchMethodException e)
         {
-            initViewMethod = null;
+            return null;
         }
     }
 
-    // JSF1.2 method
+    /**
+     * Constructor.
+     */
+    public FlowViewHandler(ViewHandler delegate)
+    {
+        this.delegate = delegate;
+    }
+
+    /**
+     * Delegate to wrapped instance. 
+     * <p>
+     * This method was added in JSF1.2. We must therefore use reflection
+     * to invoke the method on the wrapped instance. Note that this method
+     * is never invoked unless this is a JSF1.2 environment. 
+     */
     public java.lang.String calculateCharacterEncoding(FacesContext context)
     {
         try
@@ -92,7 +109,13 @@
         }
     }
 
-    // JSF1.2 method
+    /**
+     * Delegate to wrapped instance. 
+     * <p>
+     * This method was added in JSF1.2. We must therefore use reflection
+     * to invoke the method on the wrapped instance. Note that this method
+     * is never invoked unless this is a JSF1.2 environment. 
+     */
     public void initView(FacesContext context)
     throws FacesException
     {
@@ -106,58 +129,68 @@
         }
     }
 
+    /** Delegate to wrapped instance. */ 
     public Locale calculateLocale(FacesContext context)
     {
         return delegate.calculateLocale(context);
     }
 
+    /** Delegate to wrapped instance. */ 
     public String calculateRenderKitId(FacesContext context)
     {
         return delegate.calculateRenderKitId(context);
     }
 
+    /** Delegate to wrapped instance. */ 
     public String getActionURL(FacesContext context, String viewId)
     {
         return delegate.getActionURL(context, viewId);
     }
 
+    /** Delegate to wrapped instance. */ 
     public String getResourceURL(FacesContext context, String path)
     {
         return delegate.getResourceURL(context, path);
     }
 
+    /** Delegate to wrapped instance. */ 
     public void renderView(FacesContext context, UIViewRoot viewToRender) throws IOException, FacesException
     {
         delegate.renderView(context, viewToRender);
     }
 
+    /** Delegate to wrapped instance. */ 
     public void writeState(FacesContext context) throws IOException
     {
         delegate.writeState(context);
     }
 
+    /** Delegate to wrapped instance. */ 
     public UIViewRoot restoreView(FacesContext context, String viewId)
     {
-        log.debug("restoreView invoked for view " + viewId);
         return delegate.restoreView(context, viewId);
     }
 
+    /**
+     * Special createView handling for Orchestra Flows.
+     * <p>
+     * This method always delegates to the wrapped instance to create the view
+     * that the user asked for (unless we report a flow error), but we first do
+     * flow-related things like:
+     * <ul>
+     * <li>import parameters from calling to called context if this is the
+     * first page of a new flow
+     * <li>discard some conversation contexts if this view is not in the
+     * currently active flow
+     * </li>
+     * <p>
+     * Throws OrchestraException if there is a flow error, eg if the view
+     * specified is the entry point for a flow but the previous view did
+     * not do a FlowCall.
+     */
     public UIViewRoot createView(FacesContext context, String viewId)
     {
-        log.debug("createView invoked for view " + viewId);
-        FlowHandler.Selection sel = FlowHandler.processAccept(context, viewId);
-
-        if (sel.viewRoot != null)
-        {
-            return sel.viewRoot;
-        }
-        else if (sel.viewId != null)
-        {
-            return delegate.createView(context, sel.viewId);
-        }
-        else
-        {
-            return delegate.createView(context, viewId);
-        }
+        FlowHandler.processAccept(context, viewId);
+        return delegate.createView(context, viewId);
     }
 }

Modified: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowAccept.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowAccept.java?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowAccept.java (original)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowAccept.java Thu Jul 10 08:03:48 2008
@@ -83,6 +83,18 @@
         {
             throw new OrchestraException("returns is empty");
         }
+
+        for(Iterator i = params.iterator(); i.hasNext(); )
+        {
+            FlowParamAccept p = (FlowParamAccept) i.next();
+            p.validate();
+        }
+
+        for(Iterator i = returns.iterator(); i.hasNext(); )
+        {
+            FlowReturnSend p = (FlowReturnSend) i.next();
+            p.validate();
+        }
     }
 
     /**

Modified: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowCall.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowCall.java?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowCall.java (original)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowCall.java Thu Jul 10 08:03:48 2008
@@ -40,7 +40,7 @@
     List params = new ArrayList(4);  // list of FlowParamSend
     List returns = new ArrayList(4); // list of FlowReturnAccept
 
-    List onReturns = new ArrayList(4);
+    FlowOnCommit onCommit;
     List modifies = new ArrayList(4);
     
     /**
@@ -93,6 +93,18 @@
         {
             throw new OrchestraException("returns is empty");
         }
+
+        for(Iterator i = params.iterator(); i.hasNext(); )
+        {
+            FlowParamSend p = (FlowParamSend) i.next();
+            p.validate();
+        }
+
+        for(Iterator i = returns.iterator(); i.hasNext(); )
+        {
+            FlowReturnAccept p = (FlowReturnAccept) i.next();
+            p.validate();
+        }
     }
 
      /**
@@ -201,22 +213,21 @@
     }
 
     /**
-     * Return a list of method bindings that are invoked in the caller's
-     * environment after the flow has successfully committed and the return
-     * values have been successfully stored into the caller's environment.
+     * Return an object that wraps an EL expression which should be executed
+     * within the caller environment on successful commit of a called flow.
      * <p>
      * This gives the caller a chance to process the returned values before
      * it is re-rendered.
      */
-    public List getOnReturns()
+    public FlowOnCommit getOnCommit()
     {
-        return onReturns;
+        return onCommit;
     }
 
     /** For use only during object initialization. */
-    public void setOnReturns(List onReturns)
+    public void setOnCommit(FlowOnCommit onCommit)
     {
-        this.onReturns = onReturns;
+        this.onCommit = onCommit;
     }
 
     /**

Modified: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowConfig.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowConfig.java?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowConfig.java (original)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowConfig.java Thu Jul 10 08:03:48 2008
@@ -82,7 +82,6 @@
      */
     public void addFlowCall(FlowCall flowCall)
     {
-        flowCall.validate();
         String outcome = flowCall.getOutcome();
         if (flowCalls.containsKey(outcome))
         {
@@ -105,7 +104,6 @@
     /** For use only during object initialization. */
     public void setFlowAccept(FlowAccept flowAccept)
     {
-        flowAccept.validate();
         if (this.flowAccept != null)
         {
             throw new OrchestraException("Duplicate flowAccept defined");

Added: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowOnCommit.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowOnCommit.java?rev=675607&view=auto
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowOnCommit.java (added)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowOnCommit.java Thu Jul 10 08:03:48 2008
@@ -0,0 +1,88 @@
+/*
+ * 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.orchestra.flow.config;
+
+import javax.faces.context.FacesContext;
+import javax.faces.el.MethodBinding;
+
+import org.apache.myfaces.orchestra.lib.OrchestraException;
+
+
+/**
+ * Defines an action to execute in the caller environment after
+ * a flow has committed.
+ * <p>
+ * The invoked method is effectively running at the end of a postback cycle.
+ * <p>
+ * This option gives the caller a chance to process returned values.
+ * <p>
+ * Normally, the EL expression will map to a method with void return type.
+ * However if the EL expression invokes a method that returns a String, then
+ * that value is used as a navigation outcome; this allows a calling page to
+ * cause some other page to be rendered on completion of a flow. And as the
+ * values returned from the flow are available, the navigation can be dynamic
+ * depending upon those return values.
+ */
+public class FlowOnCommit
+{
+    String action;
+    
+    /** Constructor. */
+    public FlowOnCommit()
+    {
+    }
+
+    /**
+     * Check that all the properties of this object have valid values, ie
+     * whether the configuration specified by the user is valid.
+     */
+    public void validate()
+    {
+        if (action == null)
+        {
+            throw new OrchestraException("action is null");
+        }
+    }
+
+    /**
+     * An EL expression which defines what method to call.
+     * <p>
+     * Null is never returned.
+     */
+    public String getAction()
+    {
+        return action;
+    }
+
+    /** For use only during object initialization. */
+    public void setAction(String action)
+    {
+        this.action = action;
+    }
+
+    /**
+     * Execute the associated action, and return whatever the called method returns.
+     */
+    public Object execute(FacesContext facesContext)
+    {
+        MethodBinding mb = facesContext.getApplication().createMethodBinding(action, null);
+        Object o = mb.invoke(facesContext, null);
+        return o;
+    }
+}

Propchange: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowOnCommit.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/config/FlowOnCommit.java
------------------------------------------------------------------------------
    svn:executable = *

Modified: myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/digest/FlowDigester.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/digest/FlowDigester.java?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/digest/FlowDigester.java (original)
+++ myfaces/orchestra/trunk/flow/src/main/java/org/apache/myfaces/orchestra/flow/digest/FlowDigester.java Thu Jul 10 08:03:48 2008
@@ -24,6 +24,7 @@
 import org.apache.myfaces.orchestra.flow.config.FlowAccept;
 import org.apache.myfaces.orchestra.flow.config.FlowCall;
 import org.apache.myfaces.orchestra.flow.config.FlowConfig;
+import org.apache.myfaces.orchestra.flow.config.FlowOnCommit;
 import org.apache.myfaces.orchestra.flow.config.FlowParamAccept;
 import org.apache.myfaces.orchestra.flow.config.FlowParamSend;
 import org.apache.myfaces.orchestra.flow.config.FlowReturnAccept;
@@ -66,6 +67,10 @@
         d.addSetProperties("flowConfig/flowCall/return");
         d.addSetNext("flowConfig/flowCall/return", "addReturn");
 
+        d.addObjectCreate("flowConfig/flowCall/onCommit", FlowOnCommit.class);
+        d.addSetProperties("flowConfig/flowCall/onCommit");
+        d.addSetNext("flowConfig/flowCall/onCommit", "setOnCommit");
+
         d.addObjectCreate("flowConfig/flowAccept", FlowAccept.class);
         d.addSetProperties("flowConfig/flowAccept");
         d.addSetNext("flowConfig/flowAccept", "setFlowAccept");

Modified: myfaces/orchestra/trunk/flow/src/site/apt/usage.apt
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/flow/src/site/apt/usage.apt?rev=675607&r1=675606&r2=675607&view=diff
==============================================================================
--- myfaces/orchestra/trunk/flow/src/site/apt/usage.apt (original)
+++ myfaces/orchestra/trunk/flow/src/site/apt/usage.apt Thu Jul 10 08:03:48 2008
@@ -183,6 +183,7 @@
   <flowCall outcome="newAddress" service="com.acme.services.CreateAddress">
     <param name="type" src="#{userManager.addressType}"/>
     <return name="address" dst="#{user.address}"/>
+    <onCommit action="#{userManager.newAddress}"/>
   </flowCall>
 </flowConfig>
 
@@ -207,6 +208,16 @@
   specified for a return parameter of the called flow. The dst attribute is an EL
   expression that states where the named value returned by the flow should be stored.
 
+  An optional onCommit section can be defined. If present, then the specified method
+  is executed after the called flow has successfully finished and all return parameters
+  have been updated. This allows the calling page to process the returned parameters
+  after a flow has "committed". The expression normally points to a method that returns
+  void. However if it points to a method that returns a String, and a non-null value is
+  returned from that method then it is used as a navigation outcome. The onCommit
+  method can therefore cause the page rendered after flow return to be something other
+  than the caller; and as the return values are available when this function runs, it
+  can be a page dynamically chosen based on the outputs of the flow.
+
   Note that a page can trigger calls to as many different flows as it wants; multiple
   flowCall entries can exist in a single flow.xml file. This is unlike flowAccept, where
   a maximum of one entry is valid.
@@ -236,4 +247,50 @@
   and simply discards the current flow. This avoids any memory leaks for data associated
   with a flow.
 
-  
\ No newline at end of file
+Points of Interest
+~~~~~~~~~~~~~~~~~~
+
+* Passing Mutable Parameters to a Flow
+  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  
+  The flow system is designed to isolate the called flow from the caller except through
+  the specified input and return parameters. In particular, if a flow is cancelled, then
+  the caller will not be affected in any way. 
+  
+  However this is not true if the input parameters to a flow are mutable objects. In that
+  case, any modifications made by the called flow are still visible in the passed object
+  when the cancelled flow returns to the caller. This is of course just like java, where
+  a mutable object is passed to a method that then throws an exception; changes to the
+  parameter objecgt cannot be "undone".
+  
+  Therefore it is recommended that only immutable objects (or safe copies of mutable ones)
+  are passed as input parameters to flows. This isn't absolutely necessary if you know
+  that the called flow does not modify the parameter, but that cannot always be assumed. 
+  
+* Selecting Return Params
+  ~~~~~~~~~~~~~~~~~~~~~~~
+
+  The process of passing parameters from the caller to the called flow is not quite like
+  Java, but is reasonably intuitive. The way that data gets selected from the called flow
+  environment on return is somewhat odd, and requires some further explanation. The flow
+  entry point defines what outcome triggers a "commit" of the flow. Then when *any* page
+  in the flow performs a navigation using that outcome, then that is detected and the
+  EL expressions that the flow entry point defined are executed to select the data to
+  return. The EL expressions therefore do need to be written with care, so that they are
+  valid at the point that the commit outcome is used. However there is no way to force
+  a more java-style return to happen; requiring a flow to return to the caller only
+  from the entry page of the flow would be a closer analogy to Java but is too limiting
+  from a UI design viewpoint.
+  
+  In practice, flows generally only have one or two points at which they can
+  "successfully" exit, ie at which a flow commit can happen.
+
+  The process of "cancelling" a flow is of course much simpler, as no return values
+  are returned to the caller.
+
+* Persistence Contexts
+  ~~~~~~~~~~~~~~~~~~~~
+  
+  When the Persistence features of Orchestra Core are enabled, then they work in Flows
+  exactly as they do in "plain" Orchestra; each conversation has its own persistence
+  context.