svn commit: r581852 - /myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/jsf/lib/EndConversationMethodBindingFacade.java

[sk...@apache.org]

Author: skitching
Date: Thu Oct  4 04:20:05 2007
New Revision: 581852

URL: http://svn.apache.org/viewvc?rev=581852&view=rev
Log:
INCOMPATIBLE CHANGE:
Change behavior of ox:endConversation tag when null is returned by an action method; the conversation is now never
terminated in this case. Previously, it was terminated if onOutcomes was null, and not terminated otherwise.

Modified:
    myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/jsf/lib/EndConversationMethodBindingFacade.java

Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/jsf/lib/EndConversationMethodBindingFacade.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/jsf/lib/EndConversationMethodBindingFacade.java?rev=581852&r1=581851&r2=581852&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/jsf/lib/EndConversationMethodBindingFacade.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/jsf/lib/EndConversationMethodBindingFacade.java Thu Oct  4 04:20:05 2007
@@ -19,9 +19,7 @@
 
 package org.apache.myfaces.orchestra.conversation.jsf.lib;
 
-import org.apache.myfaces.orchestra.conversation.ConversationManager;
-import org.apache.myfaces.orchestra.conversation.ConversationUtils;
-import org.apache.myfaces.orchestra.conversation._ConversationUtils;
+import java.util.Collection;
 
 import javax.faces.FacesException;
 import javax.faces.component.StateHolder;
@@ -30,7 +28,9 @@
 import javax.faces.el.EvaluationException;
 import javax.faces.el.MethodBinding;
 import javax.faces.el.MethodNotFoundException;
-import java.util.Collection;
+
+import org.apache.myfaces.orchestra.conversation.ConversationManager;
+import org.apache.myfaces.orchestra.conversation.ConversationUtils;
 
 /**
  * A facade for the original method binding to deal with end conversation conditions.
@@ -64,19 +64,20 @@
 	 * @param conversation is the name of the conversation to conditionally be closed.
 	 *
 	 * @param onOutcomes is a collection of navigation strings that may be returned from the
-	 * invoked method. If the returned value <i>is</i> in this list then the named conversation
-	 * is ended (and possibly restarted), ie all beans currently in the conversation are discarded.
-	 * If the returned value is not in this list then the conversation is not affected. If this 
-	 * property is null then the conversation is always ended (except in the case where an
-	 * exception is thrown by the action method and no errorOutcome is specified). Note that if
-	 * onOutcomes is not null, then there is no way to specify that the conversation should
-	 * terminate if the action method returns null.
+	 * invoked method. The following rules are then followed to determine whether the conversation
+	 * is ended or not:
+	 * <ul>
+	 * <li>If the action returned null, then the conversation is never ended.
+	 * <li>If the onOutcomes list is null or empty then the conversation is always ended.
+	 * <li>Otherwise, the conversation ends only if the returned value is in the onOutcomes list.
+	 * </ul>
 	 *
 	 * @param original is the EL expression to be invoked.
 	 *
 	 * @param errorOutcome is a JSF navigation string to be returned if the action method
 	 * throws an exception of any kind. This navigation value is checked against the onOutcomes
-	 * values just as if the action method had actually returned this value.
+	 * values just as if the action method had actually returned this value. When not specified,
+	 * then on exception the current conversation is not terminated.
 	 */
 	public EndConversationMethodBindingFacade(
 		String conversation,
@@ -115,7 +116,6 @@
 
 	public Object invoke(FacesContext context, Object[] values) throws EvaluationException, MethodNotFoundException
 	{
-		boolean ok = true;
 		Object returnValue = null;
 		//noinspection CatchGenericClass
 		try
@@ -131,35 +131,46 @@
 
 			if (errorOutcome != null)
 			{
-				conversationManager.getMessager().setConversationException(t);
+				// Suppress the exception, and act as if errorOutcome had been returned. 
 
+				conversationManager.getMessager().setConversationException(t);
 				returnValue = errorOutcome;
 			}
 			else
 			{
-				// In this case, suppress the onOutcomes checking so the conversation
-				// doesn't get ended.
+				// When no errorOutcomes are specified, then there is nothing to check against
+				// the onOutcomes list. 
 				//
-				// TODO: perhaps the conversation should ALWAYS be ended on exception,
-				// instead of NEVER?
-				ok = false;
+				// Note that in this case the conversation is NEVER ended. It is debatable what
+				// the correct behaviour should be here. If this action wrapper was not present
+				// then the conversation would not be terminated, so an instance with no
+				// errorOutcomes specified is consistent with that. In any case, the user can
+				// easily get the alternate behaviour by simply specifying an errorOutcome and
+				// adding that to the onOutcomes list.
+
+				returnValue = null; // do not end conversation
 				throw new FacesException(t);
 			}
 		}
 		finally
 		{
-			boolean end = true;
-			if (ok)
+			boolean endConversation;
+			if (returnValue == null)
+			{
+				endConversation = false;
+			}
+			else if (onOutcomes == null || onOutcomes.isEmpty())
+			{
+				endConversation = true;
+			}
+			else
+			{
+				endConversation = onOutcomes.contains(returnValue);
+			}
+
+			if (endConversation)
 			{
-				if (onOutcomes != null && onOutcomes.size() > 0)
-				{
-					end = onOutcomes.contains(returnValue);
-				}
-
-				if (end)
-				{
-					ConversationUtils.invalidateIfExists(conversationName);
-				}
+				ConversationUtils.invalidateIfExists(conversationName);
 			}
 		}
 		return returnValue;