Extract logging logic to a service, make logs mutable

[MAILPOET-5568]

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

@@ -5,7 +5,6 @@ namespace MailPoet\Automation\Engine\Control;
 use Exception;
 use MailPoet\Automation\Engine\Data\Automation;
 use MailPoet\Automation\Engine\Data\AutomationRun;
-use MailPoet\Automation\Engine\Data\AutomationRunLog;
 use MailPoet\Automation\Engine\Data\StepRunArgs;
 use MailPoet\Automation\Engine\Data\StepValidationArgs;
 use MailPoet\Automation\Engine\Data\SubjectEntry;
@@ -16,7 +15,6 @@ use MailPoet\Automation\Engine\Integration\Action;
 use MailPoet\Automation\Engine\Integration\Payload;
 use MailPoet\Automation\Engine\Integration\Subject;
 use MailPoet\Automation\Engine\Registry;
-use MailPoet\Automation\Engine\Storage\AutomationRunLogStorage;
 use MailPoet\Automation\Engine\Storage\AutomationRunStorage;
 use MailPoet\Automation\Engine\Storage\AutomationStorage;
 use MailPoet\Automation\Engine\WordPress;
@@ -35,40 +33,35 @@ class StepHandler {
   /** @var AutomationStorage */
   private $automationStorage;
 
-  /** @var AutomationRunLogStorage */
-  private $automationRunLogStorage;
-
-  /** @var Hooks */
-  private $hooks;
-
   /** @var Registry */
   private $registry;
 
   /** @var StepRunControllerFactory */
   private $stepRunControllerFactory;
 
+  /** @var StepRunLoggerFactory */
+  private $stepRunLoggerFactory;
+
   /** @var StepScheduler */
   private $stepScheduler;
 
   public function __construct(
-    Hooks $hooks,
     SubjectLoader $subjectLoader,
     WordPress $wordPress,
     AutomationRunStorage $automationRunStorage,
-    AutomationRunLogStorage $automationRunLogStorage,
     AutomationStorage $automationStorage,
     Registry $registry,
     StepRunControllerFactory $stepRunControllerFactory,
+    StepRunLoggerFactory $stepRunLoggerFactory,
     StepScheduler $stepScheduler
   ) {
-    $this->hooks = $hooks;
     $this->subjectLoader = $subjectLoader;
     $this->wordPress = $wordPress;
     $this->automationRunStorage = $automationRunStorage;
-    $this->automationRunLogStorage = $automationRunLogStorage;
     $this->automationStorage = $automationStorage;
     $this->registry = $registry;
     $this->stepRunControllerFactory = $stepRunControllerFactory;
+    $this->stepRunLoggerFactory = $stepRunLoggerFactory;
     $this->stepScheduler = $stepScheduler;
   }
 
@@ -93,18 +86,17 @@ class StepHandler {
       return;
     }
 
-    $log = new AutomationRunLog($runId, $stepId);
+    $logger = $this->stepRunLoggerFactory->createLogger($runId, $stepId);
+    $logger->logStart();
     try {
       $this->handleStep($runId, $stepId, $runNumber);
-      $log->markCompletedSuccessfully();
+      $logger->logSuccess();
     } catch (Throwable $e) {
       $status = $e instanceof InvalidStateException && $e->getErrorCode() === 'mailpoet_automation_not_active'
         ? AutomationRun::STATUS_CANCELLED
         : AutomationRun::STATUS_FAILED;
       $this->automationRunStorage->updateStatus((int)$args['automation_run_id'], $status);
-
+      $logger->logFailure($e);
-      $log->markFailed();
-      $log->setError($e);
 
       // Action Scheduler catches only Exception instances, not other errors.
       // We need to convert them to exceptions to be processed and logged.
@@ -113,12 +105,6 @@ class StepHandler {
       }
       throw $e;
     } finally {
-      try {
-        $this->hooks->doAutomationStepAfterRun($log);
-      } catch (Throwable $e) {
-        // Ignore integration errors
-      }
-      $this->automationRunLogStorage->createAutomationRunLog($log);
       $this->postProcessAutomationRun($runId);
     }
   }

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

@@ -0,0 +1,85 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Automation\Engine\Control;
+
+use DateTimeImmutable;
+use MailPoet\Automation\Engine\Data\AutomationRunLog;
+use MailPoet\Automation\Engine\Hooks;
+use MailPoet\Automation\Engine\Storage\AutomationRunLogStorage;
+use MailPoet\InvalidStateException;
+use Throwable;
+
+class StepRunLogger {
+  /** @var AutomationRunLogStorage */
+  private $automationRunLogStorage;
+
+  /** @var Hooks */
+  private $hooks;
+
+  /** @var int */
+  private $runId;
+
+  /** @var string */
+  private $stepId;
+
+  /** @var AutomationRunLog|null */
+  private $log;
+
+  public function __construct(
+    AutomationRunLogStorage $automationRunLogStorage,
+    Hooks $hooks,
+    int $runId,
+    string $stepId
+  ) {
+    $this->automationRunLogStorage = $automationRunLogStorage;
+    $this->hooks = $hooks;
+    $this->runId = $runId;
+    $this->stepId = $stepId;
+  }
+
+  public function logStart(): void {
+    $this->getLog();
+  }
+
+  public function logSuccess(): void {
+    $log = $this->getLog();
+    $log->setStatus(AutomationRunLog::STATUS_COMPLETE);
+    $log->setCompletedAt(new DateTimeImmutable());
+    $this->triggerAfterRunHook($log);
+    $this->automationRunLogStorage->updateAutomationRunLog($log);
+  }
+
+  public function logFailure(Throwable $error): void {
+    $log = $this->getLog();
+    $log->setStatus(AutomationRunLog::STATUS_FAILED);
+    $log->setError($error);
+    $log->setCompletedAt(new DateTimeImmutable());
+    $this->triggerAfterRunHook($log);
+    $this->automationRunLogStorage->updateAutomationRunLog($log);
+  }
+
+  private function getLog(): AutomationRunLog {
+    if (!$this->log) {
+      $this->log = $this->automationRunLogStorage->getAutomationRunLogByRunAndStepId($this->runId, $this->stepId);
+    }
+
+    if (!$this->log) {
+      $log = new AutomationRunLog($this->runId, $this->stepId);
+      $id = $this->automationRunLogStorage->createAutomationRunLog($log);
+      $this->log = $this->automationRunLogStorage->getAutomationRunLog($id);
+    }
+
+    if (!$this->log) {
+      throw new InvalidStateException('Failed to create automation run log');
+    }
+    return $this->log;
+  }
+
+  private function triggerAfterRunHook(AutomationRunLog $log): void {
+    try {
+      $this->hooks->doAutomationStepAfterRun($log);
+    } catch (Throwable $e) {
+      // ignore integration errors
+    }
+  }
+}

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

