polish of view scope handling

spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java

@@ -140,6 +140,9 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 	 */
 	private Serializable messagesMemento;
 
+	/**
+	 * The flow execution outcome event.
+	 */
 	private transient Event outcome;
 
 	/**
@@ -154,20 +157,26 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 	 * @param flow the root flow of this flow execution
 	 */
 	public FlowExecutionImpl(Flow flow) {
-		setFlow(flow);
+		Assert.notNull(flow, "The flow definition is required");
+		this.flow = flow;
 		this.listeners = new FlowExecutionListeners();
 		this.attributes = CollectionUtils.EMPTY_ATTRIBUTE_MAP;
 		this.flowSessions = new LinkedList();
 		this.conversationScope = new LocalAttributeMap();
 	}
 
+	/**
+	 * Package private constructor only useful for testing restoration behavior for this object.
+	 * @param flowId the flow id
+	 * @param flowSessions the flow sessions
+	 */
 	FlowExecutionImpl(String flowId, LinkedList flowSessions) {
 		this.flowId = flowId;
 		this.flowSessions = flowSessions;
 	}
 
 	public String getCaption() {
-		return "execution of '" + flowId + "'";
+		return "execution of '" + flow.getId() + "'";
 	}
 
 	// implementing FlowExecutionContext
@@ -343,13 +352,7 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 		listeners.fireStateEntering(context, newState);
 		FlowSessionImpl session = getActiveSessionInternal();
 		State previousState = (State) session.getState();
-		if (previousState != null && previousState.isViewState()) {
+		session.setCurrentState(newState);
-			session.destroyViewScope();
-		}
-		session.setState(newState);
-		if (newState.isViewState()) {
-			session.initViewScope();
-		}
 		listeners.fireStateEntered(context, previousState);
 	}
 
@@ -370,7 +373,7 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 		flowSessions.removeLast();
 		listeners.fireSessionEnded(context, session, output);
 		if (hasEnded()) {
-			this.outcome = new Event(this, session.getState().getId(), output);
+			outcome = new Event(this, session.getState().getId(), output);
 		}
 		return session;
 	}
@@ -386,7 +389,7 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 	// package private setters for restoring transient state used by FlowExecutionImplServicesConfigurer
 
 	FlowExecutionListener[] getListeners() {
-		return this.listeners.getArray();
+		return listeners.getArray();
 	}
 
 	void setListeners(FlowExecutionListener[] listeners) {
@@ -407,28 +410,6 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 
 	// Used by FlowExecutionImplStateRestorer
 
-	/**
-	 * Restore the flow definition of this flow execution.
-	 */
-	void setFlow(Flow flow) {
-		this.flow = flow;
-		this.flowId = flow.getId();
-	}
-
-	/**
-	 * Restore conversation scope for this flow execution.
-	 */
-	void setConversationScope(MutableAttributeMap conversationScope) {
-		this.conversationScope = conversationScope;
-	}
-
-	/**
-	 * Restore the flow execution key.
-	 */
-	void setKey(FlowExecutionKey key) {
-		this.key = key;
-	}
-
 	/**
 	 * Returns the flow definition id of this flow execution.
 	 */
@@ -471,6 +452,27 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 		return flowSessions.listIterator(1);
 	}
 
+	/**
+	 * Restore the flow definition of this flow execution.
+	 */
+	void setFlow(Flow flow) {
+		this.flow = flow;
+	}
+
+	/**
+	 * Restore conversation scope for this flow execution.
+	 */
+	void setConversationScope(MutableAttributeMap conversationScope) {
+		this.conversationScope = conversationScope;
+	}
+
+	/**
+	 * Restore the flow execution key.
+	 */
+	void setKey(FlowExecutionKey key) {
+		this.key = key;
+	}
+
 	// custom serialization (implementation of Externalizable for optimized storage)
 
 	public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
