diff --git a/mailpoet/lib/AdminPages/Pages/AutomationEditor.php b/mailpoet/lib/AdminPages/Pages/AutomationEditor.php index 54b5591382..d8a8bf7e34 100644 --- a/mailpoet/lib/AdminPages/Pages/AutomationEditor.php +++ b/mailpoet/lib/AdminPages/Pages/AutomationEditor.php @@ -101,7 +101,7 @@ class AutomationEditor { $steps[$key] = [ 'key' => $step->getKey(), 'name' => $step->getName(), - 'subject_keys' => $step instanceof Trigger ? $this->subjectTransformerHandler->subjectKeysForTrigger($step) : $step->getSubjectKeys(), + 'subject_keys' => $step instanceof Trigger ? $this->subjectTransformerHandler->getSubjectKeysForTrigger($step) : $step->getSubjectKeys(), 'args_schema' => $step->getArgsSchema()->toArray(), ]; } diff --git a/mailpoet/lib/Automation/Engine/Control/SubjectTransformerHandler.php b/mailpoet/lib/Automation/Engine/Control/SubjectTransformerHandler.php index 470bde3598..fe4a1d096f 100644 --- a/mailpoet/lib/Automation/Engine/Control/SubjectTransformerHandler.php +++ b/mailpoet/lib/Automation/Engine/Control/SubjectTransformerHandler.php @@ -5,8 +5,6 @@ namespace MailPoet\Automation\Engine\Control; use MailPoet\Automation\Engine\Data\Automation; 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; @@ -21,181 +19,85 @@ class SubjectTransformerHandler { $this->registry = $registry; } - public function subjectKeysForTrigger(Trigger $trigger): array { - $subjectKeys = $trigger->getSubjectKeys(); - $possibleKeys = []; - foreach ($subjectKeys as $key) { - $possibleKeys = array_merge($possibleKeys, $this->getPossibleTransformations($key)); - } - return array_unique(array_values(array_merge($subjectKeys, $possibleKeys))); - } - - /** @return string[] */ - public function subjectKeysForAutomation(Automation $automation): array { - $configuredTriggers = array_filter( + public function getSubjectKeysForAutomation(Automation $automation): array { + $triggerData = array_values(array_filter( $automation->getSteps(), function (StepData $step): bool { return $step->getType() === StepData::TYPE_TRIGGER; } - ); - - $triggers = array_filter(array_map( - function(StepData $step): ?Step { - return $this->registry->getStep($step->getKey()); - }, - $configuredTriggers )); - $triggerKeys = []; + $triggers = array_filter(array_map( + function (StepData $step): ?Trigger { + return $this->registry->getTrigger($step->getKey()); + }, + $triggerData + )); + $all = []; foreach ($triggers as $trigger) { - $triggerKeys[$trigger->getKey()] = $trigger->getSubjectKeys(); + $all[] = $this->getSubjectKeysForTrigger($trigger); } - if (!$triggerKeys) { - return []; - } - - $triggerKeys = count($triggerKeys) === 1 ? current($triggerKeys) : array_intersect(...array_values($triggerKeys)); - $possibleKeys = []; - foreach ($triggerKeys as $key) { - $possibleKeys = array_merge($possibleKeys, $this->getPossibleTransformations($key)); - } - return array_unique(array_values(array_merge($triggerKeys, $possibleKeys))); + $all = count($all) > 1 ? array_intersect(...$all) : $all[0] ?? []; + return array_values(array_unique($all)); } - private function getPossibleTransformations(string $key, string $stopKey = null): array { - if (!$stopKey) { - $stopKey = $key; + public function getSubjectKeysForTrigger(Trigger $trigger): array { + $all = []; + foreach ($trigger->getSubjectKeys() as $key) { + $all = array_merge($all, $this->getSubjectKeysForSingleKey($key)); } - $allTransformer = $this->registry->getSubjectTransformer(); - $possibleTransformer = array_filter( - $allTransformer, - function (SubjectTransformer $transformer) use ($key): bool { - return $transformer->accepts() === $key; - } - ); - - $possibleKeys = []; - foreach ($possibleTransformer as $transformer) { - $possibleKey = $transformer->returns(); - if ($possibleKey === $stopKey) { - continue; - } - $possibleKeys[$possibleKey] = $possibleKey; - $possibleKeys = array_merge( - $possibleKeys, - $this->getPossibleTransformations($possibleKey, $stopKey) - ); - } - return $possibleKeys; + return $all; } - /** - * @param Trigger $trigger - * @param Subject ...$subjects - * @return Subject[] - */ - public function provideAllSubjects(Trigger $trigger, Subject ...$subjects): array { - $allSubjectsKeys = $this->subjectKeysForTrigger($trigger); - $allSubjectKeyTargets = array_filter( - array_diff($allSubjectsKeys, array_map( - function(Subject $subject): string { - return $subject->getKey(); - }, - $subjects - )), - function(string $target) use ($subjects): bool { - return $this->canSubjectsTransformTo($target, ...$subjects); - } - ); - - $allSubjects = []; - foreach ($subjects as $existingSubject) { - $allSubjects[$existingSubject->getKey()] = $existingSubject; - } - foreach ($allSubjectKeyTargets as $target) { - if (isset($allSubjects[$target])) { - continue; - } - foreach ($subjects as $subject) { - $transformedSubject = $this->transformSubjectTo($subject, $target); - if ($transformedSubject) { - $allSubjects[$transformedSubject->getKey()] = $transformedSubject; - } - } - } - - $allSubjectsTransformed = true; - do { - foreach ($allSubjectKeyTargets as $key) { - if (!isset($allSubjects[$key])) { - $allSubjectsTransformed = false; - break; - } - } - $allSubjects = $allSubjectsTransformed ? $allSubjects : $this->provideAllSubjects($trigger, ...array_values($allSubjects)); - } while (!$allSubjectsTransformed); - return array_values($allSubjects); - } - - private function transformSubjectTo(Subject $subject, string $target): ?Subject { - $transformerChain = $this->getTransformerChain($subject->getKey(), $target); - if (!$transformerChain) { - return null; - } - foreach ($transformerChain as $transformer) { - $subject = $transformer->transform($subject); - } - return $subject; - } - - /** - * @return SubjectTransformer[] - */ - private function getTransformerChain(string $from, string $to): array { - $transformers = $this->registry->getSubjectTransformer(); - $transformerChain = []; - - //walk the graph of transformers to find the shortest path - $queue = []; - $queue[] = [$from, []]; - while (count($queue) > 0) { - $current = array_shift($queue); - $currentKey = $current[0]; - $currentPath = $current[1]; - if ($currentKey === $to) { - $transformerChain = $currentPath; - break; - } - foreach ($transformers as $transformer) { - if ($transformer->accepts() === $currentKey) { - $newPath = $currentPath; - $newPath[] = $transformer; - $queue[] = [$transformer->returns(), $newPath]; - } - } - } - - return $transformerChain; - } - - private function canSubjectsTransformTo(string $target, Subject ...$subjects): bool { - if ( - in_array($target, array_map( - function(Subject $subject): string { - return $subject->getKey(); - }, - $subjects - )) - ) { - return true; - } - + public function getAllSubjects(Subject ...$subjects): array { + $transformerMap = $this->getTransformerMap(); + $all = []; foreach ($subjects as $subject) { - $possibleTransformations = $this->getPossibleTransformations($subject->getKey()); - if (in_array($target, $possibleTransformations)) { - return true; + $all[$subject->getKey()] = $subject; + } + + $queue = array_map( + function(Subject $subject): string { + return $subject->getKey(); + }, + $subjects + ); + while ($key = array_shift($queue)) { + foreach ($transformerMap[$key] ?? [] as $transformer) { + $newKey = $transformer->returns(); + if (!isset($all[$newKey])) { + $all[$newKey] = $transformer->transform($all[$key]); + $queue[] = $newKey; + } } } - return false; + return array_values($all); + } + + private function getTransformerMap(): array { + $transformerMap = []; + foreach ($this->registry->getSubjectTransformer() as $transformer) { + $transformerMap[$transformer->accepts()] = array_merge($transformerMap[$transformer->accepts()] ?? [], [$transformer]); + } + return $transformerMap; + } + + /** + * @return string[] + */ + private function getSubjectKeysForSingleKey(string $subjectKey): array { + $transformerMap = $this->getTransformerMap(); + $all = [$subjectKey]; + $queue = [$subjectKey]; + while ($key = array_shift($queue)) { + foreach ($transformerMap[$key] ?? [] as $transformer) { + $newKey = $transformer->returns(); + if (!in_array($newKey, $all, true)) { + $all[] = $newKey; + $queue[] = $newKey; + } + } + } + return $all; } } diff --git a/mailpoet/lib/Automation/Engine/Control/TriggerHandler.php b/mailpoet/lib/Automation/Engine/Control/TriggerHandler.php index 11b14b0e4a..04a4445932 100644 --- a/mailpoet/lib/Automation/Engine/Control/TriggerHandler.php +++ b/mailpoet/lib/Automation/Engine/Control/TriggerHandler.php @@ -59,7 +59,7 @@ class TriggerHandler { /** @param Subject[] $subjects */ public function processTrigger(Trigger $trigger, array $subjects): void { - $subjects = $this->subjectTransformerHandler->provideAllSubjects($trigger, ...$subjects); + $subjects = $this->subjectTransformerHandler->getAllSubjects(...$subjects); $automations = $this->automationStorage->getActiveAutomationsByTrigger($trigger); foreach ($automations as $automation) { $step = $automation->getTrigger($trigger->getKey()); diff --git a/mailpoet/lib/Automation/Engine/Validation/AutomationRules/ValidStepOrderRule.php b/mailpoet/lib/Automation/Engine/Validation/AutomationRules/ValidStepOrderRule.php index 5eec17a599..f5bf8639a8 100644 --- a/mailpoet/lib/Automation/Engine/Validation/AutomationRules/ValidStepOrderRule.php +++ b/mailpoet/lib/Automation/Engine/Validation/AutomationRules/ValidStepOrderRule.php @@ -45,7 +45,7 @@ class ValidStepOrderRule implements AutomationNodeVisitor { return; } - $subjectKeys = $this->subjectTransformerHandler->subjectKeysForAutomation($automation); + $subjectKeys = $this->subjectTransformerHandler->getSubjectKeysForAutomation($automation); $missingSubjectKeys = array_diff($requiredSubjectKeys, $subjectKeys); if (count($missingSubjectKeys) > 0) { throw Exceptions::missingRequiredSubjects($step, $missingSubjectKeys); diff --git a/mailpoet/tests/unit/Automation/Engine/Control/SubjectTransformerHandlerTest.php b/mailpoet/tests/unit/Automation/Engine/Control/SubjectTransformerHandlerTest.php index 37a31e3022..fa1860dcf3 100644 --- a/mailpoet/tests/unit/Automation/Engine/Control/SubjectTransformerHandlerTest.php +++ b/mailpoet/tests/unit/Automation/Engine/Control/SubjectTransformerHandlerTest.php @@ -40,7 +40,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $trigger->expects($this->any())->method('getSubjectKeys')->willReturn([$triggerSubject]); $registry = $this->createMock(Registry::class); $registry->expects($this->any())->method('getSubjectTransformer')->willReturn($transformers); - $registry->expects($this->any())->method('getStep')->willReturnCallback(function($key) use ($trigger){ + $registry->expects($this->any())->method('getTrigger')->willReturnCallback(function($key) use ($trigger){ return $key === 'trigger' ? $trigger : null; }); $testee = new SubjectTransformerHandler($registry); @@ -50,8 +50,8 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $triggerData->expects($this->any())->method('getKey')->willReturn('trigger'); $automation = $this->createMock(Automation::class); $automation->method('getSteps')->willReturn([$triggerData]); - $result = $testee->subjectKeysForAutomation($automation); - $this->assertEquals(['subject_a', 'subject_b1', 'subject_c1', 'subject_d1', 'subject_b2', 'subject_c2'], $result); + $result = $testee->getSubjectKeysForAutomation($automation); + $this->assertEquals(['subject_a', 'subject_b1', 'subject_b2', 'subject_c1', 'subject_c2', 'subject_d1'], $result); } public function testItDoesNotRunInfiniteWhileFindingAllSubjects(): void { @@ -75,7 +75,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $trigger->expects($this->any())->method('getSubjectKeys')->willReturn([$triggerSubject]); $registry = $this->createMock(Registry::class); $registry->expects($this->any())->method('getSubjectTransformer')->willReturn($transformers); - $registry->expects($this->any())->method('getStep')->willReturnCallback(function($key) use ($trigger){ + $registry->expects($this->any())->method('getTrigger')->willReturnCallback(function($key) use ($trigger){ return $key === 'trigger' ? $trigger : null; }); $testee = new SubjectTransformerHandler($registry); @@ -85,7 +85,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $triggerData->expects($this->any())->method('getKey')->willReturn('trigger'); $automation = $this->createMock(Automation::class); $automation->method('getSteps')->willReturn([$triggerData]); - $result = $testee->subjectKeysForAutomation($automation); + $result = $testee->getSubjectKeysForAutomation($automation); $this->assertEquals(['subject_a', 'subject_b', 'subject_c'], $result); } @@ -102,7 +102,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $registry = $this->createMock(Registry::class); $registry->expects($this->any())->method('getSubjectTransformer')->willReturn([]); - $registry->expects($this->any())->method('getStep')->willReturnCallback(function($key) use ($trigger1, $trigger2){ + $registry->expects($this->any())->method('getTrigger')->willReturnCallback(function($key) use ($trigger1, $trigger2){ if ($key === 'trigger1') { return $trigger1; } @@ -123,7 +123,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $automation = $this->createMock(Automation::class); $automation->method('getSteps')->willReturn([$trigger1Data, $trigger2Data]); - $result = $testee->subjectKeysForAutomation($automation); + $result = $testee->getSubjectKeysForAutomation($automation); $this->assertEquals(['b', 'c'], $result); } @@ -164,7 +164,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $trigger->expects($this->any())->method('getSubjectKeys')->willReturn(['from']); $registry = $this->createMock(Registry::class); - $registry->expects($this->any())->method('getStep')->willReturnCallback( + $registry->expects($this->any())->method('getTrigger')->willReturnCallback( function($key) { if ($key !== 'trigger') { return null; @@ -180,7 +180,7 @@ class SubjectTransformerHandlerTest extends MailPoetUnitTest { $testee = new SubjectTransformerHandler($registry); $subject = new Subject('from', ['key' => 'value']); - $subjects = $testee->provideAllSubjects($trigger, $subject); + $subjects = $testee->getAllSubjects($subject); $this->assertNotNull($subjects); $this->assertCount(3, $subjects); $this->assertSame(['from', 'middle', 'to'], array_map(function(Subject $subject): string { return $subject->getKey(); diff --git a/mailpoet/tests/unit/Automation/Engine/Validation/AutomationRules/ValidStepOrderRuleTest.php b/mailpoet/tests/unit/Automation/Engine/Validation/AutomationRules/ValidStepOrderRuleTest.php index aab3c85a90..5ff9d432dd 100644 --- a/mailpoet/tests/unit/Automation/Engine/Validation/AutomationRules/ValidStepOrderRuleTest.php +++ b/mailpoet/tests/unit/Automation/Engine/Validation/AutomationRules/ValidStepOrderRuleTest.php @@ -35,7 +35,7 @@ class ValidStepOrderRuleTest extends AutomationRuleTest { ]); $subjectTransformer = $this->createMock(SubjectTransformerHandler::class); - $subjectTransformer->expects($this->any())->method('subjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { + $subjectTransformer->expects($this->any())->method('getSubjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { return $a === $automation ? ['subject-a'] : []; }); $rule = new ValidStepOrderRule($registry, $subjectTransformer); @@ -63,7 +63,7 @@ class ValidStepOrderRuleTest extends AutomationRuleTest { ]); $subjectTransformer = $this->createMock(SubjectTransformerHandler::class); - $subjectTransformer->expects($this->any())->method('subjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { + $subjectTransformer->expects($this->any())->method('getSubjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { return $a === $automation ? ['subject-a'] : []; }); @@ -90,7 +90,7 @@ class ValidStepOrderRuleTest extends AutomationRuleTest { ]); $subjectTransformer = $this->createMock(SubjectTransformerHandler::class); - $subjectTransformer->expects($this->any())->method('subjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { + $subjectTransformer->expects($this->any())->method('getSubjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { return $a === $automation ? ['subject-a'] : []; }); $rule = new ValidStepOrderRule($registry, $subjectTransformer); @@ -113,7 +113,7 @@ class ValidStepOrderRuleTest extends AutomationRuleTest { ]); $subjectTransformer = $this->createMock(SubjectTransformerHandler::class); - $subjectTransformer->expects($this->any())->method('subjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { + $subjectTransformer->expects($this->any())->method('getSubjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { return $a === $automation ? ['subject-a', 'subject-b'] : []; }); @@ -140,7 +140,7 @@ class ValidStepOrderRuleTest extends AutomationRuleTest { ]); $subjectTransformer = $this->createMock(SubjectTransformerHandler::class); - $subjectTransformer->expects($this->any())->method('subjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { + $subjectTransformer->expects($this->any())->method('getSubjectKeysForAutomation')->willReturnCallback(function($a) use ($automation) { return $a === $automation ? ['subject-a', 'subject-b'] : []; }); $rule = new ValidStepOrderRule($registry, $subjectTransformer);