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: - *
NavigationHandler implementation and return.
- * 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:
NavigationHandler
+ * implementation and return.
- * 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); }