From 5f8e6c09354d8d03fc5c47545190e1b196909790 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Tue, 7 Mar 2017 17:02:35 +0000 Subject: [PATCH] Notify transition before calling its actions - Add new api to Transition to expose calling its actions. - Modify rest of a framework to first notify before calling transtion action. - Fixes #322 --- .../support/AbstractStateMachine.java | 8 +++++++- .../support/DefaultStateMachineExecutor.java | 4 ++-- .../transition/AbstractTransition.java | 6 +++--- .../transition/InitialTransition.java | 5 +++-- .../statemachine/transition/Transition.java | 9 ++++++++- .../statemachine/StateContextTests.java | 20 +++++++++---------- .../StateContextExpressionMethodsTests.java | 6 +++++- 7 files changed, 38 insertions(+), 20 deletions(-) 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 b0cad165..c774a38a 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 @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -307,6 +307,12 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo long now = System.currentTimeMillis(); // TODO: fix above stateContext as it's not used notifyTransitionStart(buildStateContext(Stage.TRANSITION_START, message, t, getRelayStateMachine())); + try { + t.executeTransitionActions(ctx); + } catch (Exception e) { + log.warn("Aborting as transition " + t + " caused error " + e); + return; + } notifyTransition(buildStateContext(Stage.TRANSITION, message, t, getRelayStateMachine())); if (t.getTarget().getPseudoState() != null && t.getTarget().getPseudoState().getKind() == PseudoStateKind.JOIN) { exitFromState(t.getSource(), message, t, getRelayStateMachine()); diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java index ed93e7a5..b9ee5880 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -236,7 +236,7 @@ public class DefaultStateMachineExecutor extends LifecycleObjectSupport im try { transit = t.transit(stateContext); } catch (Exception e) { - log.warn("Transition " + t + " caused error " + e); + log.warn("Aborting as transition " + t + " caused error " + e); } if (transit) { stateMachineExecutorTransit.transit(t, stateContext, queuedMessage); diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/AbstractTransition.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/AbstractTransition.java index 6067e260..18edc661 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/AbstractTransition.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/AbstractTransition.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -112,7 +112,6 @@ public abstract class AbstractTransition implements Transition { return false; } } - executeAllActions(context); return true; } @@ -155,7 +154,8 @@ public abstract class AbstractTransition implements Transition { } } - protected final void executeAllActions(StateContext context) { + @Override + public final void executeTransitionActions(StateContext context) { if (actions == null) { return; } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/InitialTransition.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/InitialTransition.java index 3d2c122b..a39cb542 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/InitialTransition.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/InitialTransition.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -64,7 +64,8 @@ public class InitialTransition extends AbstractTransition @Override public boolean transit(StateContext context) { - executeAllActions(context); + // initial itself doesn't cause further changes what + // returned true might cause. return false; } } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/Transition.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/Transition.java index ff165d50..f46af055 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/Transition.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/transition/Transition.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,6 +43,13 @@ public interface Transition { */ boolean transit(StateContext context); + /** + * Execute transition actions. + * + * @param context the state context + */ + void executeTransitionActions(StateContext context); + /** * Gets the source state of this transition. * 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 fc2baf45..aec14776 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 @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -64,8 +64,8 @@ public class StateContextTests extends AbstractStateMachineTests { assertThat(listener.contexts, hasSize(19)); assertThat(listener.contexts, contains( - hasStage(Stage.EXTENDED_STATE_CHANGED), hasStage(Stage.TRANSITION_START), + hasStage(Stage.EXTENDED_STATE_CHANGED), hasStage(Stage.TRANSITION), hasStage(Stage.STATE_ENTRY), hasStage(Stage.TRANSITION_START), @@ -85,15 +85,15 @@ public class StateContextTests extends AbstractStateMachineTests { hasStage(Stage.TRANSITION_END) )); - assertThat(listener.contexts.get(0).getStage(), is(Stage.EXTENDED_STATE_CHANGED)); + assertThat(listener.contexts.get(0).getStage(), is(Stage.TRANSITION_START)); + assertThat(listener.contexts.get(0).getTransition(), notNullValue()); + assertThat(listener.contexts.get(0).getTransition().getSource(), nullValue()); + assertThat(listener.contexts.get(0).getTransition().getTarget(), notNullValue()); + assertThat(listener.contexts.get(0).getTransition().getTarget().getId(), is(States.S0)); + assertThat(listener.contexts.get(0).getSource(), nullValue()); + assertThat(listener.contexts.get(0).getTarget(), notNullValue()); - assertThat(listener.contexts.get(1).getStage(), is(Stage.TRANSITION_START)); - assertThat(listener.contexts.get(1).getTransition(), notNullValue()); - assertThat(listener.contexts.get(1).getTransition().getSource(), nullValue()); - assertThat(listener.contexts.get(1).getTransition().getTarget(), notNullValue()); - assertThat(listener.contexts.get(1).getTransition().getTarget().getId(), is(States.S0)); - assertThat(listener.contexts.get(1).getSource(), nullValue()); - assertThat(listener.contexts.get(1).getTarget(), notNullValue()); + assertThat(listener.contexts.get(1).getStage(), is(Stage.EXTENDED_STATE_CHANGED)); assertThat(listener.contexts.get(2).getStage(), is(Stage.TRANSITION)); assertThat(listener.contexts.get(2).getTransition(), notNullValue()); diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/support/StateContextExpressionMethodsTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/support/StateContextExpressionMethodsTests.java index 6ca13bc3..0c142958 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/support/StateContextExpressionMethodsTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/support/StateContextExpressionMethodsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -104,6 +104,10 @@ public class StateContextExpressionMethodsTests { return false; } + @Override + public void executeTransitionActions(StateContext context) { + } + @Override public State getSource() { return new EnumState(SpelStates.S1);