The SessionBindingConversationManager now re-binds the ConversationContainer in the session

every time a contained conversation is unlocked (SWF-244).

spring-webflow/changelog.txt

@@ -12,6 +12,10 @@ Package org.springframework.webflow.config
 * Fixed bug in WebFlowConfigNamespaceHandler that caused the type attribute of the repository tag
   not to be read properly and throw an exception (SWF-239).
 
+Package org.springframework.webflow.conversation
+* The SessionBindingConversationManager now re-binds the ConversationContainer in the session
+  every time a contained conversation is unlocked (SWF-244).
+
 Changes in version 1.0.1 (08.01.2007)
 -------------------------------------
 

spring-webflow/src/main/java/org/springframework/webflow/conversation/Conversation.java

@@ -17,14 +17,31 @@ package org.springframework.webflow.conversation;
 
 /**
  * A service interface for working with state associated with a single logical
- * user interaction called a "conversation".
+ * user interaction called a "conversation" in the scope of a single request.
+ * Conversation objects are not thread safe and should not be shared among
+ * multiple threads.
  * <p>
  * A conversation provides a "task" context that is begun and eventually ends.
  * Between the beginning and the end attributes can be placed in and read from a
  * conversation's context.
  * <p>
- * Once begun, the conversation can be locked to obtain exclusive access to
- * manipulating it. Once the conversation is "done", it can be ended.
+ * A conversation needs to be {@link #lock() locked} to obtain exclusive
+ * access to it before it can be manipulated. Once manipulation is finished, you need to
+ * {@link #unlock() unlock} the conversation. So code interacting with a
+ * conversation always looks like this:
+ * 
+ * <pre>
+ * Conversation conv = ...;
+ * conv.lock();
+ * try {
+ *    // work with the Conversation object, calling methods like
+ *    // getAttribute(), putAttribute() and end()
+ * }
+ * finally {
+ *    conv.unlock();
+ * }
+ * </pre>
+ * 
  * <p>
  * Note that the attributes associated with a conversation are not
  * "conversation scope" as defined for a flow execution. They can be
@@ -32,12 +49,14 @@ package org.springframework.webflow.conversation;
  * conversation.
  * 
  * @author Keith Donald
+ * @author Erwin Vervaet
  */
 public interface Conversation {
 
 	/**
 	 * Returns the unique id assigned to this conversation. This id remains the
-	 * same throughout the life of the conversation.
+	 * same throughout the life of the conversation. This method can be safely
+	 * called without owning the lock of this conversation.
 	 * @return the conversation id
 	 */
 	public ConversationId getId();
@@ -50,6 +69,7 @@ public interface Conversation {
 
 	/**
 	 * Returns the conversation attribute with the specified name.
+	 * You need to aquire the lock on this conversation before calling this method.
 	 * @param name the attribute name
 	 * @return the attribute value
 	 */
@@ -57,6 +77,7 @@ public interface Conversation {
 
 	/**
 	 * Puts a conversation attribute into this context.
+	 * You need to aquire the lock on this conversation before calling this method.
 	 * @param name the attribute name
 	 * @param value the attribute value
 	 */
@@ -64,6 +85,7 @@ public interface Conversation {
 
 	/**
 	 * Removes a conversation attribute.
+	 * You need to aquire the lock on this conversation before calling this method.
 	 * @param name the attribute name
 	 */
 	public void removeAttribute(Object name);
@@ -71,11 +93,12 @@ public interface Conversation {
 	/**
 	 * Ends this conversation. This method should only be called once to
 	 * terminate the conversation and cleanup any allocated resources.
+	 * You need to aquire the lock on this conversation before calling this method.
 	 */
 	public void end();
 
 	/**
-	 * Unlock this conversation, making it available for others for
+	 * Unlock this conversation, making it available to others for
 	 * manipulation.
 	 */
 	public void unlock();

spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationId.java

@@ -18,7 +18,7 @@ package org.springframework.webflow.conversation;
 import java.io.Serializable;
 
 /**
- * An id that uniquely identifies a conversation managed by
+ * An id that uniquely identifies a conversation managed by a
  * {@link ConversationManager}.
  * 
  * @author Ben Hale

spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java

@@ -24,8 +24,10 @@ import java.util.Map;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.springframework.webflow.context.ExternalContextHolder;
 import org.springframework.webflow.conversation.Conversation;
 import org.springframework.webflow.conversation.ConversationId;
+import org.springframework.webflow.core.collection.SharedAttributeMap;
 
 /**
  * Internal {@link Conversation} implementation used by the conversation
@@ -98,8 +100,18 @@ class ContainedConversation implements Conversation, Serializable {
 	public void unlock() {
 		if (logger.isDebugEnabled()) {
 			logger.debug("Unlocking conversation " + id);
-		}				
+		}
 		lock.unlock();
+		
+		// re-bind the conversation container in the session
+		// this is required to make session replication work correctly in
+		// a clustered environment
+		// we do this after releasing the lock since we're no longer
+		// manipulating the contents of the conversation
+		SharedAttributeMap sessionMap = ExternalContextHolder.getExternalContext().getSessionMap();
+		synchronized (sessionMap.getMutex()) {
+			sessionMap.put(SessionBindingConversationManager.CONVERSATION_CONTAINER_KEY, container);
+		}
 	}
 
 	public String toString() {

spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java

@@ -33,7 +33,7 @@ import org.springframework.webflow.util.UidGenerator;
  * <p>
  * Using the {@link #setMaxConversations(int) maxConversations} property, you can
  * limit the number of concurrently active conversations allowed in a single
- * session. If the default is exceeded, the conversation manager will automatically
+ * session. If the maximum is exceeded, the conversation manager will automatically
  * end the oldest conversation. The default is 5, which should be fine for most
  * situations. Set it to -1 for no limit. Setting maxConversations to 1 allows
  * easy resource cleanup in situations where there should only be one active
@@ -48,7 +48,7 @@ public class SessionBindingConversationManager implements ConversationManager {
 	/**
 	 * Key of the session attribute holding the conversation container.
 	 */
-	private static final String CONVERSATION_CONTAINER_KEY = "webflow.conversation.container";
+	static final String CONVERSATION_CONTAINER_KEY = "webflow.conversation.container";
 	
 	/**
 	 * The conversation uid generation strategy to use.

spring-webflow/src/main/java/org/springframework/webflow/execution/repository/FlowExecutionRepository.java

@@ -95,6 +95,9 @@ public interface FlowExecutionRepository {
 	 * returned flow execution represents the restored state of an executing
 	 * flow from a point in time. This should be called to resume a persistent
 	 * flow execution.
+	 * <p>
+	 * Before calling this method, you should aquire the lock for the keyed
+	 * flow execution.
 	 * @param key the flow execution key
 	 * @return the flow execution, fully hydrated and ready to signal an event
 	 * against
@@ -107,6 +110,9 @@ public interface FlowExecutionRepository {
 	 * Place the <code>FlowExecution</code> in this repository under the
 	 * provided key. This should be called to save or update the persistent
 	 * state of an active (but paused) flow execution.
+	 * <p>
+	 * Before calling this method, you should aquire the lock for the keyed
+	 * flow execution.
 	 * @param key the flow execution key
 	 * @param flowExecution the flow execution
 	 * @throws FlowExecutionRepositoryException the flow execution could not be
@@ -118,6 +124,9 @@ public interface FlowExecutionRepository {
 	/**
 	 * Remove the flow execution from the repository. This should be called when
 	 * the flow execution ends (is no longer active).
+	 * <p>
+	 * Before calling this method, you should aquire the lock for the keyed
+	 * flow execution.
 	 * @param key the flow execution key
 	 * @throws FlowExecutionRepositoryException the flow execution could not be
 	 * removed.

spring-webflow/src/main/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepository.java

@@ -186,7 +186,13 @@ public class ContinuationFlowExecutionRepository extends AbstractConversationFlo
 	protected void onBegin(Conversation conversation) {
 		// setup a new continuation group for the conversation
 		FlowExecutionContinuationGroup continuationGroup = new FlowExecutionContinuationGroup(maxContinuations);
-		conversation.putAttribute(CONTINUATION_GROUP_ATTRIBUTE, continuationGroup);
+		conversation.lock();
+		try {
+			conversation.putAttribute(CONTINUATION_GROUP_ATTRIBUTE, continuationGroup);
+		}
+		finally {
+			conversation.unlock();
+		}
 	}
 
 	protected Serializable generateContinuationId(FlowExecution flowExecution) {

spring-webflow/src/main/java/org/springframework/webflow/executor/FlowExecutorImpl.java

@@ -207,7 +207,14 @@ public class FlowExecutorImpl implements FlowExecutor {
 			if (flowExecution.isActive()) {
 				// execution still active => store it in the repository
 				FlowExecutionKey key = executionRepository.generateKey(flowExecution);
-				executionRepository.putFlowExecution(key, flowExecution);
+				FlowExecutionLock lock = executionRepository.getLock(key);
+				lock.lock();
+				try {
+					executionRepository.putFlowExecution(key, flowExecution);
+				}
+				finally {
+					lock.unlock();
+				}
 				return new ResponseInstruction(key.toString(), flowExecution, selectedView);
 			}
 			else {

spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManagerTests.java

@@ -51,7 +51,9 @@ public class SessionBindingConversationManagerTests extends TestCase {
 				new ConversationParameters("test", "test", "test"));
 		ConversationId conversationId = conversation.getId();
 		assertNotNull(conversationManager.getConversation(conversationId));
+		conversation.lock();
 		conversation.end();
+		conversation.unlock();
 		try {
 			conversationManager.getConversation(conversationId);
 			fail("Conversation should have ben removed");
@@ -64,13 +66,17 @@ public class SessionBindingConversationManagerTests extends TestCase {
 		ExternalContextHolder.setExternalContext(new MockExternalContext());
 		Conversation conversation = conversationManager.beginConversation(
 				new ConversationParameters("test", "test", "test"));
+		conversation.lock();
 		conversation.putAttribute("testAttribute", "testValue");
 		ConversationId conversationId = conversation.getId();
 		
 		Conversation conversation2 = conversationManager.getConversation(conversationId);
 		assertSame(conversation, conversation2);
+		conversation2.lock();
 		assertEquals("testValue", conversation2.getAttribute("testAttribute"));
 		conversation.end();
+		conversation.unlock();
+		conversation2.unlock();
 	}
 	
 	public void testPassivation() throws Exception {
@@ -78,6 +84,7 @@ public class SessionBindingConversationManagerTests extends TestCase {
 		ExternalContextHolder.setExternalContext(externalContext);
 		Conversation conversation = conversationManager.beginConversation(
 				new ConversationParameters("test", "test", "test"));
+		conversation.lock();
 		conversation.putAttribute("testAttribute", "testValue");
 		ConversationId conversationId = conversation.getId();
 		ExternalContextHolder.setExternalContext(null);
@@ -92,6 +99,7 @@ public class SessionBindingConversationManagerTests extends TestCase {
 		assertNotSame(conversation, conversation2);
 		assertEquals("testValue", conversation2.getAttribute("testAttribute"));
 		conversation.end();
+		conversation.unlock();
 	}
 	
 	public void testMaxConversations() {

spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ClientContinuationFlowExecutionRepositoryTests.java

@@ -42,6 +42,8 @@ public class ClientContinuationFlowExecutionRepositoryTests extends TestCase {
 	private FlowExecution execution;
 
 	private FlowExecutionKey key;
+	
+	private FlowExecutionLock lock;
 
 	protected void setUp() throws Exception {
 		FlowDefinitionRegistry registry = new FlowDefinitionRegistryImpl();
@@ -55,41 +57,51 @@ public class ClientContinuationFlowExecutionRepositoryTests extends TestCase {
 	public void testPutExecution() {
 		key = repository.generateKey(execution);
 		assertNotNull(key);
+		lock = repository.getLock(key);
+		lock.lock();
 		repository.putFlowExecution(key, execution);
 		FlowExecution persisted = repository.getFlowExecution(key);
 		assertNotNull(persisted);
+		lock.unlock();
 	}
 
 	public void testGetNextKey() {
 		key = repository.generateKey(execution);
 		assertNotNull(key);
+		lock = repository.getLock(key);
+		lock.lock();
 		repository.putFlowExecution(key, execution);
 		FlowExecutionKey nextKey = repository.getNextKey(execution, key);
 		repository.putFlowExecution(nextKey, execution);
 		FlowExecution persisted = repository.getFlowExecution(nextKey);
 		assertNotNull(persisted);
+		lock.unlock();
 	}
 
 	public void testGetNextKeyVerifyKeyChanged() {
 		key = repository.generateKey(execution);
 		assertNotNull(key);
+		lock = repository.getLock(key);
+		lock.lock();
 		repository.putFlowExecution(key, execution);
 		FlowExecutionKey nextKey = repository.getNextKey(execution, key);
 		repository.putFlowExecution(nextKey, execution);
 		repository.getFlowExecution(key);
 		repository.getFlowExecution(nextKey);
+		lock.unlock();
 	}
 
 	public void testRemove() {
 		testPutExecution();
+		lock.lock();
 		repository.removeFlowExecution(key);
 		try {
 			repository.getFlowExecution(key);
 			fail("should've throw nsfee");
 		}
 		catch (NoSuchFlowExecutionException e) {
-
 		}
+		lock.unlock();
 	}
 
 	public void testLock() {

spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepositoryTests.java

@@ -42,6 +42,8 @@ public class ContinuationFlowExecutionRepositoryTests extends TestCase {
 	private FlowExecution execution;
 
 	private FlowExecutionKey key;
+	
+	private FlowExecutionLock lock;
 
 	protected void setUp() throws Exception {
 		FlowDefinitionRegistry registry = new FlowDefinitionRegistryImpl();
@@ -54,42 +56,52 @@ public class ContinuationFlowExecutionRepositoryTests extends TestCase {
 
 	public void testPutExecution() {
 		key = repository.generateKey(execution);
+		lock = repository.getLock(key);
+		lock.lock();
 		assertNotNull(key);
 		repository.putFlowExecution(key, execution);
 		FlowExecution persisted = repository.getFlowExecution(key);
 		assertNotNull(persisted);
+		lock.unlock();
 	}
 
 	public void testGetNextKey() {
 		key = repository.generateKey(execution);
+		lock = repository.getLock(key);
+		lock.lock();
 		assertNotNull(key);
 		repository.putFlowExecution(key, execution);
 		FlowExecutionKey nextKey = repository.getNextKey(execution, key);
 		repository.putFlowExecution(nextKey, execution);
 		FlowExecution persisted = repository.getFlowExecution(nextKey);
 		assertNotNull(persisted);
+		lock.unlock();
 	}
 
 	public void testGetNextKeyVerifyKeyChanged() {
 		key = repository.generateKey(execution);
+		lock = repository.getLock(key);
+		lock.lock();
 		assertNotNull(key);
 		repository.putFlowExecution(key, execution);
 		FlowExecutionKey nextKey = repository.getNextKey(execution, key);
 		repository.putFlowExecution(nextKey, execution);
 		repository.getFlowExecution(key);
 		repository.getFlowExecution(nextKey);
+		lock.unlock();
 	}
 
 	public void testRemove() {
 		testPutExecution();
+		lock.lock();
 		repository.removeFlowExecution(key);
 		try {
 			repository.getFlowExecution(key);
 			fail("should've throw nsfee");
 		}
 		catch (NoSuchFlowExecutionException e) {
-
 		}
+		lock.unlock();
 	}
 
 	public void testLock() {

spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/FlowExecutionContinuationGroupTests.java

@@ -32,6 +32,7 @@ import org.springframework.webflow.engine.builder.FlowBuilderException;
 import org.springframework.webflow.engine.impl.FlowExecutionImplStateRestorer;
 import org.springframework.webflow.execution.FlowExecution;
 import org.springframework.webflow.execution.repository.FlowExecutionKey;
+import org.springframework.webflow.execution.repository.FlowExecutionLock;
 import org.springframework.webflow.execution.repository.NoSuchFlowExecutionException;
 import org.springframework.webflow.execution.support.ApplicationView;
 import org.springframework.webflow.execution.support.FlowExecutionRedirect;
@@ -188,7 +189,14 @@ public class FlowExecutionContinuationGroupTests extends TestCase {
 			try {
 				FlowExecutionKey key = parseFlowExecutionKey(
 						new RequestParameterFlowExecutorArgumentHandler().extractFlowExecutionKey(externalContext));
-				return getContinuationGroup(key);
+				FlowExecutionLock lock = getLock(key);
+				lock.lock();
+				try {
+					return getContinuationGroup(key);
+				}
+				finally {
+					lock.unlock();
+				}
 			}
 			finally {
 				ExternalContextHolder.setExternalContext(null);

spring-webflow/src/test/java/org/springframework/webflow/execution/repository/support/SimpleFlowExecutionRepositoryTests.java

@@ -42,6 +42,8 @@ public class SimpleFlowExecutionRepositoryTests extends TestCase {
 	private FlowExecution execution;
 
 	private FlowExecutionKey key;
+	
+	private FlowExecutionLock lock;
 
 	protected void setUp() throws Exception {
 		FlowDefinitionRegistry registry = new FlowDefinitionRegistryImpl();
@@ -55,26 +57,34 @@ public class SimpleFlowExecutionRepositoryTests extends TestCase {
 	public void testPutExecution() {
 		key = repository.generateKey(execution);
 		assertNotNull(key);
+		lock = repository.getLock(key);
+		lock.lock();
 		repository.putFlowExecution(key, execution);
 		FlowExecution persisted = repository.getFlowExecution(key);
 		assertNotNull(persisted);
 		assertSame(execution, persisted);
+		lock.unlock();
 	}
 
 	public void testGetNextKey() {
 		key = repository.generateKey(execution);
 		assertNotNull(key);
+		lock = repository.getLock(key);
+		lock.lock();
 		repository.putFlowExecution(key, execution);
 		FlowExecutionKey nextKey = repository.getNextKey(execution, key);
 		repository.putFlowExecution(nextKey, execution);
 		FlowExecution persisted = repository.getFlowExecution(nextKey);
 		assertNotNull(persisted);
 		assertSame(execution, persisted);
+		lock.unlock();
 	}
 
 	public void testGetNextKeyVerifyKeyChanged() {
 		key = repository.generateKey(execution);
 		assertNotNull(key);
+		lock = repository.getLock(key);
+		lock.lock();
 		repository.putFlowExecution(key, execution);
 		FlowExecutionKey nextKey = repository.getNextKey(execution, key);
 		repository.putFlowExecution(nextKey, execution);
@@ -83,8 +93,8 @@ public class SimpleFlowExecutionRepositoryTests extends TestCase {
 			fail("Should've failed");
 		}
 		catch (PermissionDeniedFlowExecutionAccessException e) {
-
 		}
+		lock.unlock();
 	}
 
 	public void testGetNextKeyVerifyKeyStaysSame() {
@@ -96,14 +106,15 @@ public class SimpleFlowExecutionRepositoryTests extends TestCase {
 
 	public void testRemove() {
 		testPutExecution();
+		lock.lock();
 		repository.removeFlowExecution(key);
 		try {
 			repository.getFlowExecution(key);
 			fail("should've throw nsfee");
 		}
 		catch (NoSuchFlowExecutionException e) {
-
 		}
+		lock.unlock();
 	}
 
 	public void testLock() {