From c437e0c02508fa5d39bc4228a93484c6f3b35e06 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Tue, 14 Apr 2009 16:20:35 +0000 Subject: [PATCH] SWF-1098 --- build-spring-webflow/resources/changelog.txt | 3 +- .../impl/DefaultFlowExecutionRepository.java | 22 ++++-- .../impl/FlowExecutionSnapshotGroup.java | 10 ++- .../SimpleFlowExecutionSnapshotGroup.java | 19 ++++- .../AbstractFlowExecutionRepository.java | 76 +++++++++---------- 5 files changed, 79 insertions(+), 51 deletions(-) diff --git a/build-spring-webflow/resources/changelog.txt b/build-spring-webflow/resources/changelog.txt index e2e15329..72cd4ffb 100644 --- a/build-spring-webflow/resources/changelog.txt +++ b/build-spring-webflow/resources/changelog.txt @@ -2,7 +2,7 @@ SPRING WEB FLOW CHANGELOG ========================= http://www.springframework.org/webflow -Changes in version 2.0.7 (2009.04.09) +Changes in version 2.0.7 (2009.04.16) ------------------------------------- Bug Fixes * Fixed several issues with WebFlowMessageCodesResolver algorithm, including a JDK 1.4 compatibility issue (SWF-1064) @@ -15,6 +15,7 @@ Bug Fixes * Fixed bug where parent Model list could be shared by child in flow definition inheritance algorithm (SWF-1094). * Fixed bug where AjaxTilesView was not checking the AttributeType to determine potentially renderable Tiles Attributes (SWF-1092). * Made FlowExecutionSnapshotGroup public for serialization reasons +* Fixed bug where a flow execution snapshot id was not always incremented, which could lead to collisions between multiple windows sharing the same execution (SWF-1098). Improvements * Added org.springframework.webflow dm Server library definition for use in a dm Server deployment environment (SWF-1067) diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/DefaultFlowExecutionRepository.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/DefaultFlowExecutionRepository.java index 7bacdc78..752afd12 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/DefaultFlowExecutionRepository.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/DefaultFlowExecutionRepository.java @@ -15,6 +15,8 @@ */ package org.springframework.webflow.execution.repository.impl; +import java.io.Serializable; + import org.springframework.webflow.conversation.Conversation; import org.springframework.webflow.conversation.ConversationManager; import org.springframework.webflow.execution.FlowExecution; @@ -34,16 +36,16 @@ import org.springframework.webflow.execution.repository.snapshot.SnapshotNotFoun *

* This repository is responsible for: *

*

