Fixes multithreading bug

+ core coverage by mutation testing is now 88%
domclick · Sep 6, 2017 · 7c66601 · 7c66601
1 parent c223687
commit 7c66601
Show file tree

Hide file tree

Showing 11 changed files with 249 additions and 50 deletions.
diff --git a/dummy/pom.xml b/dummy/pom.xml
@@ -4,11 +4,23 @@
   <parent>
     <groupId>ru.sberned.statemachine</groupId>
     <artifactId>spring-flow-state-machine</artifactId>
-    <version>1.1.0</version>
+    <version>1.1.1</version>
   </parent>
   <artifactId>dummy</artifactId>
   <packaging>pom</packaging>
   <name>dummy</name>
   <description>Dummy module to be build latest</description>
+  <dependencies>
+    <dependency>
+      <groupId>ru.sberned.statemachine</groupId>
+      <artifactId>state-machine-loading</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>ru.sberned.statemachine</groupId>
+      <artifactId>state-machine-samples-simple</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+  </dependencies>
   <url>http://maven.apache.org</url>
 </project>
diff --git a/pom.xml b/pom.xml
@@ -11,7 +11,7 @@
   </parent>
   <groupId>ru.sberned.statemachine</groupId>
   <artifactId>spring-flow-state-machine</artifactId>
-  <version>1.1.0</version>
+  <version>1.1.1</version>
   <packaging>pom</packaging>
   <name>spring flow state machine</name>
   <description>Spring state machine for entities, not for application</description>

diff --git a/state-machine-core/pom.xml b/state-machine-core/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>ru.sberned.statemachine</groupId>
     <artifactId>spring-flow-state-machine</artifactId>
-    <version>1.1.0</version>
+    <version>1.1.1</version>
   </parent>
   <artifactId>state-machine-core</artifactId>
   <packaging>jar</packaging>
@@ -63,6 +63,11 @@
 
   <build>
     <plugins>
+      <plugin>
+        <groupId>org.pitest</groupId>
+        <artifactId>pitest-maven</artifactId>
+        <version>1.2.2</version>
+      </plugin>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-source-plugin</artifactId>

diff --git a/state-machine-core/src/main/java/ru/sberned/statemachine/StateMachine.java b/state-machine-core/src/main/java/ru/sberned/statemachine/StateMachine.java
@@ -40,7 +40,7 @@ public class StateMachine<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
     // To make @Transactional work
     @Autowired
     private StateMachine<ENTITY, STATE, ID> stateMachine = this;
-    private StateRepository<ENTITY, STATE, ID> stateRepository;
+    volatile private StateRepository<ENTITY, STATE, ID> stateRepository;
     @Value("${statemachine.lock.timeout.ms:5000}")
     private long lockTimeout;
 

diff --git a/state-machine-core/src/main/java/ru/sberned/statemachine/StateRepository.java b/state-machine-core/src/main/java/ru/sberned/statemachine/StateRepository.java
@@ -31,22 +31,12 @@ private void setAnyAfter(List<AfterAnyTransition<ENTITY, STATE>> anyAfter) {
         afterAllHandlers.addAll(anyAfter);
     }
 
