From dd8c4ea665ccfe0a804ae4449cf2d2dbb8fa8b2b Mon Sep 17 00:00:00 2001 From: Patrick Ruckstuhl Date: Tue, 27 Feb 2018 11:28:12 +0100 Subject: [PATCH] Lookup needed configurer instead of injecting all configurers - Also lookup Configurer in StateMachineConfiguration - Added test scenario for constructor injection issue - Fixes #522 --- .../StateMachineConfiguration.java | 16 ++--- .../StateMachineFactoryConfiguration.java | 24 ++----- .../config/ConfigurationTests.java | 63 +++++++++++++++++++ 3 files changed, 71 insertions(+), 32 deletions(-) diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineConfiguration.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineConfiguration.java index a88e17b9..c468deb7 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineConfiguration.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineConfiguration.java @@ -155,18 +155,10 @@ public class StateMachineConfiguration extends @Override public void afterPropertiesSet() throws Exception { - // do not continue without configurers, it would not work - if (getConfigurers() == null || getConfigurers().size() == 0) { - throw new BeanDefinitionStoreException( - "Cannot configure state machine due to missing configurers. Did you remember to use " + - "@EnableStateMachine with a StateMachineConfigurerAdapter."); - } - for (AnnotationConfigurer, StateMachineConfigBuilder> configurer : getConfigurers()) { - Class clazz = configurer.getClass(); - if (ClassUtils.getUserClass(clazz).getName().equals(clazzName)) { - getBuilder().apply(configurer); - } - } + AnnotationConfigurer, StateMachineConfigBuilder> configurer = + (AnnotationConfigurer, StateMachineConfigBuilder>) getBeanFactory().getBean(Class.forName(clazzName)); + getBuilder().apply(configurer); + StateMachineConfig stateMachineConfig = getBuilder().getOrBuild(); TransitionsData stateMachineTransitions = stateMachineConfig.getTransitions(); StatesData stateMachineStates = stateMachineConfig.getStates(); diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineFactoryConfiguration.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineFactoryConfiguration.java index 2c3be23c..5e236a6b 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineFactoryConfiguration.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineFactoryConfiguration.java @@ -116,7 +116,6 @@ public class StateMachineFactoryConfiguration extends FactoryBean>, BeanFactoryAware, InitializingBean { private final StateMachineConfigBuilder builder; - private List, StateMachineConfigBuilder>> configurers; private BeanFactory beanFactory; private StateMachineFactory stateMachineFactory; private String clazzName; @@ -146,18 +145,10 @@ public class StateMachineFactoryConfiguration extends @Override public void afterPropertiesSet() throws Exception { - // do not continue without configurers, it would not work - if (configurers == null || configurers.size() == 0) { - throw new BeanDefinitionStoreException( - "Cannot configure state machine due to missing configurers. Did you remember to use " + - "@EnableStateMachineFactory with a StateMachineConfigurerAdapter."); - } - for (AnnotationConfigurer, StateMachineConfigBuilder> configurer : configurers) { - Class clazz = configurer.getClass(); - if (ClassUtils.getUserClass(clazz).getName().equals(clazzName)) { - builder.apply(configurer); - } - } + AnnotationConfigurer, StateMachineConfigBuilder> configurer = + (AnnotationConfigurer, StateMachineConfigBuilder>) beanFactory.getBean(Class.forName(clazzName)); + builder.apply(configurer); + StateMachineConfig stateMachineConfig = builder.getOrBuild(); TransitionsData stateMachineTransitions = stateMachineConfig.getTransitions(); StatesData stateMachineStates = stateMachineConfig.getStates(); @@ -188,13 +179,6 @@ public class StateMachineFactoryConfiguration extends this.beanFactory = beanFactory; } - @Autowired(required=false) - protected void onConfigurers( - List, StateMachineConfigBuilder>> configurers) - throws Exception { - this.configurers = configurers; - } - } } diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/config/ConfigurationTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/config/ConfigurationTests.java index 199304d4..9621dc29 100644 --- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/config/ConfigurationTests.java +++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/config/ConfigurationTests.java @@ -31,6 +31,8 @@ import java.util.List; import org.junit.Test; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -313,6 +315,19 @@ public class ConfigurationTests extends AbstractStateMachineTests { } } } + + @Test + public void testMachinesWithDependenciesAndConstructorInjection() { + context.register(Config21.class); + context.register(Config22.class); + context.refresh(); + @SuppressWarnings("unchecked") + StateMachineFactory stateMachineFactory22 = context.getBean("stateMachineConfig22", StateMachineFactory.class); + StateMachine stateMachine22 = stateMachineFactory22.getStateMachine(); + assertThat(stateMachine22, notNullValue()); + } + + @Configuration @EnableStateMachine @@ -918,4 +933,52 @@ public class ConfigurationTests extends AbstractStateMachineTests { .event("E1"); } } + + @Configuration + @EnableStateMachineFactory(name="stateMachineConfig21") + public static class Config21 extends StateMachineConfigurerAdapter { + @Override + public void configure(StateMachineStateConfigurer states) throws Exception { + states + .withStates() + .initial("S1") + .end("S2") + .end("S3"); + } + + @Override + public void configure(StateMachineTransitionConfigurer transitions) throws Exception { + transitions + .withExternal() + .source("S1") + .target("S2") + .event("E1"); + } + } + + @Configuration + @EnableStateMachineFactory(name="stateMachineConfig22") + public static class Config22 extends StateMachineConfigurerAdapter { + @Autowired + Config22(@Qualifier("stateMachineConfig21") StateMachineFactory otherStateMachine) { + } + + @Override + public void configure(StateMachineStateConfigurer states) throws Exception { + states + .withStates() + .initial("S1") + .end("S2") + .end("S3"); + } + + @Override + public void configure(StateMachineTransitionConfigurer transitions) throws Exception { + transitions + .withExternal() + .source("S1") + .target("S2") + .event("E1"); + } + } }