From c4faa5338661b04ac96e29b56a871a7f95387d3c Mon Sep 17 00:00:00 2001 From: David Remer Date: Wed, 15 Feb 2023 08:44:17 +0200 Subject: [PATCH] Store run subjects in extra table [MAILPOET-4966] --- .../Automation/Engine/Data/AutomationRun.php | 9 +-- .../lib/Automation/Engine/Data/Subject.php | 6 +- .../Engine/Storage/AutomationRunStorage.php | 81 ++++++++++++++++--- .../Migrations/Migration_20230215_050813.php | 73 +++++++++++++++++ mailpoet/tasks/phpstan/phpstan.neon | 2 +- .../AutomationStatisticsStorageTest.php | 2 +- 6 files changed, 152 insertions(+), 21 deletions(-) create mode 100644 mailpoet/lib/Migrations/Migration_20230215_050813.php diff --git a/mailpoet/lib/Automation/Engine/Data/AutomationRun.php b/mailpoet/lib/Automation/Engine/Data/AutomationRun.php index 330c41eafb..829ad02f09 100644 --- a/mailpoet/lib/Automation/Engine/Data/AutomationRun.php +++ b/mailpoet/lib/Automation/Engine/Data/AutomationRun.php @@ -3,7 +3,6 @@ namespace MailPoet\Automation\Engine\Data; use DateTimeImmutable; -use MailPoet\Automation\Engine\Utils\Json; class AutomationRun { public const STATUS_RUNNING = 'running'; @@ -107,11 +106,9 @@ class AutomationRun { 'status' => $this->status, 'created_at' => $this->createdAt->format(DateTimeImmutable::W3C), 'updated_at' => $this->updatedAt->format(DateTimeImmutable::W3C), - 'subjects' => Json::encode( - array_map(function (Subject $subject): array { + 'subjects' => array_map(function (Subject $subject): array { return $subject->toArray(); - }, $this->subjects) - ), + }, $this->subjects), ]; } @@ -122,7 +119,7 @@ class AutomationRun { $data['trigger_key'], array_map(function (array $subject) { return Subject::fromArray($subject); - }, Json::decode($data['subjects'])) + }, $data['subjects']) ); $automationRun->id = (int)$data['id']; $automationRun->status = $data['status']; diff --git a/mailpoet/lib/Automation/Engine/Data/Subject.php b/mailpoet/lib/Automation/Engine/Data/Subject.php index 68cfd72011..d327d5377f 100644 --- a/mailpoet/lib/Automation/Engine/Data/Subject.php +++ b/mailpoet/lib/Automation/Engine/Data/Subject.php @@ -2,6 +2,8 @@ namespace MailPoet\Automation\Engine\Data; +use MailPoet\Automation\Engine\Utils\Json; + class Subject { /** @var string */ private $key; @@ -28,11 +30,11 @@ class Subject { public function toArray(): array { return [ 'key' => $this->getKey(), - 'args' => $this->getArgs(), + 'args' => Json::encode($this->getArgs()), ]; } public static function fromArray(array $data): self { - return new self($data['key'], $data['args']); + return new self($data['key'], Json::decode($data['args'])); } } diff --git a/mailpoet/lib/Automation/Engine/Storage/AutomationRunStorage.php b/mailpoet/lib/Automation/Engine/Storage/AutomationRunStorage.php index 457617b665..bc7f5b4f45 100644 --- a/mailpoet/lib/Automation/Engine/Storage/AutomationRunStorage.php +++ b/mailpoet/lib/Automation/Engine/Storage/AutomationRunStorage.php @@ -11,28 +11,60 @@ class AutomationRunStorage { /** @var string */ private $table; + /** @var string */ + private $subjectTable; + /** @var wpdb */ private $wpdb; public function __construct() { global $wpdb; $this->table = $wpdb->prefix . 'mailpoet_automation_runs'; + $this->subjectTable = $wpdb->prefix . 'mailpoet_automation_run_subjects'; $this->wpdb = $wpdb; } public function createAutomationRun(AutomationRun $automationRun): int { - $result = $this->wpdb->insert($this->table, $automationRun->toArray()); + $automationTableData = $automationRun->toArray(); + unset($automationTableData['subjects']); + $result = $this->wpdb->insert($this->table, $automationTableData); if ($result === false) { throw Exceptions::databaseError($this->wpdb->last_error); } - return $this->wpdb->insert_id; + $automationRunId = $this->wpdb->insert_id; + $subjectTableData = $automationRun->toArray()['subjects']; + + if (!$subjectTableData) { + //We allow for AutomationRuns with no subjects. + return $automationRunId; + } + + $sql = 'insert into ' . esc_sql($this->subjectTable) . ' (`automation_run_id`, `key`, `args`) values %s'; + $values = []; + foreach ($subjectTableData as $entry) { + $values[] = (string)$this->wpdb->prepare("(%d,%s,%s)", $automationRunId, $entry['key'], $entry['args']); + } + $sql = sprintf($sql, implode(',', $values)); + $result = $this->wpdb->query($sql); + if ($result === false) { + throw Exceptions::databaseError($this->wpdb->last_error); + } + + return $automationRunId; } public function getAutomationRun(int $id): ?AutomationRun { $table = esc_sql($this->table); - $query = (string)$this->wpdb->prepare("SELECT * FROM $table WHERE id = %d", $id); - $result = $this->wpdb->get_row($query, ARRAY_A); - return $result ? AutomationRun::fromArray((array)$result) : null; + $subjectTable = esc_sql($this->subjectTable); + $query = (string)$this->wpdb->prepare("SELECT * FROM $table WHERE id = %d", $id); + $data = $this->wpdb->get_row($query, ARRAY_A); + if (!is_array($data) || !$data) { + return null; + } + $query = (string)$this->wpdb->prepare("SELECT * FROM $subjectTable WHERE automation_run_id = %d", $id); + $subjects = $this->wpdb->get_results($query, ARRAY_A); + $data['subjects'] = is_array($subjects) ? $subjects : []; + return AutomationRun::fromArray((array)$data); } /** @@ -41,14 +73,39 @@ class AutomationRunStorage { */ public function getAutomationRunsForAutomation(Automation $automation): array { $table = esc_sql($this->table); - $query = (string)$this->wpdb->prepare("SELECT * FROM $table WHERE automation_id = %d", $automation->getId()); - $result = $this->wpdb->get_results($query, ARRAY_A); - return is_array($result) ? array_map( - function(array $runData): AutomationRun { + $subjectTable = esc_sql($this->subjectTable); + $query = (string)$this->wpdb->prepare("SELECT * FROM $table WHERE automation_id = %d order by id", $automation->getId()); + $automationRuns = $this->wpdb->get_results($query, ARRAY_A); + if (!is_array($automationRuns) || !$automationRuns) { + return []; + } + + $automationRunIds = array_map( + function(array $runData): int { + return (int)$runData['id']; + }, + $automationRuns + ); + + $sql = sprintf("SELECT * FROM $subjectTable WHERE automation_run_id in (%s) order by automation_run_id", implode(',', array_map(function() {return '%d'; + + }, $automationRunIds))); + + $query = (string)$this->wpdb->prepare($sql, ...$automationRunIds); + $subjects = $this->wpdb->get_results($query, ARRAY_A); + + return array_map( + function(array $runData) use ($subjects): AutomationRun { + $runData['subjects'] = array_filter( + is_array($subjects) ? $subjects : [], + function(array $subjectData) use ($runData): bool { + return (int)$subjectData['automation_run_id'] === (int)$runData['id']; + } + ); return AutomationRun::fromArray($runData); }, - $result - ) : []; + $automationRuns + ); } public function getCountForAutomation(Automation $automation, string ...$status): int { @@ -97,5 +154,7 @@ class AutomationRunStorage { public function truncate(): void { $table = esc_sql($this->table); $this->wpdb->query("TRUNCATE $table"); + $table = esc_sql($this->subjectTable); + $this->wpdb->query("TRUNCATE $table"); } } diff --git a/mailpoet/lib/Migrations/Migration_20230215_050813.php b/mailpoet/lib/Migrations/Migration_20230215_050813.php new file mode 100644 index 0000000000..5bf64724c5 --- /dev/null +++ b/mailpoet/lib/Migrations/Migration_20230215_050813.php @@ -0,0 +1,73 @@ +subjectsMigration(); + } + + private function subjectsMigration(): void { + $this->createTable('automation_run_subjects', [ + '`automation_run_id` int(11) unsigned NOT NULL', + '`key` varchar(191)', + '`args` longtext', + 'index (automation_run_id)', + ]); + $this->moveSubjectData(); + $this->dropSubjectColumn(); + } + + private function moveSubjectData(): void { + global $wpdb; + $runTable = $wpdb->prefix . 'mailpoet_automation_runs'; + $subjectTable = $wpdb->prefix . 'mailpoet_automation_run_subjects'; + if (!$this->columnExists($runTable, 'subjects')) { + return; + } + $sql = "SELECT id,subjects FROM $runTable"; + $results = $wpdb->get_results($sql, ARRAY_A); + if (!is_array($results) || !$results) { + return; + } + + foreach ($results as $result) { + $subjects = $result['subjects']; + if (!$subjects) { + continue; + } + $subjects = json_decode($subjects, true); + if (!is_array($subjects) || !$subjects) { + continue; + } + $values = []; + foreach ($subjects as $subject) { + $values[] = (string)$wpdb->prepare("(%d,%s,%s)", $result['id'], $subject['key'], json_encode($subject['args'])); + } + $sql = sprintf("INSERT INTO $subjectTable (`automation_run_id`, `key`, `args`) VALUES %s", implode(',', $values)); + if ($wpdb->query($sql) === false) { + continue; + } + + $sql = $wpdb->prepare('UPDATE ' . $runTable . ' SET subjects = NULL WHERE id = %d', $result['id']); + $wpdb->query($sql); + } + } + + private function dropSubjectColumn(): void { + global $wpdb; + $tableName = esc_sql($wpdb->prefix . 'mailpoet_automation_runs'); + if (!$this->columnExists($tableName, 'subjects')) { + return; + } + + $sql = "SELECT id,subjects FROM $tableName where subjects is not null"; + $results = $wpdb->get_results($sql, ARRAY_A); + if (is_array($results) && $results) { + return; + } + $wpdb->query("ALTER TABLE $tableName DROP COLUMN subjects"); + } +} diff --git a/mailpoet/tasks/phpstan/phpstan.neon b/mailpoet/tasks/phpstan/phpstan.neon index 64e04b7c14..5d0680a379 100644 --- a/mailpoet/tasks/phpstan/phpstan.neon +++ b/mailpoet/tasks/phpstan/phpstan.neon @@ -44,7 +44,7 @@ parameters: - '/Call to method getName\(\) on an unknown class _generated\\([a-zA-Z])*Cookie/' # codeception generate incorrect return type in ../../tests/_support/_generated - message: "#^Cannot cast string|void to string\\.$#" - count: 6 + count: 9 path: ../../lib/Automation/Engine/Storage/AutomationRunStorage.php - message: "#^Cannot cast string|void to string\\.$#" diff --git a/mailpoet/tests/integration/Automation/Engine/Storage/AutomationStatisticsStorageTest.php b/mailpoet/tests/integration/Automation/Engine/Storage/AutomationStatisticsStorageTest.php index b2648ca916..5b8bd43283 100644 --- a/mailpoet/tests/integration/Automation/Engine/Storage/AutomationStatisticsStorageTest.php +++ b/mailpoet/tests/integration/Automation/Engine/Storage/AutomationStatisticsStorageTest.php @@ -187,7 +187,7 @@ class AutomationStatisticsStorageTest extends \MailPoetTest { 'automation_id' => $automation->getId(), 'version_id' => $automation->getVersionId(), 'trigger_key' => '', - 'subjects' => "[]", + 'subjects' => [], 'id' => 0, 'status' => $status, 'created_at' => (new \DateTimeImmutable())->format(\DateTimeImmutable::W3C),