From abfa4ba16c3427cc391e378b75d0a2a1ff39a366 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Tue, 1 May 2007 14:34:40 +0000 Subject: [PATCH] SWF 305 --- spring-webflow/changelog.txt | 2 + .../executor/jsf/FlowNavigationHandler.java | 118 ++++++++++-------- .../executor/jsf/FlowPhaseListener.java | 77 +++++++----- .../executor/jsf/JsfExternalContext.java | 23 ++-- ...vigationHandlerArgumentExtractorTests.java | 10 +- 5 files changed, 130 insertions(+), 100 deletions(-) diff --git a/spring-webflow/changelog.txt b/spring-webflow/changelog.txt index eab4a7ff..0a074036 100644 --- a/spring-webflow/changelog.txt +++ b/spring-webflow/changelog.txt @@ -18,6 +18,8 @@ Package org.springframework.webflow.executor RENDER RESPONSE phase is bypassed (SWF-302). * Restored compatability with 1.0.1 allowing default navigation rules to be queried if no transition could be matched against the state of the current flow execution (SWF-303). +* JSF integration code now tears down ExternalContext thread local properly in exceptional situations and when + the RENDER_RESPONSE phase is bypassed (SWF-305). Changes in version 1.0.3 (19.04.2007) ------------------------------------- diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowNavigationHandler.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowNavigationHandler.java index 210efba1..357ce723 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowNavigationHandler.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowNavigationHandler.java @@ -23,6 +23,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.binding.mapping.AttributeMapper; import org.springframework.web.jsf.DecoratingNavigationHandler; import org.springframework.webflow.context.ExternalContext; +import org.springframework.webflow.context.ExternalContextHolder; import org.springframework.webflow.core.collection.LocalAttributeMap; import org.springframework.webflow.core.collection.MutableAttributeMap; import org.springframework.webflow.definition.FlowDefinition; @@ -35,33 +36,38 @@ import org.springframework.webflow.executor.RequestParameterInputMapper; import org.springframework.webflow.executor.support.FlowExecutorArgumentExtractor; /** - * An implementation of a JSF NavigationHandler that provides - * integration with Spring Web Flow. Responsible for delegating to Spring Web - * Flow to launch and resume flow executions, treating JSF action outcomes (like - * a command button click) as web flow events. + * An implementation of a JSF NavigationHandler that provides integration with Spring Web Flow. + * Responsible for delegating to Spring Web Flow to launch and resume flow executions, treating JSF action outcomes + * (like a command button click) as web flow events. * - * This class delegates to the standard NavigationHandler implementation when a - * navigation request does not pertain to a flow execution.

The following - * navigation handler algorithm is implemented by default:

If a flow - * execution has been restored in the current request:

If a flow execution has not - * been restored in the current request:

How the flowId and eventId arguments - * are extracted can be customized by setting a custom + * This class delegates to the standard NavigationHandler implementation when a navigation request does not pertain to a + * flow execution. + *

+ * The following navigation handler algorithm is implemented by default: + *

+ *

+ * If a flow execution has been restored in the current request: + *

+ *

+ *

+ * If a flow execution has not been restored in the current request: + *

+ *

+ * How the flowId and eventId arguments are extracted can be customized by setting a custom * {@link #setArgumentExtractor(FlowExecutorArgumentExtractor) argument extractor}. * - * Note about customization: since NavigationHandlers managed directly by the - * JSF provider cannot be benefit from DependencyInjection, See Spring's - * {@link org.springframework.web.jsf.DelegatingNavigationHandlerProxy} when you - * need to customize a FlowNavigationHandler instance. + * Note about customization: since NavigationHandlers managed directly by the JSF provider cannot be benefit from + * DependencyInjection, See Spring's {@link org.springframework.web.jsf.DelegatingNavigationHandlerProxy} when you need + * to customize a FlowNavigationHandler instance. * * @author Craig McClanahan * @author Colin Sampaleanu @@ -75,20 +81,19 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { protected final Log logger = LogFactory.getLog(getClass()); /** - * A helper for extracting parameters needed by this flow navigation - * handler. + * A helper for extracting parameters needed by this flow navigation handler. */ private FlowExecutorArgumentExtractor argumentExtractor = new FlowNavigationHandlerArgumentExtractor(); /** - * The service responsible for mapping attributes of an - * {@link ExternalContext} to a new {@link FlowExecution} during the - * {@link #launch(String, ExternalContext) launch flow} operation.

This - * allows developers to control what attributes are made available in the - * inputMap to new top-level flow executions. The starting - * execution may then choose to map that available input into its own local - * scope.

The default implementation simply exposes all request - * parameters as flow execution input attributes. May be null. + * The service responsible for mapping attributes of an {@link ExternalContext} to a new {@link FlowExecution} + * during the {@link #launch(String, ExternalContext) launch flow} operation. + *

+ * This allows developers to control what attributes are made available in the inputMap to new + * top-level flow executions. The starting execution may then choose to map that available input into its own local + * scope. + *

+ * The default implementation simply exposes all request parameters as flow execution input attributes. May be null. */ private AttributeMapper inputMapper = new RequestParameterInputMapper(); @@ -100,10 +105,8 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { } /** - * Create a new {@link FlowNavigationHandler}, wrapping the specified - * standard navigation handler implementation. - * @param originalNavigationHandler Standard NavigationHandler - * we are wrapping + * Create a new {@link FlowNavigationHandler}, wrapping the specified standard navigation handler implementation. + * @param originalNavigationHandler Standard NavigationHandler we are wrapping */ public FlowNavigationHandler(NavigationHandler originalNavigationHandler) { super(originalNavigationHandler); @@ -117,8 +120,8 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { } /** - * Sets the argument extractor to use by this navigation handler. Call to - * customize how flow id and event id arguments are extracted. + * Sets the argument extractor to use by this navigation handler. Call to customize how flow id and event id + * arguments are extracted. */ public void setArgumentExtractor(FlowExecutorArgumentExtractor argumentExtractor) { this.argumentExtractor = argumentExtractor; @@ -132,10 +135,10 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { } /** - * Sets the service responsible for mapping attributes of an - * {@link ExternalContext} to a new {@link FlowExecution} during a launch - * flow operation.

The default implementation simply exposes all request - * parameters as flow execution input attributes. May be null. + * Sets the service responsible for mapping attributes of an {@link ExternalContext} to a new {@link FlowExecution} + * during a launch flow operation. + *

+ * The default implementation simply exposes all request parameters as flow execution input attributes. May be null. * @see RequestParameterInputMapper */ public void setInputMapper(AttributeMapper inputMapper) { @@ -145,7 +148,9 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { public void handleNavigation(FacesContext facesContext, String fromAction, String outcome, NavigationHandler originalNavigationHandler) { try { - JsfExternalContext context = new JsfExternalContext(facesContext, fromAction, outcome); + JsfExternalContext context = getCurrentContext(); + // record the navigation handler context + context.handleNavigationCalled(fromAction, outcome); // first see if we need to launch a new flow execution if the flow id is present if (argumentExtractor.isFlowIdPresent(context)) { // a flow execution launch has been requested - create the new execution @@ -173,12 +178,13 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { if (argumentExtractor.isEventIdPresent(context)) { // signal the event against the current flow execution String eventId = argumentExtractor.extractEventId(context); - try { + try { FlowExecutionHolder holder = FlowExecutionHolderUtils.getFlowExecutionHolder(facesContext); ViewSelection selectedView = holder.getFlowExecution().signalEvent(eventId, context); // set the next view to render holder.setViewSelection(selectedView); - } catch (NoMatchingTransitionException e) { + } + catch (NoMatchingTransitionException e) { // not a valid event in the current state: proceed with standard navigation originalNavigationHandler.handleNavigation(facesContext, fromAction, outcome); } @@ -191,19 +197,18 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { } } catch (RuntimeException e) { - FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(facesContext); + cleanupResources(facesContext); throw e; } catch (Error e) { - FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(facesContext); + cleanupResources(facesContext); throw e; } } /** - * Factory method that creates the input attribute map for a newly created - * {@link FlowExecution}. This implementation uses the registered input - * mapper, if any. + * Factory method that creates the input attribute map for a newly created {@link FlowExecution}. This + * implementation uses the registered input mapper, if any. * @param context the external context * @return the input map, or null if no input */ @@ -220,6 +225,10 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { // helpers + private JsfExternalContext getCurrentContext() { + return (JsfExternalContext) ExternalContextHolder.getExternalContext(); + } + private FlowDefinitionLocator getLocator(JsfExternalContext context) { return FlowFacesUtils.getDefinitionLocator(context.getFacesContext()); } @@ -227,4 +236,9 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { private FlowExecutionFactory getFactory(JsfExternalContext context) { return FlowFacesUtils.getExecutionFactory(context.getFacesContext()); } + + private void cleanupResources(FacesContext context) { + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); + ExternalContextHolder.setExternalContext(null); + } } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowPhaseListener.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowPhaseListener.java index 8bef6866..573e4743 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowPhaseListener.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowPhaseListener.java @@ -115,8 +115,7 @@ public class FlowPhaseListener implements PhaseListener { *

  • Generating external URLs to redirect to on a ExternalRedirect repsonse. * * - * How arguments are extracted and how URLs are generated can be customized by setting a custom - * {{@link #setArgumentHandler(FlowExecutorArgumentHandler) argument handler}. + * How arguments are extracted and how URLs are generated can be customized by setting a custom {{@link #setArgumentHandler(FlowExecutorArgumentHandler) argument handler}. */ private FlowExecutorArgumentHandler argumentHandler = new RequestParameterFlowExecutorArgumentHandler(); @@ -224,45 +223,53 @@ public class FlowPhaseListener implements PhaseListener { public void beforePhase(PhaseEvent event) { FacesContext context = event.getFacesContext(); if (event.getPhaseId() == PhaseId.RESTORE_VIEW) { - ExternalContextHolder.setExternalContext(new JsfExternalContext(context)); - restoreFlowExecution(event.getFacesContext()); + try { + restoreFlowExecution(event.getFacesContext()); + } + catch (RuntimeException e) { + // clear the current external context only - no lock is acquired at this point + ExternalContextHolder.setExternalContext(null); + throw e; + } + catch (Error e) { + ExternalContextHolder.setExternalContext(null); + throw e; + } } else if (event.getPhaseId() == PhaseId.RENDER_RESPONSE) { if (FlowExecutionHolderUtils.isFlowExecutionRestored(event.getFacesContext())) { try { prepareResponse(getCurrentContext(), FlowExecutionHolderUtils.getFlowExecutionHolder(context)); - } catch (RuntimeException e) { - FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); + } + catch (RuntimeException e) { + cleanupResources(context); + throw e; + } + catch (Error e) { + cleanupResources(context); throw e; - } catch (Error e) { - FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); - throw e; } } } } public void afterPhase(PhaseEvent event) { - FacesContext context = event.getFacesContext(); + FacesContext context = event.getFacesContext(); if (event.getPhaseId() == PhaseId.RENDER_RESPONSE) { - try { - if (FlowExecutionHolderUtils.isFlowExecutionRestored(context)) { - FlowExecutionHolder holder = FlowExecutionHolderUtils.getFlowExecutionHolder(context); - try { - saveFlowExecution(getCurrentContext(), holder); - } - finally { - FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); - } + if (FlowExecutionHolderUtils.isFlowExecutionRestored(context)) { + try { + saveFlowExecution(getCurrentContext(), FlowExecutionHolderUtils.getFlowExecutionHolder(context)); + } + finally { + // always cleanup after save - done with flow execution request processing + cleanupResources(context); } } - finally { - ExternalContextHolder.setExternalContext(null); - } - } else { - // unlock if some other JSF artifact marked 'response complete' to short-circuit the lifecycle early + } + else { + // cleanup if some other JSF artifact marked 'response complete' to short-circuit the lifecycle early if (context.getResponseComplete()) { - FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); + cleanupResources(context); } } } @@ -270,7 +277,7 @@ public class FlowPhaseListener implements PhaseListener { protected void restoreFlowExecution(FacesContext facesContext) { JsfExternalContext context = new JsfExternalContext(facesContext); if (argumentHandler.isFlowExecutionKeyPresent(context)) { - // restore flow execution from repository so it will be available to other JSF artifacts + // restore flow execution from repository so it will be available togother JSF artifacts // (this could happen as part of a flow execution redirect or browser refresh) FlowExecutionRepository repository = getRepository(context); FlowExecutionKey flowExecutionKey = repository.parseFlowExecutionKey(argumentHandler.extractFlowExecutionKey(context)); @@ -281,12 +288,14 @@ public class FlowPhaseListener implements PhaseListener { flowExecution = repository.getFlowExecution(flowExecutionKey); if (logger.isDebugEnabled()) { logger.debug("Loaded existing flow execution key '" + flowExecutionKey + "' due to browser access " - + "[either via a flow execution redirect or direct browser refresh]"); + + "[either via a flow execution redirect or direct browser refresh]"); } - } catch (RuntimeException e) { + } + catch (RuntimeException e) { lock.unlock(); throw e; - } catch (Error e) { + } + catch (Error e) { lock.unlock(); throw e; } @@ -339,7 +348,8 @@ public class FlowPhaseListener implements PhaseListener { // no navigation event has been processed - simply refresh the execution with the same key selectedView = holder.getFlowExecution().refresh(context); holder.setViewSelection(selectedView); - } else { + } + else { // an navigation event has been processed - generate a new flow execution key if necessary generateKey(context, holder); } @@ -428,6 +438,11 @@ public class FlowPhaseListener implements PhaseListener { return (JsfExternalContext) ExternalContextHolder.getExternalContext(); } + private void cleanupResources(FacesContext context) { + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); + ExternalContextHolder.setExternalContext(null); + } + private void updateViewRoot(FacesContext facesContext, String viewId) { UIViewRoot viewRoot = facesContext.getViewRoot(); if (viewRoot == null || hasViewChanged(viewRoot, viewId)) { @@ -455,7 +470,7 @@ public class FlowPhaseListener implements PhaseListener { FlowExecutionKeyStateHolder.COMPONENT_ID); if (keyHolder == null) { keyHolder = new FlowExecutionKeyStateHolder(); - // expose as the in the view root for preservation in the tree + // expose in the view root for preservation in the component tree facesContext.getViewRoot().getChildren().add(keyHolder); } keyHolder.setFlowExecutionKey(flowExecutionKey); diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/JsfExternalContext.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/JsfExternalContext.java index 619de7cf..f26bb342 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/JsfExternalContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/JsfExternalContext.java @@ -78,19 +78,6 @@ public class JsfExternalContext implements ExternalContext { initMaps(facesContext); } - /** - * Creates a JSF External Context. - * @param facesContext the JSF faces context. - * @param actionId the action that fired - * @param outcome the action outcome - */ - public JsfExternalContext(FacesContext facesContext, String actionId, String outcome) { - this.facesContext = facesContext; - this.actionId = actionId; - this.outcome = outcome; - initMaps(facesContext); - } - /** * Initializes parameter and attribute maps from context data structures. * @param facesContext the faces context @@ -155,6 +142,16 @@ public class JsfExternalContext implements ExternalContext { return outcome; } + /** + * Records the action and outcome context information when navigation handling occurs. + * @param actionId the from action identifier + * @param outcome the action outcome + */ + public void handleNavigationCalled(String actionId, String outcome) { + this.actionId = actionId; + this.outcome = outcome; + } + /** * An accessor of a JSF session map. * @author Keith Donald diff --git a/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowNavigationHandlerArgumentExtractorTests.java b/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowNavigationHandlerArgumentExtractorTests.java index 2bb7f7f9..65de8fca 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowNavigationHandlerArgumentExtractorTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowNavigationHandlerArgumentExtractorTests.java @@ -27,19 +27,20 @@ public class FlowNavigationHandlerArgumentExtractorTests extends TestCase { protected void setUp() throws Exception { extractor = new FlowNavigationHandlerArgumentExtractor(); - facesContext = new MockFacesContext(); facesContext.setExternalContext(new MockJsfExternalContext()); } public void testExtractFlowId() { - JsfExternalContext context = new JsfExternalContext(facesContext, "action", "flowId:foo"); + JsfExternalContext context = new JsfExternalContext(facesContext); + context.handleNavigationCalled("action", "flowId:foo"); String flowId = extractor.extractFlowId(context); assertEquals("Wrong flow id", "foo", flowId); } public void testExtractFlowIdWrongFormat() { - JsfExternalContext context = new JsfExternalContext(facesContext, "action", "flow:foo"); + JsfExternalContext context = new JsfExternalContext(facesContext); + context.handleNavigationCalled("action", "bogus:foo"); try { extractor.extractFlowId(context); fail(); @@ -50,7 +51,8 @@ public class FlowNavigationHandlerArgumentExtractorTests extends TestCase { } public void testExtractEventId() { - JsfExternalContext context = new JsfExternalContext(facesContext, "action", "submit"); + JsfExternalContext context = new JsfExternalContext(facesContext); + context.handleNavigationCalled("action", "submit"); String eventId = extractor.extractEventId(context); assertEquals("Wrong event id", "submit", eventId); }