From 6e10433b1b3b881e70108ed7985ddb32cf3b2038 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Sun, 22 Mar 2015 18:25:39 +0000 Subject: [PATCH] Fixing various aspects of state handling - Factory had a problem to build machines with deep nested states. - Fixing listener handling for substates, entry and exit events. --- .../config/EnumStateMachineFactory.java | 14 +++- .../statemachine/state/StateMachineState.java | 7 +- .../support/AbstractStateMachine.java | 84 +++++++++++++++++-- .../statemachine/SubStateMachineTests.java | 6 +- 4 files changed, 97 insertions(+), 14 deletions(-) diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/EnumStateMachineFactory.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/EnumStateMachineFactory.java index a7d4f9c4..dd252a93 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/EnumStateMachineFactory.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/EnumStateMachineFactory.java @@ -120,14 +120,23 @@ public class EnumStateMachineFactory, E extends Enum> exten continue; } - if (stateData != null && ObjectUtils.nullSafeEquals(peek.getParent(), stateData.getParent())) { + boolean stackContainsSameParent = false; + Iterator> ii = stateStack.iterator(); + while (ii.hasNext()) { + StateData sd = ii.next(); + if (stateData != null && ObjectUtils.nullSafeEquals(stateData.getState(), sd.getParent())) { + stackContainsSameParent = true; + break; + } + } + + if (stateData != null && !stackContainsSameParent) { stateStack.push(stateData); continue; } Collection> stateDatas = popSameParents(stateStack); Collection> transitionsData = getTransitionData(iterator.hasNext(), stateDatas); -// Collection> transitionsData = getTransitionData(false, null); machine = buildMachine(machineMap, stateMap, stateDatas, transitionsData, getBeanFactory()); // TODO: last part in if feels a bit hack @@ -263,7 +272,6 @@ public class EnumStateMachineFactory, E extends Enum> exten } else if (stateDatas.size() == 1) { initialState = state; } -// initialState = state; states.add(state); } else { PseudoState pseudoState = null; diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/StateMachineState.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/StateMachineState.java index 855e40da..7d273874 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/StateMachineState.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/StateMachineState.java @@ -22,6 +22,7 @@ import org.springframework.messaging.Message; import org.springframework.statemachine.StateContext; import org.springframework.statemachine.StateMachine; import org.springframework.statemachine.action.Action; +import org.springframework.statemachine.support.LifecycleObjectSupport; import org.springframework.statemachine.transition.Transition; import org.springframework.statemachine.transition.TransitionKind; @@ -144,7 +145,11 @@ public class StateMachineState extends AbstractState { } } if (getPseudoState() != null && getPseudoState().getKind() == PseudoStateKind.INITIAL) { - getSubmachine().start(); + if (((LifecycleObjectSupport)getSubmachine()).isRunning()) { + getSubmachine().getState().entry(event, context); + } else { + getSubmachine().start(); + } } else { getSubmachine().getState().entry(event, 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 71be7db1..e68bb2d2 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 @@ -206,6 +206,12 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport triggerToTransitionMap.put(trigger, transition); } } + for (State state : states) { + if (state.isSubmachineState()) { + StateMachine submachine = ((AbstractState)state).getSubmachine(); + submachine.addStateListener(new StateMachineListenerRelay()); + } + } } @Override @@ -278,8 +284,6 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport } private void switchToState(State state, Message event, Transition transition) { - exitFromState(currentState, event, transition); - notifyStateChanged(currentState, state); setCurrentState(state, event, transition); // TODO: should handle triggerles transition some how differently @@ -289,19 +293,18 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport if (t.getTrigger() == null && source.equals(currentState)) { switchToState(target, event, t); } - } } void setCurrentState(State state, Message event, Transition transition) { if (states.contains(state)) { + exitFromState(currentState, event, transition); + State notifyFrom = currentState; currentState = state; entryToState(state, event, transition); + notifyStateChanged(notifyFrom, state); } else if (currentState.isSubmachineState()) { - if (transition != null && transition.getKind() == TransitionKind.EXTERNAL) { - entryToState(currentState, event, transition); - } // TODO: should find a better way to trick setting state for submachine // without a need to access package protected method via casting StateMachine submachine = ((AbstractState)currentState).getSubmachine(); @@ -315,8 +318,23 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport MessageHeaders messageHeaders = event != null ? event.getHeaders() : new MessageHeaders( new HashMap()); StateContext stateContext = new DefaultStateContext(messageHeaders, extendedState, transition, this); + + // TODO: we use this trick not to double notify + State toNotify = null; + if (state.isSubmachineState()) { + StateMachine submachine = ((AbstractState)state).getSubmachine(); + if (((LifecycleObjectSupport)submachine).isRunning()) { + toNotify = submachine.getState(); + } + } + + if (toNotify != null) { + notifyStateExited(toNotify); + } + state.exit(event != null ? event.getPayload() : null, stateContext); notifyStateExited(state); + } } @@ -326,8 +344,21 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport MessageHeaders messageHeaders = event != null ? event.getHeaders() : new MessageHeaders( new HashMap()); StateContext stateContext = new DefaultStateContext(messageHeaders, extendedState, transition, this); - state.entry(event != null ? event.getPayload() : null, stateContext); notifyStateEntered(state); + + // TODO: we use this trick not to double notify + State toNotify = null; + if (state.isSubmachineState()) { + StateMachine submachine = ((AbstractState)state).getSubmachine(); + if (((LifecycleObjectSupport)submachine).isRunning()) { + toNotify = submachine.getState(); + } + } + + state.entry(event != null ? event.getPayload() : null, stateContext); + if (toNotify != null) { + notifyStateEntered(toNotify); + } } } @@ -595,4 +626,43 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport } } + /** + * This class is used to relay listener events from a submachines which works + * as its own listener context. User only connects to main root machine and + * expects to get events for all machines from there. + */ + private class StateMachineListenerRelay implements StateMachineListener { + + @Override + public void stateChanged(State from, State to) { + stateListener.stateChanged(from, to); + } + + @Override + public void stateEntered(State state) { + stateListener.stateEntered(state); + } + + @Override + public void stateExited(State state) { + stateListener.stateExited(state); + } + + @Override + public void transition(Transition transition) { + stateListener.transition(transition); + } + + @Override + public void transitionStarted(Transition transition) { + stateListener.transitionStarted(transition); + } + + @Override + public void transitionEnded(Transition transition) { + stateListener.transitionEnded(transition); + } + + } + } diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/SubStateMachineTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/SubStateMachineTests.java index 3811ce74..589e3107 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/SubStateMachineTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/SubStateMachineTests.java @@ -231,14 +231,14 @@ public class SubStateMachineTests extends AbstractStateMachineTests { assertThat(entryActionS112.onExecuteLatch.await(1, TimeUnit.SECONDS), is(true)); assertThat(exitActionS112.onExecuteLatch.await(1, TimeUnit.SECONDS), is(false)); assertThat(entryActionS1.onExecuteLatch.await(1, TimeUnit.SECONDS), is(true)); - assertThat(exitActionS1.onExecuteLatch.await(1, TimeUnit.SECONDS), is(true)); + assertThat(exitActionS1.onExecuteLatch.await(1, TimeUnit.SECONDS), is(false)); assertThat(entryActionS111.stateContexts.size(), is(1)); assertThat(exitActionS111.stateContexts.size(), is(1)); assertThat(entryActionS112.stateContexts.size(), is(1)); assertThat(exitActionS112.stateContexts.size(), is(0)); - assertThat(entryActionS1.stateContexts.size(), is(2)); - assertThat(exitActionS1.stateContexts.size(), is(1)); + assertThat(entryActionS1.stateContexts.size(), is(1)); + assertThat(exitActionS1.stateContexts.size(), is(0)); }