@@ -483,7 +485,7 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 
 	public void writeExternal(ObjectOutput out) throws IOException {
 		out.writeBoolean(started);
-		out.writeObject(flowId);
+		out.writeObject(flow.getId());
 		out.writeObject(flowSessions);
 		out.writeObject(flashScope);
 		out.writeObject(messagesMemento);
@@ -501,7 +503,7 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 				return new ToStringCreator(this).append("flow", flow.getId()).append("flowSessions", flowSessions)
 						.append("flashScope", flashScope).toString();
 			} else {
-				return "[Unhydrated " + getCaption() + "]";
+				return "[Unhydrated execution of '" + flowId + "']";
 			}
 		}
 	}
@@ -532,13 +534,14 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable {
 
 	private FlowExecutionException wrap(Exception e) {
 		if (isActive()) {
-			FlowSessionImpl session = getActiveSessionInternal();
+			FlowSession session = getActiveSession();
-			String flowId = session.getFlowId();
+			String flowId = session.getDefinition().getId();
-			String stateId = session.getStateId();
+			String stateId = session.getState() != null ? session.getState().getId() : null;
 			return new FlowExecutionException(flowId, stateId, "Exception thrown in state '" + stateId + "' of flow '"
 					+ flowId + "'", e);
 		} else {
-			return new FlowExecutionException(flowId, null, "Exception thrown within inactive flow '" + flowId + "'", e);
+			return new FlowExecutionException(flow.getId(), null, "Exception thrown within inactive flow '"
+					+ flow.getId() + "'", e);
 		}
 	}
 

spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImplStateRestorer.java

@@ -51,36 +51,35 @@ public class FlowExecutionImplStateRestorer extends FlowExecutionImplServicesCon
 
 	public FlowExecution restoreState(FlowExecution flowExecution, FlowExecutionKey key,
 			MutableAttributeMap conversationScope, FlowExecutionKeyFactory keyFactory) {
-		FlowExecutionImpl impl = (FlowExecutionImpl) flowExecution;
+		FlowExecutionImpl execution = (FlowExecutionImpl) flowExecution;
-		if (impl.getFlowId() == null) {
+		if (execution.getFlowId() == null) {
 			throw new IllegalStateException("Cannot restore flow execution impl: the flow id is null");
 		}
-		if (impl.getFlowSessions() == null) {
+		if (execution.getFlowSessions() == null) {
 			throw new IllegalStateException("Cannot restore flow execution impl: the flowSessions list is null");
 		}
-		Flow flow = (Flow) definitionLocator.getFlowDefinition(impl.getFlowId());
+		Flow flow = (Flow) definitionLocator.getFlowDefinition(execution.getFlowId());
-		impl.setFlow(flow);
+		execution.setFlow(flow);
-		if (impl.hasSessions()) {
+		if (execution.hasSessions()) {
-			FlowSessionImpl root = impl.getRootSession();
+			FlowSessionImpl rootSession = execution.getRootSession();
-			root.setFlow(flow);
+			rootSession.setFlow(flow);
-			root.setState(flow.getStateInstance(root.getStateId()));
+			rootSession.setState(flow.getStateInstance(rootSession.getStateId()));
-			if (impl.hasSubflowSessions()) {
+			if (execution.hasSubflowSessions()) {
-				for (ListIterator it = impl.getSubflowSessionIterator(); it.hasNext();) {
+				for (ListIterator it = execution.getSubflowSessionIterator(); it.hasNext();) {
-					FlowSessionImpl subflow = (FlowSessionImpl) it.next();
+					FlowSessionImpl subflowSession = (FlowSessionImpl) it.next();
-					// TODO subflows encapsulated by top-level flow
+					Flow definition = (Flow) definitionLocator.getFlowDefinition(subflowSession.getFlowId());
-					Flow definition = (Flow) definitionLocator.getFlowDefinition(subflow.getFlowId());
+					subflowSession.setFlow(definition);
-					subflow.setFlow(definition);
+					subflowSession.setState(definition.getStateInstance(subflowSession.getStateId()));
-					subflow.setState(definition.getStateInstance(subflow.getStateId()));
 				}
 			}
 		}
-		impl.setKey(key);
+		execution.setKey(key);
 		if (conversationScope == null) {
 			conversationScope = new LocalAttributeMap();
 		}
-		impl.setConversationScope(conversationScope);
+		execution.setConversationScope(conversationScope);
-		configureServices(impl);
+		configureServices(execution);
-		impl.setKeyFactory(keyFactory);
+		execution.setKeyFactory(keyFactory);
-		return impl;
+		return execution;
 	}
 }

spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowSessionImpl.java

@@ -88,7 +88,8 @@ class FlowSessionImpl implements FlowSession, Externalizable {
 	 * @param parent this session's parent (may be null)
 	 */
 	public FlowSessionImpl(Flow flow, FlowSessionImpl parent) {
-		setFlow(flow);
+		Assert.notNull(flow, "The flow is required");
+		this.flow = flow;
 		this.parent = parent;
 	}
 
@@ -126,6 +127,18 @@ class FlowSessionImpl implements FlowSession, Externalizable {
 		return parent == null;
 	}
 
+	// public impl
+
+	public void setCurrentState(State state) {
+		if (this.state != null && this.state.isViewState()) {
+			destroyViewScope();
+		}
+		this.state = state;
+		if (this.state.isViewState()) {
+			initViewScope();
+		}
+	}
+
 	// custom serialization
 
 	public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
@@ -136,8 +149,8 @@ class FlowSessionImpl implements FlowSession, Externalizable {
 	}
 
 	public void writeExternal(ObjectOutput out) throws IOException {
-		out.writeObject(flowId);
+		out.writeObject(flow.getId());
-		out.writeObject(stateId);
+		out.writeObject(state.getId());
 		out.writeObject(scope);
 		out.writeObject(parent);
 	}
@@ -148,8 +161,7 @@ class FlowSessionImpl implements FlowSession, Externalizable {
 		return flow;
 	}
 
-	// package private setters for setting/updating internal state
+	// package private setters used by FlowExecutionImplStateRestorer for setting/updating internal state
-	// used by FlowExecutionImplStateRestorer
 
 	/**
 	 * Restores the definition of this flow session.
@@ -159,7 +171,6 @@ class FlowSessionImpl implements FlowSession, Externalizable {
 	void setFlow(Flow flow) {
 		Assert.notNull(flow, "The flow is required");
 		this.flow = flow;
-		this.flowId = flow.getId();
 	}
 
 	/**
@@ -173,39 +184,44 @@ class FlowSessionImpl implements FlowSession, Externalizable {
 		Assert.isTrue(flow == state.getOwner(),
 				"The state does not belong to the flow associated with this flow session");
 		this.state = state;
-		this.stateId = state.getId();
 	}
 
 	/**
-	 * Returns the id of the flow of this session.
+	 * Returns the de-serialized id indicating the flow id of this session.
 	 */
 	String getFlowId() {
 		return flowId;
 	}
 
 	/**
-	 * Returns the id of the current state of this session.
+	 * Returns the de-serialized id indicating the current state of this session.
 	 */
 	String getStateId() {
 		return stateId;
 	}
 
+	// internal helpers
+
 	/**
 	 * Initialize the view scope data structure.
 	 */
-	void initViewScope() {
+	private void initViewScope() {
 		scope.put(FLOW_VIEW_MAP_ATTRIBUTE, new LocalAttributeMap());
 	}
 
 	/**
 	 * Destroy the view scope data structure.
 	 */
-	void destroyViewScope() {
+	private void destroyViewScope() {
 		scope.remove(FLOW_VIEW_MAP_ATTRIBUTE);
 	}
 
 	public String toString() {
-		return new ToStringCreator(this).append("flow", flowId).append("state", stateId).append("scope", scope)
+		if (flow != null) {
-				.toString();
+			return new ToStringCreator(this).append("flow", flow.getId()).append("state",
+					state != null ? state.getId() : null).append("scope", scope).toString();
+		} else {
+			return "[Unhydrated session '" + flowId + "' in state '" + stateId + "']";
+		}
 	}
 }