From 043d2a75314e879a83f9a32b760d19f5808c66f9 Mon Sep 17 00:00:00 2001 From: George Vagenas Date: Sun, 25 Apr 2021 13:53:34 +0100 Subject: [PATCH] properly set the bean name for factory generated state machines - testing: added test case for dynamic generated id - docs: update documentation - fix: Support @WithStateMachine for machines generated using either empty id or dynamically generated id - Fixes #940 --- docs/src/reference/asciidoc/sm-context.adoc | 5 + .../config/ObjectStateMachineFactory.java | 7 + .../StateMachineHandlerCallHelper.java | 37 +++-- ...nnotationWithDefaultsWithFactoryTests.java | 144 ++++++++++++++++++ 4 files changed, 179 insertions(+), 14 deletions(-) create mode 100644 spring-statemachine-core/src/test/java/org/springframework/statemachine/annotation/MethodAnnotationWithDefaultsWithFactoryTests.java diff --git a/docs/src/reference/asciidoc/sm-context.adoc b/docs/src/reference/asciidoc/sm-context.adoc index 894cf88f..92de0bc3 100644 --- a/docs/src/reference/asciidoc/sm-context.adoc +++ b/docs/src/reference/asciidoc/sm-context.adoc @@ -48,6 +48,11 @@ include::samples/DocsConfigurationSampleTests4.java[tags=snippetAAAA] ---- ==== +When using StateMachineFactory to generate state machines the state machine using dynamic provided `id`, bean name will default to `stateMachine` it's not possible to use `@WithStateMachine (id = "some-id")` since `id` is only known at runtime. + +In such a cases, use either `@WithStateMachine` or `@WithStateMachine(name = "stateMachine")` and all state machines generated by the factory will be atached to your bean or beans. + + You can also use `@WithStateMachine` as a meta-annotation, as shown in the preceding example. In this case, you could annotate your bean with `WithMyBean`. The following example shows how to do so: diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/ObjectStateMachineFactory.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/ObjectStateMachineFactory.java index 05c2bce7..42ebe199 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/ObjectStateMachineFactory.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/ObjectStateMachineFactory.java @@ -26,6 +26,7 @@ import org.springframework.statemachine.ExtendedState; import org.springframework.statemachine.ObjectStateMachine; import org.springframework.statemachine.StateContext; import org.springframework.statemachine.StateMachine; +import org.springframework.statemachine.StateMachineSystemConstants; import org.springframework.statemachine.config.model.StateMachineModel; import org.springframework.statemachine.config.model.StateMachineModelFactory; import org.springframework.statemachine.region.Region; @@ -86,6 +87,12 @@ public class ObjectStateMachineFactory extends AbstractStateMachineFactory machine.setBeanFactory(beanFactory); } if (machine instanceof BeanNameAware) { + //When using StateMachineFactory.getStateMachine() to generate state machine, + //which means name and id are null + //in that case set name to the default `stateMachine` + if ((machineId == null || machineId.isEmpty()) && (beanName == null || beanName.isEmpty())) { + beanName = StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE; + } ((BeanNameAware)machine).setBeanName(beanName); } return machine; diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/processor/StateMachineHandlerCallHelper.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/processor/StateMachineHandlerCallHelper.java index 56b3edd2..f4e1cbf6 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/processor/StateMachineHandlerCallHelper.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/processor/StateMachineHandlerCallHelper.java @@ -32,6 +32,7 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.statemachine.StateContext; +import org.springframework.statemachine.StateMachineSystemConstants; import org.springframework.statemachine.annotation.OnEventNotAccepted; import org.springframework.statemachine.annotation.OnExtendedStateChanged; import org.springframework.statemachine.annotation.OnStateChanged; @@ -92,10 +93,10 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be // don't check name if id is set as name defaults to // 'stateMachine' and would cause additional cache entry if (StringUtils.hasText(withStateMachine.id())) { - updateCache(metaAnnotation.annotationType().getName() + withStateMachine.id(), + updateCache(metaAnnotation.annotationType().getName() +"_"+ withStateMachine.id(), new CacheEntry(handler, annotation, metaAnnotation)); } else if (StringUtils.hasText(withStateMachine.name())) { - updateCache(metaAnnotation.annotationType().getName() + withStateMachine.name(), + updateCache(metaAnnotation.annotationType().getName() +"_"+ withStateMachine.name(), new CacheEntry(handler, annotation, metaAnnotation)); } } @@ -118,7 +119,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnStateChanged.class.getName() + stateMachineId; + String cacheKey = OnStateChanged.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -138,7 +139,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnStateEntry.class.getName() + stateMachineId; + String cacheKey = OnStateEntry.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -158,7 +159,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnStateExit.class.getName() + stateMachineId; + String cacheKey = OnStateExit.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -178,7 +179,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnEventNotAccepted.class.getName() + stateMachineId; + String cacheKey = OnEventNotAccepted.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -202,7 +203,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnTransitionStart.class.getName() + stateMachineId; + String cacheKey = OnTransitionStart.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -222,7 +223,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnTransition.class.getName() + stateMachineId; + String cacheKey = OnTransition.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -242,7 +243,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnTransitionEnd.class.getName() + stateMachineId; + String cacheKey = OnTransitionEnd.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -262,7 +263,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnStateMachineStart.class.getName() + stateMachineId; + String cacheKey = OnStateMachineStart.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -278,7 +279,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnStateMachineStop.class.getName() + stateMachineId; + String cacheKey = OnStateMachineStop.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -294,7 +295,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnStateMachineError.class.getName() + stateMachineId; + String cacheKey = OnStateMachineError.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -310,7 +311,7 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be return; } List> handlersList = new ArrayList>(); - String cacheKey = OnExtendedStateChanged.class.getName() + stateMachineId; + String cacheKey = OnExtendedStateChanged.class.getName() +"_"+ stateMachineId; List list = getCacheEntries(cacheKey); if (list == null) { return; @@ -347,7 +348,15 @@ public class StateMachineHandlerCallHelper implements InitializingBean, Be } } } - return cache.get(cacheKey); + //Try to get the CacheEntry using the provided key + //Or use default machine name in the key + if (cache.containsKey(cacheKey)) { + return cache.get(cacheKey); + } else { + cacheKey = cacheKey.replaceFirst(cacheKey.substring(cacheKey.indexOf("_")+1), + StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE); + return cache.get(cacheKey); + } } private boolean annotationHandlerVariableMatch(Annotation annotation, Object key) { diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/annotation/MethodAnnotationWithDefaultsWithFactoryTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/annotation/MethodAnnotationWithDefaultsWithFactoryTests.java new file mode 100644 index 00000000..6db4dcfa --- /dev/null +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/annotation/MethodAnnotationWithDefaultsWithFactoryTests.java @@ -0,0 +1,144 @@ +/* + * Copyright 2017-2020 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 + * + * https://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.annotation; + +import org.junit.jupiter.api.Test; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.statemachine.AbstractStateMachineTests; +import org.springframework.statemachine.StateMachine; +import org.springframework.statemachine.StateMachineSystemConstants; +import org.springframework.statemachine.config.EnableStateMachineFactory; +import org.springframework.statemachine.config.EnumStateMachineConfigurerAdapter; +import org.springframework.statemachine.config.StateMachineFactory; +import org.springframework.statemachine.config.builders.StateMachineStateConfigurer; +import org.springframework.statemachine.config.builders.StateMachineTransitionConfigurer; + +import java.util.EnumSet; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.statemachine.TestUtils.doSendEventAndConsumeAll; +import static org.springframework.statemachine.TestUtils.doStartAndAssert; +import static org.springframework.statemachine.TestUtils.resolveFactory; + +public class MethodAnnotationWithDefaultsWithFactoryTests extends AbstractStateMachineTests { + + @Test + public void testMethodAnnotations() throws Exception { + context.register(BeanConfig1.class, Config1.class); + context.refresh(); + + Bean1 bean1 = context.getBean(Bean1.class); + Bean2 bean2 = context.getBean(Bean2.class); + + StateMachineFactory factory = resolveFactory(context); + StateMachine machine = factory.getStateMachine(); + doStartAndAssert(machine); + + assertThat(machine.getState().getIds()).containsExactly(TestStates.S1); + doSendEventAndConsumeAll(machine, TestEvents.E1); + assertThat(machine.getState().getIds()).containsExactly(TestStates.S2); + assertThat(bean1.counter).isEqualTo(1); + assertThat(bean2.onStateChangedLatch.await(1, TimeUnit.SECONDS)).isTrue(); + } + + @Test + public void testMethodAnnotationsWithDynamicId() throws Exception { + context.register(BeanConfig1.class, Config1.class); + context.refresh(); + + Bean1 bean1 = context.getBean(Bean1.class); + Bean2 bean2 = context.getBean(Bean2.class); + + String id = String.valueOf(System.currentTimeMillis()); + + StateMachineFactory factory = resolveFactory(context); + StateMachine machine = factory.getStateMachine(id); + doStartAndAssert(machine); + + assertThat(machine.getState().getIds()).containsExactly(TestStates.S1); + doSendEventAndConsumeAll(machine, TestEvents.E1); + assertThat(machine.getState().getIds()).containsExactly(TestStates.S2); + assertThat(bean1.counter).isEqualTo(1); + assertThat(bean2.onStateChangedLatch.await(1, TimeUnit.SECONDS)).isTrue(); + } + + @WithStateMachine + static class Bean1 { + + int counter = 0; + + @OnStateChanged (source = "S1", target = "S2") + public void onStateChanged() { + counter++; + } + } + + @WithStateMachine(id = StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE) + static class Bean2 { + + CountDownLatch onStateChangedLatch = new CountDownLatch(1); + + @OnStateChanged + public void onStateChanged() { + onStateChangedLatch.countDown(); + } + } + + @Configuration + static class BeanConfig1 { + + @Bean + public Bean1 bean1() { + return new Bean1(); + } + + @Bean + public Bean2 bean2() { + return new Bean2(); + } + } + + @Configuration + @EnableStateMachineFactory + static class Config1 extends EnumStateMachineConfigurerAdapter { + + @Override + public void configure(StateMachineStateConfigurer states) throws Exception { + states + .withStates() + .initial(TestStates.S1) + .states(EnumSet.allOf(TestStates.class)); + } + + @Override + public void configure(StateMachineTransitionConfigurer transitions) throws Exception { + transitions + .withExternal() + .source(TestStates.S1) + .target(TestStates.S2) + .event(TestEvents.E1); + } + } + + @Override + protected AnnotationConfigApplicationContext buildContext() { + return new AnnotationConfigApplicationContext(); + } +}