From 6f8edfaec493bce9b4f110e871729abcbb66743f Mon Sep 17 00:00:00 2001 From: John Oleksowicz Date: Mon, 12 Sep 2022 12:33:03 -0500 Subject: [PATCH] Make workflow run logs immutable [MAILPOET-4463] --- .../Automation/Engine/Control/StepHandler.php | 15 ++------------- .../Automation/Engine/Data/WorkflowRunLog.php | 10 ---------- .../Automation/Engine/Migrations/Migrator.php | 1 - .../Engine/Storage/WorkflowRunLogStorage.php | 15 --------------- .../Storage/WorkflowRunLogStorageTest.php | 18 ------------------ 5 files changed, 2 insertions(+), 57 deletions(-) diff --git a/mailpoet/lib/Automation/Engine/Control/StepHandler.php b/mailpoet/lib/Automation/Engine/Control/StepHandler.php index c9d1b5c1c4..0db884ccd7 100644 --- a/mailpoet/lib/Automation/Engine/Control/StepHandler.php +++ b/mailpoet/lib/Automation/Engine/Control/StepHandler.php @@ -120,7 +120,7 @@ class StepHandler { $stepType = $step->getType(); if (isset($this->stepRunners[$stepType])) { - $log = $this->createWorkflowRunLog($workflowRun, $step, $args); + $log = new WorkflowRunLog($workflowRun->getId(), $step->getId(), $args); try { $this->stepRunners[$stepType]->run($step, $workflow, $workflowRun); $log->markCompleted(); @@ -134,7 +134,7 @@ class StepHandler { } catch (Exception $e) { $log->addError($e); } - $this->workflowRunLogStorage->updateWorkflowRunLog($log); + $this->workflowRunLogStorage->createWorkflowRunLog($log); } } else { throw new InvalidStateException(); @@ -161,17 +161,6 @@ class StepHandler { // enqueue next step $this->actionScheduler->enqueue(Hooks::WORKFLOW_STEP, $nextStepArgs); - // TODO: allow long-running steps (that are not done here yet) } - - private function createWorkflowRunLog(WorkflowRun $workflowRun, Step $step, array $args): WorkflowRunLog { - $log = new WorkflowRunLog($workflowRun->getId(), $step->getId(), $args); - $logId = $this->workflowRunLogStorage->createWorkflowRunLog($log); - $workflowRunLog = $this->workflowRunLogStorage->getWorkflowRunLog($logId); - if (!$workflowRunLog instanceof WorkflowRunLog) { - throw new InvalidStateException(); - } - return $workflowRunLog; - } } diff --git a/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php b/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php index 46844a5767..357afcb3f9 100644 --- a/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php +++ b/mailpoet/lib/Automation/Engine/Data/WorkflowRunLog.php @@ -20,9 +20,6 @@ class WorkflowRunLog { /** @var DateTimeImmutable */ private $createdAt; - /** @var DateTimeImmutable */ - private $updatedAt; - /** @var DateTimeImmutable|null */ private $completedAt; @@ -58,7 +55,6 @@ class WorkflowRunLog { $now = new DateTimeImmutable(); $this->createdAt = $now; - $this->updatedAt = $now; $this->errors = []; $this->data = []; @@ -112,17 +108,12 @@ class WorkflowRunLog { return $this->createdAt; } - public function getUpdatedAt(): DateTimeImmutable { - return $this->updatedAt; - } - public function toArray(): array { return [ 'workflow_run_id' => $this->workflowRunId, 'step_id' => $this->stepId, 'status' => $this->status, 'created_at' => $this->createdAt->format(DateTimeImmutable::W3C), - 'updated_at' => $this->updatedAt->format(DateTimeImmutable::W3C), 'completed_at' => $this->completedAt ? $this->completedAt->format(DateTimeImmutable::W3C) : null, 'args' => Json::encode($this->args), 'errors' => Json::encode($this->errors), @@ -159,7 +150,6 @@ class WorkflowRunLog { $workflowRunLog->data = Json::decode($data['data']); $workflowRunLog->args = Json::decode($data['args']); $workflowRunLog->createdAt = new DateTimeImmutable($data['created_at']); - $workflowRunLog->updatedAt = new DateTimeImmutable($data['updated_at']); if ($data['completed_at']) { $workflowRunLog->completedAt = new DateTimeImmutable($data['completed_at']); diff --git a/mailpoet/lib/Automation/Engine/Migrations/Migrator.php b/mailpoet/lib/Automation/Engine/Migrations/Migrator.php index 64a12f4a72..2920eca339 100644 --- a/mailpoet/lib/Automation/Engine/Migrations/Migrator.php +++ b/mailpoet/lib/Automation/Engine/Migrations/Migrator.php @@ -69,7 +69,6 @@ class Migrator { step_id varchar(255) NOT NULL, status varchar(255) NOT NULL, created_at timestamp NOT NULL, - updated_at timestamp NOT NULL, completed_at timestamp NULL DEFAULT NULL, args longtext, errors longtext, diff --git a/mailpoet/lib/Automation/Engine/Storage/WorkflowRunLogStorage.php b/mailpoet/lib/Automation/Engine/Storage/WorkflowRunLogStorage.php index 01ae2769a7..3da176c56b 100644 --- a/mailpoet/lib/Automation/Engine/Storage/WorkflowRunLogStorage.php +++ b/mailpoet/lib/Automation/Engine/Storage/WorkflowRunLogStorage.php @@ -28,21 +28,6 @@ class WorkflowRunLogStorage { return $this->wpdb->insert_id; } - public function updateWorkflowRunLog(WorkflowRunLog $workflowRunLog): int { - $data = $workflowRunLog->toArray(); - unset($data['id']); - $data['updated_at'] = (new \DateTimeImmutable())->format(\DateTimeImmutable::W3C); - $where = ['id' => $workflowRunLog->getId()]; - - $result = $this->wpdb->update($this->table, $data, $where); - - if ($result === false) { - throw Exceptions::databaseError($this->wpdb->last_error); - } - - return $result; - } - public function getWorkflowRunLog(int $id): ?WorkflowRunLog { $table = esc_sql($this->table); $query = $this->wpdb->prepare("SELECT * FROM $table WHERE id = %d", $id); diff --git a/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowRunLogStorageTest.php b/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowRunLogStorageTest.php index 4a4f658c22..173a38e186 100644 --- a/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowRunLogStorageTest.php +++ b/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowRunLogStorageTest.php @@ -25,24 +25,6 @@ class WorkflowRunLogStorageTest extends \MailPoetTest { expect($preSave)->equals($fromDatabase->toArray()); } - public function testUpdatingLogUpdatesUpdatedAtTimestamp() { - $log = new WorkflowRunLog(1, 'step-id', []); - $reflectionClass = new \ReflectionClass(WorkflowRunLog::class); - $updatedAt = $reflectionClass->getProperty('updatedAt'); - $updatedAt->setAccessible(true); - $updatedAt->setValue($log, new \DateTimeImmutable('2022-09-07')); - $id = $this->storage->createWorkflowRunLog($log); - $log = $this->storage->getWorkflowRunLog($id); - $this->assertInstanceOf(WorkflowRunLog::class, $log); - $originalUpdatedAt = $log->getUpdatedAt(); - $log->setData('key', 'value'); - $this->assertInstanceOf(WorkflowRunLog::class, $log); - $this->storage->updateWorkflowRunLog($log); - $fromDatabase = $this->storage->getWorkflowRunLog($id); - $this->assertInstanceOf(WorkflowRunLog::class, $fromDatabase); - expect($fromDatabase->getUpdatedAt())->greaterThan($originalUpdatedAt); - } - public function testItStoresErrors() { $log = new WorkflowRunLog(1, 'step-id', []); $log->addError(new \Exception('test'));