From d68e9bb37523092e2a9abe91cfad6611443ce3e2 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Thu, 16 Aug 2007 20:50:52 +0000 Subject: [PATCH] RESOLVED - issue SWF-221: Flash scope should be a property of the Flow Execution instead of the active Flow Session http://opensource.atlassian.com/projects/spring/browse/SWF-221 --- spring-webflow/changelog.txt | 4 ++++ .../EnableScopesBeanDefinitionParser.java | 1 - .../config/scope/AbstractWebFlowScope.java | 1 - .../config/scope/ConversationScope.java | 3 +-- .../webflow/config/scope/FlashScope.java | 9 ++++----- .../webflow/config/scope/FlowScope.java | 3 +-- .../webflow/config/scope/ScopeRegistrar.java | 3 +-- .../webflow/engine/impl/FlowExecutionImpl.java | 18 ++++++++++++++---- .../webflow/engine/impl/FlowSessionImpl.java | 13 +------------ .../engine/impl/RequestControlContextImpl.java | 2 +- .../execution/FlowExecutionContext.java | 7 +++++++ .../webflow/execution/FlowSession.java | 7 ------- .../webflow/test/MockFlowExecutionContext.java | 15 ++++++++++++++- .../webflow/test/MockRequestContext.java | 2 +- .../execution/AbstractFlowExecutionTests.java | 8 +++----- .../webflow/config/scope/FlashScopeTests.java | 17 +++++++---------- .../engine/impl/FlowExecutionImplTests.java | 6 +++++- 17 files changed, 64 insertions(+), 55 deletions(-) diff --git a/spring-webflow/changelog.txt b/spring-webflow/changelog.txt index 6572bfa1..94a701e9 100644 --- a/spring-webflow/changelog.txt +++ b/spring-webflow/changelog.txt @@ -20,6 +20,10 @@ Package org.springframework.webflow.config * Added a new bean definition to a new 1.1 revision of the spring-webflow-config schema to support Web Flow scoping of beans in Spring (SWF-163). +Package org.springframework.webflow.engine +* Made flash scope a property of the FlowExecution instead of the active FlowSession. This allows flash messages to be saved across an redirect + if the messages were put there by a subflow that ended, for example (SWF-221). + Package org.springframework.webflow.context.scope * Added support for Web Flow scoping of Spring beans (SWF-163). diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/EnableScopesBeanDefinitionParser.java b/spring-webflow/src/main/java/org/springframework/webflow/config/EnableScopesBeanDefinitionParser.java index 9060adee..2d33670d 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/EnableScopesBeanDefinitionParser.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/EnableScopesBeanDefinitionParser.java @@ -24,7 +24,6 @@ import org.w3c.dom.Element; * {@link BeanDefinitionParser} for the <enable-scopes> tag. * * @author Ben Hale - * @since 1.1 */ class EnableScopesBeanDefinitionParser extends AbstractSingleBeanDefinitionParser { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/AbstractWebFlowScope.java b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/AbstractWebFlowScope.java index e8b2eaec..18bd7279 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/AbstractWebFlowScope.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/AbstractWebFlowScope.java @@ -39,7 +39,6 @@ import org.springframework.webflow.execution.FlowSession; * @see FlowExecutionContextHolder * * @author Ben Hale - * @since 1.1 */ public abstract class AbstractWebFlowScope implements Scope { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ConversationScope.java b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ConversationScope.java index 50605759..c4133c49 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ConversationScope.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ConversationScope.java @@ -20,12 +20,11 @@ import org.springframework.webflow.core.collection.MutableAttributeMap; import org.springframework.webflow.execution.FlowExecution; /** - * Conversation-backed {@link Scope} implementation. + * Conversation {@link Scope scope} implementation. * * @see FlowExecution#getConversationScope() * * @author Ben Hale - * @since 1.1 */ public class ConversationScope extends AbstractWebFlowScope { protected MutableAttributeMap getScope() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlashScope.java b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlashScope.java index 51cb6d9e..af5f5068 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlashScope.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlashScope.java @@ -17,18 +17,17 @@ package org.springframework.webflow.config.scope; import org.springframework.beans.factory.config.Scope; import org.springframework.webflow.core.collection.MutableAttributeMap; -import org.springframework.webflow.execution.FlowSession; +import org.springframework.webflow.execution.FlowExecutionContext; /** - * Flash-backed {@link Scope} implementation. + * Flash {@link Scope scope} implementation. * - * @see FlowSession#getFlashMap() + * @see FlowExecutionContext#getFlashScope() * * @author Ben Hale - * @since 1.1 */ public class FlashScope extends AbstractWebFlowScope { protected MutableAttributeMap getScope() { - return getFlowExecutionContext().getActiveSession().getFlashMap(); + return getFlowExecutionContext().getFlashScope(); } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlowScope.java b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlowScope.java index fe68264d..52fe4bfc 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlowScope.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/FlowScope.java @@ -20,12 +20,11 @@ import org.springframework.webflow.core.collection.MutableAttributeMap; import org.springframework.webflow.execution.FlowSession; /** - * Flow-backed {@link Scope} implementation. + * Flow {@link Scope scope} implementation. * * @see FlowSession#getScope() * * @author Ben Hale - * @since 1.1 */ public class FlowScope extends AbstractWebFlowScope { protected MutableAttributeMap getScope() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ScopeRegistrar.java b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ScopeRegistrar.java index ef9a0726..bf75d9ff 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ScopeRegistrar.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/scope/ScopeRegistrar.java @@ -28,14 +28,13 @@ import org.springframework.webflow.execution.ScopeType; * * @author Ben Hale * @see Scope - * @since 1.1 */ public class ScopeRegistrar implements BeanFactoryPostProcessor, Ordered { public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { - beanFactory.registerScope(ScopeType.CONVERSATION.getLabel().toLowerCase(), new ConversationScope()); beanFactory.registerScope(ScopeType.FLASH.getLabel().toLowerCase(), new FlashScope()); beanFactory.registerScope(ScopeType.FLOW.getLabel().toLowerCase(), new FlowScope()); + beanFactory.registerScope(ScopeType.CONVERSATION.getLabel().toLowerCase(), new ConversationScope()); } public int getOrder() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java index e93ff3b6..13f7ccfb 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java @@ -85,6 +85,11 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable { */ private transient FlowExecutionListeners listeners; + /** + * The flash map ("flash scope"). + */ + private MutableAttributeMap flashScope = new LocalAttributeMap(); + /** * A data structure for attributes shared by all flow sessions. *

