flow redirect bug fix

spring-webflow/src/main/java/org/springframework/webflow/action/FlowDefinitionRedirectAction.java

@@ -1,51 +1,53 @@
 package org.springframework.webflow.action;
 
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-
 import org.springframework.binding.expression.Expression;
 import org.springframework.util.Assert;
-import org.springframework.webflow.core.collection.AttributeMap;
+import org.springframework.util.StringUtils;
 import org.springframework.webflow.core.collection.LocalAttributeMap;
 import org.springframework.webflow.execution.Event;
 import org.springframework.webflow.execution.RequestContext;
 
 public class FlowDefinitionRedirectAction extends AbstractAction {
-	private Expression flowId;
+	private Expression expression;
-	private Map input;
 
-	public FlowDefinitionRedirectAction(Expression flowId, Map input) {
+	public FlowDefinitionRedirectAction(Expression expression) {
-		Assert.notNull(flowId, "The flow id to redirect to is required");
+		Assert.notNull(expression, "The flow definition redirect expression is required");
-		this.flowId = flowId;
+		this.expression = expression;
-		this.input = input;
 	}
 
 	protected Event doExecute(RequestContext context) throws Exception {
-		String flowId = (String) this.flowId.getValue(context);
+		String encodedRedirect = (String) expression.getValue(context);
-		AttributeMap input = evaluateInput(context);
+		if (encodedRedirect == null) {
-		context.getExternalContext().requestFlowDefinitionRedirect(flowId, input);
+			throw new IllegalStateException(
+					"Flow definition redirect expression evaluated to [null], the expression was " + expression);
+		}
+		// the encoded FlowDefinitionRedirect should look something like
+		// "flowDefinitionId?param0=value0&param1=value1"
+		// now parse that and build a corresponding view selection
+		int index = encodedRedirect.indexOf('?');
+		String flowDefinitionId;
+		LocalAttributeMap executionInput = null;
+		if (index != -1) {
+			flowDefinitionId = encodedRedirect.substring(0, index);
+			String[] parameters = StringUtils.delimitedListToStringArray(encodedRedirect.substring(index + 1), "&");
+			executionInput = new LocalAttributeMap(parameters.length, 1);
+			for (int i = 0; i < parameters.length; i++) {
+				String nameAndValue = parameters[i];
+				index = nameAndValue.indexOf('=');
+				if (index != -1) {
+					executionInput.put(nameAndValue.substring(0, index), nameAndValue.substring(index + 1));
+				} else {
+					executionInput.put(nameAndValue, "");
+				}
+			}
+		} else {
+			flowDefinitionId = encodedRedirect;
+		}
+		if (!StringUtils.hasText(flowDefinitionId)) {
+			// equivalent to restart
+			flowDefinitionId = context.getFlowExecutionContext().getDefinition().getId();
+		}
+		context.getExternalContext().requestFlowDefinitionRedirect(flowDefinitionId, executionInput);
 		return success();
 	}
-
-	private AttributeMap evaluateInput(RequestContext context) {
-		if (this.input == null) {
-			return null;
-		} else {
-			Map input = new HashMap();
-			for (Iterator it = this.input.entrySet().iterator(); it.hasNext();) {
-				Map.Entry entry = (Map.Entry) it.next();
-				Expression name = (Expression) entry.getKey();
-				Expression value = (Expression) entry.getValue();
-				String paramName = (String) name.getValue(context);
-				Object paramValue = value.getValue(context);
-				input.put(paramName, paramValue);
-			}
-			return new LocalAttributeMap(input);
-		}
-	}
-
-	public static FlowDefinitionRedirectAction create(String encodedFlowRedirect) {
-		throw new UnsupportedOperationException("Not yet implemented");
-	}
 }

spring-webflow/src/main/java/org/springframework/webflow/action/ViewFactoryActionAdapter.java

@@ -16,7 +16,9 @@ public class ViewFactoryActionAdapter extends AbstractAction {
 	}
 
 	protected Event doExecute(RequestContext context) throws Exception {
-		viewFactory.getView(context).render();
+		if (viewFactory != null) {
+			viewFactory.getView(context).render();
+		}
 		return new Event(this, "success");
 	}
 }

