From aa0f9b2d3f722e7787a73630ed038018332fac3d Mon Sep 17 00:00:00 2001 From: Erwin Vervaet Date: Tue, 1 May 2007 12:34:24 +0000 Subject: [PATCH] Each SessionBindingConversationManager now uses a unique key to store it's conversation container in the session (SWF-304) --- spring-webflow/changelog.txt | 4 ++++ .../impl/ContainedConversation.java | 2 +- .../impl/ConversationContainer.java | 17 ++++++++++++++- .../SessionBindingConversationManager.java | 21 ++++++++++++++----- .../impl/ConversationSizeTests.java | 12 +++++++---- .../FlowExecutionContinuationGroupTests.java | 15 +++++++++---- 6 files changed, 56 insertions(+), 15 deletions(-) diff --git a/spring-webflow/changelog.txt b/spring-webflow/changelog.txt index 23a76e05..eab4a7ff 100644 --- a/spring-webflow/changelog.txt +++ b/spring-webflow/changelog.txt @@ -9,6 +9,10 @@ Package org.springframework.webflow.config * FlowExecutorFactoryBean now has an 'inputMapper' property to conveniently configure the 'inputMapper' property of the created FlowExecutorImpl object. +Package org.springframework.webflow.conversation +* Each SessionBindingConversationManager now uses a unique key to store it's conversation container + in the session (SWF-304). + Package org.springframework.webflow.executor * JSF integration code now manages flow execution locks properly in exceptional situations and when the RENDER RESPONSE phase is bypassed (SWF-302). 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 de13180f..a071d0e4 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 @@ -110,7 +110,7 @@ class ContainedConversation implements Conversation, Serializable { // manipulating the contents of the conversation SharedAttributeMap sessionMap = ExternalContextHolder.getExternalContext().getSessionMap(); synchronized (sessionMap.getMutex()) { - sessionMap.put(SessionBindingConversationManager.CONVERSATION_CONTAINER_KEY, container); + sessionMap.put(container.getSessionKey(), container); } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java index ac64f55d..85a586a0 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/conversation/impl/ConversationContainer.java @@ -41,6 +41,11 @@ class ConversationContainer implements Serializable { * unlimited. */ private int maxConversations; + + /** + * The key of this conversation container in the session. + */ + private String sessionKey; /** * The contained conversations. A list of {@link ContainedConversation} objects. @@ -51,12 +56,22 @@ class ConversationContainer implements Serializable { * Create a new conversation container. * @param maxConversations the maximum number of allowed concurrent * conversations, -1 for unlimited + * @param sessionKey the key of this conversation container in the session */ - public ConversationContainer(int maxConversations) { + public ConversationContainer(int maxConversations, String sessionKey) { this.maxConversations = maxConversations; + this.sessionKey = sessionKey; this.conversations = new ArrayList(); } + /** + * Returns the key of this conversation container in the session. + * For package level use only. + */ + String getSessionKey() { + return sessionKey; + } + /** * Returns the current size of the conversation container: the number * of conversations contained within it. 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 d8b5bc9a..7712c919 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 @@ -24,6 +24,7 @@ import org.springframework.webflow.conversation.ConversationId; import org.springframework.webflow.conversation.ConversationManager; import org.springframework.webflow.conversation.ConversationParameters; import org.springframework.webflow.core.collection.SharedAttributeMap; +import org.springframework.webflow.util.RandomGuid; import org.springframework.webflow.util.RandomGuidUidGenerator; import org.springframework.webflow.util.UidGenerator; @@ -46,9 +47,10 @@ public class SessionBindingConversationManager implements ConversationManager { private static final Log logger = LogFactory.getLog(SessionBindingConversationManager.class); /** - * Key of the session attribute holding the conversation container. + * Generate a unique key for the session attribute holding the conversation + * container managed by this conversation manager. */ - static final String CONVERSATION_CONTAINER_KEY = "webflow.conversation.container"; + private String sessionKey = "webflow.conversation.container." + new RandomGuid().toString(); /** * The conversation uid generation strategy to use. @@ -94,6 +96,15 @@ public class SessionBindingConversationManager implements ConversationManager { public void setMaxConversations(int maxConversations) { this.maxConversations = maxConversations; } + + /** + * Returns the key this conversation manager uses to store conversation + * data in the session. The key is unique for this conversation manager instance. + * @return the session key + */ + public String getSessionKey() { + return sessionKey; + } public Conversation beginConversation(ConversationParameters conversationParameters) throws ConversationException { ConversationId conversationId = new SimpleConversationId(conversationIdGenerator.generateUid()); @@ -125,10 +136,10 @@ public class SessionBindingConversationManager implements ConversationManager { private ConversationContainer getConversationContainer() { SharedAttributeMap sessionMap = ExternalContextHolder.getExternalContext().getSessionMap(); synchronized (sessionMap.getMutex()) { - ConversationContainer container = (ConversationContainer)sessionMap.get(CONVERSATION_CONTAINER_KEY); + ConversationContainer container = (ConversationContainer)sessionMap.get(sessionKey); if (container == null) { - container = new ConversationContainer(maxConversations); - sessionMap.put(CONVERSATION_CONTAINER_KEY, container); + container = new ConversationContainer(maxConversations, sessionKey); + sessionMap.put(sessionKey, container); } return container; } diff --git a/spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/ConversationSizeTests.java b/spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/ConversationSizeTests.java index 165e4bba..6bd0293c 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/ConversationSizeTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/conversation/impl/ConversationSizeTests.java @@ -41,6 +41,7 @@ import org.springframework.webflow.test.MockExternalContext; */ public class ConversationSizeTests extends TestCase { + private SessionBindingConversationManager conversationManager; private FlowExecutor flowExecutor; protected void setUp() throws Exception { @@ -50,8 +51,11 @@ public class ConversationSizeTests extends TestCase { flowRegistryFactory.afterPropertiesSet(); FlowDefinitionRegistry flowRegistry = flowRegistryFactory.getRegistry(); + conversationManager = new SessionBindingConversationManager(); + FlowExecutorFactoryBean flowExecutorFactory = new FlowExecutorFactoryBean(); flowExecutorFactory.setDefinitionLocator(flowRegistry); + flowExecutorFactory.setConversationManager(conversationManager); flowExecutorFactory.setRepositoryType(RepositoryType.CONTINUATION); flowExecutorFactory.afterPropertiesSet(); flowExecutor = flowExecutorFactory.getFlowExecutor(); @@ -69,10 +73,10 @@ public class ConversationSizeTests extends TestCase { assertTrue(ri.getViewSelection() instanceof FlowExecutionRedirect); // alwaysRedirectOnPause // the launch has stored a ConversationContainer in the session since we're using - // SessionBindingConversationManager (the default) + // SessionBindingConversationManager assertEquals(1, session.size()); ConversationContainer conversationContainer = (ConversationContainer)session.get( - SessionBindingConversationManager.CONVERSATION_CONTAINER_KEY); + conversationManager.getSessionKey()); assertNotNull(conversationContainer); assertEquals(1, conversationContainer.size()); int initialSize = getSerializedSize(conversationContainer); @@ -83,7 +87,7 @@ public class ConversationSizeTests extends TestCase { // the refresh did not impact the size of the session assertEquals(1, session.size()); - assertSame(conversationContainer, session.get(SessionBindingConversationManager.CONVERSATION_CONTAINER_KEY)); + assertSame(conversationContainer, session.get(conversationManager.getSessionKey())); assertEquals(1, conversationContainer.size()); ri = flowExecutor.resume(ri.getFlowExecutionKey(), "end", context); @@ -92,7 +96,7 @@ public class ConversationSizeTests extends TestCase { // the conversation ended but the ConversationContainer is still in the session assertEquals(1, session.size()); - assertSame(conversationContainer, session.get(SessionBindingConversationManager.CONVERSATION_CONTAINER_KEY)); + assertSame(conversationContainer, session.get(conversationManager.getSessionKey())); assertEquals(0, conversationContainer.size()); int inactiveSize = getSerializedSize(conversationContainer); 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 dce581c0..bec8549a 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 @@ -20,6 +20,7 @@ import junit.framework.TestCase; import org.springframework.webflow.config.FlowExecutorFactoryBean; import org.springframework.webflow.context.ExternalContext; import org.springframework.webflow.context.ExternalContextHolder; +import org.springframework.webflow.conversation.ConversationManager; import org.springframework.webflow.conversation.impl.SessionBindingConversationManager; import org.springframework.webflow.definition.FlowDefinition; import org.springframework.webflow.definition.registry.FlowDefinitionLocator; @@ -113,17 +114,23 @@ public class FlowExecutionContinuationGroupTests extends TestCase { FlowDefinitionRegistry registry = new FlowDefinitionRegistryImpl(); FlowDefinition testFlow = new FlowAssembler("testFlow", new TestFlowBuilder()).assembleFlow(); registry.registerFlowDefinition(new StaticFlowDefinitionHolder(testFlow)); + + ConversationManager conversationManager = new SessionBindingConversationManager(); + FlowExecutorFactoryBean flowExecutorFactory = new FlowExecutorFactoryBean(); flowExecutorFactory.setDefinitionLocator(registry); + flowExecutorFactory.setConversationManager(conversationManager); flowExecutorFactory.afterPropertiesSet(); FlowExecutor flowExecutor = (FlowExecutor)flowExecutorFactory.getObject(); MockExternalContext externalContext = new MockExternalContext(); + + GroupGetter groupGetter = new GroupGetter(registry, conversationManager); //obtain continuation group ResponseInstruction response = flowExecutor.launch("testFlow", externalContext); externalContext.putRequestParameter("_flowExecutionKey", response.getFlowExecutionKey()); - FlowExecutionContinuationGroup group = new GroupGetter(registry).getContinuationGroup(externalContext); + FlowExecutionContinuationGroup group = groupGetter.getContinuationGroup(externalContext); assertNotNull(group); assertTrue(response.getViewSelection() instanceof FlowExecutionRedirect); @@ -151,7 +158,7 @@ public class FlowExecutionContinuationGroupTests extends TestCase { response = flowExecutor.resume(response.getFlowExecutionKey(), "end", externalContext); try { - new GroupGetter(registry).getContinuationGroup(externalContext); + groupGetter.getContinuationGroup(externalContext); fail(); } catch (NoSuchFlowExecutionException e) { @@ -180,8 +187,8 @@ public class FlowExecutionContinuationGroupTests extends TestCase { private static class GroupGetter extends ContinuationFlowExecutionRepository { - public GroupGetter(FlowDefinitionLocator definitionLocator) { - super(new FlowExecutionImplStateRestorer(definitionLocator), new SessionBindingConversationManager()); + public GroupGetter(FlowDefinitionLocator definitionLocator, ConversationManager conversationManager) { + super(new FlowExecutionImplStateRestorer(definitionLocator), conversationManager); } public FlowExecutionContinuationGroup getContinuationGroup(ExternalContext externalContext) {