From 58c0e474bfe06f9255251d49cdcf20e1d0c8deac Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Mon, 13 Apr 2009 16:19:01 +0000 Subject: [PATCH] userEventQueued; in progress, get method should not have side effects --- .../faces/webflow/JsfView.java | 21 +++++++----------- .../faces/webflow/JsfViewFactory.java | 4 ---- .../faces/webflow/JsfViewTests.java | 3 --- .../webflow/engine/ViewState.java | 22 +++++++++---------- .../support/ActionExecutingViewFactory.java | 6 ++++- .../webflow/execution/View.java | 21 +++++++++++++----- .../webflow/mvc/view/AbstractMvcView.java | 19 +++++----------- .../webflow/test/MockViewFactoryCreator.java | 4 ++++ .../webflow/engine/StubViewFactory.java | 4 ++++ 9 files changed, 51 insertions(+), 53 deletions(-) diff --git a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java index b8086bf9..dd74096e 100644 --- a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java +++ b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java @@ -47,8 +47,6 @@ public class JsfView implements View { private String viewId; - private boolean restored; - /** * Creates a new JSF view. * @param viewRoot the view root @@ -74,14 +72,6 @@ public class JsfView implements View { this.viewRoot = viewRoot; } - /** - * Sets whether or not the view root for this view was restored from storage or is new. - * @param restored true or false - */ - public void setRestored(boolean restored) { - this.restored = restored; - } - /* * Performs the standard duties of the JSF RENDER_RESPONSE phase. */ @@ -103,6 +93,10 @@ public class JsfView implements View { } } + public boolean userEventQueued() { + return requestContext.getRequestParameters().size() > 1; + } + /* * Executes postback-processing portions of the standard JSF lifecycle including APPLY_REQUEST_VALUES through * INVOKE_APPLICATION. @@ -111,10 +105,11 @@ public class JsfView implements View { FacesContext facesContext = FlowFacesContext.newInstance(requestContext, facesLifecycle); facesContext.setViewRoot(viewRoot); try { - if (restored && !facesContext.getRenderResponse() && !facesContext.getResponseComplete()) { - facesLifecycle.execute(facesContext); + facesLifecycle.execute(facesContext); + if (!hasFlowEvent()) { + requestContext.getFlashScope().put(ViewRootHolder.VIEW_ROOT_HOLDER_KEY, + new ViewRootHolder(getViewRoot())); } - requestContext.getFlashScope().put(ViewRootHolder.VIEW_ROOT_HOLDER_KEY, new ViewRootHolder(getViewRoot())); } finally { facesContext.release(); } 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 9b7c5880..db663754 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 @@ -88,7 +88,6 @@ public class JsfViewFactory implements ViewFactory { viewRoot.setLocale(context.getExternalContext().getLocale()); processTree(facesContext, viewRoot); view = createJsfView(facesContext.getViewRoot(), lifecycle, context); - view.setRestored(true); } else { if (context.inViewState()) { UIViewRoot viewRoot = viewHandler.restoreView(facesContext, viewName); @@ -99,7 +98,6 @@ public class JsfViewFactory implements ViewFactory { facesContext.setViewRoot(viewRoot); processTree(facesContext, viewRoot); view = createJsfView(viewRoot, lifecycle, context); - view.setRestored(true); } else { if (logger.isDebugEnabled()) { logger.debug("Creating UIViewRoot from '" + viewName + "'"); @@ -107,7 +105,6 @@ public class JsfViewFactory implements ViewFactory { viewRoot = viewHandler.createView(facesContext, viewName); facesContext.setViewRoot(viewRoot); view = createJsfView(viewRoot, lifecycle, context); - view.setRestored(false); } } else { if (logger.isDebugEnabled()) { @@ -117,7 +114,6 @@ public class JsfViewFactory implements ViewFactory { viewRoot.setTransient(true); facesContext.setViewRoot(viewRoot); view = createJsfView(viewRoot, lifecycle, context); - view.setRestored(false); } } if (!facesContext.getRenderResponse()) { diff --git a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java index b2117962..a651db70 100644 --- a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java +++ b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java @@ -143,7 +143,6 @@ public class JsfViewTests extends TestCase { EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashScope }); JsfView restoredView = new JsfView(existingRoot, lifecycle, context); - restoredView.setRestored(true); restoredView.processUserEvent(); @@ -173,7 +172,6 @@ public class JsfViewTests extends TestCase { EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashScope }); JsfView restoredView = new JsfView(ajaxRoot, lifecycle, context); - restoredView.setRestored(true); restoredView.processUserEvent(); @@ -201,7 +199,6 @@ public class JsfViewTests extends TestCase { EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashScope }); JsfView restoredView = new JsfView(existingRoot, lifecycle, context); - restoredView.setRestored(true); restoredView.processUserEvent(); 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 3f327847..104d7fae 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 @@ -192,13 +192,16 @@ public class ViewState extends TransitionableState { public void resume(RequestControlContext context) { restoreVariables(context); View view = viewFactory.getView(context); - view.processUserEvent(); - if (view.hasFlowEvent()) { - Event event = view.getFlowEvent(); - if (logger.isDebugEnabled()) { - logger.debug("Event '" + event.getId() + "' returned from view " + view); + if (view.userEventQueued()) { + view.processUserEvent(); + boolean stateExited = false; + if (view.hasFlowEvent()) { + Event event = view.getFlowEvent(); + if (logger.isDebugEnabled()) { + logger.debug("Event '" + event.getId() + "' returned from view " + view); + } + stateExited = context.handleEvent(event); } - boolean stateExited = context.handleEvent(event); if (!stateExited) { ExternalContext externalContext = context.getExternalContext(); if (externalContext.isResponseComplete()) { @@ -216,12 +219,7 @@ public class ViewState extends TransitionableState { } } } else { - ExternalContext externalContext = context.getExternalContext(); - if (externalContext.isResponseComplete()) { - clearFlashIfNotRedirecting(context); - } else { - renderIfAllowed(context, view); - } + renderIfAllowed(context, view); } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/support/ActionExecutingViewFactory.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/support/ActionExecutingViewFactory.java index e5faa2ea..0cf4ce00 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/support/ActionExecutingViewFactory.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/support/ActionExecutingViewFactory.java @@ -69,8 +69,12 @@ public class ActionExecutingViewFactory implements ViewFactory { } } - public void processUserEvent() { + public boolean userEventQueued() { determineEventId(requestContext); + return eventId != null; + } + + public void processUserEvent() { } public boolean hasFlowEvent() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/View.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/View.java index c8a0f5a8..33b3c7eb 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/View.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/View.java @@ -18,7 +18,7 @@ package org.springframework.webflow.execution; import java.io.IOException; /** - * Allows the client to participate in flow execution. Encapsulates behavior to send the client an appropriate response + * Allows a client to participate in flow execution. Encapsulates behavior to send the client an appropriate response * and handle the resulting event once the client responds. * * @author Keith Donald @@ -38,20 +38,29 @@ public interface View { public void render() throws IOException; /** - * Execute the view resume lifecycle. This typically results in a view model binding and validation. + * True if there is a user event queued this view should process. + * @return true if a user event is queued, false if not + */ + public boolean userEventQueued(); + + /** + * Process the queued user event. Should only be called when {@link #userEventQueued()} returns true. After calling + * this method, a flow event may be queued that should be raised in the Web Flow system. + * @see #hasFlowEvent() */ public void processUserEvent(); /** - * Returns true if an event occurred the flow system should handle. + * True if a call to {@link #processUserEvent()} raised a flow event the current state should handle. Call + * {@link #getFlowEvent()} to access the Event. * @return true if yes, false otherwise */ public boolean hasFlowEvent(); /** - * Get the user event the flow should handle. Returns an event object when {@link #hasFlowEvent()} returns - * true. Returns null otherwise - * @return the event, or null if there is no event for the flow system to handle + * Get the flow event the current state should handle. Returns an Event object when {@link #hasFlowEvent()} returns + * true. Returns null otherwise. + * @return the event, or null if there is no event for the flow system to handle */ public Event getFlowEvent(); diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java index 9cbff6eb..9f5eef96 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java @@ -45,7 +45,6 @@ import org.springframework.util.Assert; import org.springframework.validation.BindingResult; import org.springframework.validation.MessageCodesResolver; import org.springframework.web.util.WebUtils; -import org.springframework.webflow.context.ExternalContext; import org.springframework.webflow.core.collection.AttributeMap; import org.springframework.webflow.core.collection.ParameterMap; import org.springframework.webflow.definition.TransitionDefinition; @@ -185,11 +184,12 @@ public abstract class AbstractMvcView implements View { } } - public void processUserEvent() { + public boolean userEventQueued() { eventId = determineEventId(requestContext); - if (eventId == null) { - return; - } + return eventId != null; + } + + public void processUserEvent() { if (logger.isDebugEnabled()) { logger.debug("User event '" + eventId + "' raised"); } @@ -219,15 +219,6 @@ public abstract class AbstractMvcView implements View { if (mappingResults != null && hasErrors(mappingResults)) { requestContext.getFlashScope().put(ViewActionStateHolder.KEY, new ViewActionStateHolder(eventId, mappingResults)); - ExternalContext context = requestContext.getExternalContext(); - if (!context.isAjaxRequest()) { - Boolean redirectOnPause = requestContext.getFlowExecutionContext().getAttributes().getBoolean( - "alwaysRedirectOnPause"); - boolean redirectAllowed = redirectOnPause != null ? redirectOnPause.booleanValue() : false; - if (redirectAllowed) { - requestContext.getExternalContext().requestFlowExecutionRedirect(); - } - } } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/test/MockViewFactoryCreator.java b/spring-webflow/src/main/java/org/springframework/webflow/test/MockViewFactoryCreator.java index 04a60625..a19fe07c 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/test/MockViewFactoryCreator.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/test/MockViewFactoryCreator.java @@ -95,6 +95,10 @@ class MockViewFactoryCreator implements ViewFactoryCreator { return viewId; } + public boolean userEventQueued() { + return hasFlowEvent(); + } + public void processUserEvent() { // TODO - implement me as appropriate for a test environment } diff --git a/spring-webflow/src/test/java/org/springframework/webflow/engine/StubViewFactory.java b/spring-webflow/src/test/java/org/springframework/webflow/engine/StubViewFactory.java index bd2ab94e..81498b12 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/engine/StubViewFactory.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/engine/StubViewFactory.java @@ -37,6 +37,10 @@ public class StubViewFactory implements ViewFactory { context.getFlowScope().put("renderCalled", Boolean.TRUE); } + public boolean userEventQueued() { + return hasFlowEvent(); + } + public void processUserEvent() { }