@@ -153,6 +158,10 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable { return getActiveSessionInternal(); } + public MutableAttributeMap getFlashScope() { + return flashScope; + } + public MutableAttributeMap getConversationScope() { return conversationScope; } @@ -198,8 +207,8 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable { if (logger.isDebugEnabled()) { logger.debug("Resuming execution on user event '" + eventId + "'"); } + flashScope.clear(); RequestControlContext context = createControlContext(externalContext); - context.getFlashScope().clear(); getListeners().fireRequestSubmitted(context); try { try { @@ -463,17 +472,18 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable { return (State) getActiveSessionInternal().getState(); } - // custom serialization (implementation of Externalizable for optimized - // storage) + // custom serialization (implementation of Externalizable for optimized storage) public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { flowId = (String) in.readObject(); flowSessions = (LinkedList) in.readObject(); + flashScope = (MutableAttributeMap) in.readObject(); } public void writeExternal(ObjectOutput out) throws IOException { out.writeObject(flowId); out.writeObject(flowSessions); + out.writeObject(flashScope); } public String toString() { @@ -482,7 +492,7 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable { } else { if (flow != null) { return new ToStringCreator(this).append("flow", flow.getId()).append("flowSessions", flowSessions) - .toString(); + .append("flashScope", flashScope).toString(); } else { return "[Unhydrated " + getCaption() + "]"; } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowSessionImpl.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowSessionImpl.java index debe639f..c5fd990e 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowSessionImpl.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowSessionImpl.java @@ -76,11 +76,6 @@ class FlowSessionImpl implements FlowSession, Externalizable { */ private MutableAttributeMap scope = new LocalAttributeMap(); - /** - * The flash map ("flash scope"). - */ - private MutableAttributeMap flashMap = new LocalAttributeMap(); - /** * The parent session of this session (may be null if this is a root session.) */ @@ -120,10 +115,6 @@ class FlowSessionImpl implements FlowSession, Externalizable { return scope; } - public MutableAttributeMap getFlashMap() { - return flashMap; - } - public FlowSession getParent() { return parent; } @@ -139,7 +130,6 @@ class FlowSessionImpl implements FlowSession, Externalizable { stateId = (String) in.readObject(); status = (FlowSessionStatus) in.readObject(); scope = (MutableAttributeMap) in.readObject(); - flashMap = (MutableAttributeMap) in.readObject(); parent = (FlowSessionImpl) in.readObject(); } @@ -148,7 +138,6 @@ class FlowSessionImpl implements FlowSession, Externalizable { out.writeObject(stateId); out.writeObject(status); out.writeObject(scope); - out.writeObject(flashMap); out.writeObject(parent); } @@ -205,6 +194,6 @@ class FlowSessionImpl implements FlowSession, Externalizable { public String toString() { return new ToStringCreator(this).append("flow", flowId).append("state", stateId).append("scope", scope).append( - "flashMap", flashMap).append("status", status).toString(); + "status", status).toString(); } } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/RequestControlContextImpl.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/RequestControlContextImpl.java index 3abd79dd..472a4e4c 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/RequestControlContextImpl.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/RequestControlContextImpl.java @@ -111,7 +111,7 @@ class RequestControlContextImpl implements RequestControlContext { } public MutableAttributeMap getFlashScope() { - return flowExecution.getActiveSession().getFlashMap(); + return flowExecution.getFlashScope(); } public MutableAttributeMap getFlowScope() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowExecutionContext.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowExecutionContext.java index e2d22675..694275b5 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowExecutionContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowExecutionContext.java @@ -69,6 +69,13 @@ public interface FlowExecutionContext { */ public FlowSession getActiveSession() throws IllegalStateException; + /** + * Returns a mutable map for data held in "flash scope". Attributes in this map are cleared out on the next event + * signaled against this flow execution. Flash attributes survive flow execution refresh operations. + * @return flash scope + */ + public MutableAttributeMap getFlashScope(); + /** * Returns a mutable map for data held in "conversation scope". Conversation scope is a data structure that exists * for the life of this flow execution and is accessible to all flow sessions. diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowSession.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowSession.java index eca493ae..bb89d572 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowSession.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/FlowSession.java @@ -79,13 +79,6 @@ public interface FlowSession { */ public MutableAttributeMap getScope(); - /** - * Returns the local "flash map". Attributes in this map are cleared out on the next event signaled in the flow - * execution, so they survive a refresh. - * @return the flash map - */ - public MutableAttributeMap getFlashMap(); - /** * Returns the parent flow session in the current flow execution, or null if there is no parent flow * session. diff --git a/spring-webflow/src/main/java/org/springframework/webflow/test/MockFlowExecutionContext.java b/spring-webflow/src/main/java/org/springframework/webflow/test/MockFlowExecutionContext.java index a6e5d7a3..b5076783 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/test/MockFlowExecutionContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/test/MockFlowExecutionContext.java @@ -36,6 +36,8 @@ public class MockFlowExecutionContext implements FlowExecutionContext { private FlowSession activeSession; + private MutableAttributeMap flashScope = new LocalAttributeMap(); + private MutableAttributeMap conversationScope = new LocalAttributeMap(); private MutableAttributeMap attributes = new LocalAttributeMap(); @@ -78,6 +80,10 @@ public class MockFlowExecutionContext implements FlowExecutionContext { return activeSession; } + public MutableAttributeMap getFlashScope() { + return flashScope; + } + public MutableAttributeMap getConversationScope() { return conversationScope; } @@ -103,7 +109,14 @@ public class MockFlowExecutionContext implements FlowExecutionContext { } /** - * Sets flow execution (conversational) scope. + * Sets the flow execution flash scope. + */ + public void setFlashScope(MutableAttributeMap scope) { + this.flashScope = scope; + } + + /** + * Sets the flow execution conversation scope. */ public void setConversationScope(MutableAttributeMap scope) { this.conversationScope = scope; diff --git a/spring-webflow/src/main/java/org/springframework/webflow/test/MockRequestContext.java b/spring-webflow/src/main/java/org/springframework/webflow/test/MockRequestContext.java index a6cf2b9a..ac2b13f2 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/test/MockRequestContext.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/test/MockRequestContext.java @@ -103,7 +103,7 @@ public class MockRequestContext implements RequestContext { } public MutableAttributeMap getFlashScope() { - return getMockFlowExecutionContext().getActiveSession().getFlashMap(); + return getMockFlowExecutionContext().getFlashScope(); } public MutableAttributeMap getFlowScope() { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractFlowExecutionTests.java b/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractFlowExecutionTests.java index bb680e61..28fe38a6 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractFlowExecutionTests.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/test/execution/AbstractFlowExecutionTests.java @@ -350,10 +350,9 @@ public abstract class AbstractFlowExecutionTests extends TestCase { * the next user event. * @param attributeName the name of the attribute * @return the attribute value - * @since 1.0.2 */ protected Object getFlashAttribute(String attributeName) { - return getFlowExecution().getActiveSession().getFlashMap().get(attributeName); + return getFlowExecution().getFlashScope().get(attributeName); } /** @@ -362,10 +361,9 @@ public abstract class AbstractFlowExecutionTests extends TestCase { * @param attributeName the name of the attribute * @return the attribute value * @throws IllegalStateException if the attribute was not present - * @since 1.0.2 */ protected Object getRequiredFlashAttribute(String attributeName) throws IllegalStateException { - return getFlowExecution().getActiveSession().getFlashMap().getRequired(attributeName); + return getFlowExecution().getFlashScope().getRequired(attributeName); } /** @@ -376,7 +374,7 @@ public abstract class AbstractFlowExecutionTests extends TestCase { * @throws IllegalStateException if the attribute was not present or was of the wrong type */ protected Object getRequiredFlashAttribute(String attributeName, Class requiredType) throws IllegalStateException { - return getFlowExecution().getActiveSession().getFlashMap().getRequired(attributeName, requiredType); + return getFlowExecution().getFlashScope().getRequired(attributeName, requiredType); } // assert helpers diff --git a/spring-webflow/src/test/java/org/springframework/webflow/config/scope/FlashScopeTests.java b/spring-webflow/src/test/java/org/springframework/webflow/config/scope/FlashScopeTests.java index 931553c0..2ebfdab7 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/config/scope/FlashScopeTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/config/scope/FlashScopeTests.java @@ -48,19 +48,18 @@ public class FlashScopeTests extends TestCase { StubObjectFactory factory = new StubObjectFactory(); Object gotten = scope.get("name", factory); assertNotNull("Should be real object", gotten); - assertTrue("Should have added object to the map", context.getActiveSession().getFlashMap().contains("name")); + assertTrue("Should have added object to the map", context.getFlashScope().contains("name")); assertSame("Created object should have been returned", factory.getValue(), gotten); - assertSame("Created object should have been persisted", factory.getValue(), context.getActiveSession() - .getFlashMap().get("name")); + assertSame("Created object should have been persisted", factory.getValue(), context.getFlashScope().get("name")); } public void testGetVarExist() { StubObjectFactory factory = new StubObjectFactory(); Object value = new Object(); - context.getActiveSession().getFlashMap().put("name", value); + context.getFlashScope().put("name", value); Object gotten = scope.get("name", factory); assertNotNull("Should be real object", gotten); - assertTrue("Should still be in map", context.getActiveSession().getFlashMap().contains("name")); + assertTrue("Should still be in map", context.getFlashScope().contains("name")); assertSame("Persisted object should have been returned", value, gotten); assertNotSame("Created object should not have been returned", factory.getValue(), gotten); } @@ -82,17 +81,15 @@ public class FlashScopeTests extends TestCase { public void testRemoveVarMissing() { Object removed = scope.remove("name"); - assertFalse("Should have removed from object from map", context.getActiveSession().getFlashMap().contains( - "name")); + assertFalse("Should have removed from object from map", context.getFlashScope().contains("name")); assertNull("Should have returned a null object", removed); } public void testRemoveVarExist() { Object value = new Object(); - context.getActiveSession().getFlashMap().put("name", value); + context.getFlashScope().put("name", value); Object removed = scope.remove("name"); - assertFalse("Should have removed from object from map", context.getActiveSession().getFlashMap().contains( - "name")); + assertFalse("Should have removed from object from map", context.getFlashScope().contains("name")); assertSame("Should have returned the previous object", removed, value); } diff --git a/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java b/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java index 3162fa93..05169bf5 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java @@ -258,9 +258,13 @@ public class FlowExecutionImplTests extends TestCase { FlowExecution execution = new FlowExecutionImpl(new FlashScopeFlow()); MockExternalContext context = new MockExternalContext(); execution.start(null, context); + assertTrue(execution.getFlashScope().contains("flashScopedValue")); execution.refresh(context); + assertTrue(execution.getFlashScope().contains("flashScopedValue")); execution.refresh(context); - execution.signalEvent("view", context); + assertTrue(execution.getFlashScope().contains("flashScopedValue")); + execution.signalEvent("submit", context); + assertFalse(execution.getFlashScope().contains("flashScopedValue")); } public void testExceptionFromInputMapper() {