spring-webflow/src/main/java/org/springframework/webflow/engine/builder/xml/XmlFlowBuilder.java

@@ -572,7 +572,7 @@ public class XmlFlowBuilder extends AbstractFlowBuilder implements ResourceHolde
 	}
 
 	private void parseAndAddViewState(Element element, Flow flow) {
-		ViewFactory viewFactory = parseViewFactory(element);
+		ViewFactory viewFactory = parseViewFactory(element, false);
 		boolean redirect = false;
 		if (element.hasAttribute("redirect")) {
 			redirect = ((Boolean) fromStringTo(Boolean.class).execute(element.getAttribute("redirect"))).booleanValue();
@@ -607,8 +607,8 @@ public class XmlFlowBuilder extends AbstractFlowBuilder implements ResourceHolde
 		MutableAttributeMap attributes = parseAttributes(element);
 		parseAndSetSecuredAttribute(element, attributes);
 		getFlowArtifactFactory().createEndState(parseId(element), flow, parseEntryActions(element),
-				parseFinalResponseAction(element), parseOutputMapper(element), parseExceptionHandlers(element),
+				new ViewFactoryActionAdapter(parseViewFactory(element, true)), parseOutputMapper(element),
-				attributes);
+				parseExceptionHandlers(element), attributes);
 	}
 
 	private String parseId(Element element) {
@@ -640,14 +640,18 @@ public class XmlFlowBuilder extends AbstractFlowBuilder implements ResourceHolde
 		}
 	}
 
