Remove unused step runner interface

[MAILPOET-5568]

mailpoet/lib/Automation/Engine/Control/StepHandler.php

@@ -3,11 +3,9 @@
 namespace MailPoet\Automation\Engine\Control;
 
 use Exception;
-use MailPoet\Automation\Engine\Control\Steps\ActionStepRunner;
 use MailPoet\Automation\Engine\Data\Automation;
 use MailPoet\Automation\Engine\Data\AutomationRun;
 use MailPoet\Automation\Engine\Data\AutomationRunLog;
-use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Data\StepRunArgs;
 use MailPoet\Automation\Engine\Data\StepValidationArgs;
 use MailPoet\Automation\Engine\Data\SubjectEntry;
@@ -25,9 +23,6 @@ use MailPoet\Automation\Engine\WordPress;
 use Throwable;
 
 class StepHandler {
-  /** @var ActionStepRunner */
-  private $actionStepRunner;
-
   /** @var SubjectLoader */
   private $subjectLoader;
 
@@ -40,9 +35,6 @@ class StepHandler {
   /** @var AutomationStorage */
   private $automationStorage;
 
-  /** @var array<string, StepRunner> */
-  private $stepRunners = [];
-
   /** @var AutomationRunLogStorage */
   private $automationRunLogStorage;
 
@@ -52,11 +44,13 @@ class StepHandler {
   /** @var Registry */
   private $registry;
 
+  /** @var StepRunControllerFactory */
+  private $stepRunControllerFactory;
+
   /** @var StepScheduler */
   private $stepScheduler;
 
   public function __construct(
-    ActionStepRunner $actionStepRunner,
     Hooks $hooks,
     SubjectLoader $subjectLoader,
     WordPress $wordPress,
@@ -64,9 +58,9 @@ class StepHandler {
     AutomationRunLogStorage $automationRunLogStorage,
     AutomationStorage $automationStorage,
     Registry $registry,
+    StepRunControllerFactory $stepRunControllerFactory,
     StepScheduler $stepScheduler
   ) {
-    $this->actionStepRunner = $actionStepRunner;
     $this->hooks = $hooks;
     $this->subjectLoader = $subjectLoader;
     $this->wordPress = $wordPress;
@@ -74,28 +68,12 @@ class StepHandler {
     $this->automationRunLogStorage = $automationRunLogStorage;
     $this->automationStorage = $automationStorage;
     $this->registry = $registry;
+    $this->stepRunControllerFactory = $stepRunControllerFactory;
     $this->stepScheduler = $stepScheduler;
   }
 
   public function initialize(): void {
     $this->wordPress->addAction(Hooks::AUTOMATION_STEP, [$this, 'handle']);
-    $this->addStepRunner(Step::TYPE_ACTION, $this->actionStepRunner);
-    $this->wordPress->doAction(Hooks::STEP_RUNNER_INITIALIZE, [$this]);
-  }
-
-  public function addStepRunner(string $stepType, StepRunner $stepRunner): void {
-    $this->stepRunners[$stepType] = $stepRunner;
-  }
-
-  public function getStepRunners(): array {
-    return $this->stepRunners;
-  }
-
-  /**
-   * @param array<string, StepRunner> $stepRunners
-   */
-  public function setStepRunners(array $stepRunners): void {
-    $this->stepRunners = $stepRunners;
   }
 
   /** @param mixed $args */
@@ -153,18 +131,24 @@ class StepHandler {
     if (!$stepData) {
       throw Exceptions::automationStepNotFound($stepId);
     }
+
     $step = $this->registry->getStep($stepData->getKey());
-    $stepType = $stepData->getType();
+    if (!$step instanceof Action) {
-    if (isset($this->stepRunners[$stepType])) {
+      throw new InvalidStateException();
+    }
+
     $log = new AutomationRunLog($automationRun->getId(), $stepData->getId());
     try {
-        $requiredSubjects = $step instanceof Action ? $step->getSubjectKeys() : [];
+      $requiredSubjects = $step->getSubjectKeys();
       $subjectEntries = $this->getSubjectEntries($automationRun, $requiredSubjects);
       $args = new StepRunArgs($automation, $automationRun, $stepData, $subjectEntries, $stepRunNumber);
       $validationArgs = new StepValidationArgs($automation, $stepData, array_map(function (SubjectEntry $entry) {
         return $entry->getSubject();
       }, $subjectEntries));
-        $this->stepRunners[$stepType]->run($args, $validationArgs);
+
+      $step->validate($validationArgs);
+      $step->run($args, $this->stepRunControllerFactory->createController($args));
+
       $log->markCompletedSuccessfully();
     } catch (Throwable $e) {
       $log->markFailed();
@@ -178,9 +162,6 @@ class StepHandler {
       }
       $this->automationRunLogStorage->createAutomationRunLog($log);
     }
-    } else {
-      throw new InvalidStateException();
-    }
 
     // schedule next step if not scheduled by action
     if (!$this->stepScheduler->hasScheduledStep($args)) {

mailpoet/lib/Automation/Engine/Control/StepRunner.php

@@ -1,10 +0,0 @@
-<?php declare(strict_types = 1);
-
-namespace MailPoet\Automation\Engine\Control;
-
-use MailPoet\Automation\Engine\Data\StepRunArgs;
-use MailPoet\Automation\Engine\Data\StepValidationArgs;
-
-interface StepRunner {
-  public function run(StepRunArgs $runArgs, StepValidationArgs $validationArgs): void;
-}

mailpoet/lib/Automation/Engine/Control/Steps/ActionStepRunner.php

@@ -1,36 +0,0 @@
-<?php declare(strict_types = 1);
-
-namespace MailPoet\Automation\Engine\Control\Steps;
-
-use MailPoet\Automation\Engine\Control\StepRunControllerFactory;
-use MailPoet\Automation\Engine\Control\StepRunner;
-use MailPoet\Automation\Engine\Data\StepRunArgs;
-use MailPoet\Automation\Engine\Data\StepValidationArgs;
-use MailPoet\Automation\Engine\Exceptions\InvalidStateException;
-use MailPoet\Automation\Engine\Registry;
-
-class ActionStepRunner implements StepRunner {
-  /** @var Registry */
-  private $registry;
-
-  /** @var StepRunControllerFactory */
-  private $stepRunControllerFactory;
-
-  public function __construct(
-    Registry $registry,
-    StepRunControllerFactory $stepRunControllerFactory
-  ) {
-    $this->registry = $registry;
-    $this->stepRunControllerFactory = $stepRunControllerFactory;
-  }
-
-  public function run(StepRunArgs $runArgs, StepValidationArgs $validationArgs): void {
-    $action = $this->registry->getAction($runArgs->getStep()->getKey());
-    if (!$action) {
-      throw new InvalidStateException();
-    }
-
-    $action->validate($validationArgs);
-    $action->run($runArgs, $this->stepRunControllerFactory->createController($runArgs));
-  }
-}

mailpoet/lib/Automation/Engine/Hooks.php

@@ -18,7 +18,6 @@ class Hooks {
 
   public const INITIALIZE = 'mailpoet/automation/initialize';
   public const API_INITIALIZE = 'mailpoet/automation/api/initialize';
-  public const STEP_RUNNER_INITIALIZE = 'mailpoet/automation/step_runner/initialize';
   public const TRIGGER = 'mailpoet/automation/trigger';
   public const AUTOMATION_STEP = 'mailpoet/automation/step';
 

mailpoet/lib/DI/ContainerConfigurator.php

@@ -130,7 +130,6 @@ class ContainerConfigurator implements IContainerConfigurator {
     $container->autowire(\MailPoet\Automation\Engine\Control\StepScheduler::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Control\SubjectTransformerHandler::class)->setPublic(true)->setShared(false);
     $container->autowire(\MailPoet\Automation\Engine\Control\SubjectLoader::class)->setPublic(true);
-    $container->autowire(\MailPoet\Automation\Engine\Control\Steps\ActionStepRunner::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Control\TriggerHandler::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Engine::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Hooks::class)->setPublic(true);

mailpoet/tests/integration/Automation/Engine/Control/StepHandlerTest.php

@@ -3,39 +3,40 @@
 namespace MailPoet\Test\Automation\Engine\Control;
 
 use MailPoet\Automation\Engine\Control\StepHandler;
-use MailPoet\Automation\Engine\Control\StepRunner;
 use MailPoet\Automation\Engine\Data\Automation;
 use MailPoet\Automation\Engine\Data\AutomationRun;
 use MailPoet\Automation\Engine\Data\NextStep;
 use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Exceptions\InvalidStateException;
+use MailPoet\Automation\Engine\Integration\Action;
+use MailPoet\Automation\Engine\Registry;
 use MailPoet\Automation\Engine\Storage\AutomationRunStorage;
 use MailPoet\Automation\Engine\Storage\AutomationStorage;
 use MailPoet\Automation\Integrations\Core\Actions\DelayAction;
 use MailPoet\Automation\Integrations\MailPoet\Triggers\SomeoneSubscribesTrigger;
 
 class StepHandlerTest extends \MailPoetTest {
-
   /** @var AutomationStorage */
   private $automationStorage;
 
   /** @var AutomationRunStorage */
   private $automationRunStorage;
 
-  /** @var StepHandler */
-  private $testee;
-
-  /** @var array<string, StepRunner> */
-  private $originalRunners = [];
-
   public function _before() {
-    $this->testee = $this->diContainer->get(StepHandler::class);
     $this->automationStorage = $this->diContainer->get(AutomationStorage::class);
     $this->automationRunStorage = $this->diContainer->get(AutomationRunStorage::class);
-    $this->originalRunners = $this->testee->getStepRunners();
   }
 
   public function testItDoesOnlyProcessActiveAndDeactivatingAutomations() {
+    // The run method will be called twice: Once for the active automation and once for the deactivating automation.
+    $step = $this->createMock(Action::class);
+    $step->expects(self::exactly(2))->method('run');
+    $registry = $this->createMock(Registry::class);
+    $registry->expects(self::exactly(2))->method('getStep')->willReturn($step);
+    $stepHandler = $this->getServiceWithOverrides(StepHandler::class, [
+      'registry' => $registry,
+    ]);
+
     $automation = $this->createAutomation();
     $this->assertInstanceOf(Automation::class, $automation);
     $steps = $automation->getSteps();
@@ -44,13 +45,9 @@ class StepHandlerTest extends \MailPoetTest {
 
     $currentStep = current($steps);
     $this->assertInstanceOf(Step::class, $currentStep);
-    $runner = $this->createMock(StepRunner::class);
 
-    $runner->expects(self::exactly(2))->method('run'); // The run method will be called twice: Once for the active automation and once for the deactivating automation.
-
-    $this->testee->addStepRunner($currentStep->getType(), $runner);
     $this->assertSame(Automation::STATUS_ACTIVE, $automation->getStatus());
-    $this->testee->handle(['automation_run_id' => $automationRun->getId(), 'step_id' => $currentStep->getId()]);
+    $stepHandler->handle(['automation_run_id' => $automationRun->getId(), 'step_id' => $currentStep->getId()]);
     // no exception thrown.
     $newAutomationRun = $this->automationRunStorage->getAutomationRun($automationRun->getId());
     $this->assertInstanceOf(AutomationRun::class, $newAutomationRun);
@@ -58,7 +55,7 @@ class StepHandlerTest extends \MailPoetTest {
 
     $automation->setStatus(Automation::STATUS_DEACTIVATING);
     $this->automationStorage->updateAutomation($automation);
-    $this->testee->handle(['automation_run_id' => $automationRun->getId(), 'step_id' => $currentStep->getId()]);
+    $stepHandler->handle(['automation_run_id' => $automationRun->getId(), 'step_id' => $currentStep->getId()]);
     // no exception thrown.
     $newAutomationRun = $this->automationRunStorage->getAutomationRun($automationRun->getId());
     $this->assertInstanceOf(AutomationRun::class, $newAutomationRun);
@@ -78,7 +75,7 @@ class StepHandlerTest extends \MailPoetTest {
       $this->assertInstanceOf(AutomationRun::class, $automationRun);
       $error = null;
       try {
-        $this->testee->handle(['automation_run_id' => $automationRun->getId(), 'step_id' => $currentStep->getId()]);
+        $stepHandler->handle(['automation_run_id' => $automationRun->getId(), 'step_id' => $currentStep->getId()]);
       } catch (InvalidStateException $error) {
         $this->assertSame('mailpoet_automation_not_active', $error->getErrorCode(), "Automation with '$status' did not return expected error code.");
       }
@@ -92,6 +89,14 @@ class StepHandlerTest extends \MailPoetTest {
   }
 
   public function testAnDeactivatingAutomationBecomesDraftAfterLastRunIsExecuted() {
+    $step = $this->createMock(Action::class);
+    $step->expects(self::exactly(2))->method('run');
+    $registry = $this->createMock(Registry::class);
+    $registry->expects(self::exactly(2))->method('getStep')->willReturn($step);
+    $stepHandler = $this->getServiceWithOverrides(StepHandler::class, [
+      'registry' => $registry,
+    ]);
+
     $automation = $this->createAutomation();
     $this->assertInstanceOf(Automation::class, $automation);
     $automationRun1 = $this->tester->createAutomationRun($automation);
@@ -104,10 +109,8 @@ class StepHandlerTest extends \MailPoetTest {
     $steps = $automation->getSteps();
     $lastStep = end($steps);
     $this->assertInstanceOf(Step::class, $lastStep);
-    $runner = $this->createMock(StepRunner::class);
-    $this->testee->addStepRunner($lastStep->getType(), $runner);
 
-    $this->testee->handle(['automation_run_id' => $automationRun1->getId(), 'step_id' => $lastStep->getId()]);
+    $stepHandler->handle(['automation_run_id' => $automationRun1->getId(), 'step_id' => $lastStep->getId()]);
     /** @var Automation $updatedAutomation */
     $updatedAutomation = $this->automationStorage->getAutomation($automation->getId());
     /** @var AutomationRun $updatedautomationRun */
@@ -115,7 +118,7 @@ class StepHandlerTest extends \MailPoetTest {
     $this->assertSame(Automation::STATUS_DEACTIVATING, $updatedAutomation->getStatus());
     $this->assertSame(AutomationRun::STATUS_COMPLETE, $updatedautomationRun->getStatus());
 
-    $this->testee->handle(['automation_run_id' => $automationRun2->getId(), 'step_id' => $lastStep->getId()]);
+    $stepHandler->handle(['automation_run_id' => $automationRun2->getId(), 'step_id' => $lastStep->getId()]);
     /** @var Automation $updatedAutomation */
     $updatedAutomation = $this->automationStorage->getAutomation($automation->getId());
     /** @var AutomationRun $updatedautomationRun */
@@ -133,9 +136,4 @@ class StepHandlerTest extends \MailPoetTest {
       new Step('delay', Step::TYPE_ACTION, $delay->getKey(), [], [])
     );
   }
-
-  public function _after() {
-    parent::_after();
-    $this->testee->setStepRunners($this->originalRunners);
-  }
 }