From 9235345a4212f816d4da86bc87e33988cd72d783 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Sun, 15 Mar 2015 11:15:37 +0000 Subject: [PATCH] Fix state handling with substates - Fixes #23 - Transition with submachine now exists and enters super state if transition is external. --- .../config/EnumStateMachineFactory.java | 1 + .../statemachine/state/AbstractState.java | 14 +++- .../statemachine/state/StateMachineState.java | 7 +- .../support/AbstractStateMachine.java | 19 ++++- .../AbstractStateMachineTests.java | 9 +++ .../statemachine/SubStateMachineTests.java | 69 +++++++++++++++++-- 6 files changed, 109 insertions(+), 10 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 fd11efa3..a7d4f9c4 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 @@ -127,6 +127,7 @@ public class EnumStateMachineFactory, E extends Enum> exten 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 diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/AbstractState.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/AbstractState.java index b12c9f96..b39d07fd 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/AbstractState.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/AbstractState.java @@ -204,11 +204,21 @@ public abstract class AbstractState implements State { return submachine != null; } - protected StateMachine getSubmachine() { + /** + * Gets the submachine. + * + * @return the submachine or null if not set + */ + public StateMachine getSubmachine() { return submachine; } - protected Collection> getRegions() { + /** + * Gets the regions. + * + * @return the regions or empty collection if no regions + */ + public Collection> getRegions() { return regions; } 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 70ad8340..855e40da 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 @@ -121,7 +121,12 @@ public class StateMachineState extends AbstractState { @Override public void exit(E event, StateContext context) { getSubmachine().getState().exit(event, context); - getSubmachine().stop(); + // don't stop if it looks like we're coming back + // stop would cause start with entry which would + // enable default transition and state + if (context.getTransition().getSource().getId() != getSubmachine().getState().getId()) { + getSubmachine().stop(); + } Collection> actions = getExitActions(); if (actions != null && !isLocal(context)) { for (Action action : actions) { 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 2aabe401..ab0fbf4f 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 @@ -46,6 +46,7 @@ import org.springframework.statemachine.listener.StateMachineListener; import org.springframework.statemachine.processor.StateMachineHandler; import org.springframework.statemachine.processor.StateMachineOnTransitionHandler; import org.springframework.statemachine.processor.StateMachineRuntime; +import org.springframework.statemachine.state.AbstractState; import org.springframework.statemachine.state.PseudoStateKind; import org.springframework.statemachine.state.State; import org.springframework.statemachine.transition.Transition; @@ -279,8 +280,7 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport callHandlers(currentState, state, event); - currentState = state; - entryToState(state, event, transition); + setCurrentState(state, event, transition); // TODO: should handle triggerles transition some how differently for (Transition t : transitions) { @@ -294,6 +294,21 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport } + void setCurrentState(State state, Message event, Transition transition) { + if (states.contains(state)) { + currentState = state; + entryToState(state, event, transition); + } 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(); + ((AbstractStateMachine)submachine).setCurrentState(state, event, transition); + } + } + private void exitFromState(State state, Message event, Transition transition) { if (state != null) { log.trace("Exit state=[" + state + "]"); diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/AbstractStateMachineTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/AbstractStateMachineTests.java index 4534ce68..1aa303e0 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/AbstractStateMachineTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/AbstractStateMachineTests.java @@ -73,6 +73,15 @@ public abstract class AbstractStateMachineTests { E1,E2,E3,E4,EF } + public static enum TestStates2 { + BUSY, PLAYING, PAUSED, + IDLE, CLOSED, OPEN + } + + public static enum TestEvents2 { + PLAY, STOP, PAUSE, EJECT, LOAD + } + @Configuration public static class BaseConfig { 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 f94f881f..3811ce74 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 @@ -152,7 +152,7 @@ public class SubStateMachineTests extends AbstractStateMachineTests { assertThat(exitActionS1.stateContexts.size(), is(1)); } -// @Test + @Test public void testExternalTransition2() throws Exception { /** @@ -190,7 +190,7 @@ public class SubStateMachineTests extends AbstractStateMachineTests { Collection> entryActionsS112 = new ArrayList>(); entryActionsS112.add(entryActionS112); Collection> exitActionsS112 = new ArrayList>(); - exitActionsS111.add(exitActionS112); + exitActionsS112.add(exitActionS112); State stateS112 = new EnumState(TestStates.S112, null, entryActionsS112, exitActionsS112, null); // submachine 1 @@ -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(false)); + assertThat(exitActionS1.onExecuteLatch.await(1, TimeUnit.SECONDS), is(true)); 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(1)); - assertThat(exitActionS1.stateContexts.size(), is(0)); + assertThat(entryActionS1.stateContexts.size(), is(2)); + assertThat(exitActionS1.stateContexts.size(), is(1)); } @@ -389,6 +389,21 @@ public class SubStateMachineTests extends AbstractStateMachineTests { assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S10)); } + @Test + public void testStateChangeWithinMachine() { + context.register(BaseConfig.class, StateMachineEventPublisherConfiguration.class, Config3.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + EnumStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, EnumStateMachine.class); + machine.start(); + assertThat(machine, notNullValue()); + assertThat(machine.getState().getIds(), contains(TestStates2.IDLE, TestStates2.CLOSED)); + machine.sendEvent(TestEvents2.EJECT); + assertThat(machine.getState().getIds(), contains(TestStates2.IDLE, TestStates2.OPEN)); + } + @Configuration @EnableStateMachine public static class Config1 extends EnumStateMachineConfigurerAdapter { @@ -473,4 +488,48 @@ public class SubStateMachineTests extends AbstractStateMachineTests { } + @Configuration + @EnableStateMachine + static class Config3 extends EnumStateMachineConfigurerAdapter { + + @Override + public void configure(StateMachineStateConfigurer states) throws Exception { + states + .withStates() + .initial(TestStates2.IDLE) + .state(TestStates2.IDLE) + .and() + .withStates() + .parent(TestStates2.IDLE) + .initial(TestStates2.CLOSED) + .state(TestStates2.CLOSED) + .state(TestStates2.OPEN) + .and() + .withStates() + .state(TestStates2.BUSY) + .and() + .withStates() + .parent(TestStates2.BUSY) + .initial(TestStates2.PLAYING) + .state(TestStates2.PLAYING) + .state(TestStates2.PAUSED); + + } + + @Override + public void configure(StateMachineTransitionConfigurer transitions) throws Exception { + transitions + .withExternal() + .source(TestStates2.CLOSED) + .target(TestStates2.OPEN) + .event(TestEvents2.EJECT) + .and() + .withExternal() + .source(TestStates2.OPEN) + .target(TestStates2.CLOSED) + .event(TestEvents2.EJECT); + } + + } + }