From 5aa24fc4709930a76b746ca309dff97f71cc6ec0 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Tue, 1 May 2007 00:09:48 +0000 Subject: [PATCH] swf 302 and 303 --- spring-webflow/changelog.txt | 6 + .../executor/jsf/FlowExecutionHolder.java | 11 +- .../jsf/FlowExecutionHolderUtils.java | 11 ++ .../jsf/FlowExecutionKeyStateHolder.java | 24 ++- .../executor/jsf/FlowNavigationHandler.java | 178 ++++++++++-------- .../executor/jsf/FlowPhaseListener.java | 55 ++++-- 6 files changed, 175 insertions(+), 110 deletions(-) diff --git a/spring-webflow/changelog.txt b/spring-webflow/changelog.txt index 7b34a003..23a76e05 100644 --- a/spring-webflow/changelog.txt +++ b/spring-webflow/changelog.txt @@ -9,6 +9,12 @@ Package org.springframework.webflow.config * FlowExecutorFactoryBean now has an 'inputMapper' property to conveniently configure the 'inputMapper' property of the created FlowExecutorImpl object. +Package org.springframework.webflow.executor +* JSF integration code now manages flow execution locks properly in exceptional situations and when the + 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). + Changes in version 1.0.3 (19.04.2007) ------------------------------------- diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolder.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolder.java index 90da2344..7d1fb6ab 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolder.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolder.java @@ -133,11 +133,18 @@ public class FlowExecutionHolder implements Serializable { public void replaceWith(FlowExecution flowExecution) { this.flowExecutionKey = null; this.viewSelection = null; + unlockFlowExecutionIfNecessary(); + this.flowExecution = flowExecution; + } + + /** + * Unlock the held flow execution if necessary. + */ + public void unlockFlowExecutionIfNecessary() { if (flowExecutionLock != null) { flowExecutionLock.unlock(); + this.flowExecutionLock = null; } - this.flowExecutionLock = null; - this.flowExecution = flowExecution; } public String toString() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolderUtils.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolderUtils.java index 65d86bef..0bff6d69 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolderUtils.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionHolderUtils.java @@ -91,6 +91,17 @@ public class FlowExecutionHolderUtils { } } + /** + * Unlocks the current flow execution in the faces context if necessary. + * Can be safely called even if no execution is bound or one is bound but not locked. + * @param context the faces context + */ + public static void unlockCurrentFlowExecutionIfNecessary(FacesContext context) { + if (isFlowExecutionRestored(context)) { + getFlowExecutionHolder(context).unlockFlowExecutionIfNecessary(); + } + } + private static String getFlowExecutionHolderKey() { return FlowExecutionHolder.class.getName(); } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionKeyStateHolder.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionKeyStateHolder.java index a1da66dc..8cd74419 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionKeyStateHolder.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowExecutionKeyStateHolder.java @@ -36,6 +36,7 @@ import org.springframework.webflow.execution.repository.FlowExecutionRepository; * phase. * * @author Jeremy Grelle + * @author Keith Donald */ public class FlowExecutionKeyStateHolder extends UIComponentBase { @@ -116,17 +117,24 @@ public class FlowExecutionKeyStateHolder extends UIComponentBase { // restore the "current" flow execution from repository so it will be available to variable/property resolvers // and the flow navigation handler (this could happen as part of a view action like a form submission) FlowExecutionRepository repository = getRepository(context); - FlowExecutionKey key; - // restore the key from the stored flowExecutionKey - key = repository.parseFlowExecutionKey(flowExecutionKey); + // restore the key from the stored encoded key string + FlowExecutionKey key = repository.parseFlowExecutionKey(flowExecutionKey); FlowExecutionLock lock = repository.getLock(key); lock.lock(); - FlowExecution flowExecution = repository.getFlowExecution(key); - if (logger.isDebugEnabled()) { - logger.debug("Loaded existing flow execution from repository with key '" + key + "'"); + FlowExecution flowExecution; + try { + flowExecution = repository.getFlowExecution(key); + if (logger.isDebugEnabled()) { + logger.debug("Loaded existing flow execution from repository with key '" + key + "'"); + } + } catch (RuntimeException e) { + lock.unlock(); + throw e; + } catch (Error e) { + lock.unlock(); + throw e; } - FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(key, flowExecution, lock), - facesContext); + FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(key, flowExecution, lock), facesContext); } } 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 afc78fd4..210efba1 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 @@ -27,6 +27,7 @@ import org.springframework.webflow.core.collection.LocalAttributeMap; import org.springframework.webflow.core.collection.MutableAttributeMap; import org.springframework.webflow.definition.FlowDefinition; import org.springframework.webflow.definition.registry.FlowDefinitionLocator; +import org.springframework.webflow.engine.NoMatchingTransitionException; import org.springframework.webflow.execution.FlowExecution; import org.springframework.webflow.execution.FlowExecutionFactory; import org.springframework.webflow.execution.ViewSelection; @@ -34,38 +35,33 @@ 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 @@ -79,19 +75,20 @@ 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(); @@ -103,8 +100,10 @@ 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); @@ -118,8 +117,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; @@ -133,10 +132,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,49 +144,66 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { public void handleNavigation(FacesContext facesContext, String fromAction, String outcome, NavigationHandler originalNavigationHandler) { - JsfExternalContext context = new JsfExternalContext(facesContext, 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 - String flowId = argumentExtractor.extractFlowId(context); - FlowDefinition flowDefinition = getLocator(context).getFlowDefinition(flowId); - FlowExecution flowExecution = getFactory(context).createFlowExecution(flowDefinition); - // check to see if this execution was created while another was running - if (FlowExecutionHolderUtils.isFlowExecutionRestored(facesContext)) { - // replace the current flow execution with the new one - FlowExecutionHolderUtils.getFlowExecutionHolder(facesContext).replaceWith(flowExecution); - } else { - // bind the new execution as the 'current execution' - FlowExecutionHolder holder = new FlowExecutionHolder(flowExecution); - FlowExecutionHolderUtils.setFlowExecutionHolder(holder, facesContext); - } - // start the new execution - ViewSelection selectedView = flowExecution.start(createInput(context), context); - // set the starting view to render - FlowExecutionHolderUtils.getFlowExecutionHolder(facesContext).setViewSelection(selectedView); - } else { - // not a launch request - see if this is a resume request to continue an existing execution - if (FlowExecutionHolderUtils.isFlowExecutionRestored(facesContext)) { - // a flow execution has been restored - see if we need to signal an event against it - if (argumentExtractor.isEventIdPresent(context)) { - // signal the event against the current flow execution - String eventId = argumentExtractor.extractEventId(context); - FlowExecutionHolder holder = FlowExecutionHolderUtils.getFlowExecutionHolder(facesContext); - ViewSelection selectedView = holder.getFlowExecution().signalEvent(eventId, context); - // set the next view to render - holder.setViewSelection(selectedView); + try { + JsfExternalContext context = new JsfExternalContext(facesContext, 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 + String flowId = argumentExtractor.extractFlowId(context); + FlowDefinition flowDefinition = getLocator(context).getFlowDefinition(flowId); + FlowExecution flowExecution = getFactory(context).createFlowExecution(flowDefinition); + // check to see if this execution was created while another was running + if (FlowExecutionHolderUtils.isFlowExecutionRestored(facesContext)) { + // replace the current flow execution with the new one + FlowExecutionHolderUtils.getFlowExecutionHolder(facesContext).replaceWith(flowExecution); } + else { + // bind the new execution as the 'current execution' + FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(flowExecution), facesContext); + } + // start the new execution + ViewSelection selectedView = flowExecution.start(createInput(context), context); + // set the starting view to render + FlowExecutionHolderUtils.getFlowExecutionHolder(facesContext).setViewSelection(selectedView); } else { - // neither a flow launch or resume request: proceed with standard navigation - originalNavigationHandler.handleNavigation(facesContext, fromAction, outcome); + // not a launch request - see if this is a resume request to continue an existing execution + if (FlowExecutionHolderUtils.isFlowExecutionRestored(facesContext)) { + // a flow execution has been restored - see if we need to signal an event against it + if (argumentExtractor.isEventIdPresent(context)) { + // signal the event against the current flow execution + String eventId = argumentExtractor.extractEventId(context); + 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) { + // not a valid event in the current state: proceed with standard navigation + originalNavigationHandler.handleNavigation(facesContext, fromAction, outcome); + } + } + } + else { + // neither a flow launch or resume request: proceed with standard navigation + originalNavigationHandler.handleNavigation(facesContext, fromAction, outcome); + } } } + catch (RuntimeException e) { + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(facesContext); + throw e; + } + catch (Error e) { + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(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 */ 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 ecd33302..8bef6866 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 @@ -222,38 +222,48 @@ public class FlowPhaseListener implements PhaseListener { } public void beforePhase(PhaseEvent event) { + FacesContext context = event.getFacesContext(); if (event.getPhaseId() == PhaseId.RESTORE_VIEW) { - ExternalContextHolder.setExternalContext(new JsfExternalContext(event.getFacesContext())); + ExternalContextHolder.setExternalContext(new JsfExternalContext(context)); restoreFlowExecution(event.getFacesContext()); } else if (event.getPhaseId() == PhaseId.RENDER_RESPONSE) { if (FlowExecutionHolderUtils.isFlowExecutionRestored(event.getFacesContext())) { - prepareResponse(getCurrentContext(), FlowExecutionHolderUtils.getFlowExecutionHolder(event - .getFacesContext())); + try { + prepareResponse(getCurrentContext(), FlowExecutionHolderUtils.getFlowExecutionHolder(context)); + } catch (RuntimeException e) { + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); + throw e; + } catch (Error e) { + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); + throw e; + } } } } public void afterPhase(PhaseEvent event) { + FacesContext context = event.getFacesContext(); if (event.getPhaseId() == PhaseId.RENDER_RESPONSE) { try { - if (FlowExecutionHolderUtils.isFlowExecutionRestored(event.getFacesContext())) { - FlowExecutionHolder holder = FlowExecutionHolderUtils.getFlowExecutionHolder(event - .getFacesContext()); + if (FlowExecutionHolderUtils.isFlowExecutionRestored(context)) { + FlowExecutionHolder holder = FlowExecutionHolderUtils.getFlowExecutionHolder(context); try { saveFlowExecution(getCurrentContext(), holder); } finally { - if (holder.getFlowExecutionLock() != null) { - // unlock the flow execution - holder.getFlowExecutionLock().unlock(); - } + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); } } } finally { ExternalContextHolder.setExternalContext(null); } + } else { + // unlock if some other JSF artifact marked 'response complete' to short-circuit the lifecycle early + if (context.getResponseComplete()) { + FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); + } } } @@ -263,17 +273,24 @@ public class FlowPhaseListener implements PhaseListener { // restore flow execution from repository so it will be available to other 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)); + FlowExecutionKey flowExecutionKey = repository.parseFlowExecutionKey(argumentHandler.extractFlowExecutionKey(context)); FlowExecutionLock lock = repository.getLock(flowExecutionKey); lock.lock(); - FlowExecution flowExecution = repository.getFlowExecution(flowExecutionKey); - if (logger.isDebugEnabled()) { - logger.debug("Loaded existing flow execution key '" + flowExecutionKey + "' due to browser access " + FlowExecution flowExecution; + try { + 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]"); + } + } catch (RuntimeException e) { + lock.unlock(); + throw e; + } catch (Error e) { + lock.unlock(); + throw e; } - FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(flowExecutionKey, flowExecution, - lock), facesContext); + FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(flowExecutionKey, flowExecution, lock), facesContext); } else if (argumentHandler.isFlowIdPresent(context)) { // launch a new flow execution @@ -438,8 +455,8 @@ public class FlowPhaseListener implements PhaseListener { FlowExecutionKeyStateHolder.COMPONENT_ID); if (keyHolder == null) { keyHolder = new FlowExecutionKeyStateHolder(); - // expose as the first component in the view root for preservation in the tree - facesContext.getViewRoot().getChildren().add(0, keyHolder); + // expose as the in the view root for preservation in the tree + facesContext.getViewRoot().getChildren().add(keyHolder); } keyHolder.setFlowExecutionKey(flowExecutionKey); }