-	private ViewFactory parseViewFactory(Element element) {
+	private ViewFactory parseViewFactory(Element element, boolean endState) {
 		String encodedView = element.getAttribute(VIEW_ATTRIBUTE);
 		if (!StringUtils.hasText(encodedView)) {
-			encodedView = createViewId(element.getAttribute(ID_ATTRIBUTE));
+			if (endState) {
-			Expression viewName = getExpressionParser().parseExpression(encodedView,
+				return null;
-					new ParserContextImpl().eval(RequestContext.class).expect(String.class));
+			} else {
-			return getLocalContext().getViewFactoryCreator().createViewFactory(viewName,
+				encodedView = createViewId(element.getAttribute(ID_ATTRIBUTE));
-					getLocalContext().getResourceLoader());
+				Expression viewName = getExpressionParser().parseExpression(encodedView,
+						new ParserContextImpl().eval(RequestContext.class).expect(String.class));
+				return getLocalContext().getViewFactoryCreator().createViewFactory(viewName,
+						getLocalContext().getResourceLoader());
+			}
 		} else if (encodedView.startsWith(EXTERNAL_REDIRECT_PREFIX)) {
 			String encodedUrl = encodedView.substring(EXTERNAL_REDIRECT_PREFIX.length());
 			Expression externalUrl = getExpressionParser().parseExpression(encodedUrl,
@@ -655,7 +659,9 @@ public class XmlFlowBuilder extends AbstractFlowBuilder implements ResourceHolde
 			return new ActionExecutingViewFactory(new ExternalRedirectAction(externalUrl));
 		} else if (encodedView.startsWith(FLOW_DEFINITION_REDIRECT_PREFIX)) {
 			String flowRedirect = encodedView.substring(FLOW_DEFINITION_REDIRECT_PREFIX.length());
-			return new ActionExecutingViewFactory(FlowDefinitionRedirectAction.create(flowRedirect));
+			Expression expression = getExpressionParser().parseExpression(flowRedirect,
+					new ParserContextImpl().eval(RequestContext.class).expect(String.class));
+			return new ActionExecutingViewFactory(new FlowDefinitionRedirectAction(expression));
 		} else if (encodedView.startsWith(BEAN_PREFIX)) {
 			return (ViewFactory) getLocalContext().getBeanFactory().getBean(
 					encodedView.substring(BEAN_PREFIX.length()), ViewFactory.class);
@@ -1070,30 +1076,6 @@ public class XmlFlowBuilder extends AbstractFlowBuilder implements ResourceHolde
 		return null;
 	}
 
-	private Action parseFinalResponseAction(Element element) {
-		String encodedView = element.getAttribute(VIEW_ATTRIBUTE);
-		if (encodedView == null || encodedView.length() == 0) {
-			// null final responses are allowed
-			return null;
-		} else if (encodedView.startsWith(EXTERNAL_REDIRECT_PREFIX)) {
-			String encodedUrl = encodedView.substring(EXTERNAL_REDIRECT_PREFIX.length());
-			Expression externalUrl = getExpressionParser().parseExpression(encodedUrl,
-					new ParserContextImpl().eval(RequestContext.class).expect(String.class));
-			return new ExternalRedirectAction(externalUrl);
-		} else if (encodedView.startsWith(FLOW_DEFINITION_REDIRECT_PREFIX)) {
-			String flowRedirect = encodedView.substring(FLOW_DEFINITION_REDIRECT_PREFIX.length());
-			return FlowDefinitionRedirectAction.create(flowRedirect);
-		} else if (encodedView.startsWith(BEAN_PREFIX)) {
-			return (Action) getLocalContext().getBeanFactory().getBean(encodedView.substring(BEAN_PREFIX.length()),
-					Action.class);
-		} else {
-			Expression viewName = getExpressionParser().parseExpression(encodedView,
-					new ParserContextImpl().eval(RequestContext.class).expect(String.class));
-			return new ViewFactoryActionAdapter(getLocalContext().getViewFactoryCreator().createViewFactory(viewName,
-					getLocalContext().getResourceLoader()));
-		}
-	}
-
 	private FlowExecutionExceptionHandler[] parseExceptionHandlers(Element element) {
 		FlowExecutionExceptionHandler[] transitionExecutingHandlers = parseTransitionExecutingExceptionHandlers(element);
 		FlowExecutionExceptionHandler[] customHandlers = parseCustomExceptionHandlers(element);

spring-webflow/src/test/java/org/springframework/webflow/action/FlowDefinitionRedirectActionTests.java

@@ -1,8 +1,5 @@
 package org.springframework.webflow.action;
 
-import java.util.HashMap;
-import java.util.Map;
-
 import junit.framework.TestCase;
 
 import org.springframework.binding.expression.Expression;
@@ -13,10 +10,8 @@ public class FlowDefinitionRedirectActionTests extends TestCase {
 	private FlowDefinitionRedirectAction action;
 
 	public void testExecute() throws Exception {
-		Expression flowId = new StaticExpression("user");
+		Expression flowId = new StaticExpression("user?foo=bar");
-		Map input = new HashMap();
+		action = new FlowDefinitionRedirectAction(flowId);
-		input.put(new StaticExpression("foo"), new StaticExpression("bar"));
-		action = new FlowDefinitionRedirectAction(flowId, input);
 		MockRequestContext context = new MockRequestContext();
 		action.execute(context);
 		assertEquals("user", context.getMockExternalContext().getFlowRedirectFlowId());
@@ -25,7 +20,7 @@ public class FlowDefinitionRedirectActionTests extends TestCase {
 
 	public void testExecuteWithNullRequestFields() throws Exception {
 		Expression flowId = new StaticExpression("user");
-		action = new FlowDefinitionRedirectAction(flowId, null);
+		action = new FlowDefinitionRedirectAction(flowId);
 		MockRequestContext context = new MockRequestContext();
 		action.execute(context);
 		assertEquals("user", context.getMockExternalContext().getFlowRedirectFlowId());
@@ -33,7 +28,7 @@ public class FlowDefinitionRedirectActionTests extends TestCase {
 
 	public void testExecuteWithNullFlowId() throws Exception {
 		try {
-			action = new FlowDefinitionRedirectAction(null, null);
+			action = new FlowDefinitionRedirectAction(null);
 			fail("Should have failed");
 		} catch (IllegalArgumentException e) {