* 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. *
- * 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: + * + *
+ * Conversation conv = ...;
+ * conv.lock();
+ * try {
+ * // work with the Conversation object, calling methods like
+ * // getAttribute(), putAttribute() and end()
+ * }
+ * finally {
+ * conv.unlock();
+ * }
+ *
+ *
* * 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(); diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationId.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationId.java index cf464648..028d1179 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/ConversationId.java +++ b/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 diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java index 0cea1ad2..de13180f 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ContainedConversation.java +++ b/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() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java index 76d7b9c7..f70d7786 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManager.java @@ -33,7 +33,7 @@ import org.springframework.webflow.util.UidGenerator; *
* 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. diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/FlowExecutionRepository.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/FlowExecutionRepository.java index cecc7b60..41e36acd 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/FlowExecutionRepository.java +++ b/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. + *
+ * 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 FlowExecution 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.
+ *
+ * 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). + *
+ * 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. diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepository.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepository.java index 78b008cf..3f45a02a 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepository.java +++ b/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) { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/executor/FlowExecutorImpl.java b/spring-webflow/src/main/java/org/springframework/webflow/executor/FlowExecutorImpl.java index ac2a4c68..d9f1ac9f 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/executor/FlowExecutorImpl.java +++ b/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 { diff --git a/spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManagerTests.java b/spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManagerTests.java index 92b5b9ad..e5857c9c 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/SessionBindingConversationManagerTests.java +++ b/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() { diff --git a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ClientContinuationFlowExecutionRepositoryTests.java b/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ClientContinuationFlowExecutionRepositoryTests.java index 36bd4e76..4b9b9b09 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ClientContinuationFlowExecutionRepositoryTests.java +++ b/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() { diff --git a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepositoryTests.java b/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepositoryTests.java index 291e0088..c9ac2073 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/ContinuationFlowExecutionRepositoryTests.java +++ b/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() { diff --git a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/FlowExecutionContinuationGroupTests.java b/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/FlowExecutionContinuationGroupTests.java index 1b66e307..dce581c0 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/continuation/FlowExecutionContinuationGroupTests.java +++ b/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); diff --git a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/support/SimpleFlowExecutionRepositoryTests.java b/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/support/SimpleFlowExecutionRepositoryTests.java index 75b8ce0b..df641b55 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/execution/repository/support/SimpleFlowExecutionRepositoryTests.java +++ b/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() {