From 32e536cb342e40f2d61749ddb154602d969e34a5 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Tue, 1 May 2007 22:52:16 +0000 Subject: [PATCH] SWF 302 work improved logging --- ...ctConversationFlowExecutionRepository.java | 4 - .../jsf/FlowExecutionKeyStateHolder.java | 44 ++++++--- .../executor/jsf/FlowNavigationHandler.java | 10 +- .../executor/jsf/FlowPhaseListener.java | 94 ++++++++++--------- 4 files changed, 92 insertions(+), 60 deletions(-) diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractConversationFlowExecutionRepository.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractConversationFlowExecutionRepository.java index c456be19..7a4dd6fa 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractConversationFlowExecutionRepository.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractConversationFlowExecutionRepository.java @@ -118,10 +118,6 @@ public abstract class AbstractConversationFlowExecutionRepository extends Abstra } public FlowExecutionLock getLock(FlowExecutionKey key) throws FlowExecutionRepositoryException { - if (logger.isDebugEnabled()) { - logger.debug("Getting lock for flow execution with key '" + key + "'"); - } - // lock the entire conversation return new ConversationBackedFlowExecutionLock(getConversation(key)); } 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 8cd74419..a06f0a82 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 @@ -26,6 +26,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.util.StringUtils; import org.springframework.webflow.execution.FlowExecution; +import org.springframework.webflow.execution.repository.FlowExecutionAccessException; import org.springframework.webflow.execution.repository.FlowExecutionKey; import org.springframework.webflow.execution.repository.FlowExecutionLock; import org.springframework.webflow.execution.repository.FlowExecutionRepository; @@ -119,25 +120,42 @@ public class FlowExecutionKeyStateHolder extends UIComponentBase { FlowExecutionRepository repository = getRepository(context); // restore the key from the stored encoded key string FlowExecutionKey key = repository.parseFlowExecutionKey(flowExecutionKey); - FlowExecutionLock lock = repository.getLock(key); - lock.lock(); - FlowExecution flowExecution; try { - flowExecution = repository.getFlowExecution(key); - if (logger.isDebugEnabled()) { - logger.debug("Loaded existing flow execution from repository with key '" + key + "'"); + FlowExecutionLock lock = repository.getLock(key); + lock.lock(); + try { + FlowExecution flowExecution = repository.getFlowExecution(key); + if (logger.isDebugEnabled()) { + logger.debug("Loaded existing flow execution with key '" + flowExecutionKey + + "' as part of component restoration [triggered via an action event like a button click]"); + } + FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(key, flowExecution, lock), facesContext); + } + catch (RuntimeException e) { + lock.unlock(); + throw e; + } + catch (Error e) { + lock.unlock(); + throw e; } - } catch (RuntimeException e) { - lock.unlock(); - throw e; - } catch (Error e) { - lock.unlock(); - throw e; } - FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(key, flowExecution, lock), facesContext); + catch (FlowExecutionAccessException e) { + handleFlowExecutionAccessException(e, facesContext); + } } } + /** + * Hook method to handle a thrown flow execution access exception. By default this implementation simply rethrows + * the exception. Subclasses may override this method to redirect to an error page or take some other action. + * @param e the flow execution access exception + * @param context the current faces context + */ + protected void handleFlowExecutionAccessException(FlowExecutionAccessException e, FacesContext context) { + throw e; + } + /** * Save the just the current FlowExecutionKey value. */ 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 357ce723..74f27d42 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 @@ -164,7 +164,8 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { } else { // bind the new execution as the 'current execution' - FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(flowExecution), facesContext); + FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(flowExecution), + facesContext); } // start the new execution ViewSelection selectedView = flowExecution.start(createInput(context), context); @@ -185,6 +186,10 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { holder.setViewSelection(selectedView); } catch (NoMatchingTransitionException e) { + if (logger.isDebugEnabled()) { + logger.debug("No flow state transition found for event '" + eventId + + "'; falling back to standard navigation handler."); + } // not a valid event in the current state: proceed with standard navigation originalNavigationHandler.handleNavigation(facesContext, fromAction, outcome); } @@ -238,6 +243,9 @@ public class FlowNavigationHandler extends DecoratingNavigationHandler { } private void cleanupResources(FacesContext context) { + if (logger.isDebugEnabled()) { + logger.debug("Cleaning up allocated flow system resources"); + } FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); ExternalContextHolder.setExternalContext(null); } 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 fbadd08f..7b3ad638 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 @@ -38,6 +38,7 @@ import org.springframework.webflow.definition.registry.FlowDefinitionLocator; import org.springframework.webflow.execution.FlowExecution; import org.springframework.webflow.execution.FlowExecutionFactory; import org.springframework.webflow.execution.ViewSelection; +import org.springframework.webflow.execution.repository.FlowExecutionAccessException; import org.springframework.webflow.execution.repository.FlowExecutionKey; import org.springframework.webflow.execution.repository.FlowExecutionLock; import org.springframework.webflow.execution.repository.FlowExecutionRepository; @@ -177,9 +178,8 @@ public class FlowPhaseListener implements PhaseListener { /** * 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. + * during a launch flow operation. The default implementation simply exposes all request parameters as flow + * execution input attributes. May be null. * @param inputMapper the input mapper * @see RequestParameterInputMapper */ @@ -223,19 +223,10 @@ public class FlowPhaseListener implements PhaseListener { public void beforePhase(PhaseEvent event) { FacesContext context = event.getFacesContext(); if (event.getPhaseId() == PhaseId.RESTORE_VIEW) { - try { - ExternalContextHolder.setExternalContext(new JsfExternalContext(context)); - 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; - } + ExternalContextHolder.setExternalContext(new JsfExternalContext(context)); + restoreFlowExecution(event.getFacesContext()); + // we do not need to worry about clean up here since other phases will continue to run even if an exception + // occurs in restoreFlowExecution(FacesContext) } else if (event.getPhaseId() == PhaseId.RENDER_RESPONSE) { if (FlowExecutionHolderUtils.isFlowExecutionRestored(event.getFacesContext())) { @@ -243,6 +234,8 @@ public class FlowPhaseListener implements PhaseListener { prepareResponse(getCurrentContext(), FlowExecutionHolderUtils.getFlowExecutionHolder(context)); } catch (RuntimeException e) { + // we must cleanup here since this is the render response phase and the after phase callback will + // NOT run when an exception occurs (which typically does the cleanup--see below) cleanupResources(context); throw e; } @@ -262,7 +255,7 @@ public class FlowPhaseListener implements PhaseListener { saveFlowExecution(getCurrentContext(), FlowExecutionHolderUtils.getFlowExecutionHolder(context)); } finally { - // always cleanup after save - done with flow execution request processing + // always cleanup after save - we are done with flow execution request processing cleanupResources(context); } } @@ -278,29 +271,37 @@ 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 togother JSF artifacts + // restore flow execution from repository so it will be available to 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)); - FlowExecutionLock lock = repository.getLock(flowExecutionKey); - lock.lock(); - FlowExecution flowExecution; + FlowExecutionKey flowExecutionKey = repository.parseFlowExecutionKey(argumentHandler + .extractFlowExecutionKey(context)); 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]"); + FlowExecutionLock lock = repository.getLock(flowExecutionKey); + lock.lock(); + try { + FlowExecution flowExecution = repository.getFlowExecution(flowExecutionKey); + if (logger.isDebugEnabled()) { + logger.debug("Loaded existing flow execution with key '" + flowExecutionKey + + "' due to browser access " + + "[either via a flow execution redirect or direct browser refresh]"); + } + FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(flowExecutionKey, + flowExecution, lock), facesContext); + } + catch (RuntimeException e) { + lock.unlock(); + throw e; + } + catch (Error e) { + lock.unlock(); + throw e; } } - catch (RuntimeException e) { - lock.unlock(); - throw e; + catch (FlowExecutionAccessException e) { + // thrown if access to the execution could not be granted + handleFlowExecutionAccessException(e, facesContext); } - catch (Error e) { - lock.unlock(); - throw e; - } - FlowExecutionHolderUtils.setFlowExecutionHolder(new FlowExecutionHolder(flowExecutionKey, flowExecution, lock), facesContext); } else if (argumentHandler.isFlowIdPresent(context)) { // launch a new flow execution @@ -319,10 +320,21 @@ public class FlowPhaseListener implements PhaseListener { } } + /** + * Hook method to handle a thrown flow execution access exception. By default this implementation simply rethrows + * the exception. Subclasses may override this method to redirect to an error page or take some other action in the + * case where a flow execution could not be restored (for example, because the flow execution had previously ended + * or expired). + * @param e the flow execution access exception + * @param context the current faces context + */ + protected void handleFlowExecutionAccessException(FlowExecutionAccessException e, FacesContext context) { + 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. - * * @param context the external context * @return the input map, or null if no input */ @@ -339,7 +351,6 @@ public class FlowPhaseListener implements PhaseListener { /** * Prepare the appropriate JSF response (e.g. rendering a view, sending a redirect, etc). - * * @param context the context * @param holder the holder */ @@ -386,7 +397,6 @@ public class FlowPhaseListener implements PhaseListener { /** * Prepare the JSF view for rendering. - * * @param facesContext the faces context * @param holder the holder of the current flow execution */ @@ -408,7 +418,6 @@ public class FlowPhaseListener implements PhaseListener { /** * Updates the current flow execution in the repository. - * * @param context the external context * @param holder the current flow execution holder */ @@ -418,7 +427,7 @@ public class FlowPhaseListener implements PhaseListener { if (flowExecution.isActive()) { // save the flow execution out to the repository if (logger.isDebugEnabled()) { - logger.debug("Saving continuation to repository with key " + holder.getFlowExecutionKey()); + logger.debug("Saving execution to repository with key " + holder.getFlowExecutionKey()); } repository.putFlowExecution(holder.getFlowExecutionKey(), flowExecution); } @@ -440,6 +449,9 @@ public class FlowPhaseListener implements PhaseListener { } private void cleanupResources(FacesContext context) { + if (logger.isDebugEnabled()) { + logger.debug("Cleaning up allocated flow system resources"); + } FlowExecutionHolderUtils.unlockCurrentFlowExecutionIfNecessary(context); ExternalContextHolder.setExternalContext(null); } @@ -461,7 +473,6 @@ public class FlowPhaseListener implements PhaseListener { /** * Saves the flow execution key in a component in the view root for restoration on subsequent RESTORE_VIEW * operations. - * * @param facesContext the faces context exposing the view root * @param flowExecutionKey the flow execution key */ @@ -501,8 +512,7 @@ public class FlowPhaseListener implements PhaseListener { /** * Utility method needed needed only because we can not rely on JSF RequestMap supporting Map's putAll method. Tries - * putAll, falls back to individual adds - * + * putAll, falls back to individual adds. * @param targetMap the target map to add the model data to * @param map the model data to add to the target map */