Remove SubjectTransformerHandler from StepHandler

All subjects are created and persisted during AutomationRun creation. Therefore, the required
subject must exist when searching for it in the StepHandler. This commit reverses the last
changes and updates the test.

[MAILPOET-4935]

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

@@ -10,7 +10,6 @@ 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\Subject as SubjectData;
 use MailPoet\Automation\Engine\Data\SubjectEntry;
 use MailPoet\Automation\Engine\Exceptions;
 use MailPoet\Automation\Engine\Exceptions\InvalidStateException;
@@ -56,9 +55,6 @@ class StepHandler {
   /** @var Registry */
   private $registry;
 
-  /** @var SubjectTransformerHandler */
-  private $subjectTransformerHandler;
-
   public function __construct(
     ActionScheduler $actionScheduler,
     ActionStepRunner $actionStepRunner,
@@ -68,8 +64,7 @@ class StepHandler {
     AutomationRunStorage $automationRunStorage,
     AutomationRunLogStorage $automationRunLogStorage,
     AutomationStorage $automationStorage,
-    Registry $registry,
+    Registry $registry
-    SubjectTransformerHandler $subjectTransformerHandler
   ) {
     $this->actionScheduler = $actionScheduler;
     $this->actionStepRunner = $actionStepRunner;
@@ -80,7 +75,6 @@ class StepHandler {
     $this->automationRunLogStorage = $automationRunLogStorage;
     $this->automationStorage = $automationStorage;
     $this->registry = $registry;
-    $this->subjectTransformerHandler = $subjectTransformerHandler;
   }
 
   public function initialize(): void {
@@ -223,7 +217,6 @@ class StepHandler {
     $subjectEntries = [];
     foreach ($requiredSubjectKeys as $key) {
       $subjectData = $subjectDataMap[$key] ?? null;
-      $subjectData = $subjectData ?? $this->transformSubject($key, $automationRun);
       if (!$subjectData) {
         throw Exceptions::subjectDataNotFound($key, $automationRun->getId());
       }
@@ -234,13 +227,6 @@ class StepHandler {
     return $subjectEntries;
   }
 
-  /**
-   * @return SubjectData[]|null
-   */
-  private function transformSubject(string $target, AutomationRun $automationRun): ?array {
-    return $this->subjectTransformerHandler->transformSubjectData($target, $automationRun);
-  }
-
   private function postProcessAutomationRun(int $automationRunId): void {
     $automationRun = $this->automationRunStorage->getAutomationRun($automationRunId);
     if (!$automationRun) {

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

@@ -3,29 +3,22 @@
 namespace MailPoet\Automation\Engine\Control;
 
 use MailPoet\Automation\Engine\Data\Automation;
-use MailPoet\Automation\Engine\Data\AutomationRun;
 use MailPoet\Automation\Engine\Data\Step as StepData;
 use MailPoet\Automation\Engine\Data\Subject;
 use MailPoet\Automation\Engine\Integration\Step;
 use MailPoet\Automation\Engine\Integration\SubjectTransformer;
 use MailPoet\Automation\Engine\Integration\Trigger;
 use MailPoet\Automation\Engine\Registry;
-use MailPoet\Automation\Engine\Storage\AutomationStorage;
 
 class SubjectTransformerHandler {
 
   /* @var Registry */
   private $registry;
 
-  /* @var AutomationStorage */
-  private $automationStorage;
-
   public function __construct(
-    Registry $registry,
+    Registry $registry
-    AutomationStorage $automationStorage
   ) {
     $this->registry = $registry;
-    $this->automationStorage = $automationStorage;
   }
 
   public function subjectKeysForTrigger(Trigger $trigger): array {
@@ -131,27 +124,6 @@ class SubjectTransformerHandler {
     return array_values($allSubjects);
   }
 
-  /**
-   * @return Subject[]|null
-   */
-  public function transformSubjectData(string $target, AutomationRun $automationRun): ?array {
-    $automation = $this->automationStorage->getAutomation($automationRun->getAutomationId(), $automationRun->getVersionId());
-    if (!$automation || !in_array($target, $this->subjectKeysForAutomation($automation), true)) {
-      return null;
-    }
-
-    $transformedSubjects = [];
-    $subjects = $automationRun->getSubjects();
-    foreach ($subjects as $subject) {
-      $transformedSubject = $this->transformSubjectTo($subject, $target);
-      if (!$transformedSubject) {
-        continue;
-      }
-      $transformedSubjects[] = $subject;
-    }
-    return count($transformedSubjects) > 0 ? $transformedSubjects : null;
-  }
-
   private function transformSubjectTo(Subject $subject, string $target): ?Subject {
     $transformerChain = $this->getTransformerChain($subject->getKey(), $target);
     if (!$transformerChain) {

mailpoet/tests/unit/Automation/Engine/Control/SubjectTransformerHandlerTest.php

@@ -3,13 +3,11 @@
 namespace MailPoet\Automation\Engine\Control;
 
 use MailPoet\Automation\Engine\Data\Automation;
-use MailPoet\Automation\Engine\Data\AutomationRun;
 use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Data\Subject;
 use MailPoet\Automation\Engine\Integration\SubjectTransformer;
 use MailPoet\Automation\Engine\Integration\Trigger;
 use MailPoet\Automation\Engine\Registry;
-use MailPoet\Automation\Engine\Storage\AutomationStorage;
 use MailPoetUnitTest;
 
 class SubjectTransformerHandlerTest extends MailPoetUnitTest {
@@ -45,8 +43,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest {
     $registry->expects($this->any())->method('getStep')->willReturnCallback(function($key) use ($trigger){
       return $key === 'trigger' ? $trigger : null;
     });
-    $storage = $this->createMock(AutomationStorage::class);
+    $testee = new SubjectTransformerHandler($registry);
-    $testee = new SubjectTransformerHandler($registry, $storage);
 
     $triggerData = $this->createMock(Step::class);
     $triggerData->expects($this->any())->method('getType')->willReturn(Step::TYPE_TRIGGER);
@@ -81,8 +78,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest {
     $registry->expects($this->any())->method('getStep')->willReturnCallback(function($key) use ($trigger){
       return $key === 'trigger' ? $trigger : null;
     });
-    $storage = $this->createMock(AutomationStorage::class);
+    $testee = new SubjectTransformerHandler($registry);
-    $testee = new SubjectTransformerHandler($registry, $storage);
 
     $triggerData = $this->createMock(Step::class);
     $triggerData->expects($this->any())->method('getType')->willReturn(Step::TYPE_TRIGGER);
@@ -115,8 +111,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest {
       }
       return null;
     });
-    $storage = $this->createMock(AutomationStorage::class);
+    $testee = new SubjectTransformerHandler($registry);
-    $testee = new SubjectTransformerHandler($registry, $storage);
 
     $trigger1Data = $this->createMock(Step::class);
     $trigger1Data->expects($this->any())->method('getType')->willReturn(Step::TYPE_TRIGGER);
@@ -132,7 +127,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest {
     $this->assertEquals(['b', 'c'], $result);
   }
 
-  public function testItTransformsSubjects(): void {
+  public function testItProvidesAllSubjects(): void {
 
     $subjectTransformerStart = $this->createMock(SubjectTransformer::class);
     $subjectTransformerStart->expects($this->any())->method('accepts')->willReturn('from');
@@ -154,15 +149,19 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest {
       return $subject;
     });
 
+    $unrelatedTransformer = $this->createMock(SubjectTransformer::class);
+    $unrelatedTransformer->expects($this->any())->method('accepts')->willReturn('unrelated');
+    $unrelatedTransformer->expects($this->never())->method('returns')->willReturn('some-other-unrelated');
+    $unrelatedTransformer->expects($this->never())->method('transform');
+
+
     $transformer = [
       $subjectTransformerEnd,
       $subjectTransformerStart,
+      $unrelatedTransformer,
     ];
-    $automation = $this->createMock(Automation::class);
+    $trigger = $this->createMock(Trigger::class);
-    $triggerStep = $this->createMock(Step::class);
+    $trigger->expects($this->any())->method('getSubjectKeys')->willReturn(['from']);
-    $triggerStep->expects($this->any())->method('getType')->willReturn(Step::TYPE_TRIGGER);
-    $triggerStep->expects($this->any())->method('getKey')->willReturn('trigger');
-    $automation->expects($this->any())->method('getSteps')->willReturn([$triggerStep]);
 
     $registry = $this->createMock(Registry::class);
     $registry->expects($this->any())->method('getStep')->willReturnCallback(
@@ -178,22 +177,14 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest {
       }
     );
     $registry->expects($this->any())->method('getSubjectTransformer')->willReturn($transformer);
-    $storage = $this->createMock(AutomationStorage::class);
+    $testee = new SubjectTransformerHandler($registry);
-    $storage->expects($this->any())->method('getAutomation')->willReturnCallback(function($id) use ($automation) {
-      return $id === 1 ? $automation : null;
-    });
-    $testee = new SubjectTransformerHandler($registry, $storage);
 
     $subject = new Subject('from', ['key' => 'value']);
-    $automationRun = $this->createMock(AutomationRun::class);
+    $subjects = $testee->provideAllSubjects($trigger, $subject);
-    $automationRun->expects($this->any())->method('getAutomationId')->willReturn(1);
-    $automationRun->expects($this->any())->method('getVersionId')->willReturn(1);
-    $automationRun->expects($this->any())->method('getSubjects')->willReturn([$subject]);
-    $subjects = $testee->transformSubjectData('to', $automationRun);
     $this->assertNotNull($subjects);
-    $this->assertCount(1, $subjects);
+    $this->assertCount(3, $subjects);
-    $subject = current($subjects);
+    $this->assertSame(['from', 'middle', 'to'], array_map(function(Subject $subject): string { return $subject->getKey();
-    $this->assertInstanceOf(Subject::class, $subject);
+
-    $this->assertEquals('to', $subject->getKey());
+    }, $subjects));
   }
 }