From 12137222a8dbf6436433226feadc1ea442bb274d Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Fri, 6 May 2016 17:24:54 +0100 Subject: [PATCH] Join state should not be entered - Fix join pseudostate to be transient. - Fix tests which uses wrong concepts. - Fixes #222 --- .../buildtests/ForkJoinEntryExitTests.java | 23 ++++++++++ .../config/AbstractStateMachineFactory.java | 14 +++++- .../statemachine/state/JoinPseudoState.java | 9 +++- .../statemachine/state/RegionState.java | 1 - .../support/AbstractStateMachine.java | 3 +- .../statemachine/StateMachineTests.java | 4 +- .../statemachine/state/JoinStateTests.java | 8 ++-- .../recipes/TasksHandlerTests.java | 44 +++++++++---------- .../src/test/java/demo/tasks/TasksTests.java | 12 ++--- 9 files changed, 79 insertions(+), 39 deletions(-) diff --git a/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/ForkJoinEntryExitTests.java b/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/ForkJoinEntryExitTests.java index 3c5d8591..8d6b71af 100644 --- a/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/ForkJoinEntryExitTests.java +++ b/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/ForkJoinEntryExitTests.java @@ -49,6 +49,29 @@ public class ForkJoinEntryExitTests extends AbstractBuildTests { plan.test(); } + @Test + @SuppressWarnings("unchecked") + public void testJoinExits() throws Exception { + context.register(Config1.class); + context.refresh(); + StateMachine stateMachine = context.getBean(StateMachine.class); + + StateMachineTestPlan plan = + StateMachineTestPlanBuilder.builder() + .stateMachine(stateMachine) + .step().expectState("S1").and() + .step() + .sendEvent("E1") + .expectStates("S2", "S210", "S220").and() + .step() + .sendEvent("E2") + .sendEvent("E3") + .expectStateEntered(3) + .expectStates("S3").and() + .build(); + plan.test(); + } + @Configuration @EnableStateMachine public static class Config1 extends StateMachineConfigurerAdapter { diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/AbstractStateMachineFactory.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/AbstractStateMachineFactory.java index 51b50e78..f55f19c3 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/AbstractStateMachineFactory.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/AbstractStateMachineFactory.java @@ -631,7 +631,19 @@ public abstract class AbstractStateMachineFactory extends LifecycleObjectS joins.add(stateMap.get(fs)); } } - JoinPseudoState pseudoState = new JoinPseudoState(joins); + S ss = null; + Collection> transitions = stateMachineTransitions.getTransitions(); + for (TransitionData tt : transitions) { + if (tt.getSource() == s) { + ss = tt.getTarget(); + break; + } + } + StateHolder holder = new StateHolder(stateMap.get(ss)); + if (holder.getState() == null) { + holderMap.put(ss, holder); + } + JoinPseudoState pseudoState = new JoinPseudoState(joins, holder); state = buildStateInternal(stateData.getState(), stateData.getDeferred(), stateData.getEntryActions(), stateData.getExitActions(), pseudoState); states.add(state); diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java index 79a0940d..929f6daf 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java @@ -21,6 +21,7 @@ import java.util.List; import org.springframework.statemachine.StateContext; import org.springframework.statemachine.listener.StateMachineListenerAdapter; import org.springframework.statemachine.state.PseudoStateContext.PseudoAction; +import org.springframework.util.Assert; /** * Join implementation of a {@link PseudoState}. @@ -34,22 +35,26 @@ public class JoinPseudoState extends AbstractPseudoState { private final List> joins; private volatile JoinTracker tracker; + private final StateHolder state; /** * Instantiates a new join pseudo state. * * @param joins the joins + * @param state the holder for target state */ - public JoinPseudoState(List> joins) { + public JoinPseudoState(List> joins, StateHolder state) { super(PseudoStateKind.JOIN); + Assert.notNull(state, "Holder must be set"); this.joins = joins; + this.state = state; } @Override public State entry(StateContext context) { tracker = new JoinTracker(this, new ArrayList>(joins)); context.getStateMachine().addStateListener(tracker); - return null; + return state.getState(); } @Override diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/RegionState.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/RegionState.java index f6438ea5..3681b3c7 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/RegionState.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/RegionState.java @@ -146,7 +146,6 @@ public class RegionState extends AbstractState { @Override public void entry(StateContext context) { - System.out.println("XXXXXX " + context); if (join != null) { join.entry(context); } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java index 7b6196eb..b34b4463 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java @@ -751,7 +751,7 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo private State followLinkedPseudoStates(State state, StateContext stateContext) { PseudoStateKind kind = state.getPseudoState() != null ? state.getPseudoState().getKind() : null; - if (kind == PseudoStateKind.INITIAL || kind == PseudoStateKind.JOIN || kind == PseudoStateKind.FORK) { + if (kind == PseudoStateKind.INITIAL || kind == PseudoStateKind.FORK) { return state; } else if (kind != null) { State toState = state.getPseudoState().entry(stateContext); @@ -776,6 +776,7 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo State toState = findStateWithPseudoState(pseudoState); StateContext stateContext = buildStateContext(Stage.STATE_EXIT, null, null, getRelayStateMachine()); pseudoState.exit(stateContext); + toState = followLinkedPseudoStates(toState, stateContext); switchToState(toState, null, null, getRelayStateMachine()); } }); diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateMachineTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateMachineTests.java index 404bcfba..b3454943 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateMachineTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateMachineTests.java @@ -146,10 +146,10 @@ public class StateMachineTests extends AbstractStateMachineTests { assertThat(listener.stateChangedCount, is(1)); assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S2, TestStates.S21, TestStates.S30)); - listener.reset(3); + listener.reset(2); machine.sendEvent(TestEvents.E3); assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(3)); + assertThat(listener.stateChangedCount, is(2)); assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S4)); } diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/state/JoinStateTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/state/JoinStateTests.java index 58ab86aa..5e787513 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/state/JoinStateTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/state/JoinStateTests.java @@ -70,10 +70,10 @@ public class JoinStateTests extends AbstractStateMachineTests { assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); assertThat(listener.stateChangedCount, is(1)); - listener.reset(3); + listener.reset(2); machine.sendEvent(TestEvents.E3); assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(3)); + assertThat(listener.stateChangedCount, is(2)); assertThat(machine.getState().getIds(), contains(TestStates.S4)); } @@ -144,10 +144,10 @@ public class JoinStateTests extends AbstractStateMachineTests { assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); assertThat(listener.stateChangedCount, is(1)); - listener.reset(3); + listener.reset(2); machine.sendEvent(TestEvents.E3); assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(3)); + assertThat(listener.stateChangedCount, is(2)); assertThat(machine.getState().getIds(), contains(TestStates.S4)); } diff --git a/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java b/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java index 7472a5cd..c0228682 100644 --- a/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java +++ b/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java @@ -47,7 +47,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(10, 0, 0); + listener.reset(9, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -56,7 +56,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(10)); + assertThat(listener.stateChangedCount, is(9)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(3)); @@ -71,7 +71,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(12, 0, 0); + listener.reset(11, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -80,7 +80,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(12)); + assertThat(listener.stateChangedCount, is(11)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_ERROR, TasksHandler.STATE_MANUAL)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(3)); @@ -95,7 +95,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(12, 0, 0); + listener.reset(11, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -104,7 +104,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(12)); + assertThat(listener.stateChangedCount, is(11)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_ERROR, TasksHandler.STATE_MANUAL)); listener.reset(0, 0, 0, 0, 1); @@ -139,11 +139,11 @@ public class TasksHandlerTests { assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); assertThat(listener.stateChangedCount, is(1)); - listener.reset(11, 0, 0); + listener.reset(10, 0, 0); handler.runTasks(); assertThat(listener.stateChangedLatch.await(4, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(11)); + assertThat(listener.stateChangedCount, is(10)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); } @@ -156,7 +156,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(10, 0, 0); + listener.reset(9, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -165,7 +165,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(12, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(10)); + assertThat(listener.stateChangedCount, is(9)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(3)); @@ -195,11 +195,11 @@ public class TasksHandlerTests { assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); assertThat(listener.stateChangedCount, is(1)); - listener.reset(21, 0, 0); + listener.reset(20, 0, 0); handler.runTasks(); assertThat(listener.stateChangedLatch.await(10, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(21)); + assertThat(listener.stateChangedCount, is(20)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(9)); @@ -216,7 +216,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(10, 0, 0); + listener.reset(9, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -226,7 +226,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(10)); + assertThat(listener.stateChangedCount, is(9)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); assertThat(tasksListener.onTasksStartedLatch.await(1, TimeUnit.SECONDS), is(true)); @@ -253,7 +253,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(12, 0, 0); + listener.reset(11, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -263,7 +263,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(12)); + assertThat(listener.stateChangedCount, is(11)); assertThat(tasksListener.onTasksStartedLatch.await(1, TimeUnit.SECONDS), is(true)); assertThat(tasksListener.onTasksStarted, is(1)); @@ -287,7 +287,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(12, 0, 0); + listener.reset(11, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -297,7 +297,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(12)); + assertThat(listener.stateChangedCount, is(11)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_ERROR, TasksHandler.STATE_MANUAL)); listener.reset(1, 0, 0); @@ -322,7 +322,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(10, 0, 0); + listener.reset(9, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -333,7 +333,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(10)); + assertThat(listener.stateChangedCount, is(9)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(3)); @@ -361,7 +361,7 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(12, 0, 0); + listener.reset(11, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); @@ -372,7 +372,7 @@ public class TasksHandlerTests { handler.runTasks(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(12)); + assertThat(listener.stateChangedCount, is(11)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_ERROR, TasksHandler.STATE_MANUAL)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(3)); diff --git a/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java b/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java index 15741697..5ddb22f6 100644 --- a/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java +++ b/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java @@ -61,7 +61,7 @@ public class TasksTests { @Test public void testRunOnce() throws InterruptedException { - listener.reset(9, 0, 0); + listener.reset(8, 0, 0); tasks.run(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); assertThat(machine.getState().getIds(), contains(States.READY)); @@ -71,7 +71,7 @@ public class TasksTests { @Test public void testRunTwice() throws InterruptedException { - listener.reset(9, 0, 0); + listener.reset(8, 0, 0); tasks.run(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); assertThat(machine.getState().getIds(), contains(States.READY)); @@ -79,7 +79,7 @@ public class TasksTests { Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(3)); - listener.reset(9, 0, 0); + listener.reset(8, 0, 0); tasks.run(); assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); assertThat(machine.getState().getIds(), contains(States.READY)); @@ -90,17 +90,17 @@ public class TasksTests { @Test public void testFailAutomaticFix() throws InterruptedException { - listener.reset(11, 0, 0); + listener.reset(10, 0, 0); tasks.fail("T1"); tasks.run(); assertThat(listener.stateChangedLatch.await(6, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(11)); + assertThat(listener.stateChangedCount, is(10)); assertThat(machine.getState().getIds(), contains(States.READY)); } @Test public void testFailManualFix() throws InterruptedException { - listener.reset(11, 0, 0); + listener.reset(10, 0, 0); tasks.fail("T2"); tasks.run(); assertThat(listener.stateChangedLatch.await(6, TimeUnit.SECONDS), is(true));