From 457c924e676aa15f8d6da90c86568e8185c70edc Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Thu, 8 Dec 2016 12:15:52 +0000 Subject: [PATCH] Remove some EventNotAccepted events - Now only trying to notify those from a root machine as same events from childs are kinda pointless and just create noise. - Fixes #287 --- .../access/StateMachineAccess.java | 6 + .../config/AbstractStateMachineFactory.java | 22 ++ .../support/AbstractStateMachine.java | 14 ++ .../statemachine/StateContextTests.java | 24 +- .../access/StateMachineAccessTests.java | 4 + .../event/StateMachineEventTests.java | 223 ++++++++++++++++++ 6 files changed, 270 insertions(+), 23 deletions(-) diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/access/StateMachineAccess.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/access/StateMachineAccess.java index 5554bb59..1aa0b07a 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/access/StateMachineAccess.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/access/StateMachineAccess.java @@ -76,4 +76,10 @@ public interface StateMachineAccess { */ void setForwardedInitialEvent(Message message); + /** + * Sets the parent machine. + * + * @param stateMachine the state machine + */ + void setParentMachine(StateMachine stateMachine); } 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 815b1a20..33b057ea 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 @@ -322,6 +322,28 @@ public abstract class AbstractStateMachineFactory extends LifecycleObjectS holder.getValue().setState(stateMap.get(holder.getKey())); } + // set parent machines for each built machine + for (Entry> mme : machineMap.entrySet()) { + StateMachine m = null; + if (mme.getKey() != null) { + Object sParent = null; + for (StateData sd : stateMachineModel.getStatesData().getStateData()) { + if (ObjectUtils.nullSafeEquals(sd.getState(), mme.getKey())) { + sParent = sd.getParent(); + break; + } + } + m = machineMap.get(sParent); + } + final StateMachine mm = m; + mme.getValue().getStateMachineAccessor().doWithRegion(new StateMachineFunction>(){ + + @Override + public void apply(StateMachineAccess function) { + function.setParentMachine(mm); + } + }); + } return delegateAutoStartup(machine); } 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 4ff55bd6..b0cad165 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 @@ -116,6 +116,8 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo private final Object lock = new Object(); + private StateMachine parentMachine; + /** * Instantiates a new abstract state machine. * @@ -234,6 +236,13 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo return accepted; } + @Override + protected void notifyEventNotAccepted(StateContext stateContext) { + if (parentMachine == null) { + super.notifyEventNotAccepted(stateContext); + } + } + @Override public boolean sendEvent(E event) { return sendEvent(MessageBuilder.withPayload(event).build()); @@ -521,6 +530,11 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo this.relay = stateMachine; } + @Override + public void setParentMachine(StateMachine parentMachine) { + this.parentMachine = parentMachine; + } + @Override protected void stateChangedInRelay() { // TODO: temp tweak, see super diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateContextTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateContextTests.java index bddf1516..fc2baf45 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateContextTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/StateContextTests.java @@ -23,7 +23,6 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; -import static org.hamcrest.Matchers.sameInstance; import java.util.ArrayList; import java.util.Map; @@ -168,36 +167,15 @@ public class StateContextTests extends AbstractStateMachineTests { // all nested machines sends these assertThat(listener.contexts, contains( - hasStage(Stage.EVENT_NOT_ACCEPTED), - hasStage(Stage.EVENT_NOT_ACCEPTED), hasStage(Stage.EVENT_NOT_ACCEPTED) )); - assertThat(listener.contexts.get(0).getStage(), is(Stage.EVENT_NOT_ACCEPTED)); assertThat(listener.contexts.get(0).getTransition(), nullValue()); assertThat(listener.contexts.get(0).getEvent(), is(Events.J)); assertThat(listener.contexts.get(0).getSource(), notNullValue()); - assertThat(listener.contexts.get(0).getSource().getId(), is(States.S11)); + assertThat(listener.contexts.get(0).getSource().getId(), is(States.S0)); assertThat(listener.contexts.get(0).getTarget(), nullValue()); - - assertThat(listener.contexts.get(1).getStage(), is(Stage.EVENT_NOT_ACCEPTED)); - assertThat(listener.contexts.get(1).getTransition(), nullValue()); - assertThat(listener.contexts.get(1).getEvent(), is(Events.J)); - assertThat(listener.contexts.get(1).getSource(), notNullValue()); - assertThat(listener.contexts.get(1).getSource().getId(), is(States.S1)); - assertThat(listener.contexts.get(1).getTarget(), nullValue()); - - assertThat(listener.contexts.get(2).getStage(), is(Stage.EVENT_NOT_ACCEPTED)); - assertThat(listener.contexts.get(2).getTransition(), nullValue()); - assertThat(listener.contexts.get(2).getEvent(), is(Events.J)); - assertThat(listener.contexts.get(2).getSource(), notNullValue()); - assertThat(listener.contexts.get(2).getSource().getId(), is(States.S0)); - assertThat(listener.contexts.get(2).getTarget(), nullValue()); - - // TODO: I wonder if these should be different machines - assertThat(listener.contexts.get(0).getStateMachine(), sameInstance(listener.contexts.get(1).getStateMachine())); - assertThat(listener.contexts.get(0).getStateMachine(), sameInstance(listener.contexts.get(2).getStateMachine())); } static class TestStateMachineListener extends StateMachineListenerAdapter { diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/access/StateMachineAccessTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/access/StateMachineAccessTests.java index 590b8648..84731d70 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/access/StateMachineAccessTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/access/StateMachineAccessTests.java @@ -106,6 +106,10 @@ public class StateMachineAccessTests { this.relay = stateMachine; } + @Override + public void setParentMachine(StateMachine stateMachine) { + } + @Override public void resetStateMachine(StateMachineContext stateMachineContext) { } diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/event/StateMachineEventTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/event/StateMachineEventTests.java index ad3e8fac..d5f14bbe 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/event/StateMachineEventTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/event/StateMachineEventTests.java @@ -124,6 +124,141 @@ public class StateMachineEventTests extends AbstractStateMachineTests { assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S12)); } + @Test + public void testEventNotAcceptedS1() throws Exception { + context.register(BaseConfig.class, Config3.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + ObjectStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class); + TestListener listener = new TestListener(); + machine.addStateListener(listener); + + machine.start(); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + + listener.reset(1); + boolean accepted = machine.sendEvent(TestEvents.E2); + assertThat(accepted, is(false)); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + assertThat(listener.eventNotAcceptedLatch.await(1, TimeUnit.SECONDS), is(true)); + assertThat(listener.eventNotAccepted.size(), is(1)); + } + + @Test + public void testEventAcceptedS1() throws Exception { + context.register(BaseConfig.class, Config3.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + ObjectStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class); + TestListener listener = new TestListener(); + machine.addStateListener(listener); + + machine.start(); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + + listener.reset(1); + boolean accepted = machine.sendEvent(TestEvents.E1); + assertThat(accepted, is(true)); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + assertThat(listener.eventNotAcceptedLatch.await(1, TimeUnit.SECONDS), is(false)); + assertThat(listener.eventNotAccepted.size(), is(0)); + } + + @Test + public void testEventAcceptedS1NoS1Transition() throws Exception { + context.register(BaseConfig.class, Config4.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + ObjectStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class); + TestListener listener = new TestListener(); + machine.addStateListener(listener); + + machine.start(); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + + listener.reset(1); + boolean accepted = machine.sendEvent(TestEvents.E1); + assertThat(accepted, is(true)); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + assertThat(listener.eventNotAcceptedLatch.await(1, TimeUnit.SECONDS), is(false)); + assertThat(listener.eventNotAccepted.size(), is(0)); + } + + @Test + public void testEventAcceptedS1GuardAllow() throws Exception { + context.register(BaseConfig.class, Config3.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + ObjectStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class); + TestListener listener = new TestListener(); + machine.addStateListener(listener); + + machine.start(); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + machine.getExtendedState().getVariables().put("S1E1", true); + + listener.reset(1); + boolean accepted = machine.sendEvent(TestEvents.E1); + assertThat(accepted, is(true)); + assertThat(machine.getState().getIds(), contains(TestStates.S2)); + assertThat(listener.eventNotAcceptedLatch.await(1, TimeUnit.SECONDS), is(false)); + assertThat(listener.eventNotAccepted.size(), is(0)); + } + + @Test + public void testEventAcceptedS11GuardAllow() throws Exception { + context.register(BaseConfig.class, Config3.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + ObjectStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class); + TestListener listener = new TestListener(); + machine.addStateListener(listener); + + machine.start(); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + machine.getExtendedState().getVariables().put("S11E1", true); + + listener.reset(1); + boolean accepted = machine.sendEvent(TestEvents.E1); + assertThat(accepted, is(true)); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S12)); + assertThat(listener.eventNotAcceptedLatch.await(1, TimeUnit.SECONDS), is(false)); + assertThat(listener.eventNotAccepted.size(), is(0)); + } + + @Test + public void testEventAcceptedS111GuardAllow() throws Exception { + context.register(BaseConfig.class, Config3.class); + context.refresh(); + assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE)); + @SuppressWarnings("unchecked") + ObjectStateMachine machine = + context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class); + TestListener listener = new TestListener(); + machine.addStateListener(listener); + + machine.start(); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S111)); + machine.getExtendedState().getVariables().put("S111E1", true); + + listener.reset(1); + boolean accepted = machine.sendEvent(TestEvents.E1); + assertThat(accepted, is(true)); + assertThat(machine.getState().getIds(), contains(TestStates.S1, TestStates.S11, TestStates.S112)); + assertThat(listener.eventNotAcceptedLatch.await(1, TimeUnit.SECONDS), is(false)); + assertThat(listener.eventNotAccepted.size(), is(0)); + } + @Configuration @EnableStateMachine static class Config1 extends EnumStateMachineConfigurerAdapter { @@ -216,6 +351,90 @@ public class StateMachineEventTests extends AbstractStateMachineTests { } + @Configuration + @EnableStateMachine + static class Config3 extends EnumStateMachineConfigurerAdapter { + + @Override + public void configure(StateMachineStateConfigurer states) throws Exception { + states + .withStates() + .initial(TestStates.S1) + .state(TestStates.S2) + .and() + .withStates() + .parent(TestStates.S1) + .initial(TestStates.S11) + .state(TestStates.S12) + .and() + .withStates() + .parent(TestStates.S11) + .initial(TestStates.S111) + .state(TestStates.S112); + } + + @Override + public void configure(StateMachineTransitionConfigurer transitions) throws Exception { + transitions + .withExternal() + .source(TestStates.S1) + .target(TestStates.S2) + .event(TestEvents.E1) + .guardExpression("extendedState.variables.containsKey('S1E1')") + .and() + .withExternal() + .source(TestStates.S11) + .target(TestStates.S12) + .event(TestEvents.E1) + .guardExpression("extendedState.variables.containsKey('S11E1')") + .and() + .withExternal() + .source(TestStates.S111) + .target(TestStates.S112) + .event(TestEvents.E1) + .guardExpression("extendedState.variables.containsKey('S111E1')"); + } + } + + @Configuration + @EnableStateMachine + static class Config4 extends EnumStateMachineConfigurerAdapter { + + @Override + public void configure(StateMachineStateConfigurer states) throws Exception { + states + .withStates() + .initial(TestStates.S1) + .state(TestStates.S2) + .and() + .withStates() + .parent(TestStates.S1) + .initial(TestStates.S11) + .state(TestStates.S12) + .and() + .withStates() + .parent(TestStates.S11) + .initial(TestStates.S111) + .state(TestStates.S112); + } + + @Override + public void configure(StateMachineTransitionConfigurer transitions) throws Exception { + transitions + .withExternal() + .source(TestStates.S11) + .target(TestStates.S12) + .event(TestEvents.E1) + .guardExpression("extendedState.variables.containsKey('S11E1')") + .and() + .withExternal() + .source(TestStates.S111) + .target(TestStates.S112) + .event(TestEvents.E1) + .guardExpression("extendedState.variables.containsKey('S111E1')"); + } + } + static class TestEventListener implements ApplicationListener { volatile CountDownLatch onEventLatch = new CountDownLatch(6); @@ -254,6 +473,10 @@ public class StateMachineEventTests extends AbstractStateMachineTests { eventNotAcceptedLatch.countDown(); } + public void reset(int c1) { + eventNotAcceptedLatch = new CountDownLatch(c1); + eventNotAccepted.clear(); + } } }