Guard should not break with Throwable
- Now catching all exceptions and throwables with evaluate and return false in those cases. - Fixes #206
This commit is contained in:
@@ -17,6 +17,8 @@ package org.springframework.statemachine.state;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
import org.springframework.statemachine.StateContext;
|
||||
import org.springframework.statemachine.guard.Guard;
|
||||
import org.springframework.util.Assert;
|
||||
@@ -31,6 +33,7 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public class ChoicePseudoState<S, E> implements PseudoState<S, E> {
|
||||
|
||||
private final static Log log = LogFactory.getLog(ChoicePseudoState.class);
|
||||
private final List<ChoiceStateData<S, E>> choices;
|
||||
|
||||
/**
|
||||
@@ -52,7 +55,7 @@ public class ChoicePseudoState<S, E> implements PseudoState<S, E> {
|
||||
State<S, E> s = null;
|
||||
for (ChoiceStateData<S, E> c : choices) {
|
||||
s = c.getState();
|
||||
if (c.guard != null && c.guard.evaluate(context)) {
|
||||
if (c.guard != null && evaluateInternal(c.guard, context)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -67,6 +70,15 @@ public class ChoicePseudoState<S, E> implements PseudoState<S, E> {
|
||||
public void addPseudoStateListener(PseudoStateListener<S, E> listener) {
|
||||
}
|
||||
|
||||
private boolean evaluateInternal(Guard<S, E> guard, StateContext<S, E> context) {
|
||||
try {
|
||||
return guard.evaluate(context);
|
||||
} catch (Throwable t) {
|
||||
log.warn("Deny guard due to throw as GUARD should not error", t);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Data class wrapping choice {@link State} and {@link Guard}
|
||||
* together.
|
||||
|
||||
@@ -17,6 +17,8 @@ package org.springframework.statemachine.state;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
import org.springframework.statemachine.StateContext;
|
||||
import org.springframework.statemachine.guard.Guard;
|
||||
import org.springframework.util.Assert;
|
||||
@@ -31,6 +33,7 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public class JunctionPseudoState<S, E> implements PseudoState<S, E> {
|
||||
|
||||
private final static Log log = LogFactory.getLog(JunctionPseudoState.class);
|
||||
private final List<JunctionStateData<S, E>> junctions;
|
||||
|
||||
/**
|
||||
@@ -52,7 +55,7 @@ public class JunctionPseudoState<S, E> implements PseudoState<S, E> {
|
||||
State<S, E> s = null;
|
||||
for (JunctionStateData<S, E> j : junctions) {
|
||||
s = j.getState();
|
||||
if (j.guard != null && j.guard.evaluate(context)) {
|
||||
if (j.guard != null && evaluateInternal(j.guard, context)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -67,6 +70,15 @@ public class JunctionPseudoState<S, E> implements PseudoState<S, E> {
|
||||
public void addPseudoStateListener(PseudoStateListener<S, E> listener) {
|
||||
}
|
||||
|
||||
private boolean evaluateInternal(Guard<S, E> guard, StateContext<S, E> context) {
|
||||
try {
|
||||
return guard.evaluate(context);
|
||||
} catch (Throwable t) {
|
||||
log.warn("Deny guard due to throw as GUARD should not error", t);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Data class wrapping choice {@link State} and {@link Guard}
|
||||
* together.
|
||||
|
||||
@@ -17,6 +17,8 @@ package org.springframework.statemachine.transition;
|
||||
|
||||
import java.util.Collection;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
import org.springframework.statemachine.StateContext;
|
||||
import org.springframework.statemachine.action.Action;
|
||||
import org.springframework.statemachine.guard.Guard;
|
||||
@@ -35,6 +37,8 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public abstract class AbstractTransition<S, E> implements Transition<S, E> {
|
||||
|
||||
private final static Log log = LogFactory.getLog(AbstractTransition.class);
|
||||
|
||||
private final State<S,E> source;
|
||||
|
||||
private final State<S,E> target;
|
||||
@@ -90,7 +94,12 @@ public abstract class AbstractTransition<S, E> implements Transition<S, E> {
|
||||
@Override
|
||||
public boolean transit(StateContext<S, E> context) {
|
||||
if (guard != null) {
|
||||
if (!guard.evaluate(context)) {
|
||||
try {
|
||||
if (!guard.evaluate(context)) {
|
||||
return false;
|
||||
}
|
||||
} catch (Throwable t) {
|
||||
log.warn("Deny guard due to throw as GUARD should not error", t);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,6 +33,7 @@ import org.springframework.statemachine.AbstractStateMachineTests.TestEvents;
|
||||
import org.springframework.statemachine.AbstractStateMachineTests.TestGuard;
|
||||
import org.springframework.statemachine.AbstractStateMachineTests.TestStates;
|
||||
import org.springframework.statemachine.ObjectStateMachine;
|
||||
import org.springframework.statemachine.StateContext;
|
||||
import org.springframework.statemachine.StateMachineSystemConstants;
|
||||
import org.springframework.statemachine.config.EnableStateMachine;
|
||||
import org.springframework.statemachine.config.EnumStateMachineConfigurerAdapter;
|
||||
@@ -41,7 +42,7 @@ import org.springframework.statemachine.config.builders.StateMachineTransitionCo
|
||||
|
||||
/**
|
||||
* Tests for state machine guards.
|
||||
*
|
||||
*
|
||||
* @author Janne Valkealahti
|
||||
*
|
||||
*/
|
||||
@@ -57,12 +58,12 @@ public class GuardTests {
|
||||
TestAction testAction = ctx.getBean("testAction", TestAction.class);
|
||||
assertThat(testGuard, notNullValue());
|
||||
assertThat(testAction, notNullValue());
|
||||
|
||||
|
||||
machine.start();
|
||||
machine.sendEvent(TestEvents.E1);
|
||||
machine.sendEvent(TestEvents.E1);
|
||||
assertThat(testGuard.onEvaluateLatch.await(2, TimeUnit.SECONDS), is(true));
|
||||
assertThat(testAction.onExecuteLatch.await(2, TimeUnit.SECONDS), is(true));
|
||||
|
||||
|
||||
ctx.close();
|
||||
}
|
||||
|
||||
@@ -76,18 +77,35 @@ public class GuardTests {
|
||||
TestAction testAction = ctx.getBean("testAction", TestAction.class);
|
||||
assertThat(testGuard, notNullValue());
|
||||
assertThat(testAction, notNullValue());
|
||||
|
||||
|
||||
machine.start();
|
||||
assertThat(machine.getState().getIds(), contains(TestStates.S1));
|
||||
|
||||
machine.sendEvent(TestEvents.E1);
|
||||
|
||||
machine.sendEvent(TestEvents.E1);
|
||||
assertThat(testGuard.onEvaluateLatch.await(2, TimeUnit.SECONDS), is(true));
|
||||
assertThat(testAction.onExecuteLatch.await(2, TimeUnit.SECONDS), is(false));
|
||||
assertThat(machine.getState().getIds(), contains(TestStates.S1));
|
||||
|
||||
|
||||
ctx.close();
|
||||
}
|
||||
|
||||
|
||||
@SuppressWarnings({ "unchecked" })
|
||||
@Test
|
||||
public void testGuardThrows() throws Exception {
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Config3.class);
|
||||
ObjectStateMachine<TestStates,TestEvents> machine =
|
||||
ctx.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class);
|
||||
machine.start();
|
||||
assertThat(machine.getState().getIds(), contains(TestStates.S1));
|
||||
machine.sendEvent(TestEvents.E1);
|
||||
assertThat(machine.getState().getIds(), contains(TestStates.S1));
|
||||
machine.sendEvent(TestEvents.E2);
|
||||
assertThat(machine.getState().getIds(), contains(TestStates.S1));
|
||||
machine.sendEvent(TestEvents.E3);
|
||||
assertThat(machine.getState().getIds(), contains(TestStates.S2));
|
||||
ctx.close();
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@EnableStateMachine
|
||||
public static class Config1 extends EnumStateMachineConfigurerAdapter<TestStates, TestEvents> {
|
||||
@@ -116,12 +134,12 @@ public class GuardTests {
|
||||
public TestAction testAction() {
|
||||
return new TestAction();
|
||||
}
|
||||
|
||||
|
||||
@Bean
|
||||
public TestGuard testGuard() {
|
||||
return new TestGuard(true);
|
||||
}
|
||||
|
||||
|
||||
@Bean
|
||||
public TaskExecutor taskExecutor() {
|
||||
return new SyncTaskExecutor();
|
||||
@@ -152,12 +170,12 @@ public class GuardTests {
|
||||
.action(testAction())
|
||||
.guard(testGuard());
|
||||
}
|
||||
|
||||
|
||||
@Bean
|
||||
public TestGuard testGuard() {
|
||||
return new TestGuard(false);
|
||||
}
|
||||
|
||||
|
||||
@Bean
|
||||
public TestAction testAction() {
|
||||
return new TestAction();
|
||||
@@ -169,5 +187,61 @@ public class GuardTests {
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
|
||||
@Configuration
|
||||
@EnableStateMachine
|
||||
public static class Config3 extends EnumStateMachineConfigurerAdapter<TestStates, TestEvents> {
|
||||
|
||||
@Override
|
||||
public void configure(StateMachineStateConfigurer<TestStates, TestEvents> states) throws Exception {
|
||||
states
|
||||
.withStates()
|
||||
.initial(TestStates.S1)
|
||||
.state(TestStates.S1)
|
||||
.state(TestStates.S2);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void configure(StateMachineTransitionConfigurer<TestStates, TestEvents> transitions) throws Exception {
|
||||
transitions
|
||||
.withExternal()
|
||||
.source(TestStates.S1)
|
||||
.target(TestStates.S2)
|
||||
.event(TestEvents.E1)
|
||||
.guard(testGuard1())
|
||||
.and()
|
||||
.withExternal()
|
||||
.source(TestStates.S1)
|
||||
.target(TestStates.S2)
|
||||
.event(TestEvents.E2)
|
||||
.guard(testGuard2())
|
||||
.and()
|
||||
.withExternal()
|
||||
.source(TestStates.S1)
|
||||
.target(TestStates.S2)
|
||||
.event(TestEvents.E3);
|
||||
}
|
||||
|
||||
@Bean
|
||||
public Guard<TestStates, TestEvents> testGuard1() {
|
||||
return new Guard<TestStates, TestEvents>() {
|
||||
|
||||
@Override
|
||||
public boolean evaluate(StateContext<TestStates, TestEvents> context) {
|
||||
throw new RuntimeException();
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@Bean
|
||||
public Guard<TestStates, TestEvents> testGuard2() {
|
||||
return new Guard<TestStates, TestEvents>() {
|
||||
|
||||
@Override
|
||||
public boolean evaluate(StateContext<TestStates, TestEvents> context) {
|
||||
throw new Error();
|
||||
}
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user