From e91a3bf5d35bff89685c03ceb31dd8341e65fb50 Mon Sep 17 00:00:00 2001 From: Erwin Vervaet Date: Wed, 13 Jun 2007 07:58:10 +0000 Subject: [PATCH] Code review and polishing. --- spring-webflow/changelog.txt | 16 ++++++----- .../jsf/FlowExecutionHolderUtils.java | 12 +++++--- .../executor/jsf/FlowSystemCleanupFilter.java | 19 ++++++------- ...bstractExternalizedFlowExecutionTests.java | 28 +++++++++++-------- .../jsf/FlowSystemCleanupFilterTests.java | 15 ++++++++++ 5 files changed, 56 insertions(+), 34 deletions(-) diff --git a/spring-webflow/changelog.txt b/spring-webflow/changelog.txt index 71bbc07e..2a09a50b 100644 --- a/spring-webflow/changelog.txt +++ b/spring-webflow/changelog.txt @@ -26,8 +26,9 @@ Package org.springframework.webflow.engine * Added name(String, Action) method to AbstractFlowBuilder for convenient creation of named actions. * AnnotatedAction now has a convenience putAttribute(String, Object) method. * Added annotate(Action) method to AbstractFlowBuilder. -* Added populateLocalContext(Flow, GenericApplicationContext, Resource[]) hook method to XmlFlowBuilder to allow for control over - the registration of beans needed locally by a flow definition. Useful for testing. (SWF-307). +* Added populateLocalContext(Flow, GenericApplicationContext, Resource[]) hook method to XmlFlowBuilder + to allow for control over the registration of beans needed locally by a flow definition. + Useful for testing (SWF-307). Package org.springframework.webflow.executor * JSF integration code now manages flow execution locks properly in exceptional situations and when the @@ -36,13 +37,14 @@ Package org.springframework.webflow.executor 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). -* JSF users can now install a fallback FlowSystemCleanupFilter to ensure that no matter what happens in the JSF environment flow - resources are cleaned up after request processing (SWF-306). +* JSF users can now install a fallback FlowSystemCleanupFilter to ensure that no matter what happens in the + JSF environment, flow resources are cleaned up after request processing (SWF-306). Package org.springframework.webflow.test -* Added the ability to attach multiple flow execution listeners to a test case (SWF-334). -* Relaxed 'final' qualifier on AbstractXmlFlowExecutionTests#createFlowBuilder(FlowServiceLocator). Overriding this method is useful - for customizing the builder's population of the bean factory local to the flow definition (SWF-307). +* Added the ability to attach multiple flow execution listeners to a flow execution test case (SWF-334). +* Relaxed 'final' qualifier on AbstractXmlFlowExecutionTests#createFlowBuilder(FlowServiceLocator). + Overriding this method is useful for customizing the builder's population of the bean factory local to the + flow definition (SWF-307). Changes in version 1.0.3 (19.04.2007) ------------------------------------- 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 2ec4eb05..522dc1d4 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 @@ -26,7 +26,7 @@ import org.springframework.webflow.execution.FlowExecution; *