-    private void setUnhandledMessageProcessor(UnhandledMessageProcessor<ID, STATE> unhandledMessageProcessor) {
-        this.unhandledMessageProcessor = unhandledMessageProcessor;
-    }
-
     List<BeforeTransition<ENTITY>> getBefore(STATE from, STATE to) {
-        if (isValidTransition(from, to)) {
-            return stateMap.get(to).get(from).getBeforeHandlers();
-        }
-        return new ArrayList<>();
+        return stateMap.get(to).get(from).getBeforeHandlers();
     }
 
     List<AfterTransition<ENTITY>> getAfter(STATE from, STATE to) {
-        if (isValidTransition(from, to)) {
-            return stateMap.get(to).get(from).getAfterHandlers();
-        }
-        return new ArrayList<>();
+        return stateMap.get(to).get(from).getAfterHandlers();
     }
 
     List<BeforeAnyTransition<ENTITY, STATE>> getBeforeAll() {
@@ -61,28 +51,40 @@ UnhandledMessageProcessor<ID, STATE> getUnhandledMessageProcessor() {
         return unhandledMessageProcessor;
     }
 
-    private class Processors {
-        private List<BeforeTransition<ENTITY>> beforeHandlers = new ArrayList<>();
-        private List<AfterTransition<ENTITY>> afterHandlers = new ArrayList<>();
+    private void setUnhandledMessageProcessor(UnhandledMessageProcessor<ID, STATE> unhandledMessageProcessor) {
+        this.unhandledMessageProcessor = unhandledMessageProcessor;
+    }
 
-        private List<BeforeTransition<ENTITY>> getBeforeHandlers() {
-            return beforeHandlers;
-        }
+    @SuppressWarnings("unchecked")
+    public interface From<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
+        To<ENTITY, STATE, ID> from(STATE... states);
+    }
 
-        private List<AfterTransition<ENTITY>> getAfterHandlers() {
-            return afterHandlers;
-        }
+
+    @SuppressWarnings("unchecked")
+    public interface To<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
+        CompleteTransition<ENTITY, STATE, ID> to(STATE... states);
     }
 
+    @SuppressWarnings("unchecked")
+    public interface CompleteTransition<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
+        CompleteTransition<ENTITY, STATE, ID> before(BeforeTransition<ENTITY>... handlers);
+
+        CompleteTransition<ENTITY, STATE, ID> after(AfterTransition<ENTITY>... handlers);
+
+        From<ENTITY, STATE, ID> and();
+
+        StateRepository<ENTITY, STATE, ID> build();
+    }
 
