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 fc187c78c..dfb68c908 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 @@ -18,7 +18,6 @@ import io.nflow.engine.internal.dao.PollingRaceConditionException; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.PeriodicLogger; -import io.nflow.engine.service.WorkflowDefinitionService; @Component @SuppressFBWarnings(value = "MDM_RANDOM_SEED", justification = "rand does not need to be secure") @@ -35,7 +34,6 @@ public class WorkflowDispatcher implements Runnable { private final WorkflowInstanceExecutor executor; private final WorkflowInstanceDao workflowInstances; private final WorkflowStateProcessorFactory stateProcessorFactory; - private final WorkflowDefinitionService workflowDefinitions; private final ExecutorDao executorDao; private final long sleepTimeMillis; private final int stuckThreadThresholdSeconds; @@ -44,12 +42,10 @@ public class WorkflowDispatcher implements Runnable { @Inject @SuppressFBWarnings(value = "WEM_WEAK_EXCEPTION_MESSAGING", justification = "Transaction support exception message is fine") public WorkflowDispatcher(WorkflowInstanceExecutor executor, WorkflowInstanceDao workflowInstances, - WorkflowStateProcessorFactory stateProcessorFactory, WorkflowDefinitionService workflowDefinitions, ExecutorDao executorDao, - Environment env) { + WorkflowStateProcessorFactory stateProcessorFactory, ExecutorDao executorDao, Environment env) { this.executor = executor; this.workflowInstances = workflowInstances; this.stateProcessorFactory = stateProcessorFactory; - this.workflowDefinitions = workflowDefinitions; this.executorDao = executorDao; this.sleepTimeMillis = env.getRequiredProperty("nflow.dispatcher.sleep.ms", Long.class); this.stuckThreadThresholdSeconds = env.getRequiredProperty("nflow.executor.stuckThreadThreshold.seconds", Integer.class); @@ -63,7 +59,6 @@ public WorkflowDispatcher(WorkflowInstanceExecutor executor, WorkflowInstanceDao public void run() { logger.info("Starting."); try { - workflowDefinitions.postProcessWorkflowDefinitions(); running = true; while (!shutdownRequested) { if (paused) { 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 a3b3646c1..45b8fabec 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 @@ -27,14 +27,12 @@ public class WorkflowDefinitionService { private volatile Map> workflowDefinitions = new LinkedHashMap<>(); private final WorkflowDefinitionDao workflowDefinitionDao; - private final boolean persistWorkflowDefinitions; - private final boolean autoInit; + private final boolean autoPersistDefinitions; @Inject public WorkflowDefinitionService(WorkflowDefinitionDao workflowDefinitionDao, Environment env) { this.workflowDefinitionDao = workflowDefinitionDao; - this.persistWorkflowDefinitions = env.getRequiredProperty("nflow.definition.persist", Boolean.class); - this.autoInit = env.getRequiredProperty("nflow.autoinit", Boolean.class); + this.autoPersistDefinitions = env.getRequiredProperty("nflow.definition.autopersist", Boolean.class); } /** @@ -58,18 +56,19 @@ public List> getWorkflowDefi } /** - * 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. + * Persist all loaded workflow definitions to database if nflow.definition.autopersist is false. If nflow.definition.autopersist + * is true, definitions are persisted automatically when they are added to managed definitions. */ - public void postProcessWorkflowDefinitions() { - if (!autoInit && persistWorkflowDefinitions) { + public void persistWorkflowDefinitions() { + if (!autoPersistDefinitions) { workflowDefinitions.values().forEach(workflowDefinitionDao::storeWorkflowDefinition); } } /** - * Add given workflow definition to managed definitions. Persist given definition if nflow.autoinit and nflow.definition.persist - * are true. + * Add given workflow definition to managed definitions. Persist given definition to database if nflow.definition.autopersist is + * true. If nflow.definition.autopersist is false, call persistWorkflowDefinitions manually if needed to persist the + * definitions. * * @param wd * The workflow definition to be added. @@ -86,7 +85,7 @@ public void addWorkflowDefinition(AbstractWorkflowDefinition new WorkflowDispatcher(executor, workflowInstances, executorFactory, workflowDefinitions, executorDao, env)); + assertThrows(BeanCreationException.class, + () -> new WorkflowDispatcher(executor, workflowInstances, executorFactory, executorDao, env)); } @Test @@ -252,7 +249,7 @@ class ExceptionOnPoolShutdownIsNotPropagated extends MultithreadedTestCase { @Override public void initialize() { poolSpy = Mockito.spy(executor); - dispatcher = new WorkflowDispatcher(poolSpy, workflowInstances, executorFactory, workflowDefinitions, executorDao, env); + dispatcher = new WorkflowDispatcher(poolSpy, workflowInstances, executorFactory, executorDao, env); } public void threadDispatcher() { 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 4d655e0c6..aa8a32403 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 @@ -34,18 +34,16 @@ public class WorkflowDefinitionServiceTest extends BaseNflowTest { @BeforeEach public void setup() { lenient().when(workflowDefinition.getType()).thenReturn("dummy"); + initializeService(true); } - 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); + private void initializeService(boolean autoPersistDefinition) { + when(env.getRequiredProperty("nflow.definition.autopersist", Boolean.class)).thenReturn(autoPersistDefinition); service = new WorkflowDefinitionService(workflowDefinitionDao, env); } @Test - public void addedDefinitionIsStoredWhenAutoInitIsTrue() { - initializeService(true, true); - + public void addedDefinitionIsStoredWhenAutoPersistDefinitionIsTrue() { service.addWorkflowDefinition(workflowDefinition); verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); @@ -53,8 +51,8 @@ public void addedDefinitionIsStoredWhenAutoInitIsTrue() { } @Test - public void addedDefinitionIsNotStoredWhenAutoInitIsFalse() { - initializeService(true, false); + public void addedDefinitionIsNotStoredWhenAutoPersistDefinitionIsFalse() { + initializeService(false); service.addWorkflowDefinition(workflowDefinition); @@ -63,53 +61,31 @@ public void addedDefinitionIsNotStoredWhenAutoInitIsFalse() { } @Test - public void addedDefinitionIsNotStoredWhenDefinitionPersistIsFalse() { - initializeService(false, true); - + public void persistWorkflowDefinitionStoresDefinitionsWhenAutoPersistDefinitionIsFalse() { + initializeService(false); service.addWorkflowDefinition(workflowDefinition); - verifyZeroInteractions(workflowDefinitionDao); - assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); - } - - @Test - public void definitionsAreStoredDuringPostProcessingWhenAutoInitIsFalse() { - initializeService(true, false); - service.addWorkflowDefinition(workflowDefinition); - - service.postProcessWorkflowDefinitions(); + service.persistWorkflowDefinitions(); verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); } @Test - public void definitionsAreNotStoredDuringPostProcessingWhenAutoInitIsTrue() { - initializeService(true, true); + public void persistWorkflowDefinitionDoesNotStoreDefinitionsWhenAutoPersistDefinitionIsTrue() { service.addWorkflowDefinition(workflowDefinition); verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); - service.postProcessWorkflowDefinitions(); + service.persistWorkflowDefinitions(); 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); + verify(workflowDefinitionDao).storeWorkflowDefinition(workflowDefinition); IllegalStateException thrown = assertThrows(IllegalStateException.class, () -> service.addWorkflowDefinition(workflowDefinition)); @@ -118,22 +94,19 @@ public void addingDuplicatDefinitionThrowsException() { assertThat(thrown.getMessage(), containsString("Both " + className + " and " + className + " define same workflow type: dummy")); assertThat(service.getWorkflowDefinitions().size(), is(equalTo(1))); - } - - @Test - public void getWorkflowDefinitionReturnsNullWhenTypeIsNotFound() { - initializeService(true, true); - - assertThat(service.getWorkflowDefinition("notFound"), is(nullValue())); + verifyNoMoreInteractions(workflowDefinitionDao); } @Test public void getWorkflowDefinitionReturnsDefinitionWhenTypeIsFound() { - initializeService(true, true); - service.addWorkflowDefinition(workflowDefinition); assertThat(service.getWorkflowDefinition("dummy"), is(instanceOf(DummyTestWorkflow.class))); } + @Test + public void getWorkflowDefinitionReturnsNullWhenTypeIsNotFound() { + assertThat(service.getWorkflowDefinition("dummy"), is(nullValue())); + } + } 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 e09a8642e..9fda588ef 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 @@ -57,7 +57,7 @@ static class ContextConfiguration { @Bean @Primary public Environment env() { - return new MockEnvironment().withProperty("nflow.definition.persist", "true").withProperty("nflow.autoinit", "true"); + return new MockEnvironment().withProperty("nflow.definition.autopersist", "true"); } @Bean diff --git a/nflow-netty/src/test/java/io/nflow/netty/StartNflowTest.java b/nflow-netty/src/test/java/io/nflow/netty/StartNflowTest.java index 308d942c6..831b63f47 100644 --- a/nflow-netty/src/test/java/io/nflow/netty/StartNflowTest.java +++ b/nflow-netty/src/test/java/io/nflow/netty/StartNflowTest.java @@ -30,7 +30,7 @@ public void startNflowNetty() throws Exception { Map properties = new HashMap<>(); properties.put("nflow.db.create_on_startup", false); properties.put("nflow.autostart", false); - properties.put("nflow.autoinit", false); + properties.put("nflow.definition.autopersist", false); ApplicationContext ctx = startNflow.startNetty(7500, "local", "", properties); assertNotNull(testListener.applicationContextEvent); @@ -39,7 +39,7 @@ public void startNflowNetty() throws Exception { assertEquals("externallyDefinedExecutorGroup", ctx.getEnvironment().getProperty("nflow.executor.group")); assertEquals("false", ctx.getEnvironment().getProperty("nflow.db.create_on_startup")); assertEquals("false", ctx.getEnvironment().getProperty("nflow.autostart")); - assertEquals("false", ctx.getEnvironment().getProperty("nflow.autoinit")); + assertEquals("false", ctx.getEnvironment().getProperty("nflow.definition.autopersist")); } } diff --git a/nflow-tests/src/test/java/io/nflow/tests/SkipAutoStartTest.java b/nflow-tests/src/test/java/io/nflow/tests/SkipAutoStartTest.java index f34f23bc0..0d3f5ee23 100644 --- a/nflow-tests/src/test/java/io/nflow/tests/SkipAutoStartTest.java +++ b/nflow-tests/src/test/java/io/nflow/tests/SkipAutoStartTest.java @@ -2,26 +2,28 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; -import io.nflow.tests.extension.NflowServerConfig; -import io.nflow.tests.extension.NflowServerExtension; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.extension.ExtendWith; +import io.nflow.tests.extension.NflowServerConfig; +import io.nflow.tests.extension.NflowServerExtension; + @ExtendWith(NflowServerExtension.class) @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class SkipAutoStartTest extends AbstractNflowTest { - // When nflow.autoinit, nflow.autostart and nflow.db.create_on_startup are false - // no database access should happen. This test fails if SQL statements are - // issued during bean initialization. - public static NflowServerConfig server = new NflowServerConfig.Builder() - .prop("nflow.autoinit", "false") - .prop("nflow.autostart", "false") - .prop("nflow.db.create_on_startup", "false") - .build(); + /** + * When nflow.definition.autopersist, nflow.autostart and nflow.db.create_on_startup are false no database access should happen. + * This test fails if SQL statements are issued during bean initialization. + */ + public static NflowServerConfig server = new NflowServerConfig.Builder() // + .prop("nflow.definition.autopersist", "false") // + .prop("nflow.autostart", "false") // + .prop("nflow.db.create_on_startup", "false") // + .build(); public SkipAutoStartTest() { super(server);