Added a Servlet Filter to ensure that all exits out of JSF end with cleanup (SWF-306)

spring-webflow-samples/sellitem-jsf/src/main/webapp/WEB-INF/web.xml

@@ -23,6 +23,18 @@
         <listener-class>org.apache.myfaces.webapp.StartupServletContextListener</listener-class>
     </listener>
 
+    <!-- Guarantees that under all JSF exit conditions, the flow context will be cleaned up -->
+    <filter>
+    	<filter-name>Flow System Cleanup Filter</filter-name>
+    	<filter-class>org.springframework.webflow.executor.jsf.FlowSystemCleanupFilter</filter-class>
+    </filter>
+
+	<!-- Filters all request to *.faces to the Flow System Cleanup Filter for guarenteed cleanup -->
+	<filter-mapping>
+		<filter-name>Flow System Cleanup Filter</filter-name>
+		<url-pattern>*.faces</url-pattern>
+	</filter-mapping>
+
     <!-- The front controller for the JSF application, responsible for handling all application requests -->
     <servlet>
         <servlet-name>Faces Servlet</servlet-name>

spring-webflow/changelog.txt

@@ -34,6 +34,8 @@ 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 can now use a FlowSystemCleanupFilter to ensure that no matter what happens in JSF, the request context
+  is cleaned up properly (SWF-306).
 
 Changes in version 1.0.3 (19.04.2007)
 -------------------------------------

spring-webflow/src/main/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilter.java

@@ -0,0 +1,88 @@
+/*
+ * 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;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.springframework.web.filter.OncePerRequestFilter;
+import org.springframework.webflow.context.ExternalContextHolder;
+import org.springframework.webflow.execution.FlowExecutionContextHolder;
+
+/**
+ * A servlet filter used to guarantee that webflow context information is
+ * cleaned up in a JSF environment.
+ * 
+ * @author Ben Hale
+ * @since 1.1
+ */
+public class FlowSystemCleanupFilter extends OncePerRequestFilter {
+
+	protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+			throws ServletException, IOException {
+		try {
+			chain.doFilter(request, response);
+		} finally {
+			cleanupCurrentFlowExecution(request);
+			ExternalContextHolder.setExternalContext(null);
+		}
+	}
+
+	/**
+	 * Cleans up the current flow execution in the request context if necessary.
+	 * Specifically, handles unlocking the execution if necessary, setting the
+	 * holder to null, and cleaning up the flow execution context thread local.
+	 * Can be safely called even if no execution is bound or one is bound but
+	 * not locked.
+	 * @param request the servlet request
+	 */
+	private void cleanupCurrentFlowExecution(ServletRequest request) {
+		if (isFlowExecutionRestored(request)) {
+			FlowExecutionContextHolder.setFlowExecutionContext(null);
+			getFlowExecutionHolder(request).unlockFlowExecutionIfNecessary();
+			request.removeAttribute(getFlowExecutionHolderKey());
+		}
+	}
+
+	/**
+	 * Returns true if the flow execution has been restored in the current
+	 * thread.
+	 * @param request the servlet request
+	 * @return true if restored, false otherwise
+	 */
+	private boolean isFlowExecutionRestored(ServletRequest request) {
+		return getFlowExecutionHolder(request) != null;
+	}
+
+	/**
+	 * Returns the current flow execution holder for the given servlet request.
+	 * @param request the servlet request
+	 * @return the flow execution holder, or <code>null</code> if none set.
+	 */
+	private FlowExecutionHolder getFlowExecutionHolder(ServletRequest request) {
+		return (FlowExecutionHolder) request.getAttribute(getFlowExecutionHolderKey());
+	}
+
+	private static String getFlowExecutionHolderKey() {
+		return FlowExecutionHolder.class.getName();
+	}
+
+}

spring-webflow/src/test/java/org/springframework/webflow/executor/jsf/FlowSystemCleanupFilterTests.java

@@ -0,0 +1,85 @@
+package org.springframework.webflow.executor.jsf;
+
+import java.io.IOException;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+
+import junit.framework.TestCase;
+
+import org.springframework.mock.web.MockFilterChain;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.webflow.context.ExternalContextHolder;
+import org.springframework.webflow.engine.impl.FlowExecutionImpl;
+import org.springframework.webflow.execution.FlowExecutionContextHolder;
+import org.springframework.webflow.test.MockExternalContext;
+
+public class FlowSystemCleanupFilterTests extends TestCase {
+
+	private FlowSystemCleanupFilter filter;
+
+	private ServletRequest request;
+
+	private ServletResponse response;
+
+	private FilterChain chain;
+
+	protected void setUp() throws Exception {
+		filter = new FlowSystemCleanupFilter();
+		request = new MockHttpServletRequest();
+		request.setAttribute(getFlowExecutionHolderKey(), new FlowExecutionHolder(new FlowExecutionImpl()));
+		response = new MockHttpServletResponse();
+		chain = new MockFilterChain();
+		FlowExecutionContextHolder.setFlowExecutionContext(new FlowExecutionImpl());
+		ExternalContextHolder.setExternalContext(new MockExternalContext());
+	}
+
+	public void testCleanup() throws ServletException, IOException {
+		filter.doFilter(request, response, chain);
+
+		assertNull("Should have cleaned up the flow execution", request.getAttribute(getFlowExecutionHolderKey()));
+		try {
+			FlowExecutionContextHolder.getFlowExecutionContext();
+			fail("Should have an empty holder");
+		} catch (IllegalStateException e) {
+		}
+		try {
+			ExternalContextHolder.getExternalContext();
+			fail("Should have an empty holder");
+		} catch (IllegalStateException e) {
+		}
+	}
+
+	public void testExceptionThrown() throws ServletException, IOException {
+		try {
+			filter.doFilter(request, response, new ExceptionThrowingMockFilterChain());
+		} catch (RuntimeException e) {
+			assertNull("Should have cleaned up the flow execution", request.getAttribute(getFlowExecutionHolderKey()));
+			try {
+				FlowExecutionContextHolder.getFlowExecutionContext();
+				fail("Should have an empty holder");
+			} catch (IllegalStateException e1) {
+			}
+			try {
+				ExternalContextHolder.getExternalContext();
+				fail("Should have an empty holder");
+			} catch (IllegalStateException e1) {
+			}
+		}
+	}
+
+	private static String getFlowExecutionHolderKey() {
+		return FlowExecutionHolder.class.getName();
+	}
+
+	private class ExceptionThrowingMockFilterChain extends MockFilterChain {
+
+		public void doFilter(ServletRequest request, ServletResponse response) {
+			throw new RuntimeException();
+		}
+
+	}
+}