From 381e5e2091fb96ede2c02ceeab1bcf48580881a0 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Sun, 13 Jul 2008 04:41:06 +0000 Subject: [PATCH] end-state bug fix --- .../faces/webflow/JsfViewFactory.java | 21 ++++++----- .../webflow/context/ExternalContext.java | 36 +++++++------------ .../portlet/PortletExternalContext.java | 15 ++------ .../servlet/ServletExternalContext.java | 14 ++------ .../webflow/engine/EndState.java | 2 +- .../webflow/engine/ViewState.java | 2 +- .../builder/model/FlowModelFlowBuilder.java | 10 ++++-- .../mvc/servlet/FlowHandlerAdapter.java | 2 +- .../webflow/test/MockExternalContext.java | 7 ++-- .../portlet/PortletExternalContextTests.java | 8 +---- .../servlet/ServletExternalContextTests.java | 10 ++---- .../mvc/servlet/FlowHandlerAdapterTests.java | 2 +- 12 files changed, 48 insertions(+), 81 deletions(-) diff --git a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java index e62c4b0a..82494bd0 100644 --- a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java +++ b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java @@ -97,26 +97,29 @@ public class JsfViewFactory implements ViewFactory { UIViewRoot viewRoot = viewHandler.restoreView(facesContext, viewName); if (viewRoot != null) { if (logger.isDebugEnabled()) { - logger.debug("View root restored for '" + viewName + "'"); + logger.debug("UIViewRoot restored for '" + viewName + "'"); } + facesContext.setViewRoot(viewRoot); + processTree(facesContext, viewRoot); view = createJsfView(viewRoot, lifecycle, context); - facesContext.setViewRoot(view.getViewRoot()); - processTree(facesContext, view.getViewRoot()); view.setRestored(true); } else { if (logger.isDebugEnabled()) { - logger.debug("Creating view root for '" + viewName + "'"); + logger.debug("Creating UIViewRoot from '" + viewName + "'"); } - view = createJsfView(viewHandler.createView(facesContext, viewName), lifecycle, context); - facesContext.setViewRoot(view.getViewRoot()); + viewRoot = viewHandler.createView(facesContext, viewName); + facesContext.setViewRoot(viewRoot); + view = createJsfView(viewRoot, lifecycle, context); view.setRestored(false); } } else { if (logger.isDebugEnabled()) { - logger.debug("Creating view root for '" + viewName + "'"); + logger.debug("Creating transient UIViewRoot from '" + viewName + "'"); } - view = createJsfView(viewHandler.createView(facesContext, viewName), lifecycle, context); - facesContext.setViewRoot(view.getViewRoot()); + UIViewRoot viewRoot = viewHandler.createView(facesContext, viewName); + viewRoot.setTransient(true); + facesContext.setViewRoot(viewRoot); + view = createJsfView(viewRoot, lifecycle, context); view.setRestored(false); } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/context/ExternalContext.java b/spring-webflow/src/main/java/org/springframework/webflow/context/ExternalContext.java index e0e99a94..eafce00b 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/context/ExternalContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/context/ExternalContext.java @@ -137,34 +137,31 @@ public interface ExternalContext { */ public Writer getResponseWriter(); + /** + * Is a response allowed to be written for this request? + * @return true if yes, false otherwise + */ + public boolean isResponseAllowed(); + /** * Request that a flow execution redirect be performed by the calling environment. Typically called from within a * flow execution to request a refresh operation, usually to support "refresh after event processing" behavior. - * Calling this method commits the response. - * @see #isResponseCommitted() - * @throws IllegalStateException if a response has already been committed */ - public void requestFlowExecutionRedirect() throws IllegalStateException; + public void requestFlowExecutionRedirect(); /** * Request that a flow definition redirect be performed by the calling environment. Typically called from within a * flow execution end state to request starting a new, independent execution of a flow in a chain-like manner. - * Calling this method commits the response. - * @see #isResponseCommitted() * @param flowId the id of the flow definition to redirect to * @param input input to pass the flow; this input is generally encoded the url to launch the flow - * @throws IllegalStateException if a response has already been committed */ - public void requestFlowDefinitionRedirect(String flowId, MutableAttributeMap input) throws IllegalStateException; + public void requestFlowDefinitionRedirect(String flowId, MutableAttributeMap input); /** - * Request a redirect to an arbitrary resource location. May not be supported in some environments. Calling this - * method commits the response. - * @see #isResponseCommitted() + * Request a redirect to an arbitrary resource location. May not be supported in some environments. * @param location the location of the resource to redirect to - * @throws IllegalStateException if a response has already been committed */ - public void requestExternalRedirect(String location) throws IllegalStateException; + public void requestExternalRedirect(String location); /** * Request that the redirect response requested be sent to the client in a manner that causes the client to issue @@ -179,20 +176,13 @@ public interface ExternalContext { * Called by flow artifacts such as View states and end states to indicate they handled the response, typically by * writing out content to the response stream. Setting this flag allows this external context to know the response * was handled, and that it not need to take additional response handling action itself. - * @throws IllegalStateException if a response has already been committed */ - public void recordResponseCommitted() throws IllegalStateException; + public void recordResponseComplete(); /** - * Has the response been committed? + * Has the response been completed via a call to {@link #recordResponseComplete()}? * @return true if yes, false otherwise */ - public boolean isResponseCommitted(); - - /** - * Is a response allowed to be written for this request? - * @return true if yes, false otherwise - */ - public boolean isResponseAllowed(); + public boolean isResponseComplete(); } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/context/portlet/PortletExternalContext.java b/spring-webflow/src/main/java/org/springframework/webflow/context/portlet/PortletExternalContext.java index 719ebe09..f9018b39 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/context/portlet/PortletExternalContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/context/portlet/PortletExternalContext.java @@ -28,7 +28,6 @@ import javax.portlet.PortletResponse; import javax.portlet.RenderRequest; import javax.portlet.RenderResponse; -import org.springframework.util.Assert; import org.springframework.webflow.context.ExternalContext; import org.springframework.webflow.core.collection.LocalAttributeMap; import org.springframework.webflow.core.collection.LocalParameterMap; @@ -93,7 +92,7 @@ public class PortletExternalContext implements ExternalContext { /** * A flag indicating if the flow committed the response. Set to true by requesting an execution redirect, definition - * redirect, external redirect, or by calling {@link ExternalContext#recordResponseCommitted()} + * redirect, external redirect, or by calling {@link ExternalContext#recordResponseComplete()} */ private boolean responseCommitted; @@ -227,27 +226,23 @@ public class PortletExternalContext implements ExternalContext { return isRenderPhase(); } - public boolean isResponseCommitted() { + public boolean isResponseComplete() { return responseCommitted; } - public void recordResponseCommitted() { - assertResponseNotCommitted(); + public void recordResponseComplete() { responseCommitted = true; } public void requestFlowExecutionRedirect() { - recordResponseCommitted(); flowExecutionRedirectRequested = true; } public void requestExternalRedirect(String uri) { - recordResponseCommitted(); externalRedirectUrl = uri; } public void requestFlowDefinitionRedirect(String flowId, MutableAttributeMap input) { - recordResponseCommitted(); flowDefinitionRedirectFlowId = flowId; flowDefinitionRedirectFlowInput = input; } @@ -344,8 +339,4 @@ public class PortletExternalContext implements ExternalContext { } } - private void assertResponseNotCommitted() throws IllegalStateException { - Assert.isTrue(!responseCommitted, "A response has already been committed to this ExternalContext"); - } - } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/context/servlet/ServletExternalContext.java b/spring-webflow/src/main/java/org/springframework/webflow/context/servlet/ServletExternalContext.java index 348557b2..2e38a6db 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/context/servlet/ServletExternalContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/context/servlet/ServletExternalContext.java @@ -24,7 +24,6 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.springframework.util.Assert; import org.springframework.webflow.context.ExternalContext; import org.springframework.webflow.core.collection.LocalAttributeMap; import org.springframework.webflow.core.collection.LocalParameterMap; @@ -79,7 +78,7 @@ public class ServletExternalContext implements ExternalContext { /** * A flag indicating if the flow committed the response. Set to true by requesting an execution redirect, definition - * redirect, external redirect, or by calling {@link ExternalContext#recordResponseCommitted()} + * redirect, external redirect, or by calling {@link ExternalContext#recordResponseComplete()} */ private boolean responseCommitted; @@ -220,27 +219,23 @@ public class ServletExternalContext implements ExternalContext { return true; } - public boolean isResponseCommitted() { + public boolean isResponseComplete() { return responseCommitted; } - public void recordResponseCommitted() { - assertResponseNotCommitted(); + public void recordResponseComplete() { responseCommitted = true; } public void requestFlowExecutionRedirect() { - recordResponseCommitted(); flowExecutionRedirectRequested = true; } public void requestExternalRedirect(String location) { - recordResponseCommitted(); externalRedirectUrl = location; } public void requestFlowDefinitionRedirect(String flowId, MutableAttributeMap input) { - recordResponseCommitted(); flowDefinitionRedirectFlowId = flowId; flowDefinitionRedirectFlowInput = input; } @@ -346,7 +341,4 @@ public class ServletExternalContext implements ExternalContext { this.flowUrlHandler = flowUrlHandler; } - private void assertResponseNotCommitted() throws IllegalStateException { - Assert.isTrue(!responseCommitted, "A response has already been committed to this ExternalContext"); - } } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/EndState.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/EndState.java index 4e6c7fb1..52ba7ab2 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/EndState.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/EndState.java @@ -98,7 +98,7 @@ public class EndState extends State { // entire flow execution is ending; issue the final response if (finalResponseAction != null && context.getExternalContext().isResponseAllowed()) { ActionExecutor.execute(finalResponseAction, context); - context.getExternalContext().recordResponseCommitted(); + context.getExternalContext().recordResponseComplete(); } context.endActiveFlowSession(getId(), createSessionOutput(context)); } else { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/ViewState.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/ViewState.java index 3dc2f7fd..075efa95 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/ViewState.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/ViewState.java @@ -243,8 +243,8 @@ public class ViewState extends TransitionableState { } context.getFlashScope().clear(); context.getMessageContext().clearMessages(); + context.getExternalContext().recordResponseComplete(); context.viewRendered(view); - context.getExternalContext().recordResponseCommitted(); } private void restoreVariables(RequestContext context) { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/builder/model/FlowModelFlowBuilder.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/builder/model/FlowModelFlowBuilder.java index 6b6c5992..8b08cbc5 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/builder/model/FlowModelFlowBuilder.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/builder/model/FlowModelFlowBuilder.java @@ -570,9 +570,15 @@ public class FlowModelFlowBuilder extends AbstractFlowBuilder { attributes.put("commit", fromStringTo(Boolean.class).execute(state.getCommit())); } parseAndPutSecured(state.getSecured(), attributes); + Action finalResponseAction; + ViewFactory viewFactory = parseViewFactory(state.getView(), state.getId(), true); + if (viewFactory != null) { + finalResponseAction = new ViewFactoryActionAdapter(viewFactory); + } else { + finalResponseAction = null; + } getLocalContext().getFlowArtifactFactory().createEndState(state.getId(), flow, - parseActions(state.getOnEntryActions()), - new ViewFactoryActionAdapter(parseViewFactory(state.getView(), state.getId(), true)), + parseActions(state.getOnEntryActions()), finalResponseAction, parseFlowOutputMapper(state.getOutputs()), parseExceptionHandlers(state.getExceptionHandlers(), null), attributes); } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java index 1c3ccfd7..cde4af64 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java @@ -253,7 +253,7 @@ public class FlowHandlerAdapter extends WebContentGenerator implements HandlerAd protected void defaultHandleExecutionOutcome(String flowId, FlowExecutionOutcome outcome, ServletExternalContext context, HttpServletRequest request, HttpServletResponse response) throws IOException { - if (!context.isResponseCommitted()) { + if (!context.isResponseComplete()) { // by default, just start the flow over passing the output as input if (logger.isDebugEnabled()) { logger.debug("Ended flow '" + flowId + "' did not commit a response; " diff --git a/spring-webflow/src/main/java/org/springframework/webflow/test/MockExternalContext.java b/spring-webflow/src/main/java/org/springframework/webflow/test/MockExternalContext.java index 30aff798..1fd85e84 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/test/MockExternalContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/test/MockExternalContext.java @@ -154,27 +154,24 @@ public class MockExternalContext implements ExternalContext { return true; } - public boolean isResponseCommitted() { + public boolean isResponseComplete() { return responseCommitted; } - public void recordResponseCommitted() throws IllegalStateException { + public void recordResponseComplete() throws IllegalStateException { responseCommitted = true; } public void requestFlowExecutionRedirect() { - recordResponseCommitted(); flowExecutionRedirectRequested = true; } public void requestFlowDefinitionRedirect(String flowId, MutableAttributeMap input) { - recordResponseCommitted(); flowDefinitionRedirectFlowId = flowId; flowDefinitionRedirectFlowInput = input; } public void requestExternalRedirect(String uri) { - recordResponseCommitted(); externalRedirectUrl = uri; } diff --git a/spring-webflow/src/test/java/org/springframework/webflow/context/portlet/PortletExternalContextTests.java b/spring-webflow/src/test/java/org/springframework/webflow/context/portlet/PortletExternalContextTests.java index 929ecb53..37cf7da7 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/context/portlet/PortletExternalContextTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/context/portlet/PortletExternalContextTests.java @@ -68,25 +68,22 @@ public class PortletExternalContextTests extends TestCase { } public void testNotResponseCommitted() { - assertFalse(context.isResponseCommitted()); + assertFalse(context.isResponseComplete()); } public void testCommitExecutionRedirect() { context.requestFlowExecutionRedirect(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowExecutionRedirectRequested()); } public void testCommitFlowRedirect() { context.requestFlowDefinitionRedirect("foo", null); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowDefinitionRedirectRequested()); assertEquals("foo", context.getFlowRedirectFlowId()); } public void testCommitExternalRedirect() { context.requestExternalRedirect("foo"); - assertTrue(context.isResponseCommitted()); assertTrue(context.getExternalRedirectRequested()); assertEquals("foo", context.getExternalRedirectUrl()); } @@ -94,7 +91,6 @@ public class PortletExternalContextTests extends TestCase { public void testCommitExecutionRedirectPopup() { context.requestFlowExecutionRedirect(); context.requestRedirectInPopup(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowExecutionRedirectRequested()); assertTrue(context.getRedirectInPopup()); } @@ -102,7 +98,6 @@ public class PortletExternalContextTests extends TestCase { public void testCommitFlowRedirectPopup() { context.requestFlowDefinitionRedirect("foo", null); context.requestRedirectInPopup(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowDefinitionRedirectRequested()); assertEquals("foo", context.getFlowRedirectFlowId()); assertTrue(context.getRedirectInPopup()); @@ -111,7 +106,6 @@ public class PortletExternalContextTests extends TestCase { public void testCommitExternalRedirectPopup() { context.requestExternalRedirect("foo"); context.requestRedirectInPopup(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getExternalRedirectRequested()); assertEquals("foo", context.getExternalRedirectUrl()); assertTrue(context.getRedirectInPopup()); diff --git a/spring-webflow/src/test/java/org/springframework/webflow/context/servlet/ServletExternalContextTests.java b/spring-webflow/src/test/java/org/springframework/webflow/context/servlet/ServletExternalContextTests.java index 42be908d..5568a48c 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/context/servlet/ServletExternalContextTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/context/servlet/ServletExternalContextTests.java @@ -66,25 +66,22 @@ public class ServletExternalContextTests extends TestCase { } public void testNotResponseCommitted() { - assertFalse(context.isResponseCommitted()); + assertFalse(context.isResponseComplete()); } public void testCommitExecutionRedirect() { context.requestFlowExecutionRedirect(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowExecutionRedirectRequested()); } public void testCommitFlowRedirect() { context.requestFlowDefinitionRedirect("foo", null); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowDefinitionRedirectRequested()); assertEquals("foo", context.getFlowRedirectFlowId()); } public void testCommitExternalRedirect() { context.requestExternalRedirect("foo"); - assertTrue(context.isResponseCommitted()); assertTrue(context.getExternalRedirectRequested()); assertEquals("foo", context.getExternalRedirectUrl()); } @@ -92,7 +89,6 @@ public class ServletExternalContextTests extends TestCase { public void testCommitExecutionRedirectPopup() { context.requestFlowExecutionRedirect(); context.requestRedirectInPopup(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowExecutionRedirectRequested()); assertTrue(context.getRedirectInPopup()); } @@ -100,7 +96,6 @@ public class ServletExternalContextTests extends TestCase { public void testCommitFlowRedirectPopup() { context.requestFlowDefinitionRedirect("foo", null); context.requestRedirectInPopup(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getFlowDefinitionRedirectRequested()); assertEquals("foo", context.getFlowRedirectFlowId()); assertTrue(context.getRedirectInPopup()); @@ -109,7 +104,6 @@ public class ServletExternalContextTests extends TestCase { public void testCommitExternalRedirectPopup() { context.requestExternalRedirect("foo"); context.requestRedirectInPopup(); - assertTrue(context.isResponseCommitted()); assertTrue(context.getExternalRedirectRequested()); assertEquals("foo", context.getExternalRedirectUrl()); assertTrue(context.getRedirectInPopup()); @@ -119,4 +113,4 @@ public class ServletExternalContextTests extends TestCase { assertTrue(context.isResponseAllowed()); } -} +} \ No newline at end of file diff --git a/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java b/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java index b4b136e8..f2dd0169 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java @@ -166,7 +166,7 @@ public class FlowHandlerAdapterTests extends TestCase { flowExecutor.resumeExecution("12345", context); LocalAttributeMap output = new LocalAttributeMap(); output.put("bar", "baz"); - context.recordResponseCommitted(); + context.recordResponseComplete(); FlowExecutionOutcome outcome = new FlowExecutionOutcome("finish", output); FlowExecutionResult result = FlowExecutionResult.createEndedResult("foo", outcome); EasyMock.expectLastCall().andReturn(result);