From e9cb3af78505e2f74d0db46396c77580d5bf8f07 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Sat, 21 May 2016 09:54:21 +0100 Subject: [PATCH] Fix possible NPE - getState() now handles null better when checking running state when possibly going to return final state. - Fixes #227 --- .../buildtests/EndSmokeTests.java | 118 ++++++++++++++++++ .../statemachine/buildtests/end-smoke.di | 2 + .../buildtests/end-smoke.notation | 92 ++++++++++++++ .../statemachine/buildtests/end-smoke.uml | 16 +++ .../support/AbstractStateMachine.java | 9 +- 5 files changed, 233 insertions(+), 4 deletions(-) create mode 100644 spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/EndSmokeTests.java create mode 100644 spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.di create mode 100644 spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.notation create mode 100644 spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.uml diff --git a/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/EndSmokeTests.java b/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/EndSmokeTests.java new file mode 100644 index 00000000..51e03cfe --- /dev/null +++ b/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/EndSmokeTests.java @@ -0,0 +1,118 @@ +/* + * Copyright 2016 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.statemachine.buildtests; + +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; + +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.statemachine.StateMachine; +import org.springframework.statemachine.config.EnableStateMachine; +import org.springframework.statemachine.config.StateMachineConfigurerAdapter; +import org.springframework.statemachine.config.builders.StateMachineConfigurationConfigurer; +import org.springframework.statemachine.config.builders.StateMachineModelConfigurer; +import org.springframework.statemachine.config.model.StateMachineModelFactory; +import org.springframework.statemachine.test.StateMachineTestPlan; +import org.springframework.statemachine.test.StateMachineTestPlanBuilder; +import org.springframework.statemachine.uml.UmlStateMachineModelFactory; + +public class EndSmokeTests extends AbstractBuildTests { + + @Test + @SuppressWarnings("unchecked") + public void testEndState() throws Exception { + context.register(Config1.class); + context.refresh(); + StateMachine stateMachine = context.getBean(StateMachine.class); + + TestThread t = new TestThread(stateMachine); + t.start(); + + StateMachineTestPlan plan = + StateMachineTestPlanBuilder.builder() + .stateMachine(stateMachine) + .step().expectState("S1").and() + .step().sendEvent("E1").expectStateChanged(1).expectState("SF").and() + .build(); + plan.test(); + + t.runWhile = false; + t.join(); + assertThat(t.e, nullValue()); + assertThat(t.ok, is(true)); + } + + private static class TestThread extends Thread { + + StateMachine stateMachine; + boolean ok = true; + boolean runWhile = true; + Exception e = null; + + public TestThread(StateMachine stateMachine) { + this.stateMachine = stateMachine; + } + + @Override + public void run() { + while (runWhile) { + try { + if (stateMachine.getState() == null) { + ok = false; + runWhile = false; + } + } catch (Exception e) { + this.e = e; + ok = false; + runWhile = false; + } + } + } + } + + @Configuration + @EnableStateMachine + public static class Config1 extends StateMachineConfigurerAdapter { + + @Override + public void configure(StateMachineConfigurationConfigurer config) throws Exception { + config + .withConfiguration() + .autoStartup(true); + } + + @Override + public void configure(StateMachineModelConfigurer model) throws Exception { + model + .withModel() + .factory(modelFactory()); + } + + @Bean + public StateMachineModelFactory modelFactory() { + return new UmlStateMachineModelFactory("classpath:org/springframework/statemachine/buildtests/end-smoke.uml"); + } + } + + @Override + protected AnnotationConfigApplicationContext buildContext() { + return new AnnotationConfigApplicationContext(); + } +} diff --git a/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.di b/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.di new file mode 100644 index 00000000..bf9abab3 --- /dev/null +++ b/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.di @@ -0,0 +1,2 @@ + + diff --git a/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.notation b/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.notation new file mode 100644 index 00000000..2a2ede5a --- /dev/null +++ b/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.notation @@ -0,0 +1,92 @@ + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.uml b/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.uml new file mode 100644 index 00000000..5b42acc5 --- /dev/null +++ b/spring-statemachine-build-tests/src/test/resources/org/springframework/statemachine/buildtests/end-smoke.uml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + 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 1db52fc6..05f2594c 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 @@ -165,7 +165,7 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo public State getState() { // if we're complete assume we're stopped // and state was stashed into lastState - if (isComplete()) { + if (lastState != null && isComplete()) { return lastState; } else { return currentState; @@ -388,11 +388,12 @@ public abstract class AbstractStateMachine extends StateMachineObjectSuppo @Override public boolean isComplete() { - if (currentState == null) { + State s = currentState; + if (s == null) { return !isRunning(); } else { - return currentState != null && currentState.getPseudoState() != null - && currentState.getPseudoState().getKind() == PseudoStateKind.END; + return s != null && s.getPseudoState() != null + && s.getPseudoState().getKind() == PseudoStateKind.END; } }