diff --git a/ordo-api/src/main/java/com/jetlumen/ordo/api/ActionHandler.java b/ordo-api/src/main/java/com/jetlumen/ordo/api/ActionHandler.java new file mode 100644 index 0000000..7385356 --- /dev/null +++ b/ordo-api/src/main/java/com/jetlumen/ordo/api/ActionHandler.java @@ -0,0 +1,16 @@ +package com.jetlumen.ordo.api; + +/** + * Executes a named action step against the running instance's context. Hosts supply a + * singleton implementation (same pattern as {@link RoutingCondition}); the database only stores + * the {@code actionKey} string. + */ +@FunctionalInterface +public interface ActionHandler { + void execute(String actionKey, ProcessContext context); + + static ActionHandler noop() { + return (key, context) -> { + }; + } +} diff --git a/ordo-api/src/main/java/com/jetlumen/ordo/api/ApprovalStep.java b/ordo-api/src/main/java/com/jetlumen/ordo/api/ApprovalStep.java index 0fef95e..2e0b062 100644 --- a/ordo-api/src/main/java/com/jetlumen/ordo/api/ApprovalStep.java +++ b/ordo-api/src/main/java/com/jetlumen/ordo/api/ApprovalStep.java @@ -6,28 +6,44 @@ import java.util.Objects; import java.util.Set; /** - * A single, named approval step in a process definition. A step has one or more candidate - * assignees; when there is more than one candidate, {@link #policy()} decides whether any - * single approval is enough ({@link ApprovalPolicy#ANY}) or every candidate must approve - * ({@link ApprovalPolicy#ALL}). + * A named step in a process definition. Approval steps have one or more candidate assignees; + * action steps have an {@code actionKey} invoked by the host {@link ActionHandler}. */ -public record ApprovalStep(String id, String name, List candidates, ApprovalPolicy policy) { +public record ApprovalStep(String id, String name, List candidates, ApprovalPolicy policy, StepKind kind, + String actionKey) { public ApprovalStep { requireText(id, "step id"); requireText(name, "step name"); + kind = kind == null ? StepKind.APPROVAL : kind; + Objects.requireNonNull(policy, "policy must not be null"); Objects.requireNonNull(candidates, "candidates must not be null"); candidates = List.copyOf(candidates); - if (candidates.isEmpty()) { - throw new IllegalArgumentException("a step must have at least one candidate"); - } - Set distinct = new HashSet<>(); - for (String candidate : candidates) { - requireText(candidate, "candidate"); - if (!distinct.add(candidate)) { - throw new IllegalArgumentException("duplicate candidate in step " + id + ": " + candidate); + if (kind == StepKind.ACTION) { + if (!candidates.isEmpty()) { + throw new IllegalArgumentException("an action step must not have candidates"); + } + requireText(actionKey, "action key"); + actionKey = actionKey.strip(); + } else { + if (actionKey != null && !actionKey.isBlank()) { + throw new IllegalArgumentException("an approval step must not have an action key"); + } + actionKey = null; + if (candidates.isEmpty()) { + throw new IllegalArgumentException("a step must have at least one candidate"); + } + Set distinct = new HashSet<>(); + for (String candidate : candidates) { + requireText(candidate, "candidate"); + if (!distinct.add(candidate)) { + throw new IllegalArgumentException("duplicate candidate in step " + id + ": " + candidate); + } } } - Objects.requireNonNull(policy, "policy must not be null"); + } + + public ApprovalStep(String id, String name, List candidates, ApprovalPolicy policy) { + this(id, name, candidates, policy, StepKind.APPROVAL, null); } /** Convenience factory for the common case of a single, fixed approver. */ @@ -35,6 +51,10 @@ public record ApprovalStep(String id, String name, List candidates, Appr return new ApprovalStep(id, name, List.of(assignee), ApprovalPolicy.ANY); } + public static ApprovalStep action(String id, String name, String actionKey) { + return new ApprovalStep(id, name, List.of(), ApprovalPolicy.ANY, StepKind.ACTION, actionKey); + } + static void requireText(String value, String field) { if (value == null || value.isBlank()) { throw new IllegalArgumentException(field + " must not be blank"); diff --git a/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinition.java b/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinition.java index 0b506ce..bd14f41 100644 --- a/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinition.java +++ b/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinition.java @@ -14,7 +14,7 @@ public record ProcessDefinition(String id, String name, List steps ApprovalStep.requireText(name, "definition name"); steps = List.copyOf(steps); if (steps.isEmpty()) { - throw new IllegalArgumentException("a definition must contain at least one approval step"); + throw new IllegalArgumentException("a definition must contain at least one step"); } Set ids = new HashSet<>(); for (ApprovalStep step : steps) { diff --git a/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinitionParser.java b/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinitionParser.java index 157b962..409634f 100644 --- a/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinitionParser.java +++ b/ordo-api/src/main/java/com/jetlumen/ordo/api/ProcessDefinitionParser.java @@ -37,7 +37,7 @@ public final class ProcessDefinitionParser { private static ProcessDefinition toDefinition(DefinitionDocument document) { if (document.steps() == null || document.steps().isEmpty()) { - throw new IllegalArgumentException("a definition must contain at least one approval step"); + throw new IllegalArgumentException("a definition must contain at least one step"); } List steps = new ArrayList<>(document.steps().size()); for (StepDocument step : document.steps()) { @@ -45,7 +45,9 @@ public final class ProcessDefinitionParser { throw new IllegalArgumentException("step must not be null"); } ApprovalPolicy policy = step.policy() == null ? ApprovalPolicy.ANY : step.policy(); - steps.add(new ApprovalStep(step.id(), step.name(), step.candidates(), policy)); + StepKind kind = step.kind() == null ? StepKind.APPROVAL : step.kind(); + List candidates = step.candidates() == null ? List.of() : step.candidates(); + steps.add(new ApprovalStep(step.id(), step.name(), candidates, policy, kind, step.action())); } rotateStartStep(steps, document.startStep()); List transitions = new ArrayList<>(); @@ -90,7 +92,13 @@ public final class ProcessDefinitionParser { List transitions) { } - private record StepDocument(String id, String name, List candidates, ApprovalPolicy policy) { + private record StepDocument( + String id, + String name, + List candidates, + ApprovalPolicy policy, + StepKind kind, + @JsonProperty("action") String action) { } private record TransitionDocument( diff --git a/ordo-api/src/main/java/com/jetlumen/ordo/api/StepKind.java b/ordo-api/src/main/java/com/jetlumen/ordo/api/StepKind.java new file mode 100644 index 0000000..616a7e5 --- /dev/null +++ b/ordo-api/src/main/java/com/jetlumen/ordo/api/StepKind.java @@ -0,0 +1,5 @@ +package com.jetlumen.ordo.api; + +public enum StepKind { + APPROVAL, ACTION +} diff --git a/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionParserTest.java b/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionParserTest.java index e945498..3e22279 100644 --- a/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionParserTest.java +++ b/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionParserTest.java @@ -153,4 +153,25 @@ class ProcessDefinitionParserTest { void rejectsMalformedJson() { assertThrows(IllegalArgumentException.class, () -> ProcessDefinitionParser.fromJson("{")); } + + @Test + void parsesActionStepsAndDefaultsKindToApproval() { + String json = """ + { + "id": "leave", + "name": "Leave request", + "steps": [ + { "id": "manager", "name": "Manager approval", "candidates": ["maria"] }, + { "id": "notify", "name": "Notify HR", "kind": "ACTION", "action": "leave-approved-mail" } + ], + "transitions": [ + { "from": "manager", "to": "notify" }, + { "from": "notify", "to": null } + ] + } + """; + ProcessDefinition definition = ProcessDefinitionParser.fromJson(json); + assertEquals(StepKind.APPROVAL, definition.steps().getFirst().kind()); + assertEquals(ApprovalStep.action("notify", "Notify HR", "leave-approved-mail"), definition.steps().get(1)); + } } diff --git a/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionTest.java b/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionTest.java index 0e6df42..2178b1e 100644 --- a/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionTest.java +++ b/ordo-api/src/test/java/com/jetlumen/ordo/api/ProcessDefinitionTest.java @@ -94,4 +94,28 @@ class ProcessDefinitionTest { StepTransition.end("hr"), StepTransition.always("manager", "hr")), definition.transitions()); } + + @Test + void rejectsActionStepsWithCandidatesOrBlankActionKey() { + assertThrows(IllegalArgumentException.class, + () -> new ApprovalStep("notify", "Notify", List.of("maria"), ApprovalPolicy.ANY, StepKind.ACTION, + "mail")); + assertThrows(IllegalArgumentException.class, () -> ApprovalStep.action("notify", "Notify", " ")); + } + + @Test + void rejectsApprovalStepsWithAnActionKey() { + assertThrows(IllegalArgumentException.class, + () -> new ApprovalStep("manager", "Manager", List.of("maria"), ApprovalPolicy.ANY, StepKind.APPROVAL, + "mail")); + } + + @Test + void acceptsADefinitionThatIsOnlyActionSteps() { + ProcessDefinition definition = new ProcessDefinition("notify", "Notify", List.of( + ApprovalStep.action("mail", "Send mail", "leave-approved-mail")), + List.of(StepTransition.end("mail"))); + assertEquals(StepKind.ACTION, definition.steps().getFirst().kind()); + assertEquals("leave-approved-mail", definition.steps().getFirst().actionKey()); + } } diff --git a/ordo-core/src/main/java/com/jetlumen/ordo/core/DefaultOrdoEngine.java b/ordo-core/src/main/java/com/jetlumen/ordo/core/DefaultOrdoEngine.java index f36ee28..43fc3b3 100644 --- a/ordo-core/src/main/java/com/jetlumen/ordo/core/DefaultOrdoEngine.java +++ b/ordo-core/src/main/java/com/jetlumen/ordo/core/DefaultOrdoEngine.java @@ -1,5 +1,6 @@ package com.jetlumen.ordo.core; +import com.jetlumen.ordo.api.ActionHandler; import com.jetlumen.ordo.api.ApprovalPolicy; import com.jetlumen.ordo.api.ApprovalStep; import com.jetlumen.ordo.api.ApprovalTask; @@ -10,6 +11,7 @@ import com.jetlumen.ordo.api.ProcessDefinition; import com.jetlumen.ordo.api.ProcessInstance; import com.jetlumen.ordo.api.ProcessStatus; import com.jetlumen.ordo.api.RoutingCondition; +import com.jetlumen.ordo.api.StepKind; import com.jetlumen.ordo.api.StepTransition; import com.jetlumen.ordo.api.TaskAction; import com.jetlumen.ordo.api.TaskStatus; @@ -30,6 +32,9 @@ import com.jetlumen.ordo.api.repository.ProcessInstanceRepository; import java.time.Clock; import java.time.Instant; +import java.lang.System.Logger; +import java.lang.System.Logger.Level; +import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Objects; @@ -44,22 +49,27 @@ import java.util.UUID; * when several JVMs share the same storage. */ public final class DefaultOrdoEngine implements OrdoEngine { + private static final int MAX_CONSECUTIVE_ACTIONS = 32; + private static final Logger LOG = System.getLogger("ordo"); + private final Clock clock; private final AssigneeResolver assigneeResolver; private final RoutingCondition routingCondition; + private final ActionHandler actionHandler; private final TransactionExecutor transactionExecutor; private final ProcessDefinitionRepository definitionRepository; private final ProcessInstanceRepository instanceRepository; private final ApprovalTaskRepository taskRepository; public DefaultOrdoEngine(Clock clock, AssigneeResolver assigneeResolver, RoutingCondition routingCondition, - TransactionExecutor transactionExecutor, + ActionHandler actionHandler, TransactionExecutor transactionExecutor, ProcessDefinitionRepository definitionRepository, ProcessInstanceRepository instanceRepository, ApprovalTaskRepository taskRepository) { this.clock = Objects.requireNonNull(clock, "clock must not be null"); this.assigneeResolver = Objects.requireNonNull(assigneeResolver, "assigneeResolver must not be null"); this.routingCondition = Objects.requireNonNull(routingCondition, "routingCondition must not be null"); + this.actionHandler = Objects.requireNonNull(actionHandler, "actionHandler must not be null"); this.transactionExecutor = Objects.requireNonNull(transactionExecutor, "transactionExecutor must not be null"); this.definitionRepository = Objects.requireNonNull(definitionRepository, "definitionRepository must not be null"); this.instanceRepository = Objects.requireNonNull(instanceRepository, "instanceRepository must not be null"); @@ -93,26 +103,32 @@ public final class DefaultOrdoEngine implements OrdoEngine { public synchronized ProcessInstance start(String definitionId, String initiator, ProcessContext context) { requireText(initiator, "initiator"); Objects.requireNonNull(context, "context must not be null"); - return transactionExecutor.execute(() -> { + List queued = new ArrayList<>(); + ProcessInstance instance = transactionExecutor.execute(() -> { ProcessDefinition definition = requireDefinition(definitionId); Instant now = clock.instant(); - ProcessInstance instance = new ProcessInstance(nextId(), definition.id(), initiator, + ProcessInstance started = new ProcessInstance(nextId(), definition.id(), initiator, ProcessStatus.RUNNING, now, null, context); - instanceRepository.insert(instance); - createStepTasks(instance, definition.steps().getFirst(), now); - return instance; + instanceRepository.insert(started); + enterStep(started, definition, definition.steps().getFirst(), now, queued); + return started; }); + runQueuedActions(queued); + return instanceRepository.findById(instance.id()).orElse(instance); } @Override public synchronized ApprovalTask approve(String taskId, String actor, String comment) { - return transactionExecutor.execute(() -> { + List queued = new ArrayList<>(); + ApprovalTask completed = transactionExecutor.execute(() -> { ApprovalTask task = requirePendingTaskForActor(taskId, actor); Instant now = clock.instant(); ApprovalTask completedTask = completeTask(task, TaskStatus.APPROVED, new TaskAction(actor, comment, now)); - advanceAfterDecision(completedTask, now); + advanceAfterDecision(completedTask, now, queued); return completedTask; }); + runQueuedActions(queued); + return completed; } @Override @@ -121,7 +137,7 @@ public final class DefaultOrdoEngine implements OrdoEngine { ApprovalTask task = requirePendingTaskForActor(taskId, actor); Instant now = clock.instant(); ApprovalTask completedTask = completeTask(task, TaskStatus.REJECTED, new TaskAction(actor, comment, now)); - advanceAfterDecision(completedTask, now); + advanceAfterDecision(completedTask, now, List.of()); return completedTask; }); } @@ -217,7 +233,7 @@ public final class DefaultOrdoEngine implements OrdoEngine { * Decides whether the step (and the process instance) can move on after a single candidate * task was approved or rejected, applying the step's {@link ApprovalPolicy}. */ - private void advanceAfterDecision(ApprovalTask completedTask, Instant now) { + private void advanceAfterDecision(ApprovalTask completedTask, Instant now, List queued) { ProcessInstance instance = requireInstance(completedTask.instanceId()); ProcessDefinition definition = requireDefinition(instance.definitionId()); ApprovalStep step = requireStep(definition, completedTask.stepId()); @@ -225,7 +241,7 @@ public final class DefaultOrdoEngine implements OrdoEngine { if (completedTask.status() == TaskStatus.APPROVED && step.policy() == ApprovalPolicy.ANY) { skipPendingSiblings(siblings, completedTask.id(), now); - advanceOrComplete(instance, definition, step, now); + advanceOrComplete(instance, definition, step, now, queued); return; } @@ -249,16 +265,46 @@ public final class DefaultOrdoEngine implements OrdoEngine { // ALL policy: only advance once every candidate has approved. boolean allApproved = siblings.stream().allMatch(sibling -> sibling.status() == TaskStatus.APPROVED); if (allApproved) { - advanceOrComplete(instance, definition, step, now); + advanceOrComplete(instance, definition, step, now, queued); } } - private void advanceOrComplete(ProcessInstance instance, ProcessDefinition definition, ApprovalStep step, Instant now) { + private void advanceOrComplete(ProcessInstance instance, ProcessDefinition definition, ApprovalStep step, + Instant now, List queued) { StepTransition matched = resolveTransition(definition, step, instance); if (matched.toStepId() == null) { completeInstance(instance, ProcessStatus.APPROVED, now); } else { - createStepTasks(instance, requireStep(definition, matched.toStepId()), now); + enterStep(instance, definition, requireStep(definition, matched.toStepId()), now, queued); + } + } + + private void enterStep(ProcessInstance instance, ProcessDefinition definition, ApprovalStep start, Instant now, + List queued) { + ApprovalStep current = start; + for (int hops = 0; hops < MAX_CONSECUTIVE_ACTIONS; hops++) { + if (current.kind() == StepKind.APPROVAL) { + createStepTasks(instance, current, now); + return; + } + queued.add(new PendingAction(current.actionKey(), instance.context())); + StepTransition matched = resolveTransition(definition, current, instance); + if (matched.toStepId() == null) { + completeInstance(instance, ProcessStatus.APPROVED, now); + return; + } + current = requireStep(definition, matched.toStepId()); + } + throw new IllegalStateException("too many consecutive action steps in instance: " + instance.id()); + } + + private void runQueuedActions(List queued) { + for (PendingAction pending : queued) { + try { + actionHandler.execute(pending.actionKey(), pending.context()); + } catch (RuntimeException e) { + LOG.log(Level.WARNING, "action failed: " + pending.actionKey(), e); + } } } @@ -324,4 +370,7 @@ public final class DefaultOrdoEngine implements OrdoEngine { throw new IllegalArgumentException(name + " must not be blank"); } } + + private record PendingAction(String actionKey, ProcessContext context) { + } } diff --git a/ordo-core/src/main/java/com/jetlumen/ordo/core/InMemoryOrdoEngine.java b/ordo-core/src/main/java/com/jetlumen/ordo/core/InMemoryOrdoEngine.java index 14e925b..bdb4f7f 100644 --- a/ordo-core/src/main/java/com/jetlumen/ordo/core/InMemoryOrdoEngine.java +++ b/ordo-core/src/main/java/com/jetlumen/ordo/core/InMemoryOrdoEngine.java @@ -1,5 +1,6 @@ package com.jetlumen.ordo.core; +import com.jetlumen.ordo.api.ActionHandler; import com.jetlumen.ordo.api.ApprovalTask; import com.jetlumen.ordo.api.AssigneeResolver; import com.jetlumen.ordo.api.OrdoEngine; @@ -20,31 +21,37 @@ public final class InMemoryOrdoEngine implements OrdoEngine { private final DefaultOrdoEngine delegate; public InMemoryOrdoEngine() { - this(Clock.systemUTC(), AssigneeResolver.direct(), RoutingCondition.always()); + this(Clock.systemUTC(), AssigneeResolver.direct(), RoutingCondition.always(), ActionHandler.noop()); } public InMemoryOrdoEngine(Clock clock) { - this(clock, AssigneeResolver.direct(), RoutingCondition.always()); + this(clock, AssigneeResolver.direct(), RoutingCondition.always(), ActionHandler.noop()); } public InMemoryOrdoEngine(AssigneeResolver assigneeResolver) { - this(Clock.systemUTC(), assigneeResolver, RoutingCondition.always()); + this(Clock.systemUTC(), assigneeResolver, RoutingCondition.always(), ActionHandler.noop()); } public InMemoryOrdoEngine(Clock clock, AssigneeResolver assigneeResolver) { - this(clock, assigneeResolver, RoutingCondition.always()); + this(clock, assigneeResolver, RoutingCondition.always(), ActionHandler.noop()); } public InMemoryOrdoEngine(RoutingCondition routingCondition) { - this(Clock.systemUTC(), AssigneeResolver.direct(), routingCondition); + this(Clock.systemUTC(), AssigneeResolver.direct(), routingCondition, ActionHandler.noop()); } public InMemoryOrdoEngine(AssigneeResolver assigneeResolver, RoutingCondition routingCondition) { - this(Clock.systemUTC(), assigneeResolver, routingCondition); + this(Clock.systemUTC(), assigneeResolver, routingCondition, ActionHandler.noop()); } public InMemoryOrdoEngine(Clock clock, AssigneeResolver assigneeResolver, RoutingCondition routingCondition) { - this.delegate = new DefaultOrdoEngine(clock, assigneeResolver, routingCondition, new NoopTransactionExecutor(), + this(clock, assigneeResolver, routingCondition, ActionHandler.noop()); + } + + public InMemoryOrdoEngine(Clock clock, AssigneeResolver assigneeResolver, RoutingCondition routingCondition, + ActionHandler actionHandler) { + this.delegate = new DefaultOrdoEngine(clock, assigneeResolver, routingCondition, actionHandler, + new NoopTransactionExecutor(), new InMemoryProcessDefinitionRepository(), new InMemoryProcessInstanceRepository(), new InMemoryApprovalTaskRepository()); diff --git a/ordo-core/src/test/java/com/jetlumen/ordo/core/InMemoryOrdoEngineTest.java b/ordo-core/src/test/java/com/jetlumen/ordo/core/InMemoryOrdoEngineTest.java index cd2cf3e..da17e0d 100644 --- a/ordo-core/src/test/java/com/jetlumen/ordo/core/InMemoryOrdoEngineTest.java +++ b/ordo-core/src/test/java/com/jetlumen/ordo/core/InMemoryOrdoEngineTest.java @@ -1,5 +1,6 @@ package com.jetlumen.ordo.core; +import com.jetlumen.ordo.api.AssigneeResolver; import com.jetlumen.ordo.api.ApprovalPolicy; import com.jetlumen.ordo.api.ApprovalStep; import com.jetlumen.ordo.api.ApprovalTask; @@ -23,6 +24,7 @@ import com.jetlumen.ordo.api.exception.UnauthorizedTaskOperationException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.time.Clock; import java.util.List; import java.util.Map; @@ -386,6 +388,65 @@ class InMemoryOrdoEngineTest { assertTrue(routingEngine.findPendingTasksByInstanceId(low.id()).isEmpty()); } + @Test + void runsActionStepsAfterApprovalThenCreatesTheNextApprovalTask() { + List executed = new java.util.ArrayList<>(); + InMemoryOrdoEngine actionEngine = new InMemoryOrdoEngine(Clock.systemUTC(), AssigneeResolver.direct(), + RoutingCondition.always(), (key, context) -> executed.add(key)); + actionEngine.register(new ProcessDefinition("leave", "Leave request", List.of( + ApprovalStep.single("manager", "Manager approval", "maria"), + ApprovalStep.action("notify", "Notify HR", "leave-approved-mail"), + ApprovalStep.single("hr", "HR approval", "henry")), + List.of( + StepTransition.always("manager", "notify"), + StepTransition.always("notify", "hr"), + StepTransition.end("hr")))); + + var instance = actionEngine.start("leave", "alice"); + actionEngine.approve(actionEngine.findPendingTasksByInstanceId(instance.id()).getFirst().id(), "maria"); + + assertEquals(List.of("leave-approved-mail"), executed); + ApprovalTask hrTask = actionEngine.findPendingTasksByInstanceId(instance.id()).getFirst(); + assertEquals("hr", hrTask.stepId()); + assertEquals(ProcessStatus.RUNNING, actionEngine.findInstance(instance.id()).orElseThrow().status()); + } + + @Test + void continuesWhenAnActionHandlerThrows() { + InMemoryOrdoEngine actionEngine = new InMemoryOrdoEngine(Clock.systemUTC(), AssigneeResolver.direct(), + RoutingCondition.always(), (key, context) -> { + throw new IllegalStateException("mail failed"); + }); + actionEngine.register(new ProcessDefinition("leave", "Leave request", List.of( + ApprovalStep.single("manager", "Manager approval", "maria"), + ApprovalStep.action("notify", "Notify HR", "leave-approved-mail")), + List.of( + StepTransition.always("manager", "notify"), + StepTransition.end("notify")))); + + var instance = actionEngine.start("leave", "alice"); + actionEngine.approve(actionEngine.findPendingTasksByInstanceId(instance.id()).getFirst().id(), "maria"); + assertEquals(ProcessStatus.APPROVED, actionEngine.findInstance(instance.id()).orElseThrow().status()); + } + + @Test + void startEntersAnActionStepThenStopsOnTheFollowingApproval() { + List executed = new java.util.ArrayList<>(); + InMemoryOrdoEngine actionEngine = new InMemoryOrdoEngine(Clock.systemUTC(), AssigneeResolver.direct(), + RoutingCondition.always(), (key, context) -> executed.add(key)); + actionEngine.register(new ProcessDefinition("leave", "Leave request", List.of( + ApprovalStep.action("notify", "Notify manager", "leave-submitted-mail"), + ApprovalStep.single("manager", "Manager approval", "maria")), + List.of( + StepTransition.always("notify", "manager"), + StepTransition.end("manager")))); + + var instance = actionEngine.start("leave", "alice"); + assertEquals(List.of("leave-submitted-mail"), executed); + assertEquals("manager", actionEngine.findPendingTasksByInstanceId(instance.id()).getFirst().stepId()); + assertEquals(ProcessStatus.RUNNING, instance.status()); + } + @Test void throwsWhenNoTransitionMatches() { InMemoryOrdoEngine routingEngine = new InMemoryOrdoEngine((key, context) -> false); diff --git a/ordo-spring-boot-autoconfigure/src/main/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfiguration.java b/ordo-spring-boot-autoconfigure/src/main/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfiguration.java index 223934f..ded1321 100644 --- a/ordo-spring-boot-autoconfigure/src/main/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfiguration.java +++ b/ordo-spring-boot-autoconfigure/src/main/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfiguration.java @@ -1,5 +1,6 @@ package com.jetlumen.ordo.spring; +import com.jetlumen.ordo.api.ActionHandler; import com.jetlumen.ordo.api.AssigneeResolver; import com.jetlumen.ordo.api.OrdoEngine; import com.jetlumen.ordo.api.RoutingCondition; @@ -65,6 +66,12 @@ public class OrdoJdbcAutoConfiguration { return RoutingCondition.always(); } + @Bean + @ConditionalOnMissingBean + public ActionHandler ordoActionHandler() { + return ActionHandler.noop(); + } + @Bean @ConditionalOnMissingBean @DependsOnDatabaseInitialization @@ -101,12 +108,14 @@ public class OrdoJdbcAutoConfiguration { public OrdoEngine ordoEngine(Clock ordoClock, AssigneeResolver ordoAssigneeResolver, RoutingCondition ordoRoutingCondition, + ActionHandler actionHandler, TransactionExecutor ordoTransactionExecutor, ProcessDefinitionRepository ordoProcessDefinitionRepository, ProcessInstanceRepository ordoProcessInstanceRepository, ApprovalTaskRepository ordoApprovalTaskRepository) { - return new DefaultOrdoEngine(ordoClock, ordoAssigneeResolver, ordoRoutingCondition, ordoTransactionExecutor, - ordoProcessDefinitionRepository, ordoProcessInstanceRepository, ordoApprovalTaskRepository); + return new DefaultOrdoEngine(ordoClock, ordoAssigneeResolver, ordoRoutingCondition, actionHandler, + ordoTransactionExecutor, ordoProcessDefinitionRepository, ordoProcessInstanceRepository, + ordoApprovalTaskRepository); } @Bean diff --git a/ordo-spring-boot-autoconfigure/src/test/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfigurationTest.java b/ordo-spring-boot-autoconfigure/src/test/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfigurationTest.java index b047d26..d553ba0 100644 --- a/ordo-spring-boot-autoconfigure/src/test/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfigurationTest.java +++ b/ordo-spring-boot-autoconfigure/src/test/java/com/jetlumen/ordo/spring/OrdoJdbcAutoConfigurationTest.java @@ -1,5 +1,6 @@ package com.jetlumen.ordo.spring; +import com.jetlumen.ordo.api.ActionHandler; import com.jetlumen.ordo.api.ApprovalStep; import com.jetlumen.ordo.api.ApprovalTask; import com.jetlumen.ordo.api.AssigneeResolver; @@ -20,6 +21,7 @@ import java.util.List; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class OrdoJdbcAutoConfigurationTest { @@ -138,6 +140,17 @@ class OrdoJdbcAutoConfigurationTest { }); } + @Test + void honoursUserDefinedActionHandler() { + withDataSourceRunner.withUserConfiguration(CustomActionHandlerConfig.class) + .run(context -> { + assertThat(context).hasSingleBean(OrdoEngine.class); + assertThatThrownBy(() -> context.getBean(ActionHandler.class).execute("any", null)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("custom-action"); + }); + } + @Configuration static class CustomAssigneeResolverConfig { @Bean @@ -153,4 +166,14 @@ class OrdoJdbcAutoConfigurationTest { return (key, context) -> true; } } + + @Configuration + static class CustomActionHandlerConfig { + @Bean + ActionHandler ordoActionHandler() { + return (key, context) -> { + throw new IllegalStateException("custom-action"); + }; + } + } } diff --git a/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepository.java b/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepository.java index bf28257..c41c7ad 100644 --- a/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepository.java +++ b/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepository.java @@ -25,7 +25,8 @@ public final class JdbcProcessDefinitionRepository implements ProcessDefinitionR private static final String INSERT_DEFINITION = "INSERT INTO ordo_process_definition (id, name) VALUES (?, ?)"; private static final String INSERT_STEP = - "INSERT INTO ordo_approval_step (definition_id, step_id, step_name, policy, step_order) VALUES (?, ?, ?, ?, ?)"; + "INSERT INTO ordo_approval_step (definition_id, step_id, step_name, policy, step_order, kind, action_key)" + + " VALUES (?, ?, ?, ?, ?, ?, ?)"; private static final String INSERT_CANDIDATE = "INSERT INTO ordo_step_candidate (definition_id, step_id, candidate, candidate_order) VALUES (?, ?, ?, ?)"; private static final String INSERT_TRANSITION = @@ -41,7 +42,8 @@ public final class JdbcProcessDefinitionRepository implements ProcessDefinitionR private static final String SELECT_DEFINITION = "SELECT id, name FROM ordo_process_definition WHERE id = ?"; private static final String SELECT_STEPS = - "SELECT step_id, step_name, policy FROM ordo_approval_step WHERE definition_id = ? ORDER BY step_order"; + "SELECT step_id, step_name, policy, kind, action_key FROM ordo_approval_step" + + " WHERE definition_id = ? ORDER BY step_order"; private static final String SELECT_CANDIDATES = "SELECT step_id, candidate FROM ordo_step_candidate WHERE definition_id = ? ORDER BY step_id, candidate_order"; private static final String SELECT_TRANSITIONS = @@ -76,9 +78,17 @@ public final class JdbcProcessDefinitionRepository implements ProcessDefinitionR Objects.requireNonNull(definition, "definition must not be null"); Connection connection = connectionProvider.getConnection(); try { - if (!insertDefinitionRow(connection, definition)) { + // PostgreSQL aborts the current transaction on unique-constraint violations, + // so upsert must not probe existence via a failing INSERT. + if (definitionExists(connection, definition.id())) { updateDefinitionName(connection, definition); deleteGraph(connection, definition.id()); + } else { + try (PreparedStatement insert = connection.prepareStatement(INSERT_DEFINITION)) { + insert.setString(1, definition.id()); + insert.setString(2, definition.name()); + insert.executeUpdate(); + } } insertGraph(connection, definition); } catch (SQLException e) { @@ -124,7 +134,8 @@ public final class JdbcProcessDefinitionRepository implements ProcessDefinitionR List steps = new ArrayList<>(); for (StepRow row : stepRows) { List candidates = candidatesByStep.getOrDefault(row.stepId(), List.of()); - steps.add(new ApprovalStep(row.stepId(), row.stepName(), candidates, row.policy())); + steps.add(new ApprovalStep(row.stepId(), row.stepName(), candidates, row.policy(), row.kind(), + row.actionKey())); } List transitions = new ArrayList<>(); try (PreparedStatement selectTransitions = connection.prepareStatement(SELECT_TRANSITIONS)) { @@ -145,6 +156,15 @@ public final class JdbcProcessDefinitionRepository implements ProcessDefinitionR } } + private static boolean definitionExists(Connection connection, String definitionId) throws SQLException { + try (PreparedStatement select = connection.prepareStatement(SELECT_DEFINITION)) { + select.setString(1, definitionId); + try (ResultSet resultSet = select.executeQuery()) { + return resultSet.next(); + } + } + } + private static boolean insertDefinitionRow(Connection connection, ProcessDefinition definition) throws SQLException { try (PreparedStatement insertDefinition = connection.prepareStatement(INSERT_DEFINITION)) { insertDefinition.setString(1, definition.id()); @@ -191,6 +211,8 @@ public final class JdbcProcessDefinitionRepository implements ProcessDefinitionR insertStep.setString(3, step.name()); insertStep.setString(4, step.policy().name()); insertStep.setInt(5, stepOrder++); + insertStep.setString(6, step.kind().name()); + insertStep.setString(7, step.actionKey()); insertStep.executeUpdate(); } int candidateOrder = 0; diff --git a/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/mapper/ApprovalStepMapper.java b/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/mapper/ApprovalStepMapper.java index 468434e..538c804 100644 --- a/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/mapper/ApprovalStepMapper.java +++ b/ordo-storage-jdbc/src/main/java/com/jetlumen/ordo/storage/jdbc/mapper/ApprovalStepMapper.java @@ -1,6 +1,7 @@ package com.jetlumen.ordo.storage.jdbc.mapper; import com.jetlumen.ordo.api.ApprovalPolicy; +import com.jetlumen.ordo.api.StepKind; import java.sql.ResultSet; import java.sql.SQLException; @@ -16,9 +17,11 @@ public final class ApprovalStepMapper { public static StepRow readRow(ResultSet resultSet) throws SQLException { return new StepRow(resultSet.getString("step_id"), resultSet.getString("step_name"), - ApprovalPolicy.valueOf(resultSet.getString("policy"))); + ApprovalPolicy.valueOf(resultSet.getString("policy")), + StepKind.valueOf(resultSet.getString("kind")), + resultSet.getString("action_key")); } - public record StepRow(String stepId, String stepName, ApprovalPolicy policy) { + public record StepRow(String stepId, String stepName, ApprovalPolicy policy, StepKind kind, String actionKey) { } } diff --git a/ordo-storage-jdbc/src/main/resources/db/migration/V4__add_step_kind_and_action_key.sql b/ordo-storage-jdbc/src/main/resources/db/migration/V4__add_step_kind_and_action_key.sql new file mode 100644 index 0000000..23e628b --- /dev/null +++ b/ordo-storage-jdbc/src/main/resources/db/migration/V4__add_step_kind_and_action_key.sql @@ -0,0 +1,3 @@ +-- Adds ACTION step kind and an optional action_key (host ActionHandler lookup). +ALTER TABLE ordo_approval_step ADD COLUMN kind VARCHAR(16) NOT NULL DEFAULT 'APPROVAL'; +ALTER TABLE ordo_approval_step ADD COLUMN action_key VARCHAR(255); diff --git a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcOrdoEngineIntegrationTest.java b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcOrdoEngineIntegrationTest.java index d7eb340..271a1c0 100644 --- a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcOrdoEngineIntegrationTest.java +++ b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcOrdoEngineIntegrationTest.java @@ -1,5 +1,6 @@ package com.jetlumen.ordo.storage.jdbc; +import com.jetlumen.ordo.api.ActionHandler; import com.jetlumen.ordo.api.ApprovalStep; import com.jetlumen.ordo.api.ApprovalTask; import com.jetlumen.ordo.api.AssigneeResolver; @@ -119,7 +120,7 @@ class JdbcOrdoEngineIntegrationTest { @Test void rollsBackTheWholeApprovalWhenNoRouteMatches() { OrdoEngine failingEngine = new DefaultOrdoEngine(Clock.fixed(NOW, ZoneOffset.UTC), - AssigneeResolver.direct(), (key, context) -> false, + AssigneeResolver.direct(), (key, context) -> false, ActionHandler.noop(), new JdbcTransactionExecutor(connectionProvider), new JdbcProcessDefinitionRepository(connectionProvider), new JdbcProcessInstanceRepository(connectionProvider), @@ -197,6 +198,7 @@ class JdbcOrdoEngineIntegrationTest { private OrdoEngine newEngine(AssigneeResolver assigneeResolver) { return new DefaultOrdoEngine(Clock.fixed(NOW, ZoneOffset.UTC), assigneeResolver, RoutingCondition.always(), + ActionHandler.noop(), new JdbcTransactionExecutor(connectionProvider), new JdbcProcessDefinitionRepository(connectionProvider), new JdbcProcessInstanceRepository(connectionProvider), diff --git a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcPostgresIntegrationTest.java b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcPostgresIntegrationTest.java index aa8f326..4a894a6 100644 --- a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcPostgresIntegrationTest.java +++ b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcPostgresIntegrationTest.java @@ -1,5 +1,6 @@ package com.jetlumen.ordo.storage.jdbc; +import com.jetlumen.ordo.api.ActionHandler; import com.jetlumen.ordo.api.ApprovalStep; import com.jetlumen.ordo.api.ApprovalTask; import com.jetlumen.ordo.api.AssigneeResolver; @@ -219,6 +220,7 @@ class JdbcPostgresIntegrationTest { private OrdoEngine newEngine(AssigneeResolver assigneeResolver) { return new DefaultOrdoEngine(Clock.fixed(NOW, ZoneOffset.UTC), assigneeResolver, RoutingCondition.always(), + ActionHandler.noop(), new JdbcTransactionExecutor(connectionProvider), new JdbcProcessDefinitionRepository(connectionProvider), new JdbcProcessInstanceRepository(connectionProvider), diff --git a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepositoryTest.java b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepositoryTest.java index cc048a4..d047382 100644 --- a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepositoryTest.java +++ b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcProcessDefinitionRepositoryTest.java @@ -2,6 +2,7 @@ package com.jetlumen.ordo.storage.jdbc; import com.jetlumen.ordo.api.ApprovalStep; import com.jetlumen.ordo.api.ProcessDefinition; +import com.jetlumen.ordo.api.StepKind; import com.jetlumen.ordo.api.StepTransition; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -72,6 +73,21 @@ class JdbcProcessDefinitionRepositoryTest { assertEquals(definition, repository.findById("expense").orElseThrow()); } + @Test + void insertsAndReadsBackActionStepsWithoutCandidates() { + ProcessDefinition definition = new ProcessDefinition("notify", "Notify", List.of( + ApprovalStep.single("manager", "Manager approval", "maria"), + ApprovalStep.action("mail", "Send mail", "leave-approved-mail")), + List.of( + StepTransition.always("manager", "mail"), + StepTransition.end("mail"))); + + assertTrue(repository.insertIfAbsent(definition)); + assertEquals(definition, repository.findById("notify").orElseThrow()); + assertEquals(StepKind.ACTION, repository.findById("notify").orElseThrow().steps().get(1).kind()); + assertEquals("leave-approved-mail", repository.findById("notify").orElseThrow().steps().get(1).actionKey()); + } + private static ProcessDefinition definition(String id, String name) { return ProcessDefinition.linear(id, name, List.of(ApprovalStep.single("lead", "Lead approval", "lee"))); } diff --git a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcTestSupport.java b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcTestSupport.java index c0eca19..3f35c29 100644 --- a/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcTestSupport.java +++ b/ordo-storage-jdbc/src/test/java/com/jetlumen/ordo/storage/jdbc/JdbcTestSupport.java @@ -16,7 +16,8 @@ final class JdbcTestSupport { private static final String[] MIGRATIONS = { "/db/migration/V1__create_ordo_tables.sql", "/db/migration/V2__add_step_candidates_and_policy.sql", - "/db/migration/V3__add_step_transitions.sql" + "/db/migration/V3__add_step_transitions.sql", + "/db/migration/V4__add_step_kind_and_action_key.sql" }; private static final String[] SCHEMA_SQL = loadSchemas();