From b8f9a28e66c8664210dcced5958779a5fe1ffeca Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Sun, 2 Jun 2019 21:07:25 +0300 Subject: [PATCH 1/9] separate definition scanning from definition service --- .../java/io/nflow/engine/NflowEngine.java | 2 +- .../internal/executor/WorkflowDispatcher.java | 5 -- .../internal/executor/WorkflowLifecycle.java | 3 +- .../WorkflowDefinitionClassNameScanner.java | 54 +++++++++++++ .../service/WorkflowDefinitionService.java | 56 ++----------- .../WorkflowDefinitionSpringBeanScanner.java | 35 +++++++++ .../executor/WorkflowLifecycleTest.java | 3 +- ...orkflowDefinitionClassNameScannerTest.java | 34 ++++++++ .../WorkflowDefinitionServiceTest.java | 78 +++++++++++-------- ...rkflowDefinitionServiceWithSpringTest.java | 18 ++++- ...rkflowDefinitionSpringBeanScannerTest.java | 28 +++++++ 11 files changed, 221 insertions(+), 95 deletions(-) create mode 100644 nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java create mode 100644 nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java create mode 100644 nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java create mode 100644 nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java diff --git a/nflow-engine/src/main/java/io/nflow/engine/NflowEngine.java b/nflow-engine/src/main/java/io/nflow/engine/NflowEngine.java index 2795f776b..b9b5418ad 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/NflowEngine.java +++ b/nflow-engine/src/main/java/io/nflow/engine/NflowEngine.java @@ -62,7 +62,7 @@ public NflowEngine(DataSource dataSource, SQLVariants sqlVariants, workflowLifecycle = ctx.getBean(WorkflowLifecycle.class); workflowDefinitionService = ctx.getBean(WorkflowDefinitionService.class); - workflowDefinitionService.setWorkflowDefinitions(workflowDefinitions); + workflowDefinitions.forEach(workflowDefinitionService::addWorkflowDefinition); archiveService = ctx.getBean(ArchiveService.class); healthCheckService = ctx.getBean(HealthCheckService.class); diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java index e0680828f..79c954d9b 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java @@ -2,7 +2,6 @@ import static org.slf4j.LoggerFactory.getLogger; -import java.io.IOException; import java.util.List; import java.util.Random; import java.util.concurrent.CountDownLatch; @@ -76,7 +75,6 @@ public void run() { } else { try { executor.waitUntilQueueSizeLowerThanThreshold(executorDao.getMaxWaitUntil()); - if (!shutdownRequested) { if (executorDao.tick()) { workflowInstances.recoverWorkflowInstancesFromDeadNodes(); @@ -98,9 +96,6 @@ public void run() { } } } - - } catch (IOException | ReflectiveOperationException e) { - logger.error("Fetching workflow definitions failed", e); } finally { running = false; shutdownPool(); diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java index 739a4a247..901e8dc29 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java @@ -2,7 +2,6 @@ import static org.slf4j.LoggerFactory.getLogger; -import java.io.IOException; import java.util.concurrent.ThreadFactory; import javax.inject.Inject; @@ -26,7 +25,7 @@ public class WorkflowLifecycle implements SmartLifecycle { @Inject public WorkflowLifecycle(WorkflowDefinitionService workflowDefinitions, WorkflowDispatcher dispatcher, - @NFlow ThreadFactory nflowThreadFactory, Environment env) throws IOException, ReflectiveOperationException { + @NFlow ThreadFactory nflowThreadFactory, Environment env) { this.dispatcher = dispatcher; this.workflowDefinitions = workflowDefinitions; if (env.getRequiredProperty("nflow.autoinit", Boolean.class)) { diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java new file mode 100644 index 000000000..c196674b5 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java @@ -0,0 +1,54 @@ +package io.nflow.engine.service; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.slf4j.LoggerFactory.getLogger; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; + +import javax.inject.Inject; + +import org.slf4j.Logger; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.io.AbstractResource; +import org.springframework.stereotype.Component; + +import io.nflow.engine.config.NFlow; +import io.nflow.engine.workflow.definition.AbstractWorkflowDefinition; +import io.nflow.engine.workflow.definition.WorkflowState; + +/** + * Service for managing workflow definitions. + */ +@Component +public class WorkflowDefinitionClassNameScanner { + + private static final Logger logger = getLogger(WorkflowDefinitionClassNameScanner.class); + + private final WorkflowDefinitionService workflowDefinitionService; + + @Inject + public WorkflowDefinitionClassNameScanner(WorkflowDefinitionService workflowDefinitionService) { + this.workflowDefinitionService = workflowDefinitionService; + } + + @Autowired(required = false) + public void setWorkflowDefinitions(@NFlow AbstractResource classNameListing) throws IOException, ReflectiveOperationException { + if (classNameListing == null) { + logger.info("No non-Spring workflow definitions"); + } else { + try (BufferedReader br = new BufferedReader(new InputStreamReader(classNameListing.getInputStream(), UTF_8))) { + String row; + while ((row = br.readLine()) != null) { + logger.info("Preparing workflow {}", row); + @SuppressWarnings("unchecked") + Class> clazz = (Class>) Class + .forName(row); + workflowDefinitionService.addWorkflowDefinition(clazz.getDeclaredConstructor().newInstance()); + } + } + } + } + +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java index f79d2a2ba..99ae4b56c 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java @@ -1,13 +1,8 @@ package io.nflow.engine.service; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.slf4j.LoggerFactory.getLogger; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStreamReader; import java.util.ArrayList; -import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -15,12 +10,9 @@ import javax.inject.Inject; import org.slf4j.Logger; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.env.Environment; -import org.springframework.core.io.AbstractResource; import org.springframework.stereotype.Component; -import io.nflow.engine.config.NFlow; import io.nflow.engine.internal.dao.WorkflowDefinitionDao; import io.nflow.engine.workflow.definition.AbstractWorkflowDefinition; import io.nflow.engine.workflow.definition.WorkflowState; @@ -33,31 +25,17 @@ public class WorkflowDefinitionService { private static final Logger logger = getLogger(WorkflowDefinitionService.class); - private AbstractResource nonSpringWorkflowsListing; private final Map> workflowDefinitions = new LinkedHashMap<>(); private final WorkflowDefinitionDao workflowDefinitionDao; private final boolean persistWorkflowDefinitions; + private final boolean autoInit; + @Inject public WorkflowDefinitionService(WorkflowDefinitionDao workflowDefinitionDao, Environment env) { this.workflowDefinitionDao = workflowDefinitionDao; this.persistWorkflowDefinitions = env.getRequiredProperty("nflow.definition.persist", Boolean.class); - } - - /** - * Add given workflow definitions to the managed definitions. - * @param workflowDefinitions The workflow definitions to be added. - */ - @Autowired(required = false) - public void setWorkflowDefinitions(Collection> workflowDefinitions) { - for (AbstractWorkflowDefinition wd : workflowDefinitions) { - addWorkflowDefinition(wd); - } - } - - @Autowired(required = false) - public void setWorkflowDefinitions(@NFlow AbstractResource nflowNonSpringWorkflowsListing) { - this.nonSpringWorkflowsListing = nflowNonSpringWorkflowsListing; + this.autoInit = env.getRequiredProperty("nflow.autoinit", Boolean.class); } /** @@ -80,31 +58,10 @@ public List> getWorkflowDefi /** * Add workflow definitions from the nflowNonSpringWorkflowsListing resource and persist * all loaded workflow definitions. - * @throws IOException when workflow definitions can not be read from the resource. - * @throws ReflectiveOperationException when the workflow definition can not be instantiated. */ - public void postProcessWorkflowDefinitions() throws IOException, ReflectiveOperationException { - if (nonSpringWorkflowsListing == null) { - logger.info("No non-Spring workflow definitions"); - } else { - initNonSpringWorkflowDefinitions(); - } + public void postProcessWorkflowDefinitions() { if (persistWorkflowDefinitions) { - for (AbstractWorkflowDefinition definition : workflowDefinitions.values()) { - workflowDefinitionDao.storeWorkflowDefinition(definition); - } - } - } - - private void initNonSpringWorkflowDefinitions() throws IOException, ReflectiveOperationException { - try (BufferedReader br = new BufferedReader(new InputStreamReader(nonSpringWorkflowsListing.getInputStream(), UTF_8))) { - String row; - while ((row = br.readLine()) != null) { - logger.info("Preparing workflow {}", row); - @SuppressWarnings("unchecked") - Class> clazz = (Class>) Class.forName(row); - addWorkflowDefinition(clazz.getDeclaredConstructor().newInstance()); - } + workflowDefinitions.values().forEach(workflowDefinitionDao::storeWorkflowDefinition); } } @@ -114,6 +71,9 @@ public void addWorkflowDefinition(AbstractWorkflowDefinition> workflowDefinitions) { + workflowDefinitions.forEach(workflowDefinitionService::addWorkflowDefinition); + } + +} diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java index cd1c4ddd3..a140efb3e 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java @@ -7,7 +7,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.io.IOException; import java.util.concurrent.ThreadFactory; import org.junit.jupiter.api.BeforeEach; @@ -36,7 +35,7 @@ public class WorkflowLifecycleTest { private WorkflowLifecycle lifecycle; @BeforeEach - public void setup() throws IOException, ReflectiveOperationException { + public void setup() { when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(TRUE); when(env.getRequiredProperty("nflow.autostart", Boolean.class)).thenReturn(TRUE); when(threadFactory.newThread(dispatcher)).thenReturn(dispatcherThread); diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java new file mode 100644 index 000000000..f0cfb56e5 --- /dev/null +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java @@ -0,0 +1,34 @@ +package io.nflow.engine.service; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.ByteArrayInputStream; + +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.springframework.core.io.ClassPathResource; + +import io.nflow.engine.internal.executor.BaseNflowTest; + +public class WorkflowDefinitionClassNameScannerTest extends BaseNflowTest { + + @Mock + private ClassPathResource nonSpringWorkflowListing; + @Mock + private WorkflowDefinitionService workflowDefinitionService; + private WorkflowDefinitionClassNameScanner scanner; + + @Test + public void definitionIsAdded() throws Exception { + String dummyTestClassname = DummyTestWorkflow.class.getName(); + ByteArrayInputStream bis = new ByteArrayInputStream(dummyTestClassname.getBytes(UTF_8)); + when(nonSpringWorkflowListing.getInputStream()).thenReturn(bis); + scanner = new WorkflowDefinitionClassNameScanner(workflowDefinitionService); + scanner.setWorkflowDefinitions(nonSpringWorkflowListing); + verify(workflowDefinitionService).addWorkflowDefinition(any(DummyTestWorkflow.class)); + } + +} diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java index d969c7d0e..49a34bb4c 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java @@ -1,69 +1,85 @@ package io.nflow.engine.service; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; -import java.io.ByteArrayInputStream; -import java.util.List; - import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.springframework.core.env.Environment; -import org.springframework.core.io.ClassPathResource; import io.nflow.engine.internal.dao.WorkflowDefinitionDao; import io.nflow.engine.internal.executor.BaseNflowTest; -import io.nflow.engine.workflow.definition.AbstractWorkflowDefinition; -import io.nflow.engine.workflow.definition.WorkflowState; public class WorkflowDefinitionServiceTest extends BaseNflowTest { - @Mock - private ClassPathResource nonSpringWorkflowListing; @Mock private WorkflowDefinitionDao workflowDefinitionDao; @Mock private Environment env; + @Mock + private DummyTestWorkflow workflowDefinition; private WorkflowDefinitionService service; @BeforeEach public void setup() throws Exception { when(env.getRequiredProperty("nflow.definition.persist", Boolean.class)).thenReturn(true); - String dummyTestClassname = DummyTestWorkflow.class.getName(); - ByteArrayInputStream bis = new ByteArrayInputStream(dummyTestClassname.getBytes(UTF_8)); - when(nonSpringWorkflowListing.getInputStream()).thenReturn(bis); + when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(true); + lenient().when(workflowDefinition.getType()).thenReturn("dummy"); service = new WorkflowDefinitionService(workflowDefinitionDao, env); - service.setWorkflowDefinitions(nonSpringWorkflowListing); - assertThat(service.getWorkflowDefinitions().size(), is(equalTo(0))); - service.postProcessWorkflowDefinitions(); + } + + @Test + public void addedDefinitionIsStoredWhenAutoinitIsTrue() { + service.addWorkflowDefinition(workflowDefinition); + + verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); + assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); + } + + @Test + public void addedDefinitionIsNotStoredWhenAutoinitIsFalse() { + when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(false); + service = new WorkflowDefinitionService(workflowDefinitionDao, env); + + service.addWorkflowDefinition(workflowDefinition); + + verifyZeroInteractions(workflowDefinitionDao); assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); - verify(workflowDefinitionDao).storeWorkflowDefinition(eq(service.getWorkflowDefinitions().get(0))); } @Test - public void initDuplicateWorkflows() throws Exception { - String dummyTestClassname = DummyTestWorkflow.class.getName(); - ByteArrayInputStream bis = new ByteArrayInputStream((dummyTestClassname + "\n" + dummyTestClassname).getBytes(UTF_8)); - when(nonSpringWorkflowListing.getInputStream()).thenReturn(bis); - IllegalStateException thrown = assertThrows(IllegalStateException.class, () -> service.postProcessWorkflowDefinitions()); - assertThat(thrown.getMessage(), containsString("Both io.nflow.engine.service.DummyTestWorkflow and io.nflow.engine.service.DummyTestWorkflow define same workflow type: dummy")); + public void definitionsAreStoredDuringPostProcessing() { + when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(false); + service = new WorkflowDefinitionService(workflowDefinitionDao, env); + service.addWorkflowDefinition(workflowDefinition); + + service.postProcessWorkflowDefinitions(); + + verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); + assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); } @Test - public void demoWorkflowLoadedSuccessfully() { - List> definitions = service.getWorkflowDefinitions(); - assertThat(definitions.size(), is(equalTo(1))); + public void addingDuplicatDefinitionThrowsException() { + service.addWorkflowDefinition(workflowDefinition); + + IllegalStateException thrown = assertThrows(IllegalStateException.class, + () -> service.addWorkflowDefinition(workflowDefinition)); + + String className = workflowDefinition.getClass().getName(); + assertThat(thrown.getMessage(), + containsString("Both " + className + " and " + className + " define same workflow type: dummy")); + assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); } @Test @@ -73,13 +89,9 @@ public void getWorkflowDefinitionReturnsNullWhenTypeIsNotFound() { @Test public void getWorkflowDefinitionReturnsDefinitionWhenTypeIsFound() { + service.addWorkflowDefinition(workflowDefinition); + assertThat(service.getWorkflowDefinition("dummy"), is(instanceOf(DummyTestWorkflow.class))); } - @Test - public void nonSpringWorkflowsAreOptional() throws Exception { - service = new WorkflowDefinitionService(workflowDefinitionDao, env); - service.postProcessWorkflowDefinitions(); - assertEquals(0, service.getWorkflowDefinitions().size()); - } } diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java index 5107e2400..98b9d0cd0 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java @@ -50,6 +50,8 @@ @DirtiesContext public class WorkflowDefinitionServiceWithSpringTest { + static WorkflowDefinitionService workflowDefinitionService; + @Configuration @Profile("nflow-engine-test") @ComponentScan(basePackageClasses = SpringDummyTestWorkflow.class) @@ -57,13 +59,13 @@ static class ContextConfiguration { @Bean @Primary public Environment env() { - return new MockEnvironment().withProperty("nflow.definition.persist", "true"); + return new MockEnvironment().withProperty("nflow.definition.persist", "true").withProperty("nflow.autoinit", "true"); } @Bean @NFlow public AbstractResource nflowNonSpringWorkflowsListing() { - return mock(AbstractResource.class); + return null; } @Bean @@ -145,15 +147,23 @@ public WorkflowInstanceFactory workflowInstanceFactory() { return mock(WorkflowInstanceFactory.class); } + @Bean + public WorkflowDefinitionService workflowDefinitionService(WorkflowDefinitionDao workflowDefinitionDao, Environment env) { + workflowDefinitionService = new WorkflowDefinitionService(workflowDefinitionDao, env); + return workflowDefinitionService; + } + } + @SuppressWarnings("unused") @Autowired - private WorkflowDefinitionService service; + private WorkflowDefinitionSpringBeanScanner scanner; @Test public void springWorkflowDefinitionsAreDetected() { - List> definitions = service.getWorkflowDefinitions(); + List> definitions = workflowDefinitionService.getWorkflowDefinitions(); assertThat(definitions.size(), is(equalTo(1))); assertThat(definitions.get(0).getType(), is(new SpringDummyTestWorkflow().getType())); } + } diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java new file mode 100644 index 000000000..ecc05d7c9 --- /dev/null +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java @@ -0,0 +1,28 @@ +package io.nflow.engine.service; + +import static java.util.Arrays.asList; +import static org.mockito.Mockito.verify; + +import org.junit.jupiter.api.Test; +import org.mockito.Mock; + +import io.nflow.engine.internal.executor.BaseNflowTest; + +public class WorkflowDefinitionSpringBeanScannerTest extends BaseNflowTest { + + @Mock + private WorkflowDefinitionService workflowDefinitionService; + @Mock + private DummyTestWorkflow definition; + private WorkflowDefinitionSpringBeanScanner scanner; + + @Test + public void definitionIsAdded() { + scanner = new WorkflowDefinitionSpringBeanScanner(workflowDefinitionService); + + scanner.setWorkflowDefinitions(asList(definition)); + + verify(workflowDefinitionService).addWorkflowDefinition(definition); + } + +} From cd98351f51bde926aec6ede35f5b30bf5986686a Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Sun, 2 Jun 2019 23:53:10 +0300 Subject: [PATCH 2/9] use constructor injection in WorkflowInstanceService --- .../engine/service/WorkflowInstanceService.java | 17 ++++------------- .../service/WorkflowInstanceServiceTest.java | 11 +++++++---- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowInstanceService.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowInstanceService.java index 6c24cf9a5..225b23eec 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowInstanceService.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowInstanceService.java @@ -32,24 +32,15 @@ public class WorkflowInstanceService { private static final Logger logger = getLogger(WorkflowInstanceService.class); - private WorkflowDefinitionService workflowDefinitionService; + private final WorkflowDefinitionService workflowDefinitionService; private final WorkflowInstanceDao workflowInstanceDao; - private WorkflowInstancePreProcessor workflowInstancePreProcessor; + private final WorkflowInstancePreProcessor workflowInstancePreProcessor; @Inject - public WorkflowInstanceService(WorkflowInstanceDao workflowInstanceDao) { + public WorkflowInstanceService(WorkflowInstanceDao workflowInstanceDao, WorkflowDefinitionService workflowDefinitionService, + WorkflowInstancePreProcessor workflowInstancePreProcessor) { this.workflowInstanceDao = workflowInstanceDao; - } - - // constructor injection won't work here - @Inject - public void setWorkflowDefinitionService(WorkflowDefinitionService workflowDefinitionService) { this.workflowDefinitionService = workflowDefinitionService; - } - - // constructor injection won't work here - @Inject - public void setWorkflowInstancePreProcessor(WorkflowInstancePreProcessor workflowInstancePreProcessor) { this.workflowInstancePreProcessor = workflowInstancePreProcessor; } diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowInstanceServiceTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowInstanceServiceTest.java index d279ffc95..d0c76a505 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowInstanceServiceTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowInstanceServiceTest.java @@ -15,7 +15,12 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.util.Collections; import java.util.List; @@ -61,9 +66,7 @@ public class WorkflowInstanceServiceTest extends BaseNflowTest { public void setup() { WorkflowDefinition dummyWorkflow = new DummyTestWorkflow(); lenient().doReturn(dummyWorkflow).when(workflowDefinitions).getWorkflowDefinition("dummy"); - service = new WorkflowInstanceService(workflowInstanceDao); - service.setWorkflowDefinitionService(workflowDefinitions); - service.setWorkflowInstancePreProcessor(workflowInstancePreProcessor); + service = new WorkflowInstanceService(workflowInstanceDao, workflowDefinitions, workflowInstancePreProcessor); setCurrentMillisFixed(currentTimeMillis()); } From 6f7110ba1868c8364cb127398e85cc8c1afe179f Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 3 Jun 2019 00:06:35 +0300 Subject: [PATCH 3/9] update javadocs --- .../service/WorkflowDefinitionClassNameScanner.java | 8 +++++++- .../nflow/engine/service/WorkflowDefinitionService.java | 8 ++++++-- .../service/WorkflowDefinitionSpringBeanScanner.java | 6 +++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java index c196674b5..a4970b83c 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java @@ -19,7 +19,7 @@ import io.nflow.engine.workflow.definition.WorkflowState; /** - * Service for managing workflow definitions. + * Register workflow definitions defined in the class name listing resource. */ @Component public class WorkflowDefinitionClassNameScanner { @@ -33,6 +33,12 @@ public WorkflowDefinitionClassNameScanner(WorkflowDefinitionService workflowDefi this.workflowDefinitionService = workflowDefinitionService; } + /** + * Register workflow definitions defined in the class name listing resource. + * @param classNameListing The resource containing the workflow definition class names. + * @throws IOException When reading the resource fails. + * @throws ReflectiveOperationException When creating a workflow definition instance fails. + */ @Autowired(required = false) public void setWorkflowDefinitions(@NFlow AbstractResource classNameListing) throws IOException, ReflectiveOperationException { if (classNameListing == null) { diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java index 99ae4b56c..43e1b1786 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java @@ -56,8 +56,7 @@ public List> getWorkflowDefi } /** - * Add workflow definitions from the nflowNonSpringWorkflowsListing resource and persist - * all loaded workflow definitions. + * Persist all loaded workflow definitions if needed. */ public void postProcessWorkflowDefinitions() { if (persistWorkflowDefinitions) { @@ -65,6 +64,11 @@ public void postProcessWorkflowDefinitions() { } } + /** + * Add given workflow definition to managed definitions. Persist given definition if needed. + * @param wd The workflow definition to be added. + * @throws IllegalStateException When a definition with the same type has already been added. + */ public void addWorkflowDefinition(AbstractWorkflowDefinition wd) { AbstractWorkflowDefinition conflict = workflowDefinitions.put(wd.getType(), wd); if (conflict != null) { diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java index 6e5b2e679..dd4586e5d 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java @@ -11,7 +11,7 @@ import io.nflow.engine.workflow.definition.WorkflowState; /** - * Service for managing workflow definitions. + * Register workflow definitions defined as Spring beans. */ @Component public class WorkflowDefinitionSpringBeanScanner { @@ -24,8 +24,8 @@ public WorkflowDefinitionSpringBeanScanner(WorkflowDefinitionService workflowDef } /** - * Add given workflow definitions to the managed definitions. - * @param workflowDefinitions The workflow definitions to be added. + * Register workflow definitions defined as Spring beans. + * @param workflowDefinitions The workflow definitions to be registered. */ @Autowired(required = false) public void setWorkflowDefinitions(Collection> workflowDefinitions) { From fd267cf4005eae7c4480c57b0908e4b7d6d131ab Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 3 Jun 2019 00:53:23 +0300 Subject: [PATCH 4/9] use constructor injection in WorkflowDefinitionSpringBeanScanner --- .../WorkflowDefinitionSpringBeanScanner.java | 15 ++------------- .../WorkflowDefinitionSpringBeanScannerTest.java | 5 +---- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java index dd4586e5d..f84794a37 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScanner.java @@ -4,7 +4,6 @@ import javax.inject.Inject; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import io.nflow.engine.workflow.definition.AbstractWorkflowDefinition; @@ -16,19 +15,9 @@ @Component public class WorkflowDefinitionSpringBeanScanner { - private final WorkflowDefinitionService workflowDefinitionService; - @Inject - public WorkflowDefinitionSpringBeanScanner(WorkflowDefinitionService workflowDefinitionService) { - this.workflowDefinitionService = workflowDefinitionService; - } - - /** - * Register workflow definitions defined as Spring beans. - * @param workflowDefinitions The workflow definitions to be registered. - */ - @Autowired(required = false) - public void setWorkflowDefinitions(Collection> workflowDefinitions) { + public WorkflowDefinitionSpringBeanScanner(WorkflowDefinitionService workflowDefinitionService, + Collection> workflowDefinitions) { workflowDefinitions.forEach(workflowDefinitionService::addWorkflowDefinition); } diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java index ecc05d7c9..2ed715940 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionSpringBeanScannerTest.java @@ -14,13 +14,10 @@ public class WorkflowDefinitionSpringBeanScannerTest extends BaseNflowTest { private WorkflowDefinitionService workflowDefinitionService; @Mock private DummyTestWorkflow definition; - private WorkflowDefinitionSpringBeanScanner scanner; @Test public void definitionIsAdded() { - scanner = new WorkflowDefinitionSpringBeanScanner(workflowDefinitionService); - - scanner.setWorkflowDefinitions(asList(definition)); + new WorkflowDefinitionSpringBeanScanner(workflowDefinitionService, asList(definition)); verify(workflowDefinitionService).addWorkflowDefinition(definition); } From 061e8e19ad0aa68d5f9ad91b7305f750a0942ca9 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 3 Jun 2019 01:07:47 +0300 Subject: [PATCH 5/9] use constructor injection in WorkflowDefinitionClassNameScanner --- .../WorkflowDefinitionClassNameScanner.java | 18 +++--------------- ...WorkflowDefinitionClassNameScannerTest.java | 14 +++++++++++--- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java index a4970b83c..cca0980f8 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionClassNameScanner.java @@ -7,10 +7,10 @@ import java.io.IOException; import java.io.InputStreamReader; +import javax.annotation.Nullable; import javax.inject.Inject; import org.slf4j.Logger; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.io.AbstractResource; import org.springframework.stereotype.Component; @@ -26,21 +26,9 @@ public class WorkflowDefinitionClassNameScanner { private static final Logger logger = getLogger(WorkflowDefinitionClassNameScanner.class); - private final WorkflowDefinitionService workflowDefinitionService; - @Inject - public WorkflowDefinitionClassNameScanner(WorkflowDefinitionService workflowDefinitionService) { - this.workflowDefinitionService = workflowDefinitionService; - } - - /** - * Register workflow definitions defined in the class name listing resource. - * @param classNameListing The resource containing the workflow definition class names. - * @throws IOException When reading the resource fails. - * @throws ReflectiveOperationException When creating a workflow definition instance fails. - */ - @Autowired(required = false) - public void setWorkflowDefinitions(@NFlow AbstractResource classNameListing) throws IOException, ReflectiveOperationException { + public WorkflowDefinitionClassNameScanner(WorkflowDefinitionService workflowDefinitionService, + @Nullable @NFlow AbstractResource classNameListing) throws IOException, ReflectiveOperationException { if (classNameListing == null) { logger.info("No non-Spring workflow definitions"); } else { diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java index f0cfb56e5..0c28d4d50 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionClassNameScannerTest.java @@ -3,6 +3,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; @@ -19,16 +20,23 @@ public class WorkflowDefinitionClassNameScannerTest extends BaseNflowTest { private ClassPathResource nonSpringWorkflowListing; @Mock private WorkflowDefinitionService workflowDefinitionService; - private WorkflowDefinitionClassNameScanner scanner; @Test public void definitionIsAdded() throws Exception { String dummyTestClassname = DummyTestWorkflow.class.getName(); ByteArrayInputStream bis = new ByteArrayInputStream(dummyTestClassname.getBytes(UTF_8)); when(nonSpringWorkflowListing.getInputStream()).thenReturn(bis); - scanner = new WorkflowDefinitionClassNameScanner(workflowDefinitionService); - scanner.setWorkflowDefinitions(nonSpringWorkflowListing); + + new WorkflowDefinitionClassNameScanner(workflowDefinitionService, nonSpringWorkflowListing); + verify(workflowDefinitionService).addWorkflowDefinition(any(DummyTestWorkflow.class)); } + @Test + public void listingResourceIsOptional() throws Exception { + new WorkflowDefinitionClassNameScanner(workflowDefinitionService, null); + + verifyZeroInteractions(workflowDefinitionService); + } + } From a9a9e6e38f759be09cc455dbfec4158f0659e355 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 3 Jun 2019 11:54:07 +0300 Subject: [PATCH 6/9] move definition persisting logic completely to workflow definition service --- .../internal/executor/WorkflowDispatcher.java | 6 +- .../internal/executor/WorkflowLifecycle.java | 11 +--- .../service/WorkflowDefinitionService.java | 27 +++++--- .../executor/WorkflowLifecycleTest.java | 7 +-- .../WorkflowDefinitionServiceTest.java | 62 ++++++++++++++++--- ...rkflowDefinitionServiceWithSpringTest.java | 13 ++-- 6 files changed, 76 insertions(+), 50 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java index 79c954d9b..fc187c78c 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java @@ -40,7 +40,6 @@ public class WorkflowDispatcher implements Runnable { private final long sleepTimeMillis; private final int stuckThreadThresholdSeconds; private final Random rand = new Random(); - private final boolean autoInit; @Inject @SuppressFBWarnings(value = "WEM_WEAK_EXCEPTION_MESSAGING", justification = "Transaction support exception message is fine") @@ -54,7 +53,6 @@ public WorkflowDispatcher(WorkflowInstanceExecutor executor, WorkflowInstanceDao this.executorDao = executorDao; this.sleepTimeMillis = env.getRequiredProperty("nflow.dispatcher.sleep.ms", Long.class); this.stuckThreadThresholdSeconds = env.getRequiredProperty("nflow.executor.stuckThreadThreshold.seconds", Integer.class); - this.autoInit = env.getRequiredProperty("nflow.autoinit", Boolean.class); if (!executorDao.isTransactionSupportEnabled()) { throw new BeanCreationException("Transaction support must be enabled"); @@ -65,9 +63,7 @@ public WorkflowDispatcher(WorkflowInstanceExecutor executor, WorkflowInstanceDao public void run() { logger.info("Starting."); try { - if (!autoInit) { - workflowDefinitions.postProcessWorkflowDefinitions(); - } + workflowDefinitions.postProcessWorkflowDefinitions(); running = true; while (!shutdownRequested) { if (paused) { diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java index 901e8dc29..c7b278b98 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowLifecycle.java @@ -12,27 +12,18 @@ import org.springframework.stereotype.Component; import io.nflow.engine.config.NFlow; -import io.nflow.engine.service.WorkflowDefinitionService; @Component public class WorkflowLifecycle implements SmartLifecycle { private static final Logger logger = getLogger(WorkflowLifecycle.class); - private final WorkflowDefinitionService workflowDefinitions; private final WorkflowDispatcher dispatcher; private final boolean autoStart; private final Thread dispatcherThread; @Inject - public WorkflowLifecycle(WorkflowDefinitionService workflowDefinitions, WorkflowDispatcher dispatcher, - @NFlow ThreadFactory nflowThreadFactory, Environment env) { + public WorkflowLifecycle(WorkflowDispatcher dispatcher, @NFlow ThreadFactory nflowThreadFactory, Environment env) { this.dispatcher = dispatcher; - this.workflowDefinitions = workflowDefinitions; - if (env.getRequiredProperty("nflow.autoinit", Boolean.class)) { - this.workflowDefinitions.postProcessWorkflowDefinitions(); - } else { - logger.info("nFlow engine autoinit disabled (system property nflow.autoinit=false)"); - } autoStart = env.getRequiredProperty("nflow.autostart", Boolean.class); dispatcherThread = nflowThreadFactory.newThread(dispatcher); dispatcherThread.setName("nflow-dispatcher"); diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java index 43e1b1786..eb07ebd3b 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java @@ -30,7 +30,6 @@ public class WorkflowDefinitionService { private final boolean persistWorkflowDefinitions; private final boolean autoInit; - @Inject public WorkflowDefinitionService(WorkflowDefinitionDao workflowDefinitionDao, Environment env) { this.workflowDefinitionDao = workflowDefinitionDao; @@ -40,7 +39,9 @@ public WorkflowDefinitionService(WorkflowDefinitionDao workflowDefinitionDao, En /** * Return the workflow definition that matches the give workflow type name. - * @param type Workflow definition type. + * + * @param type + * Workflow definition type. * @return The workflow definition or null if not found. */ public AbstractWorkflowDefinition getWorkflowDefinition(String type) { @@ -49,6 +50,7 @@ public AbstractWorkflowDefinition getWorkflowDefinition(String type) { /** * Return all managed workflow definitions. + * * @return List of workflow definitions. */ public List> getWorkflowDefinitions() { @@ -56,28 +58,33 @@ public List> getWorkflowDefi } /** - * Persist all loaded workflow definitions if needed. + * Persist all loaded workflow definitions if nflow.autoinit is false and nflow.definition.persist is true. If nflow.autoinit is + * true, definitions are persisted when they are added to managed definitions. */ public void postProcessWorkflowDefinitions() { - if (persistWorkflowDefinitions) { + if (!autoInit && persistWorkflowDefinitions) { workflowDefinitions.values().forEach(workflowDefinitionDao::storeWorkflowDefinition); } } /** - * Add given workflow definition to managed definitions. Persist given definition if needed. - * @param wd The workflow definition to be added. - * @throws IllegalStateException When a definition with the same type has already been added. + * Add given workflow definition to managed definitions. Persist given definition if nflow.autoinit and nflow.definition.persist + * are true. + * + * @param wd + * The workflow definition to be added. + * @throws IllegalStateException + * When a definition with the same type has already been added. */ public void addWorkflowDefinition(AbstractWorkflowDefinition wd) { AbstractWorkflowDefinition conflict = workflowDefinitions.put(wd.getType(), wd); if (conflict != null) { - throw new IllegalStateException("Both " + wd.getClass().getName() + " and " + conflict.getClass().getName() + - " define same workflow type: " + wd.getType()); + throw new IllegalStateException("Both " + wd.getClass().getName() + " and " + conflict.getClass().getName() + + " define same workflow type: " + wd.getType()); } if (autoInit && persistWorkflowDefinitions) { workflowDefinitionDao.storeWorkflowDefinition(wd); } - logger.info("Added workflow type: {} ({})", wd.getType(), wd.getClass().getName()); + logger.info("Added workflow type: {} ({})", wd.getType(), wd.getClass().getName()); } } diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java index a140efb3e..e5207c7cc 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowLifecycleTest.java @@ -16,8 +16,6 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.env.Environment; -import io.nflow.engine.service.WorkflowDefinitionService; - @ExtendWith(MockitoExtension.class) public class WorkflowLifecycleTest { @@ -29,17 +27,14 @@ public class WorkflowLifecycleTest { private Environment env; @Mock private Thread dispatcherThread; - @Mock - private WorkflowDefinitionService workflowDefinitions; private WorkflowLifecycle lifecycle; @BeforeEach public void setup() { - when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(TRUE); when(env.getRequiredProperty("nflow.autostart", Boolean.class)).thenReturn(TRUE); when(threadFactory.newThread(dispatcher)).thenReturn(dispatcherThread); - lifecycle = new WorkflowLifecycle(workflowDefinitions, dispatcher, threadFactory, env); + lifecycle = new WorkflowLifecycle(dispatcher, threadFactory, env); } @Test diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java index 49a34bb4c..4d655e0c6 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; @@ -31,15 +32,20 @@ public class WorkflowDefinitionServiceTest extends BaseNflowTest { private WorkflowDefinitionService service; @BeforeEach - public void setup() throws Exception { - when(env.getRequiredProperty("nflow.definition.persist", Boolean.class)).thenReturn(true); - when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(true); + public void setup() { lenient().when(workflowDefinition.getType()).thenReturn("dummy"); + } + + private void initializeService(boolean definitionPersist, boolean autoInit) { + when(env.getRequiredProperty("nflow.definition.persist", Boolean.class)).thenReturn(definitionPersist); + when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(autoInit); service = new WorkflowDefinitionService(workflowDefinitionDao, env); } @Test - public void addedDefinitionIsStoredWhenAutoinitIsTrue() { + public void addedDefinitionIsStoredWhenAutoInitIsTrue() { + initializeService(true, true); + service.addWorkflowDefinition(workflowDefinition); verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); @@ -47,9 +53,8 @@ public void addedDefinitionIsStoredWhenAutoinitIsTrue() { } @Test - public void addedDefinitionIsNotStoredWhenAutoinitIsFalse() { - when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(false); - service = new WorkflowDefinitionService(workflowDefinitionDao, env); + public void addedDefinitionIsNotStoredWhenAutoInitIsFalse() { + initializeService(true, false); service.addWorkflowDefinition(workflowDefinition); @@ -58,9 +63,18 @@ public void addedDefinitionIsNotStoredWhenAutoinitIsFalse() { } @Test - public void definitionsAreStoredDuringPostProcessing() { - when(env.getRequiredProperty("nflow.autoinit", Boolean.class)).thenReturn(false); - service = new WorkflowDefinitionService(workflowDefinitionDao, env); + public void addedDefinitionIsNotStoredWhenDefinitionPersistIsFalse() { + initializeService(false, true); + + service.addWorkflowDefinition(workflowDefinition); + + verifyZeroInteractions(workflowDefinitionDao); + assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); + } + + @Test + public void definitionsAreStoredDuringPostProcessingWhenAutoInitIsFalse() { + initializeService(true, false); service.addWorkflowDefinition(workflowDefinition); service.postProcessWorkflowDefinitions(); @@ -69,8 +83,32 @@ public void definitionsAreStoredDuringPostProcessing() { assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); } + @Test + public void definitionsAreNotStoredDuringPostProcessingWhenAutoInitIsTrue() { + initializeService(true, true); + service.addWorkflowDefinition(workflowDefinition); + verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); + + service.postProcessWorkflowDefinitions(); + + verifyNoMoreInteractions(workflowDefinitionDao); + assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); + } + + @Test + public void definitionsAreNotStoredDuringPostProcessingWhenDefinitionPersistIsFalse() { + initializeService(false, false); + service.addWorkflowDefinition(workflowDefinition); + + service.postProcessWorkflowDefinitions(); + + verifyZeroInteractions(workflowDefinitionDao); + assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); + } + @Test public void addingDuplicatDefinitionThrowsException() { + initializeService(true, true); service.addWorkflowDefinition(workflowDefinition); IllegalStateException thrown = assertThrows(IllegalStateException.class, @@ -84,11 +122,15 @@ public void addingDuplicatDefinitionThrowsException() { @Test public void getWorkflowDefinitionReturnsNullWhenTypeIsNotFound() { + initializeService(true, true); + assertThat(service.getWorkflowDefinition("notFound"), is(nullValue())); } @Test public void getWorkflowDefinitionReturnsDefinitionWhenTypeIsFound() { + initializeService(true, true); + service.addWorkflowDefinition(workflowDefinition); assertThat(service.getWorkflowDefinition("dummy"), is(instanceOf(DummyTestWorkflow.class))); diff --git a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java index 98b9d0cd0..e09a8642e 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/service/WorkflowDefinitionServiceWithSpringTest.java @@ -50,8 +50,6 @@ @DirtiesContext public class WorkflowDefinitionServiceWithSpringTest { - static WorkflowDefinitionService workflowDefinitionService; - @Configuration @Profile("nflow-engine-test") @ComponentScan(basePackageClasses = SpringDummyTestWorkflow.class) @@ -147,21 +145,18 @@ public WorkflowInstanceFactory workflowInstanceFactory() { return mock(WorkflowInstanceFactory.class); } - @Bean - public WorkflowDefinitionService workflowDefinitionService(WorkflowDefinitionDao workflowDefinitionDao, Environment env) { - workflowDefinitionService = new WorkflowDefinitionService(workflowDefinitionDao, env); - return workflowDefinitionService; - } - } + @Autowired + private WorkflowDefinitionService service; + @SuppressWarnings("unused") @Autowired private WorkflowDefinitionSpringBeanScanner scanner; @Test public void springWorkflowDefinitionsAreDetected() { - List> definitions = workflowDefinitionService.getWorkflowDefinitions(); + List> definitions = service.getWorkflowDefinitions(); assertThat(definitions.size(), is(equalTo(1))); assertThat(definitions.get(0).getType(), is(new SpringDummyTestWorkflow().getType())); } From f6155b12c0023eef8e6119bb72f2be0479501fe4 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 3 Jun 2019 15:03:09 +0300 Subject: [PATCH 7/9] synchronize access to workflowDefinitions --- .../engine/service/WorkflowDefinitionService.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java index eb07ebd3b..a3b3646c1 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java +++ b/nflow-engine/src/main/java/io/nflow/engine/service/WorkflowDefinitionService.java @@ -25,7 +25,7 @@ public class WorkflowDefinitionService { private static final Logger logger = getLogger(WorkflowDefinitionService.class); - private final Map> workflowDefinitions = new LinkedHashMap<>(); + private volatile Map> workflowDefinitions = new LinkedHashMap<>(); private final WorkflowDefinitionDao workflowDefinitionDao; private final boolean persistWorkflowDefinitions; private final boolean autoInit; @@ -77,10 +77,14 @@ public void postProcessWorkflowDefinitions() { * When a definition with the same type has already been added. */ public void addWorkflowDefinition(AbstractWorkflowDefinition wd) { - AbstractWorkflowDefinition conflict = workflowDefinitions.put(wd.getType(), wd); - if (conflict != null) { - throw new IllegalStateException("Both " + wd.getClass().getName() + " and " + conflict.getClass().getName() - + " define same workflow type: " + wd.getType()); + synchronized (this) { + Map> newDefinitions = new LinkedHashMap<>(workflowDefinitions); + AbstractWorkflowDefinition conflict = newDefinitions.put(wd.getType(), wd); + if (conflict != null) { + throw new IllegalStateException("Both " + wd.getClass().getName() + " and " + conflict.getClass().getName() + + " define same workflow type: " + wd.getType()); + } + workflowDefinitions = newDefinitions; } if (autoInit && persistWorkflowDefinitions) { workflowDefinitionDao.storeWorkflowDefinition(wd); From ccf5842a0266f1353ded8c5a0ae9a82d5204b72b Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Thu, 6 Jun 2019 22:25:49 +0300 Subject: [PATCH 8/9] update changelog --- CHANGELOG.md | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 384f67572..f306540f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,12 @@ -## 5.7.1-SNAPSHOT (future release) +## 6.0.0-SNAPSHOT (future release) **Highlights** +- Use constructor injection instead of field or setter injection in nFlow classes +- Separate workflow definition scanning from `WorkflowDefinitionService` **Details** +- `nflow-engine` + - Separate workflow definition scanning from `WorkflowDefinitionService` by introducing `WorkflowDefinitionSpringBeanScanner` and `WorkflowDefinitionClassNameScanner`. This allows breaking the circular dependency when a workflow definition uses `WorkflowInstanceService` (which depends on `WorkflowDefinitionService`, which depended on all workflow definitions). This enabled using constructor injection in all nFlow classes. ## 5.7.0 (2019-06-06) @@ -20,20 +24,20 @@ - Moved default implementations for `WorkflowExecutorListener` interface methods from the abstract class to the interface. - Fixed bug with dropping non-existent index in PostgreSQL create script. - Dependency updates: - - reactor.netty 0.8.8.RELEASE - - jetty 9.4.18.v20190429 - - javassist 3.25.0-GA - - mysql-connector-java 8.0.16 - - mssql-jdbc 7.2.2.jre8 - - metrics 4.1.0 - - spring 5.1.7.RELEASE - - hibernate.validator 6.0.15.Final - - cxf 3.3.2 - - joda-time 2.10.2 - - commons-lang3 3.9 - - jackson 2.9.9 - - junit 5.4.1 - - mockito 2.27.0 + - reactor.netty 0.8.8.RELEASE + - jetty 9.4.18.v20190429 + - javassist 3.25.0-GA + - mysql-connector-java 8.0.16 + - mssql-jdbc 7.2.2.jre8 + - metrics 4.1.0 + - spring 5.1.7.RELEASE + - hibernate.validator 6.0.15.Final + - cxf 3.3.2 + - joda-time 2.10.2 + - commons-lang3 3.9 + - jackson 2.9.9 + - junit 5.4.1 + - mockito 2.27.0 - `nflow-explorer` - Dependency updates From ce71de0e6c268240239545d5e0d62037d5eee757 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Thu, 6 Jun 2019 22:26:19 +0300 Subject: [PATCH 9/9] update version to 6.0.0-SNAPSHOT --- nflow-engine/pom.xml | 2 +- nflow-explorer/pom.xml | 2 +- nflow-jetty/pom.xml | 2 +- nflow-metrics/pom.xml | 2 +- nflow-netty/pom.xml | 2 +- nflow-perf-test/pom.xml | 2 +- nflow-rest-api-common/pom.xml | 2 +- nflow-rest-api-jax-rs/pom.xml | 2 +- nflow-rest-api-spring-web/pom.xml | 2 +- nflow-server-common/pom.xml | 2 +- nflow-tests/pom.xml | 2 +- pom.xml | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/nflow-engine/pom.xml b/nflow-engine/pom.xml index 5b222e244..618428c1e 100644 --- a/nflow-engine/pom.xml +++ b/nflow-engine/pom.xml @@ -13,7 +13,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT diff --git a/nflow-explorer/pom.xml b/nflow-explorer/pom.xml index db6bd9988..9dd757166 100644 --- a/nflow-explorer/pom.xml +++ b/nflow-explorer/pom.xml @@ -13,7 +13,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT .. diff --git a/nflow-jetty/pom.xml b/nflow-jetty/pom.xml index 44e30b4bf..715282438 100644 --- a/nflow-jetty/pom.xml +++ b/nflow-jetty/pom.xml @@ -13,7 +13,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT UTF-8 diff --git a/nflow-metrics/pom.xml b/nflow-metrics/pom.xml index c0b4ef924..e68127157 100644 --- a/nflow-metrics/pom.xml +++ b/nflow-metrics/pom.xml @@ -12,7 +12,7 @@ io.nflow nflow-root - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT diff --git a/nflow-netty/pom.xml b/nflow-netty/pom.xml index 88f55e46f..85d082ca8 100644 --- a/nflow-netty/pom.xml +++ b/nflow-netty/pom.xml @@ -13,7 +13,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT UTF-8 diff --git a/nflow-perf-test/pom.xml b/nflow-perf-test/pom.xml index de525d2a4..671f9720a 100644 --- a/nflow-perf-test/pom.xml +++ b/nflow-perf-test/pom.xml @@ -13,7 +13,7 @@ io.nflow nflow-root - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT diff --git a/nflow-rest-api-common/pom.xml b/nflow-rest-api-common/pom.xml index 5065fa8d6..fc2ee83aa 100644 --- a/nflow-rest-api-common/pom.xml +++ b/nflow-rest-api-common/pom.xml @@ -12,7 +12,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT diff --git a/nflow-rest-api-jax-rs/pom.xml b/nflow-rest-api-jax-rs/pom.xml index 6f0d66e37..0bfa59db0 100644 --- a/nflow-rest-api-jax-rs/pom.xml +++ b/nflow-rest-api-jax-rs/pom.xml @@ -12,7 +12,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT diff --git a/nflow-rest-api-spring-web/pom.xml b/nflow-rest-api-spring-web/pom.xml index 8a823d9b2..b4a31ff42 100644 --- a/nflow-rest-api-spring-web/pom.xml +++ b/nflow-rest-api-spring-web/pom.xml @@ -12,7 +12,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT diff --git a/nflow-server-common/pom.xml b/nflow-server-common/pom.xml index 7e8406467..efdb821ff 100644 --- a/nflow-server-common/pom.xml +++ b/nflow-server-common/pom.xml @@ -13,7 +13,7 @@ nflow-root io.nflow - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT UTF-8 diff --git a/nflow-tests/pom.xml b/nflow-tests/pom.xml index 528bf226e..9e307ca76 100644 --- a/nflow-tests/pom.xml +++ b/nflow-tests/pom.xml @@ -12,7 +12,7 @@ io.nflow nflow-root - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT diff --git a/pom.xml b/pom.xml index 4b416c283..b8fd3b96b 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ nflow-root pom nFlow Root - 5.7.1-SNAPSHOT + 6.0.0-SNAPSHOT http://nflow.io