-    public static class StateRepositoryBuilder<ENTITY extends HasStateAndId<ID, STATE>, STATE , ID> {
+    public static class StateRepositoryBuilder<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
         private StateRepository<ENTITY, STATE, ID> stateRepository;
 
         private StateRepositoryBuilder() {
             stateRepository = new StateRepository<>();
         }
 
-        public static <ENTITY extends HasStateAndId<ID, STATE>, STATE , ID> StateRepositoryBuilder<ENTITY, STATE, ID> configure() {
+        public static <ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> StateRepositoryBuilder<ENTITY, STATE, ID> configure() {
             return new StateRepositoryBuilder<>();
         }
 
@@ -115,25 +117,17 @@ public From<ENTITY, STATE, ID> defineTransitions() {
         }
     }
 
-    @SuppressWarnings("unchecked")
-    public interface From<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
-        To<ENTITY, STATE, ID> from(STATE... states);
-    }
-
-    @SuppressWarnings("unchecked")
-    public interface To<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
-        CompleteTransition<ENTITY, STATE, ID> to(STATE... states);
-    }
-
-    @SuppressWarnings("unchecked")
-    public interface CompleteTransition<ENTITY extends HasStateAndId<ID, STATE>, STATE, ID> {
-        CompleteTransition<ENTITY, STATE, ID> before(BeforeTransition<ENTITY>... handlers);
-
-        CompleteTransition<ENTITY, STATE, ID> after(AfterTransition<ENTITY>... handlers);
+    private class Processors {
+        private List<BeforeTransition<ENTITY>> beforeHandlers = new ArrayList<>();
+        private List<AfterTransition<ENTITY>> afterHandlers = new ArrayList<>();
 
-        From<ENTITY, STATE, ID> and();
+        private List<BeforeTransition<ENTITY>> getBeforeHandlers() {
+            return beforeHandlers;
+        }
 
-        StateRepository<ENTITY, STATE, ID> build();
+        private List<AfterTransition<ENTITY>> getAfterHandlers() {
+            return afterHandlers;
+        }
     }
 
     public class StateTransition implements To<ENTITY, STATE, ID>, From<ENTITY, STATE, ID>, CompleteTransition<ENTITY, STATE, ID> {

diff --git a/state-machine-core/src/test/java/ru/sberned/statemachine/StateMachineTests.java b/state-machine-core/src/test/java/ru/sberned/statemachine/StateMachineTests.java
@@ -72,6 +72,15 @@ public void testCorrectStatesNoHandlers() {
         verify(onTransition, timeout(500).times(1)).moveToState(CustomState.STATE1, new Item("1", CustomState.START));
     }
 
+    @Test
+    public void shouldMoveToStatePreservingInfo() {
+        StateRepository<Item, CustomState, String> stateHolder = getDefaultTransition();
+
+        stateMachine.setStateRepository(stateHolder);
+        publisher.publishEvent(new StateChangedEvent("1", CustomState.STATE1, "info"));
+        verify(onTransition, timeout(500).times(1)).moveToState(CustomState.STATE1, new Item("1", CustomState.START), "info");
+    }
+
     @Test
     public void testCorrectStatesWithHandlersInOrder() throws Exception {
         StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
@@ -99,6 +108,80 @@ public void testCorrectStatesWithHandlersInOrder() throws Exception {
         inOrder.verify(afterTransition2, times(1)).afterTransition(item);
     }
 
+    @Test
+    public void noActionsOnAbsentBefore() throws Exception {
+        StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
+                .setAvailableStates(EnumSet.allOf(CustomState.class))
+                .defineTransitions()
+                .from(CustomState.START)
+                .to(CustomState.STATE1)
+                .build();
+
+        stateMachine.setStateRepository(stateHolder);
+
+        Item item = new Item("1", CustomState.START);
+
+        stateMachine.handleMessage("1", CustomState.STATE1, null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void emptyFromShouldCauseError() throws Exception {
+        StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
+                .setAvailableStates(EnumSet.allOf(CustomState.class))
+                .defineTransitions()
+                .from()
+                .to(CustomState.STATE1)
+                .build();
+
+        stateMachine.setStateRepository(stateHolder);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void emptyToShouldCauseError() throws Exception {
+        StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
+                .setAvailableStates(EnumSet.allOf(CustomState.class))
+                .defineTransitions()
+                .from(CustomState.START)
+                .to()
+                .build();
+
+        stateMachine.setStateRepository(stateHolder);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void nullFromStateShouldCauseError() throws Exception {
+        StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
+                .setAvailableStates(EnumSet.allOf(CustomState.class))
+                .defineTransitions()
+                .from(null)
+                .to(CustomState.STATE1)
+                .build();
+
+        stateMachine.setStateRepository(stateHolder);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void nullToStateShouldCauseError() throws Exception {
+        StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
+                .setAvailableStates(EnumSet.allOf(CustomState.class))
+                .defineTransitions()
+                .from(CustomState.START)
+                .to(null)
+                .build();
+        stateMachine.setStateRepository(stateHolder);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void shouldThrowExceptionOnUseOfUnavailableState() throws Exception {
+        StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
+                .setAvailableStates(Collections.singleton(CustomState.START))
+                .defineTransitions()
+                .from(CustomState.START)
+                .to(CustomState.STATE1)
+                .build();
+        stateMachine.setStateRepository(stateHolder);
+    }
+
     @Test
     public void testConflictingEventsLeadToOnlyOneStateChange() throws InterruptedException {
         StateRepository<Item, CustomState, String> stateHolder = StateRepositoryBuilder.<Item, CustomState, String>configure()
@@ -172,6 +255,17 @@ public void testExecutionResultSuccess() throws ExecutionException, InterruptedE
         assertTrue(results.get("1").get());
     }
 
+    @Test
+    public void shoudReturnEmptyMapOnNullOrEmptyEvents() throws ExecutionException, InterruptedException {
+        StateRepository<Item, CustomState, String> stateHolder = getDefaultTransition();
+
+        stateMachine.setStateRepository(stateHolder);
+        Map<String, Future<Boolean>> results = stateMachine.changeState(Collections.EMPTY_LIST, CustomState.STATE1, null);
+        assertTrue(results.isEmpty());
+        results = stateMachine.changeState(null, CustomState.STATE1, null);
+        assertTrue(results.isEmpty());
+    }
+
     @Test
     public void testExecutionResultFail() throws ExecutionException, InterruptedException {
         StateRepository<Item, CustomState, String> stateHolder = getDefaultTransition();