* By default, the current flow execution holder is stored associated with the current thread in the * {@link FacesContext}'s {@link ExternalContext#getRequestMap()}. - * + * * @author Keith Donald */ public class FlowExecutionHolderUtils { @@ -71,7 +71,7 @@ public class FlowExecutionHolderUtils { return null; } } - + /** * Returns the current required flow execution in the given faces context. * @param context faces context @@ -102,8 +102,12 @@ public class FlowExecutionHolderUtils { context.getExternalContext().getRequestMap().remove(getFlowExecutionHolderKey()); } } - - private static String getFlowExecutionHolderKey() { + + /** + * Returns the key used to index the flow execution holder in the request + * attributes. + */ + static String getFlowExecutionHolderKey() { return FlowExecutionHolder.class.getName(); } } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilter.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilter.java index 3ccb993e..cca65412 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilter.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilter.java @@ -27,10 +27,10 @@ import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.webflow.context.ExternalContextHolder; /** - * A servlet filter used to guarantee that webflow context information is - * cleaned up in a JSF environment. Most useful to ensure no possibility + * A servlet filter used to guarantee that web flow context information is + * cleaned up in a JSF environment. Most useful to ensure no possibility * of a flow execution remaining locked due to an uncaught JSF exception. - * + * * @author Ben Hale */ public class FlowSystemCleanupFilter extends OncePerRequestFilter { @@ -39,7 +39,8 @@ public class FlowSystemCleanupFilter extends OncePerRequestFilter { throws ServletException, IOException { try { chain.doFilter(request, response); - } finally { + } + finally { cleanupCurrentFlowExecution(request); ExternalContextHolder.setExternalContext(null); } @@ -55,7 +56,7 @@ public class FlowSystemCleanupFilter extends OncePerRequestFilter { private void cleanupCurrentFlowExecution(ServletRequest request) { if (isFlowExecutionRestored(request)) { getFlowExecutionHolder(request).unlockFlowExecutionIfNecessary(); - request.removeAttribute(getFlowExecutionHolderKey()); + request.removeAttribute(FlowExecutionHolderUtils.getFlowExecutionHolderKey()); } } @@ -72,13 +73,9 @@ public class FlowSystemCleanupFilter extends OncePerRequestFilter { /** * Returns the current flow execution holder for the given servlet request. * @param request the servlet request - * @return the flow execution holder, or null if none set. + * @return the flow execution holder, or null if none set */ private FlowExecutionHolder getFlowExecutionHolder(ServletRequest request) { - return (FlowExecutionHolder) request.getAttribute(getFlowExecutionHolderKey()); - } - - private static String getFlowExecutionHolderKey() { - return FlowExecutionHolder.class.getName(); + return (FlowExecutionHolder) request.getAttribute(FlowExecutionHolderUtils.getFlowExecutionHolderKey()); } } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractExternalizedFlowExecutionTests.java b/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractExternalizedFlowExecutionTests.java index dc7b4eda..1466fb11 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractExternalizedFlowExecutionTests.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractExternalizedFlowExecutionTests.java @@ -34,7 +34,7 @@ import org.springframework.webflow.test.MockFlowServiceLocator; /** * Base class for flow integration tests that verify an externalized flow definition executes as expected. Supports * caching of the flow definition built from an externalized resource to speed up test execution. - * + * * @author Keith Donald */ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlowExecutionTests { @@ -68,7 +68,7 @@ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlo } /** - * Internal helper that return the flow execution factory used by the test cast to a + * Internal helper that returns the flow execution factory used by the test cast to a * {@link FlowExecutionImplFactory}. */ private FlowExecutionImplFactory getFlowExecutionImplFactory() { @@ -100,8 +100,8 @@ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlo } /** - * Set a single listener to be attached to the flow execution the next time one is {@link #startFlow() started} by this - * test. Useful for attaching a listener that does test assertions during the execution of the flow. + * Set a single listener to be attached to the flow execution the next time one is {@link #startFlow() started} by + * this test. Useful for attaching a listener that does test assertions during the execution of the flow. * @param executionListener the listener to attach */ protected void setFlowExecutionListener(FlowExecutionListener executionListener) { @@ -113,12 +113,16 @@ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlo * Set the listeners to be attached to the flow execution the next time one is {@link #startFlow() started} by this * test. Useful for attaching listeners that do test assertions during the execution of the flow. * @param executionListeners the listeners to attach + * @since 1.0.4 */ protected void setFlowExecutionListeners(FlowExecutionListener[] executionListeners) { getFlowExecutionImplFactory().setExecutionListenerLoader( new StaticFlowExecutionListenerLoader(executionListeners)); } + /** + * Returns the flow definition being tested. + */ protected final FlowDefinition getFlowDefinition() { if (isCacheFlowDefinition() && cachedFlowDefinition != null) { return cachedFlowDefinition; @@ -132,7 +136,7 @@ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlo } /** - * Returns the flow artifact factory to use during flow definition construction time for accessing externally + * Returns the flow service locator to use during flow definition construction time for accessing externally * managed flow artifacts such as actions and flows to be used as subflows. *

* This implementation just creates a {@link MockFlowServiceLocator} and populates it with services by calling @@ -155,7 +159,7 @@ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlo } /** - * Factory method to assemble another flow definition from a resource. Called by {@link #getFlowDefinition()} to + * Factory method to assemble a flow definition from a resource. Called by {@link #getFlowDefinition()} to * create the "main" flow to test. May also be called by subclasses to create subflow definitions whose executions * should also be exercised by this test. * @param resource the flow definition resource @@ -177,18 +181,17 @@ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlo * protected FlowDefinitionResource getFlowDefinitionResource() { * return createFlowDefinitionResource("/WEB-INF/flows/order-flow.xml"); * } - * + * * @return the flow definition resource */ protected abstract FlowDefinitionResource getFlowDefinitionResource(); /** - * Factory method to create the builder that will build the flow definition whose execution will be tested. Subclasses must - * override. - * + * Factory method to create the builder that will build the flow definition whose execution will be tested. + * Subclasses must implement. + *

* A subclass may return a builder that sets up mock implementations of services needed locally by the flow * definition at runtime. - * * @param resource the externalized flow definition resource location * @param serviceLocator the flow service locator * @return the flow builder that will build the flow to be tested @@ -217,7 +220,8 @@ public abstract class AbstractExternalizedFlowExecutionTests extends AbstractFlo } /** - * Convenient factory method that creates a {@link FlowDefinitionResource} from a file. + * Convenient factory method that creates a {@link FlowDefinitionResource} from a file. Typically + * called by subclasses overriding {@link #getFlowDefinitionResource()}. * @param file the file * @return the flow definition resource */ diff --git a/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilterTests.java b/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilterTests.java index 32f0cf6e..1425fbb7 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilterTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilterTests.java @@ -1,3 +1,18 @@ +/* + * Copyright 2004-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.webflow.executor.jsf; import java.io.IOException;