From e472c00b1cdf7ba38bbe80997b3a25ba8608a9a5 Mon Sep 17 00:00:00 2001 From: Jan Jakes Date: Wed, 14 Sep 2022 15:10:18 +0200 Subject: [PATCH] Implerment depth-first pre-order workflow graph walker with plug-in node visitors [MAILPOET-4629] --- mailpoet/lib/Automation/Engine/Exceptions.php | 8 ++ .../Validation/WorkflowGraph/WorkflowNode.php | 31 +++++ .../WorkflowGraph/WorkflowNodeVisitor.php | 13 ++ .../WorkflowGraph/WorkflowWalker.php | 81 +++++++++++ mailpoet/lib/DI/ContainerConfigurator.php | 1 + .../WorkflowGraph/WorkflowWalkerTest.php | 130 ++++++++++++++++++ 6 files changed, 264 insertions(+) create mode 100644 mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNode.php create mode 100644 mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNodeVisitor.php create mode 100644 mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowWalker.php create mode 100644 mailpoet/tests/unit/Automation/Engine/Validation/WorkflowGraph/WorkflowWalkerTest.php diff --git a/mailpoet/lib/Automation/Engine/Exceptions.php b/mailpoet/lib/Automation/Engine/Exceptions.php index e4743ad735..852d60bf57 100644 --- a/mailpoet/lib/Automation/Engine/Exceptions.php +++ b/mailpoet/lib/Automation/Engine/Exceptions.php @@ -23,6 +23,7 @@ class Exceptions { private const SUBJECT_LOAD_FAILED = 'mailpoet_automation_workflow_subject_load_failed'; private const MULTIPLE_SUBJECTS_FOUND = 'mailpoet_automation_multiple_subjects_found'; private const WORKFLOW_STRUCTURE_MODIFICATION_NOT_SUPPORTED = 'mailpoet_automation_workflow_structure_modification_not_supported'; + private const WORKFLOW_STRUCTURE_NOT_VALID = 'mailpoet_automation_workflow_structure_not_valid'; public function __construct() { throw new InvalidStateException( @@ -139,4 +140,11 @@ class Exceptions { ->withErrorCode(self::WORKFLOW_STRUCTURE_MODIFICATION_NOT_SUPPORTED) ->withMessage(__("Workflow structure modification not supported.", 'mailpoet')); } + + public static function workflowStructureNotValid(string $detail): UnexpectedValueException { + return UnexpectedValueException::create() + ->withErrorCode(self::WORKFLOW_STRUCTURE_NOT_VALID) + // translators: %s is a detailed information + ->withMessage(sprintf(__("Invalid workflow structure: %s", 'mailpoet'), $detail)); + } } diff --git a/mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNode.php b/mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNode.php new file mode 100644 index 0000000000..f7cc9a027f --- /dev/null +++ b/mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNode.php @@ -0,0 +1,31 @@ +step = $step; + $this->parents = $parents; + } + + public function getStep(): Step { + return $this->step; + } + + /** @return Step[] */ + public function getParents(): array { + return $this->parents; + } +} diff --git a/mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNodeVisitor.php b/mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNodeVisitor.php new file mode 100644 index 0000000000..c792018935 --- /dev/null +++ b/mailpoet/lib/Automation/Engine/Validation/WorkflowGraph/WorkflowNodeVisitor.php @@ -0,0 +1,13 @@ +getSteps(); + $root = $steps['root'] ?? null; + if (!$root) { + throw Exceptions::workflowStructureNotValid(__("Workflow must contain a 'root' step", 'mailpoet')); + } + + foreach ($visitors as $visitor) { + $visitor->initialize($workflow); + } + + foreach ($this->walkStepsDepthFirstPreOrder($steps, $root) as $record) { + [$step, $parents] = $record; + foreach ($visitors as $visitor) { + $visitor->visitNode($workflow, new WorkflowNode($step, array_values($parents))); + } + } + + foreach ($visitors as $visitor) { + $visitor->complete($workflow); + } + } + + /** + * @param array $steps + * @return Generator}> + */ + private function walkStepsDepthFirstPreOrder(array $steps, Step $root): Generator { + /** @var array{0: Step, 1: array}[] $stack */ + $stack = [ + [$root, []], + ]; + + do { + $record = array_pop($stack); + if (!$record) { + throw new InvalidStateException(); + } + yield $record; + [$step, $parents] = $record; + + foreach (array_reverse($step->getNextSteps()) as $nextStepData) { + $nextStepId = $nextStepData->getId(); + $nextStep = $steps[$nextStepId] ?? null; + if (!$nextStep) { + throw $this->createStepNotFoundException($nextStepId, $step->getId()); + } + + $nextStepParents = array_merge($parents, [$step->getId() => $step]); + if (isset($nextStepParents[$nextStepId])) { + continue; // cycle detected, do not enter the path again + } + array_push($stack, [$nextStep, $nextStepParents]); + } + } while (count($stack) > 0); + } + + private function createStepNotFoundException(string $stepId, string $parentStepId): UnexpectedValueException { + return Exceptions::workflowStructureNotValid( + // translators: %1$s is ID of the step not found, %2$s is ID of the step that references it + sprintf( + __("Step with ID '%1\$s' not found (referenced from '%2\$s')", 'mailpoet'), + $stepId, + $parentStepId + ) + ); + } +} diff --git a/mailpoet/lib/DI/ContainerConfigurator.php b/mailpoet/lib/DI/ContainerConfigurator.php index 94ace6f351..799129c93b 100644 --- a/mailpoet/lib/DI/ContainerConfigurator.php +++ b/mailpoet/lib/DI/ContainerConfigurator.php @@ -128,6 +128,7 @@ class ContainerConfigurator implements IContainerConfigurator { $container->autowire(\MailPoet\Automation\Engine\Storage\WorkflowRunLogStorage::class)->setPublic(true); $container->autowire(\MailPoet\Automation\Engine\Storage\WorkflowTemplateStorage::class)->setPublic(true); $container->autowire(\MailPoet\Automation\Engine\Storage\WorkflowStorage::class)->setPublic(true); + $container->autowire(\MailPoet\Automation\Engine\Validation\WorkflowGraph\WorkflowWalker::class)->setPublic(true); $container->autowire(\MailPoet\Automation\Engine\WordPress::class)->setPublic(true); // Automation - API endpoints $container->autowire(\MailPoet\Automation\Engine\Endpoints\Workflows\WorkflowsGetEndpoint::class)->setPublic(true); diff --git a/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowGraph/WorkflowWalkerTest.php b/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowGraph/WorkflowWalkerTest.php new file mode 100644 index 0000000000..a2f83225c6 --- /dev/null +++ b/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowGraph/WorkflowWalkerTest.php @@ -0,0 +1,130 @@ +createWorkflow([]); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage("Invalid workflow structure: Workflow must contain a 'root' step"); + $this->walkWorkflow($workflow); + } + + public function testNonRootStepMissing(): void { + $workflow = $this->createWorkflow(['root' => ['a']]); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage("Invalid workflow structure: Step with ID 'a' not found (referenced from 'root')"); + $this->walkWorkflow($workflow); + } + + public function testSimpleWorkflow(): void { + $workflow = $this->createWorkflow([ + 'root' => ['a'], + 'a' => ['b'], + 'b' => ['c'], + 'c' => [], + ]); + + $path = $this->walkWorkflow($workflow); + $this->assertSame([ + ['root', []], + ['a', ['root']], + ['b', ['root', 'a']], + ['c', ['root', 'a', 'b']], + ], $path); + } + + public function testMultiBranchWorkflow(): void { + $workflow = $this->createWorkflow([ + 'root' => ['a1', 'a2'], + 'a1' => ['b1', 'b2'], + 'a2' => ['c'], + 'b1' => ['d'], + 'b2' => [], + 'c' => [], + 'd' => [], + ]); + + $path = $this->walkWorkflow($workflow); + $this->assertSame([ + ['root', []], + ['a1', ['root']], + ['b1', ['root', 'a1']], + ['d', ['root', 'a1', 'b1']], + ['b2', ['root', 'a1']], + ['a2', ['root']], + ['c', ['root', 'a2']], + ], $path); + } + + + public function testCyclicWorkflow(): void { + $workflow = $this->createWorkflow([ + 'root' => ['a', 'root'], + 'a' => ['b'], + 'b' => ['c'], + 'c' => ['a', 'd'], + 'd' => ['d'], + ]); + + $path = $this->walkWorkflow($workflow); + $this->assertSame([ + ['root', []], + ['a', ['root']], + ['b', ['root', 'a']], + ['c', ['root', 'a', 'b']], + ['d', ['root', 'a', 'b', 'c']], + ], $path); + } + + private function createStep(string $id, array $nextStepIds): Step { + return new Step( + $id, + 'test-type', + 'test-key', + [], + array_map(function (string $id) { + return new NextStep($id); + }, $nextStepIds)); + } + + private function createWorkflow(array $steps): Workflow { + $stepMap = []; + foreach ($steps as $id => $nextStepIds) { + $stepMap[$id] = $this->createStep($id, $nextStepIds); + } + return $this->make(Workflow::class, ['getSteps' => $stepMap]); + } + + private function walkWorkflow(Workflow $workflow): array { + $visitor = new class implements WorkflowNodeVisitor { + public $nodes = []; + + public function initialize(Workflow $workflow): void { + $this->nodes = []; + } + + public function visitNode(Workflow $workflow, WorkflowNode $node): void { + $this->nodes[] = $node; + } + + public function complete(Workflow $workflow): void {} + }; + + $walker = new WorkflowWalker(); + $walker->walk($workflow, [$visitor]); + return array_map(function (WorkflowNode $node) { + return [$node->getStep()->getId(), array_map(function (Step $parent) { + return $parent->getId(); + }, $node->getParents())]; + }, $visitor->nodes); + } +}