From 911901cfb627ef118bf7419711f6d5b5bfccf892 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Sat, 18 Apr 2015 19:39:46 +0100 Subject: [PATCH] Fix base support for region config - Fixes #46 - Add some region/group info in a state configurer so that we know which states are in a same reqion. Just a simple uuid for now. - Send machine started event at end of start and properly register listener with regions. - Fix factory not to mess things up with submachines so correctly track if we have multiple initial states. --- .../statemachine/StateMachine.java | 8 -- .../config/EnumStateMachineFactory.java | 135 ++++++++++-------- .../statemachine/config/StateData.java | 17 ++- .../configurers/DefaultStateConfigurer.java | 13 +- .../statemachine/region/Region.java | 9 ++ .../statemachine/state/RegionState.java | 4 +- .../support/AbstractStateMachine.java | 8 +- .../statemachine/RegionMachineTests.java | 130 ++++++++++++++++- 8 files changed, 245 insertions(+), 79 deletions(-) diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachine.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachine.java index b0b0a6e0..f45f7b2c 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachine.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachine.java @@ -15,7 +15,6 @@ */ package org.springframework.statemachine; -import org.springframework.statemachine.listener.StateMachineListener; import org.springframework.statemachine.region.Region; import org.springframework.statemachine.state.State; @@ -44,11 +43,4 @@ public interface StateMachine extends Region { */ ExtendedState getExtendedState(); - /** - * Adds the state listener. - * - * @param listener the listener - */ - void addStateListener(StateMachineListener listener); - } 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 2202d7fd..93287ff7 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 @@ -94,30 +94,11 @@ public class EnumStateMachineFactory, E extends Enum> exten // find a correct mappings because they use state id's, not actual // states. final Map> stateMap = new HashMap>(); - - Tree> tree = new Tree>(); - - for (StateData stateData : stateMachineStates.getStateDatas()) { - Object id = stateData.getState(); - Object parent = stateData.getParent(); - tree.add(stateData, id, parent); - } - - TreeTraverser>> traverser = new TreeTraverser>>() { - @Override - public Iterable>> children(Node> root) { - return root.getChildren(); - } - }; - Stack> regionStack = new Stack>(); Stack> stateStack = new Stack>(); - - Iterable>> postOrderTraversal = traverser.postOrderTraversal(tree.getRoot()); - Iterator>> iterator = postOrderTraversal.iterator(); - Map> machineMap = new HashMap>(); + Iterator>> iterator = buildStateDataIterator(); while (iterator.hasNext()) { Node> node = iterator.next(); StateData stateData = node.getData(); @@ -145,58 +126,48 @@ public class EnumStateMachineFactory, E extends Enum> exten } Collection> stateDatas = popSameParents(stateStack); + int initialCount = getInitialCount(stateDatas); + Collection>> regionsStateDatas = splitIntoRegions(stateDatas); Collection> transitionsData = getTransitionData(iterator.hasNext(), stateDatas); - machine = buildMachine(machineMap, stateMap, stateDatas, transitionsData, getBeanFactory(), contextEvents, defaultExtendedState); - // TODO: last part in if feels a bit hack - // (!peek.isInitial() && !machineMap.containsKey(peek.getParent())) - if (peek.isInitial() || (!peek.isInitial() && !machineMap.containsKey(peek.getParent()))) { - machineMap.put(peek.getParent(), machine); - } - if (peek.getParent() == null) { - regionStack.push(new MachineStackItem(machine, peek.getParent(), peek)); - } - stateStack.push(stateData); - } - - // TODO: usage of initials is a temporary fix to workaround for missing - // full support of regions - int initials = 0; - if (regionStack.size() > 1) { - Collection> transitionsData = resolveTransitionData2(stateMachineTransitions.getTransitions()); - Collection> stateDatas = new ArrayList>(); - Iterator> i = regionStack.iterator(); - while (i.hasNext()) { - MachineStackItem next = i.next(); - stateDatas.add(next.stateData); - if (next.stateData.isInitial()) { - initials++; + if (initialCount > 1) { + for (Collection> regionStateDatas : regionsStateDatas) { + machine = buildMachine(machineMap, stateMap, regionStateDatas, transitionsData, getBeanFactory(), + contextEvents, defaultExtendedState); + if (peek.isInitial() || (!peek.isInitial() && !machineMap.containsKey(peek.getParent()))) { + machineMap.put(peek.getParent(), machine); + } + regionStack.push(new MachineStackItem(machine, peek.getParent(), peek)); } - } - if (initials == 1) { - machine = buildMachine(machineMap, stateMap, stateDatas, transitionsData, getBeanFactory(), contextEvents, defaultExtendedState); - } - } - - if (initials > 1) { - Collection> regions = new ArrayList>(); - for (MachineStackItem si : regionStack) { - if (si.parent == null) { + Collection> regions = new ArrayList>(); + for (MachineStackItem si : regionStack) { regions.add(si.machine); } - } - if (regions.size() > 1) { - RegionState rstate = new RegionState(null, regions); + RegionState rstate = new RegionState(null, regions, null, null, null, new DefaultPseudoState(PseudoStateKind.INITIAL)); Collection> states = new ArrayList>(); states.add(rstate); - EnumStateMachine m = new EnumStateMachine(states, null, rstate, + EnumStateMachine m = new EnumStateMachine(states, new ArrayList>(), rstate, null, null, defaultExtendedState); if (contextEvents != null) { m.setContextEventsEnabled(contextEvents); } + if (getBeanFactory() != null) { + m.setBeanFactory(getBeanFactory()); + } + m.afterPropertiesSet(); + machine = m; + + + } else { + machine = buildMachine(machineMap, stateMap, stateDatas, transitionsData, getBeanFactory(), contextEvents, defaultExtendedState); + if (peek.isInitial() || (!peek.isInitial() && !machineMap.containsKey(peek.getParent()))) { + machineMap.put(peek.getParent(), machine); + } } + + stateStack.push(stateData); } return machine; @@ -206,6 +177,29 @@ public class EnumStateMachineFactory, E extends Enum> exten this.contextEvents = contextEvents; } + private int getInitialCount(Collection> stateDatas) { + int count = 0; + for (StateData stateData : stateDatas) { + if (stateData.isInitial()) { + count++; + } + } + return count; + } + + private Collection>> splitIntoRegions(Collection> stateDatas) { + Map>> map = new HashMap>>(); + for (StateData stateData : stateDatas) { + Collection> c = map.get(stateData.getRegion()); + if (c == null) { + c = new ArrayList>(); + } + c.add(stateData); + map.put(stateData.getRegion(), c); + } + return map.values(); + } + private Collection> getTransitionData(boolean roots, Collection> stateDatas) { if (roots) { return resolveTransitionData(stateMachineTransitions.getTransitions(), stateDatas); @@ -335,6 +329,10 @@ public class EnumStateMachineFactory, E extends Enum> exten } if (transitionData.getKind() == TransitionKind.EXTERNAL) { + // TODO can we do this? + if (stateMap.get(source) == null && stateMap.get(target) == null) { + continue; + } DefaultExternalTransition transition = new DefaultExternalTransition(stateMap.get(source), stateMap.get(target), transitionData.getActions(), event, transitionData.getGuard(), trigger); transitions.add(transition); @@ -364,4 +362,25 @@ public class EnumStateMachineFactory, E extends Enum> exten return machine; } + private Iterator>> buildStateDataIterator() { + Tree> tree = new Tree>(); + + for (StateData stateData : stateMachineStates.getStateDatas()) { + Object id = stateData.getState(); + Object parent = stateData.getParent(); + tree.add(stateData, id, parent); + } + + TreeTraverser>> traverser = new TreeTraverser>>() { + @Override + public Iterable>> children(Node> root) { + return root.getChildren(); + } + }; + + Iterable>> postOrderTraversal = traverser.postOrderTraversal(tree.getRoot()); + Iterator>> iterator = postOrderTraversal.iterator(); + return iterator; + } + } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateData.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateData.java index 7abd3c69..20a45214 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateData.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateData.java @@ -33,6 +33,7 @@ import org.springframework.statemachine.state.State; public class StateData { private Object parent; + private Object region; private S state; private Collection deferred; private Collection> entryActions; @@ -41,13 +42,14 @@ public class StateData { private Action initialAction; private boolean end = false; - public StateData(Object parent, S state, Collection deferred, + public StateData(Object parent, Object region, S state, Collection deferred, Collection> entryActions, Collection> exitActions) { this.state = state; this.deferred = deferred; this.entryActions = entryActions; this.exitActions = exitActions; this.parent = parent; + this.region = region; } public S getState() { @@ -74,6 +76,14 @@ public class StateData { this.parent = parent; } + public Object getRegion() { + return region; + } + + public void setRegion(Object region) { + this.region = region; + } + public boolean isInitial() { return initial; } @@ -100,8 +110,9 @@ public class StateData { @Override public String toString() { - return "StateData [parent=" + parent + ", state=" + state + ", deferred=" + deferred + ", entryActions=" - + entryActions + ", exitActions=" + exitActions + ", initial=" + initial + ", end=" + end + "]"; + return "StateData [parent=" + parent + ", region=" + region + ", state=" + state + ", deferred=" + deferred + + ", entryActions=" + entryActions + ", exitActions=" + exitActions + ", initial=" + initial + ", end=" + + end + "]"; } } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configurers/DefaultStateConfigurer.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configurers/DefaultStateConfigurer.java index 6cb5ad19..047685b7 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configurers/DefaultStateConfigurer.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configurers/DefaultStateConfigurer.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Set; +import java.util.UUID; import org.springframework.statemachine.action.Action; import org.springframework.statemachine.config.StateData; @@ -27,12 +28,22 @@ import org.springframework.statemachine.config.builders.StateMachineStateConfigu import org.springframework.statemachine.config.builders.StateMachineStates; import org.springframework.statemachine.config.common.annotation.AnnotationConfigurerAdapter; +/** + * Default implementation of a {@link StateConfigurer}. + * + * @author Janne Valkealahti + * + * @param the type of state + * @param the type of event + */ public class DefaultStateConfigurer extends AnnotationConfigurerAdapter, StateMachineStateConfigurer, StateMachineStateBuilder> implements StateConfigurer { private Object parent; + private final Object region = UUID.randomUUID().toString(); + private final Collection> incomplete = new ArrayList>(); private S initialState; @@ -131,7 +142,7 @@ public class DefaultStateConfigurer private void addIncomplete(Object parent, S state, Collection deferred, Collection> entryActions, Collection> exitActions) { - incomplete.add(new StateData(parent, state, deferred, entryActions, exitActions)); + incomplete.add(new StateData(parent, region, state, deferred, entryActions, exitActions)); } } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/region/Region.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/region/Region.java index 7919580b..5e5e1218 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/region/Region.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/region/Region.java @@ -18,6 +18,7 @@ package org.springframework.statemachine.region; import java.util.Collection; import org.springframework.messaging.Message; +import org.springframework.statemachine.listener.StateMachineListener; import org.springframework.statemachine.state.State; import org.springframework.statemachine.transition.Transition; @@ -87,4 +88,12 @@ public interface Region { * @return true, if complete */ boolean isComplete(); + + /** + * Adds the state listener. + * + * @param listener the listener + */ + void addStateListener(StateMachineListener listener); + } 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 0113f668..02b26edd 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 @@ -134,7 +134,9 @@ public class RegionState extends AbstractState { } } else { for (Region region : getRegions()) { - region.getState().entry(event, context); + if (region.getState() != null) { + region.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 4eebe2fd..d3f51632 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 @@ -49,6 +49,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.region.Region; import org.springframework.statemachine.state.AbstractState; import org.springframework.statemachine.state.PseudoStateKind; import org.springframework.statemachine.state.State; @@ -198,6 +199,11 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport if (state.isSubmachineState()) { StateMachine submachine = ((AbstractState)state).getSubmachine(); submachine.addStateListener(new StateMachineListenerRelay()); + } else if (state.isOrthogonal()) { + Collection> regions = ((AbstractState)state).getRegions(); + for (Region region : regions) { + region.addStateListener(new StateMachineListenerRelay()); + } } } } @@ -205,7 +211,6 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport @Override protected void doStart() { super.doStart(); - notifyStateMachineStarted(this); registerTriggerListener(); switchToState(initialState, initialEvent, null, this); // TODO: for now execute outside of switchToState @@ -214,6 +219,7 @@ public abstract class AbstractStateMachine extends LifecycleObjectSupport initialEvent != null ? initialEvent.getHeaders() : null, extendedState, initialTransition, this); initialTransition.transit(stateContext); } + notifyStateMachineStarted(this); } @Override diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/RegionMachineTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/RegionMachineTests.java index 0cd1bb88..0772c63b 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/RegionMachineTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/RegionMachineTests.java @@ -17,19 +17,28 @@ package org.springframework.statemachine; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import java.util.ArrayList; import java.util.Collection; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import org.junit.Test; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; import org.springframework.core.task.SyncTaskExecutor; -import org.springframework.statemachine.AbstractStateMachineTests.TestEntryAction; -import org.springframework.statemachine.AbstractStateMachineTests.TestEvents; -import org.springframework.statemachine.AbstractStateMachineTests.TestExitAction; -import org.springframework.statemachine.AbstractStateMachineTests.TestStates; import org.springframework.statemachine.action.Action; +import org.springframework.statemachine.config.EnableStateMachine; +import org.springframework.statemachine.config.EnumStateMachineConfigurerAdapter; +import org.springframework.statemachine.config.builders.StateMachineStateConfigurer; +import org.springframework.statemachine.config.builders.StateMachineTransitionConfigurer; +import org.springframework.statemachine.event.StateMachineEventPublisherConfiguration; +import org.springframework.statemachine.listener.StateMachineListenerAdapter; import org.springframework.statemachine.region.Region; import org.springframework.statemachine.state.DefaultPseudoState; import org.springframework.statemachine.state.EnumState; @@ -47,10 +56,15 @@ import org.springframework.statemachine.trigger.EventTrigger; * @author Janne Valkealahti * */ -public class RegionMachineTests { +public class RegionMachineTests extends AbstractStateMachineTests { + + @Override + protected AnnotationConfigApplicationContext buildContext() { + return new AnnotationConfigApplicationContext(); + } @Test - public void testSimpleRegion() throws Exception { + public void testSimpleRegionBuildRaw() throws Exception { PseudoState pseudoState = new DefaultPseudoState(PseudoStateKind.INITIAL); TestEntryAction entryActionS1 = new TestEntryAction("S1"); TestExitAction exitActionS1 = new TestExitAction("S1"); @@ -113,7 +127,7 @@ public class RegionMachineTests { } @Test - public void testMultiRegion() throws Exception { + public void testMultiRegionBuildRaw() throws Exception { SyncTaskExecutor taskExecutor = new SyncTaskExecutor(); PseudoState pseudoState = new DefaultPseudoState(PseudoStateKind.INITIAL); State stateSI = new EnumState(TestStates.SI); @@ -205,4 +219,106 @@ public class RegionMachineTests { assertThat(exitActionS112.stateContexts.size(), is(0)); } + @Test + public void testMultiRegion() throws Exception { + context.register(BaseConfig.class, StateMachineEventPublisherConfiguration.class, Config1.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + EnumStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, EnumStateMachine.class); + assertThat(machine, notNullValue()); + TestStateMachineListener listener = context.getBean(TestStateMachineListener.class); + machine.addStateListener(listener); + machine.start(); + assertThat(listener.stateMachineStartedLatch.await(5, TimeUnit.SECONDS), is(true)); + assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S10, TestStates.S20)); + + listener.reset(2, 0); + machine.sendEvent(TestEvents.E1); + assertThat(listener.stateChangedLatch.await(5, TimeUnit.SECONDS), is(true)); + assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S11, TestStates.S21)); + + listener.reset(1, 0); + machine.sendEvent(TestEvents.E2); + assertThat(listener.stateChangedLatch.await(5, TimeUnit.SECONDS), is(true)); + assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S10, TestStates.S21)); + + listener.reset(1, 0); + machine.sendEvent(TestEvents.E3); + assertThat(listener.stateChangedLatch.await(5, TimeUnit.SECONDS), is(true)); + assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S10, TestStates.S20)); + } + + @Configuration + @EnableStateMachine + static class Config1 extends EnumStateMachineConfigurerAdapter { + + @Override + public void configure(StateMachineStateConfigurer states) throws Exception { + states + .withStates() + .initial(TestStates.S10) + .state(TestStates.S10) + .state(TestStates.S11) + .and() + .withStates() + .initial(TestStates.S20) + .state(TestStates.S20) + .state(TestStates.S21); + } + + @Override + public void configure(StateMachineTransitionConfigurer transitions) throws Exception { + transitions + .withExternal() + .source(TestStates.S10) + .target(TestStates.S11) + .event(TestEvents.E1) + .and() + .withExternal() + .source(TestStates.S11) + .target(TestStates.S10) + .event(TestEvents.E2) + .and() + .withExternal() + .source(TestStates.S20) + .target(TestStates.S21) + .event(TestEvents.E1) + .and() + .withExternal() + .source(TestStates.S21) + .target(TestStates.S20) + .event(TestEvents.E3); + } + + @Bean + public TestStateMachineListener testStateMachineListener() { + return new TestStateMachineListener(); + } + + } + + private static class TestStateMachineListener extends StateMachineListenerAdapter { + + volatile CountDownLatch stateChangedLatch = new CountDownLatch(0); + volatile CountDownLatch stateMachineStartedLatch = new CountDownLatch(3); + + @Override + public void stateChanged(State from, State to) { + stateChangedLatch.countDown(); + } + + @Override + public void stateMachineStarted(StateMachine stateMachine) { + stateMachineStartedLatch.countDown(); + } + + void reset(int c1, int c2) { + stateChangedLatch = new CountDownLatch(c1); + stateMachineStartedLatch = new CountDownLatch(c2); + } + + } + }