@@ -0,0 +1,26 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Automation\Engine\Control;
+
+use MailPoet\Automation\Engine\Hooks;
+use MailPoet\Automation\Engine\Storage\AutomationRunLogStorage;
+
+class StepRunLoggerFactory {
+  /** @var AutomationRunLogStorage */
+  private $automationRunLogStorage;
+
+  /** @var Hooks */
+  private $hooks;
+
+  public function __construct(
+    AutomationRunLogStorage $automationRunLogStorage,
+    Hooks $hooks
+  ) {
+    $this->automationRunLogStorage = $automationRunLogStorage;
+    $this->hooks = $hooks;
+  }
+
+  public function createLogger(int $runId, string $stepId): StepRunLogger {
+    return new StepRunLogger($this->automationRunLogStorage, $this->hooks, $runId, $stepId);
+  }
+}

mailpoet/lib/Automation/Engine/Data/AutomationRunLog.php

@@ -12,6 +12,12 @@ class AutomationRunLog {
   public const STATUS_COMPLETE = 'complete';
   public const STATUS_FAILED = 'failed';
 
+  public const STATUS_ALL = [
+    self::STATUS_RUNNING,
+    self::STATUS_COMPLETE,
+    self::STATUS_FAILED,
+  ];
+
   /** @var int */
   private $id;
 
@@ -71,6 +77,13 @@ class AutomationRunLog {
     return $this->status;
   }
 
+  public function setStatus(string $status): void {
+    if (!in_array($status, self::STATUS_ALL, true)) {
+      throw new InvalidArgumentException("Invalid status '$status'.");
+    }
+    $this->status = $status;
+  }
+
   public function getError(): array {
     return $this->error;
   }
@@ -83,6 +96,10 @@ class AutomationRunLog {
     return $this->completedAt;
   }
 
+  public function setCompletedAt(DateTimeImmutable $completedAt): void {
+    $this->completedAt = $completedAt;
+  }
+
   /**
    * @param string $key
    * @param mixed $value
@@ -111,16 +128,6 @@ class AutomationRunLog {
     ];
   }
 
-  public function markCompletedSuccessfully(): void {
-    $this->status = self::STATUS_COMPLETE;
-    $this->completedAt = new DateTimeImmutable();
-  }
-
-  public function markFailed(): void {
-    $this->status = self::STATUS_FAILED;
-    $this->completedAt = new DateTimeImmutable();
-  }
-
   public function setError(Throwable $error): void {
     $error = [
       'message' => $error->getMessage(),

mailpoet/lib/Automation/Engine/Storage/AutomationRunLogStorage.php

@@ -28,6 +28,13 @@ class AutomationRunLogStorage {
     return $this->wpdb->insert_id;
   }
 
+  public function updateAutomationRunLog(AutomationRunLog $automationRunLog): void {
+    $result = $this->wpdb->update($this->table, $automationRunLog->toArray(), ['id' => $automationRunLog->getId()]);
+    if ($result === false) {
+      throw Exceptions::databaseError($this->wpdb->last_error);
+    }
+  }
+
   public function getAutomationRunStatisticsForAutomationInTimeFrame(int $automationId, string $status, \DateTimeImmutable $after, \DateTimeImmutable $before, int $versionId = null): array {
     $logTable = esc_sql($this->table);
     $runTable = esc_sql($this->wpdb->prefix . 'mailpoet_automation_runs');
@@ -68,6 +75,16 @@ class AutomationRunLogStorage {
     return null;
   }
 
+  public function getAutomationRunLogByRunAndStepId(int $runId, string $stepId): ?AutomationRunLog {
+    $table = esc_sql($this->table);
+    $query = $this->wpdb->prepare("SELECT * FROM $table WHERE automation_run_id = %d AND step_id = %s", $runId, $stepId);
+    if (!is_string($query)) {
+      throw InvalidStateException::create();
+    }
+    $result = $this->wpdb->get_row($query, ARRAY_A);
+    return $result ? AutomationRunLog::fromArray((array)$result) : null;
+  }
+
   /**
    * @param int $automationRunId
    * @return AutomationRunLog[]

mailpoet/lib/DI/ContainerConfigurator.php

@@ -127,6 +127,7 @@ class ContainerConfigurator implements IContainerConfigurator {
     $container->autowire(\MailPoet\Automation\Engine\Control\FilterHandler::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Control\StepHandler::class)->setPublic(true);
     $container->autowire(\MailPoet\Automation\Engine\Control\StepRunControllerFactory::class)->setPublic(true);
+    $container->autowire(\MailPoet\Automation\Engine\Control\StepRunLoggerFactory::class)->setPublic(true);
     $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);