From 7c6660192e2e8a0585749fff527dfb5e366aea02 Mon Sep 17 00:00:00 2001 From: asm0dey Date: Wed, 6 Sep 2017 18:25:28 +0300 Subject: [PATCH] Fixes multithreading bug + core coverage by mutation testing is now 88% --- dummy/pom.xml | 14 ++- pom.xml | 2 +- state-machine-core/pom.xml | 7 +- .../ru/sberned/statemachine/StateMachine.java | 2 +- .../sberned/statemachine/StateRepository.java | 74 +++++++------- .../statemachine/StateMachineTests.java | 94 ++++++++++++++++++ .../StateMachineUnhandledMessagesTests.java | 98 ++++++++++++++++++- state-machine-samples/pom.xml | 2 +- .../state-machine-loading/pom.xml | 2 +- .../state-machine-samples-simple/pom.xml | 2 +- state-machine-starter/pom.xml | 2 +- 11 files changed, 249 insertions(+), 50 deletions(-) diff --git a/dummy/pom.xml b/dummy/pom.xml index 7e7a8b5..97c7b25 100644 --- a/dummy/pom.xml +++ b/dummy/pom.xml @@ -4,11 +4,23 @@ ru.sberned.statemachine spring-flow-state-machine - 1.1.0 + 1.1.1 dummy pom dummy Dummy module to be build latest + + + ru.sberned.statemachine + state-machine-loading + ${project.version} + + + ru.sberned.statemachine + state-machine-samples-simple + ${project.version} + + http://maven.apache.org diff --git a/pom.xml b/pom.xml index 61f1ea9..cc5f4d2 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ ru.sberned.statemachine spring-flow-state-machine - 1.1.0 + 1.1.1 pom spring flow state machine Spring state machine for entities, not for application diff --git a/state-machine-core/pom.xml b/state-machine-core/pom.xml index 94e1076..813c8f1 100644 --- a/state-machine-core/pom.xml +++ b/state-machine-core/pom.xml @@ -5,7 +5,7 @@ ru.sberned.statemachine spring-flow-state-machine - 1.1.0 + 1.1.1 state-machine-core jar @@ -63,6 +63,11 @@ + + org.pitest + pitest-maven + 1.2.2 + org.apache.maven.plugins maven-source-plugin 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 index 6f5aa00..7ebb6af 100644 --- 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, STATE, ID> { // To make @Transactional work @Autowired private StateMachine stateMachine = this; - private StateRepository stateRepository; + volatile private StateRepository 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 index 513574f..27905ec 100644 --- 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> anyAfter) { afterAllHandlers.addAll(anyAfter); } - private void setUnhandledMessageProcessor(UnhandledMessageProcessor unhandledMessageProcessor) { - this.unhandledMessageProcessor = unhandledMessageProcessor; - } - List> 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> 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> getBeforeAll() { @@ -61,28 +51,40 @@ UnhandledMessageProcessor getUnhandledMessageProcessor() { return unhandledMessageProcessor; } - private class Processors { - private List> beforeHandlers = new ArrayList<>(); - private List> afterHandlers = new ArrayList<>(); + private void setUnhandledMessageProcessor(UnhandledMessageProcessor unhandledMessageProcessor) { + this.unhandledMessageProcessor = unhandledMessageProcessor; + } - private List> getBeforeHandlers() { - return beforeHandlers; - } + @SuppressWarnings("unchecked") + public interface From, STATE, ID> { + To from(STATE... states); + } - private List> getAfterHandlers() { - return afterHandlers; - } + + @SuppressWarnings("unchecked") + public interface To, STATE, ID> { + CompleteTransition to(STATE... states); } + @SuppressWarnings("unchecked") + public interface CompleteTransition, STATE, ID> { + CompleteTransition before(BeforeTransition... handlers); + + CompleteTransition after(AfterTransition... handlers); + + From and(); + + StateRepository build(); + } - public static class StateRepositoryBuilder, STATE , ID> { + public static class StateRepositoryBuilder, STATE, ID> { private StateRepository stateRepository; private StateRepositoryBuilder() { stateRepository = new StateRepository<>(); } - public static , STATE , ID> StateRepositoryBuilder configure() { + public static , STATE, ID> StateRepositoryBuilder configure() { return new StateRepositoryBuilder<>(); } @@ -115,25 +117,17 @@ public From defineTransitions() { } } - @SuppressWarnings("unchecked") - public interface From, STATE, ID> { - To from(STATE... states); - } - - @SuppressWarnings("unchecked") - public interface To, STATE, ID> { - CompleteTransition to(STATE... states); - } - - @SuppressWarnings("unchecked") - public interface CompleteTransition, STATE, ID> { - CompleteTransition before(BeforeTransition... handlers); - - CompleteTransition after(AfterTransition... handlers); + private class Processors { + private List> beforeHandlers = new ArrayList<>(); + private List> afterHandlers = new ArrayList<>(); - From and(); + private List> getBeforeHandlers() { + return beforeHandlers; + } - StateRepository build(); + private List> getAfterHandlers() { + return afterHandlers; + } } public class StateTransition implements To, From, CompleteTransition { 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 index 612cd45..80db324 100644 --- 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 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 stateHolder = StateRepositoryBuilder.configure() @@ -99,6 +108,80 @@ public void testCorrectStatesWithHandlersInOrder() throws Exception { inOrder.verify(afterTransition2, times(1)).afterTransition(item); } + @Test + public void noActionsOnAbsentBefore() throws Exception { + StateRepository stateHolder = StateRepositoryBuilder.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 stateHolder = StateRepositoryBuilder.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 stateHolder = StateRepositoryBuilder.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 stateHolder = StateRepositoryBuilder.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 stateHolder = StateRepositoryBuilder.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 stateHolder = StateRepositoryBuilder.configure() + .setAvailableStates(Collections.singleton(CustomState.START)) + .defineTransitions() + .from(CustomState.START) + .to(CustomState.STATE1) + .build(); + stateMachine.setStateRepository(stateHolder); + } + @Test public void testConflictingEventsLeadToOnlyOneStateChange() throws InterruptedException { StateRepository stateHolder = StateRepositoryBuilder.configure() @@ -172,6 +255,17 @@ public void testExecutionResultSuccess() throws ExecutionException, InterruptedE assertTrue(results.get("1").get()); } + @Test + public void shoudReturnEmptyMapOnNullOrEmptyEvents() throws ExecutionException, InterruptedException { + StateRepository stateHolder = getDefaultTransition(); + + stateMachine.setStateRepository(stateHolder); + Map> 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 stateHolder = getDefaultTransition(); diff --git a/state-machine-core/src/test/java/ru/sberned/statemachine/StateMachineUnhandledMessagesTests.java b/state-machine-core/src/test/java/ru/sberned/statemachine/StateMachineUnhandledMessagesTests.java index 14f8eb8..7a45b7c 100644 --- a/state-machine-core/src/test/java/ru/sberned/statemachine/StateMachineUnhandledMessagesTests.java +++ b/state-machine-core/src/test/java/ru/sberned/statemachine/StateMachineUnhandledMessagesTests.java @@ -8,6 +8,7 @@ import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.context.ApplicationEventPublisher; import org.springframework.test.context.junit4.SpringRunner; +import ru.sberned.statemachine.processor.UnableToProcessException; import ru.sberned.statemachine.processor.UnhandledMessageProcessor; import ru.sberned.statemachine.state.StateChangedEvent; import ru.sberned.statemachine.state.StateChanger; @@ -17,11 +18,14 @@ import java.util.Collections; import java.util.EnumSet; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; import static org.mockito.Mockito.*; import static ru.sberned.statemachine.processor.UnhandledMessageProcessor.IssueType.*; -import static ru.sberned.statemachine.util.CustomState.STATE1; -import static ru.sberned.statemachine.util.CustomState.STATE2; +import static ru.sberned.statemachine.util.CustomState.*; /** * Created by Evgeniya Patuk (jpatuk@gmail.com) on 17/06/2017. @@ -79,6 +83,23 @@ public void testUnhandledMessageProcessorTimeout() throws InterruptedException { verify(processor, timeout(3000).times(1)).process("1", STATE2, TIMEOUT, null); } + @Test + public void testInterruptedException() throws InterruptedException { + StateRepository stateHolder = StateRepository.StateRepositoryBuilder.configure() + .setAvailableStates(EnumSet.allOf(CustomState.class)) + .setUnhandledMessageProcessor(processor) + .defineTransitions() + .from(CustomState.START) + .to(STATE1) + .build(); + StateMachine stateMachine = new StateMachine<>(stateProvider, (state, item, infos) -> { + }, key -> new ErroringLockProvider()); + stateMachine.setStateRepository(stateHolder); + Map dummy = stateMachine.changeState(Collections.singletonList("1"), STATE1, "dummy"); + + verify(processor, timeout(2000).times(1)).process(eq("1"), eq(STATE1), eq(INTERRUPTED_EXCEPTION), any(InterruptedException.class)); + } + @Test public void testUnhandledMessageProcessorInvalidState() throws InterruptedException { StateRepository stateHolder = getDefaultTransition(processor); @@ -89,6 +110,14 @@ public void testUnhandledMessageProcessorInvalidState() throws InterruptedExcept verify(processor, timeout(1500).times(1)).process("1", STATE2, INVALID_TRANSITION, null); } + @Test(expected = NullPointerException.class) + public void shouldFailIfCalledDirectlyWithNullEvent() throws InterruptedException { + StateRepository stateHolder = getDefaultTransition(processor); + + stateMachine.setStateRepository(stateHolder); + stateMachine.handleStateChanged(null); + } + @Test public void testUnhandledMessageProcessorExecutionException() throws InterruptedException { StateRepository stateHolder = getDefaultTransition(processor); @@ -102,6 +131,40 @@ public void testUnhandledMessageProcessorExecutionException() throws Interrupted verify(processor, timeout(1500).times(1)).process("1", STATE1, EXECUTION_EXCEPTION, ex); } + @Test + public void shouldFailIfBeforeFails() throws InterruptedException { + StateRepository stateHolder = StateRepository.StateRepositoryBuilder.configure() + .setAvailableStates(EnumSet.of(CustomState.START, CustomState.FINISH)) + .setUnhandledMessageProcessor(processor) + .defineTransitions() + .from(CustomState.START) + .to(CustomState.FINISH) + .before(item -> false) + .build(); + stateMachine.setStateRepository(stateHolder); + + publisher.publishEvent(new StateChangedEvent("1", FINISH)); + + verify(processor, timeout(1500).times(1)).process(eq("1"), eq(FINISH), eq(EXECUTION_EXCEPTION), any(UnableToProcessException.class)); + } + + @Test + public void shouldFailIfBeforeAnyFails() throws InterruptedException { + StateRepository stateHolder = StateRepository.StateRepositoryBuilder.configure() + .setAvailableStates(EnumSet.of(CustomState.START, CustomState.FINISH)) + .setUnhandledMessageProcessor(processor) + .setAnyBefore((item, customState) -> false) + .defineTransitions() + .from(CustomState.START) + .to(CustomState.FINISH) + .build(); + stateMachine.setStateRepository(stateHolder); + + publisher.publishEvent(new StateChangedEvent("1", FINISH)); + + verify(processor, timeout(1500).times(1)).process(eq("1"), eq(FINISH), eq(EXECUTION_EXCEPTION), any(UnableToProcessException.class)); + } + @Test public void testStateNotPresentInStateHolder() throws InterruptedException { StateRepository stateHolder = StateRepository.StateRepositoryBuilder.configure() @@ -118,4 +181,35 @@ public void testStateNotPresentInStateHolder() throws InterruptedException { verify(processor, timeout(1500).times(1)).process("1", STATE2, INVALID_TRANSITION, null); } + + private static class ErroringLockProvider implements Lock { + @Override + public void lock() { + } + + @Override + public void lockInterruptibly() throws InterruptedException { + throw new InterruptedException(); + } + + @Override + public boolean tryLock() { + return false; + } + + @Override + public boolean tryLock(long time, TimeUnit unit) throws InterruptedException { + throw new InterruptedException(); + } + + @Override + public void unlock() { + + } + + @Override + public Condition newCondition() { + return null; + } + } } diff --git a/state-machine-samples/pom.xml b/state-machine-samples/pom.xml index 43ff750..6541e5e 100644 --- a/state-machine-samples/pom.xml +++ b/state-machine-samples/pom.xml @@ -4,7 +4,7 @@ ru.sberned.statemachine spring-flow-state-machine - 1.1.0 + 1.1.1 state-machine-samples pom diff --git a/state-machine-samples/state-machine-loading/pom.xml b/state-machine-samples/state-machine-loading/pom.xml index d8039bf..a03e4d4 100644 --- a/state-machine-samples/state-machine-loading/pom.xml +++ b/state-machine-samples/state-machine-loading/pom.xml @@ -5,7 +5,7 @@ ru.sberned.statemachine state-machine-samples - 1.1.0 + 1.1.1 state-machine-loading spring flow state machine loading sample diff --git a/state-machine-samples/state-machine-samples-simple/pom.xml b/state-machine-samples/state-machine-samples-simple/pom.xml index cbd5442..58fb816 100644 --- a/state-machine-samples/state-machine-samples-simple/pom.xml +++ b/state-machine-samples/state-machine-samples-simple/pom.xml @@ -5,7 +5,7 @@ ru.sberned.statemachine state-machine-samples - 1.1.0 + 1.1.1 state-machine-samples-simple diff --git a/state-machine-starter/pom.xml b/state-machine-starter/pom.xml index 302fb02..af01174 100644 --- a/state-machine-starter/pom.xml +++ b/state-machine-starter/pom.xml @@ -5,7 +5,7 @@ ru.sberned.statemachine spring-flow-state-machine - 1.1.0 + 1.1.1 spring-flow-state-machine-starter