- * This repository implementation also provides support for execution invalidation after completion, where once - * a logical flow execution completes, it and all of its snapshots are removed. This cleans up memory and prevents the + * This repository implementation also provides support for execution invalidation after completion, where once a + * logical flow execution completes, it and all of its snapshots are removed. This cleans up memory and prevents the * possibility of duplicate submission after completion. * * @author Keith Donald @@ -88,6 +90,12 @@ public class DefaultFlowExecutionRepository extends AbstractSnapshottingFlowExec this.maxSnapshots = maxSnapshots; } + // supporting flow execution key factory impl + + protected Serializable nextSnapshotId(Serializable executionId) { + return getSnapshotGroup(getConversation(executionId)).nextSnapshotId(); + } + // implementing flow execution repository public FlowExecution getFlowExecution(FlowExecutionKey key) { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/FlowExecutionSnapshotGroup.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/FlowExecutionSnapshotGroup.java index 156aa978..c52f845b 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/FlowExecutionSnapshotGroup.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/FlowExecutionSnapshotGroup.java @@ -14,8 +14,8 @@ import org.springframework.webflow.execution.repository.snapshot.SnapshotNotFoun public interface FlowExecutionSnapshotGroup { /** - * Returns the snapshot with the provided id, or null if no such snapshot exists with - * that id. + * Returns the snapshot with the provided id, or null if no such snapshot exists with that + * id. * @param snapshotId the snapshot id * @return the continuation * @throws SnapshotNotFoundException if the id does not match a continuation in this group @@ -54,4 +54,10 @@ public interface FlowExecutionSnapshotGroup { */ public int getSnapshotCount(); + /** + * Gets the next snapshot id for new snapshot to add to this group. + * @return the next snapshot id + */ + public Serializable nextSnapshotId(); + } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/SimpleFlowExecutionSnapshotGroup.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/SimpleFlowExecutionSnapshotGroup.java index 8a3f39f1..0fb62506 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/SimpleFlowExecutionSnapshotGroup.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/impl/SimpleFlowExecutionSnapshotGroup.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.LinkedList; import java.util.Map; +import org.springframework.core.JdkVersion; import org.springframework.webflow.execution.repository.snapshot.FlowExecutionSnapshot; import org.springframework.webflow.execution.repository.snapshot.SnapshotNotFoundException; @@ -43,10 +44,15 @@ class SimpleFlowExecutionSnapshotGroup implements FlowExecutionSnapshotGroup, Se private LinkedList snapshotIds = new LinkedList(); /** - * The maximum number of snapshots allowed in this group. + * The maximum number of snapshots allowed in this group. -1 indicates no max limit. */ private int maxSnapshots = -1; + /** + * The snapshot id sequence ensuring unique snapshot ids within this group; snapshot ids start at 1. + */ + private int snapshotIdSequence = 1; + /** * Returns the maximum number of snapshots allowed in this group. */ @@ -103,6 +109,17 @@ class SimpleFlowExecutionSnapshotGroup implements FlowExecutionSnapshotGroup, Se return snapshotIds.size(); } + public Serializable nextSnapshotId() { + Integer nextSnapshotId; + if (JdkVersion.isAtLeastJava15()) { + nextSnapshotId = Integer.valueOf(snapshotIdSequence); + } else { + nextSnapshotId = new Integer(snapshotIdSequence); + } + snapshotIdSequence++; + return nextSnapshotId; + } + /** * Has the maximum number of snapshots in this group been exceeded? */ diff --git a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractFlowExecutionRepository.java b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractFlowExecutionRepository.java index 3c523bac..1c088835 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractFlowExecutionRepository.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/execution/repository/support/AbstractFlowExecutionRepository.java @@ -19,7 +19,6 @@ import java.io.Serializable; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.core.JdkVersion; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.webflow.conversation.Conversation; @@ -57,8 +56,6 @@ public abstract class AbstractFlowExecutionRepository implements FlowExecutionRe */ protected final Log logger = LogFactory.getLog(getClass()); - private static final Integer ONE = new Integer(1); - private ConversationManager conversationManager; private boolean alwaysGenerateNewNextKey = true; @@ -97,11 +94,17 @@ public abstract class AbstractFlowExecutionRepository implements FlowExecutionRe // implementing flow execution key factory public FlowExecutionKey getKey(FlowExecution execution) { - if (execution.getKey() == null) { + CompositeFlowExecutionKey key = (CompositeFlowExecutionKey) execution.getKey(); + if (key == null) { Conversation conversation = beginConversation(execution); - return new CompositeFlowExecutionKey(conversation.getId(), ONE); + ConversationId executionId = conversation.getId(); + return new CompositeFlowExecutionKey(executionId, nextSnapshotId(executionId)); } else { - return getNextKey(execution); + if (alwaysGenerateNewNextKey) { + return new CompositeFlowExecutionKey(key.getExecutionId(), nextSnapshotId(key.getExecutionId())); + } else { + return execution.getKey(); + } } } @@ -122,12 +125,6 @@ public abstract class AbstractFlowExecutionRepository implements FlowExecutionRe return new ConversationBackedFlowExecutionLock(getConversation(key)); } - // abstract repository methods to be overridden by subclasses - - public abstract FlowExecution getFlowExecution(FlowExecutionKey key) throws FlowExecutionRepositoryException; - - public abstract void putFlowExecution(FlowExecution flowExecution) throws FlowExecutionRepositoryException; - public void removeFlowExecution(FlowExecution flowExecution) throws FlowExecutionRepositoryException { assertKeySet(flowExecution); if (logger.isDebugEnabled()) { @@ -136,11 +133,24 @@ public abstract class AbstractFlowExecutionRepository implements FlowExecutionRe endConversation(flowExecution); } + // abstract repository methods to be overridden by subclasses + + /** + * The next snapshot id to use for a {@link FlowExecution} instance. Called when {@link #getKey(FlowExecution) + * getting a flow execution key}. + * @return the id of the flow execution + */ + protected abstract Serializable nextSnapshotId(Serializable executionId); + + public abstract FlowExecution getFlowExecution(FlowExecutionKey key) throws FlowExecutionRepositoryException; + + public abstract void putFlowExecution(FlowExecution flowExecution) throws FlowExecutionRepositoryException; + // hooks for use in subclasses /** - * Factory method that maps a new flow execution to a descriptive - * {@link ConversationParameters conversation parameters} object. + * Factory method that maps a new flow execution to a descriptive {@link ConversationParameters conversation + * parameters} object. * @param flowExecution the new flow execution * @return the conversation parameters object to pass to the conversation manager when the conversation is started */ @@ -150,35 +160,29 @@ public abstract class AbstractFlowExecutionRepository implements FlowExecutionRe } /** - * Gets the next key to assign to the flow execution. - * @param execution - * @return the next flow execution - */ - protected FlowExecutionKey getNextKey(FlowExecution execution) { - if (alwaysGenerateNewNextKey) { - CompositeFlowExecutionKey currentKey = (CompositeFlowExecutionKey) execution.getKey(); - Integer currentSnapshotId = (Integer) currentKey.getSnapshotId(); - return new CompositeFlowExecutionKey(currentKey.getExecutionId(), nextSnapshotId(currentSnapshotId)); - } else { - return execution.getKey(); - } - } - - /** - * Returns the conversation governing the execution of the {@link FlowExecution} with the provided key. + * Returns the conversation governing the {@link FlowExecution} with the provided key. * @param key the flow execution key * @return the governing conversation * @throws NoSuchFlowExecutionException when the conversation for identified flow execution cannot be found */ protected Conversation getConversation(FlowExecutionKey key) throws NoSuchFlowExecutionException { try { - ConversationId conversationId = (ConversationId) ((CompositeFlowExecutionKey) key).getExecutionId(); - return conversationManager.getConversation(conversationId); + return getConversation(((CompositeFlowExecutionKey) key).getExecutionId()); } catch (NoSuchConversationException e) { throw new NoSuchFlowExecutionException(key, e); } } + /** + * Returns the conversation governing the logical flow execution with the given execution id. + * @param executionId the flow execution id + * @return the governing conversation + * @throws NoSuchConversationException when the conversation for identified flow execution cannot be found + */ + protected Conversation getConversation(Serializable executionId) throws NoSuchConversationException { + return conversationManager.getConversation((ConversationId) executionId); + } + /** * Assert that a flow execution key has been assigned to the execution. * @param execution the flow execution @@ -200,14 +204,6 @@ public abstract class AbstractFlowExecutionRepository implements FlowExecutionRe return conversation; } - private Integer nextSnapshotId(Integer currentSnapshotId) { - if (JdkVersion.isAtLeastJava15()) { - return Integer.valueOf(currentSnapshotId.intValue() + 1); - } else { - return new Integer(currentSnapshotId.intValue() + 1); - } - } - private ConversationId parseExecutionId(String encodedId, String encodedKey) throws BadlyFormattedFlowExecutionKeyException { try {