Convert WorkflowStepValidator to node-visitor-based rules

[MAILPOET-4659]

mailpoet/lib/Automation/Engine/Validation/WorkflowRules/UnknownStepRule.php

@@ -1,22 +1,19 @@
 <?php declare(strict_types = 1);
 
-namespace MailPoet\Automation\Engine\Validation;
+namespace MailPoet\Automation\Engine\Validation\WorkflowRules;
 
-use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Data\Workflow;
 use MailPoet\Automation\Engine\Exceptions;
 use MailPoet\Automation\Engine\Exceptions\InvalidStateException;
 use MailPoet\Automation\Engine\Registry;
 use MailPoet\Automation\Engine\Storage\WorkflowStorage;
-use MailPoet\Validator\Validator;
+use MailPoet\Automation\Engine\Validation\WorkflowGraph\WorkflowNode;
+use MailPoet\Automation\Engine\Validation\WorkflowGraph\WorkflowNodeVisitor;
 
-class WorkflowStepsValidator {
+class UnknownStepRule implements WorkflowNodeVisitor {
   /** @var Registry */
   private $registry;
 
-  /** @var Validator */
-  private $validator;
-
   /** @var WorkflowStorage */
   private $workflowStorage;
 
@@ -25,36 +22,31 @@ class WorkflowStepsValidator {
 
   public function __construct(
     Registry $registry,
-    Validator $validator,
     WorkflowStorage $workflowStorage
   ) {
-    $this->validator = $validator;
     $this->registry = $registry;
     $this->workflowStorage = $workflowStorage;
   }
 
-  public function validateSteps(Workflow $workflow): void {
-    foreach ($workflow->getSteps() as $step) {
-      $this->validateStep($workflow, $step);
-    }
+  public function initialize(Workflow $workflow): void {
+    $this->cachedExistingWorkflow = false;
   }
 
-  private function validateStep(Workflow $workflow, Step $step): void {
+  public function visitNode(Workflow $workflow, WorkflowNode $node): void {
+    $step = $node->getStep();
     $registryStep = $this->registry->getStep($step->getKey());
+
+    // step not registered (e.g. plugin was deactivated) - allow saving it only if it hasn't changed
     if (!$registryStep) {
-      // step not registered (e.g. plugin was deactivated) - allow saving it only if it hasn't changed
       $currentWorkflow = $this->getCurrentWorkflow($workflow);
       $currentStep = $currentWorkflow ? ($currentWorkflow->getSteps()[$step->getId()] ?? null) : null;
       if (!$currentStep || $step->toArray() !== $currentStep->toArray()) {
         throw Exceptions::workflowStepModifiedWhenUnknown($step);
       }
-      return;
     }
+  }
 
-    // full validation for active workflows
-    if ($workflow->getStatus() === Workflow::STATUS_ACTIVE) {
-      $this->validator->validate($registryStep->getArgsSchema(), $step->getArgs());
-    }
+  public function complete(Workflow $workflow): void {
   }
 
   private function getCurrentWorkflow(Workflow $workflow): ?Workflow {

mailpoet/lib/Automation/Engine/Validation/WorkflowRules/ValidStepArgsRule.php

@@ -0,0 +1,44 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Automation\Engine\Validation\WorkflowRules;
+
+use MailPoet\Automation\Engine\Data\Workflow;
+use MailPoet\Automation\Engine\Registry;
+use MailPoet\Automation\Engine\Validation\WorkflowGraph\WorkflowNode;
+use MailPoet\Automation\Engine\Validation\WorkflowGraph\WorkflowNodeVisitor;
+use MailPoet\Validator\Validator;
+
+class ValidStepArgsRule implements WorkflowNodeVisitor {
+  /** @var Registry */
+  private $registry;
+
+  /** @var Validator */
+  private $validator;
+
+  public function __construct(
+    Registry $registry,
+    Validator $validator
+  ) {
+    $this->registry = $registry;
+    $this->validator = $validator;
+  }
+
+  public function initialize(Workflow $workflow): void {
+  }
+
+  public function visitNode(Workflow $workflow, WorkflowNode $node): void {
+    $step = $node->getStep();
+    $registryStep = $this->registry->getStep($step->getKey());
+    if (!$registryStep) {
+      return;
+    }
+
+    // validate args schema only for active workflows
+    if ($workflow->getStatus() !== Workflow::STATUS_ACTIVE) {
+      $this->validator->validate($registryStep->getArgsSchema(), $step->getArgs());
+    }
+  }
+
+  public function complete(Workflow $workflow): void {
+  }
+}

mailpoet/lib/Automation/Engine/Validation/WorkflowValidator.php

@@ -11,24 +11,30 @@ use MailPoet\Automation\Engine\Validation\WorkflowRules\NoJoinRule;
 use MailPoet\Automation\Engine\Validation\WorkflowRules\NoSplitRule;
 use MailPoet\Automation\Engine\Validation\WorkflowRules\NoUnreachableStepsRule;
 use MailPoet\Automation\Engine\Validation\WorkflowRules\TriggersUnderRootRule;
+use MailPoet\Automation\Engine\Validation\WorkflowRules\UnknownStepRule;
+use MailPoet\Automation\Engine\Validation\WorkflowRules\ValidStepArgsRule;
 
 class WorkflowValidator {
-  /** @var WorkflowStepsValidator */
-  private $stepsValidator;
-
   /** @var WorkflowWalker */
   private $workflowWalker;
 
+  /** @var ValidStepArgsRule */
+  private $validStepArgsRule;
+
+  /** @var UnknownStepRule */
+  private $unknownStepRule;
+
   public function __construct(
-    WorkflowStepsValidator $stepsValidator,
+    UnknownStepRule $unknownStepRule,
+    ValidStepArgsRule $validStepArgsRule,
     WorkflowWalker $workflowWalker
   ) {
+    $this->unknownStepRule = $unknownStepRule;
+    $this->validStepArgsRule = $validStepArgsRule;
     $this->workflowWalker = $workflowWalker;
-    $this->stepsValidator = $stepsValidator;
   }
 
   public function validate(Workflow $workflow): void {
-    // validate graph
     $this->workflowWalker->walk($workflow, [
       new NoUnreachableStepsRule(),
       new ConsistentStepMapRule(),
@@ -37,9 +43,8 @@ class WorkflowValidator {
       new NoCycleRule(),
       new NoJoinRule(),
       new NoSplitRule(),
+      $this->unknownStepRule,
+      $this->validStepArgsRule,
     ]);
-
-    // validate steps
-    $this->stepsValidator->validateSteps($workflow);
   }
 }

mailpoet/lib/DI/ContainerConfigurator.php

@@ -130,7 +130,8 @@ class ContainerConfigurator implements IContainerConfigurator {
     $container->autowire(\MailPoet\Automation\Engine\Storage\WorkflowStorage::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Storage\WorkflowStatisticsStorage::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Validation\WorkflowGraph\WorkflowWalker::class)->setPublic(true);
-    $container->autowire(\MailPoet\Automation\Engine\Validation\WorkflowStepsValidator::class)->setPublic(true);
+    $container->autowire(\MailPoet\Automation\Engine\Validation\WorkflowRules\UnknownStepRule::class)->setPublic(true);
+    $container->autowire(\MailPoet\Automation\Engine\Validation\WorkflowRules\ValidStepArgsRule::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Validation\WorkflowValidator::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\WordPress::class)->setPublic(true);
     // Automation - API endpoints