Introduce phpstan-type for MailerLog data

[MAILPOET-4122]

mailpoet/lib/Mailer/MailerLog.php

@@ -4,13 +4,33 @@ namespace MailPoet\Mailer;
 
 use MailPoet\Settings\SettingsController;
 
+/**
+ * @phpstan-type MailerLogError array{
+ *    "error_code"?: non-empty-string,
+ *    "error_message": string,
+ *    "operation": string
+ *   }
+ * @phpstan-type MailerLogData array{
+ *   "sent": array<string,int>,
+ *   "started": int,
+ *   "status": ?string,
+ *   "retry_attempt": ?int,
+ *   "retry_at": ?int,
+ *   "error": ?MailerLogError
+ * }
+ */
+
 class MailerLog {
   const SETTING_NAME = 'mta_log';
   const STATUS_PAUSED = 'paused';
   const RETRY_ATTEMPTS_LIMIT = 3;
   const RETRY_INTERVAL = 120; // seconds
 
-  public static function getMailerLog($mailerLog = false) {
+  /**
+   * @param MailerLogData|null $mailerLog
+   * @return MailerLogData
+   */
+  public static function getMailerLog(array $mailerLog = null): array {
     if ($mailerLog) return $mailerLog;
     $settings = SettingsController::getInstance();
     $mailerLog = $settings->get(self::SETTING_NAME);
@@ -25,7 +45,10 @@ class MailerLog {
     return $mailerLog;
   }
 
-  public static function createMailerLog() {
+  /**
+   * @return MailerLogData
+   */
+  public static function createMailerLog(): array {
     $mailerLog = [
       'sent' => [],
       'started' => time(),
@@ -39,18 +62,30 @@ class MailerLog {
     return $mailerLog;
   }
 
-  public static function resetMailerLog() {
+  /**
+   * @return MailerLogData
+   */
+  public static function resetMailerLog(): array {
     return self::createMailerLog();
   }
 
-  public static function updateMailerLog($mailerLog) {
+  /**
+   * @param MailerLogData $mailerLog
+   * @return MailerLogData
+   */
+  public static function updateMailerLog(array $mailerLog): array {
     $mailerLog = self::removeOutdatedSentInformationFromMailerlog($mailerLog);
     $settings = SettingsController::getInstance();
     $settings->set(self::SETTING_NAME, $mailerLog);
     return $mailerLog;
   }
 
-  public static function enforceExecutionRequirements($mailerLog = false) {
+  /**
+   * @param MailerLogData|null $mailerLog
+   * @return null
+   * @throws \Exception
+   */
+  public static function enforceExecutionRequirements(array $mailerLog = null) {
     $mailerLog = self::getMailerLog($mailerLog);
     if ($mailerLog['retry_attempt'] === self::RETRY_ATTEMPTS_LIMIT) {
       $mailerLog = self::pauseSending($mailerLog);
@@ -70,16 +105,24 @@ class MailerLog {
     if (self::isSendingLimitReached($mailerLog)) {
       throw new \Exception(__('Sending frequency limit has been reached.', 'mailpoet'));
     }
+    return null;
   }
 
-  public static function pauseSending($mailerLog) {
+  /**
+   * @param MailerLogData $mailerLog
+   * @return MailerLogData
+   */
+  public static function pauseSending($mailerLog): array {
     $mailerLog['status'] = self::STATUS_PAUSED;
     $mailerLog['retry_attempt'] = null;
     $mailerLog['retry_at'] = null;
     return self::updateMailerLog($mailerLog);
   }
 
-  public static function resumeSending() {
+  /**
+   * @return MailerLogData
+   */
+  public static function resumeSending(): array {
     return self::resetMailerLog();
   }
 
@@ -92,7 +135,7 @@ class MailerLog {
    *
    * @throws \Exception
    */
-  public static function processNonBlockingError($operation, $errorMessage, $retryInterval = self::RETRY_INTERVAL) {
+  public static function processNonBlockingError(string $operation, string $errorMessage, int $retryInterval = self::RETRY_INTERVAL) {
     $mailerLog = self::getMailerLog();
     $mailerLog['retry_at'] = time() + $retryInterval;
     $mailerLog = self::setError($mailerLog, $operation, $errorMessage);
@@ -110,7 +153,13 @@ class MailerLog {
    *
    * @throws \Exception
    */
-  public static function processError($operation, $errorMessage, $errorCode = null, $pauseSending = false, $throttledBatchSize = null) {
+  public static function processError(
+    string $operation,
+    string $errorMessage,
+    string $errorCode = null,
+    bool $pauseSending = false,
+    int $throttledBatchSize = null
+  ) {
     $mailerLog = self::getMailerLog();
     if (!isset($throttledBatchSize) || $throttledBatchSize === 1) {
       $mailerLog['retry_attempt']++;
@@ -124,7 +173,19 @@ class MailerLog {
     self::enforceExecutionRequirements();
   }
 
-  public static function setError($mailerLog, $operation, $errorMessage, $errorCode = null) {
+  /**
+   * @param MailerLogData $mailerLog
+   * @param string $operation
+   * @param string $errorMessage
+   * @param string|null $errorCode
+   * @return MailerLogData
+   */
+  public static function setError(
+    array $mailerLog,
+    string $operation,
+    string $errorMessage,
+    string $errorCode = null
+  ): array {
     $mailerLog['error'] = [
       'operation' => $operation,
       'error_message' => $errorMessage,
@@ -135,19 +196,26 @@ class MailerLog {
     return $mailerLog;
   }
 
-  public static function getError($mailerLog = false) {
+  /**
+   * @param MailerLogData|null $mailerLog
+   * @return MailerLogError|null
+   */
+  public static function getError(array $mailerLog = null): ?array {
     $mailerLog = self::getMailerLog($mailerLog);
     return isset($mailerLog['error']) ? $mailerLog['error'] : null;
   }
 
-  public static function incrementSentCount() {
+  /**
+   * @return MailerLogData|null
+   */
+  public static function incrementSentCount(): ?array {
     $mailerLog = self::getMailerLog();
     // do not increment count if sending limit is reached
     if (self::isSendingLimitReached($mailerLog)) {
-      return;
+      return null;
     }
     // clear previous retry count, errors, etc.
-    if ($mailerLog['error']) {
+    if ($mailerLog['error'] !== null) {
       $mailerLog = self::clearSendingErrorLog($mailerLog);
     }
 
@@ -159,14 +227,22 @@ class MailerLog {
     return self::updateMailerLog($mailerLog);
   }
 
-  public static function clearSendingErrorLog($mailerLog) {
+  /**
+   * @param MailerLogData $mailerLog
+   * @return MailerLogData
+   */
+  public static function clearSendingErrorLog(array $mailerLog): array {
     $mailerLog['retry_attempt'] = null;
     $mailerLog['retry_at'] = null;
     $mailerLog['error'] = null;
     return self::updateMailerLog($mailerLog);
   }
 
-  public static function isSendingLimitReached($mailerLog = false) {
+  /**
+   * @param MailerLogData|null $mailerLog
+   * @return bool
+   */
+  public static function isSendingLimitReached(array $mailerLog = null): bool {
     $settings = SettingsController::getInstance();
     $mailerConfig = $settings->get(Mailer::MAILER_CONFIG_SETTING_NAME);
     // do not enforce sending limit for MailPoet's sending method
@@ -183,7 +259,12 @@ class MailerLog {
     return $sent >= $frequencyLimit;
   }
 
-  public static function sentSince(int $sinceSeconds, array $mailerLog = null): int {
+  /**
+   * @param int $sinceSeconds
+   * @param MailerLogData|null $mailerLog
+   * @return int
+   */
+  private static function sentSince(int $sinceSeconds, array $mailerLog = null): int {
 
     $sinceDate = date('Y-m-d H:i:s', time() - $sinceSeconds);
     $mailerLog = self::getMailerLog($mailerLog);
@@ -202,8 +283,8 @@ class MailerLog {
   /**
    * Clears "sent" section of the mailer log from outdated entries.
    *
-   * @param array|null $mailerLog
+   * @param MailerLogData|null $mailerLog
-   * @return array
+   * @return MailerLogData
    */
   private static function removeOutdatedSentInformationFromMailerlog(array $mailerLog = null): array {
 
@@ -223,12 +304,20 @@ class MailerLog {
     return $mailerLog;
   }
 
-  public static function sentEntriesDate(int $timestamp = null): string {
+  /**
+   * @param int|null $timestamp
+   * @return string
+   */
+  private static function sentEntriesDate(int $timestamp = null): string {
 
     return date('Y-m-d H:i:s', $timestamp ?? time());
   }
 
-  public static function isSendingPaused($mailerLog = false) {
+  /**
+   * @param MailerLogData|null $mailerLog
+   * @return bool
+   */
+  public static function isSendingPaused(array $mailerLog = null): bool {
     $mailerLog = self::getMailerLog($mailerLog);
     return $mailerLog['status'] === self::STATUS_PAUSED;
   }

mailpoet/tasks/phpstan/phpstan-7-baseline.neon

@@ -650,11 +650,6 @@ parameters:
 			count: 1
 			path: ../../lib/Listing/ListingRepository.php
 
-		-
-			message: "#^Variable \\$mailer in empty\\(\\) is never defined\\.$#"
-			count: 1
-			path: ../../lib/Mailer/MailerLog.php
-
 		-
 			message: "#^Parameter \\#1 \\$timeout of method MailPoetVendor\\\\Swift_Transport_EsmtpTransport\\:\\:setTimeout\\(\\) expects int, mixed given\\.$#"
 			count: 1

mailpoet/tasks/phpstan/phpstan-8-baseline.neon

@@ -650,11 +650,6 @@ parameters:
 			count: 1
 			path: ../../lib/Listing/ListingRepository.php
 
-		-
-			message: "#^Variable \\$mailer in empty\\(\\) is never defined\\.$#"
-			count: 1
-			path: ../../lib/Mailer/MailerLog.php
-
 		-
 			message: "#^Parameter \\#1 \\$timeout of method MailPoetVendor\\\\Swift_Transport_EsmtpTransport\\:\\:setTimeout\\(\\) expects int, mixed given\\.$#"
 			count: 1

mailpoet/tests/integration/API/JSON/v1/MailerTest.php

@@ -14,7 +14,8 @@ use MailPoet\Settings\SettingsController;
 class MailerTest extends \MailPoetTest {
   public function testItResumesSending() {
     // create mailer log with a "paused" status
-    $mailerLog = ['status' => MailerLog::STATUS_PAUSED];
+    $mailerLog = MailerLog::getMailerLog();
+    $mailerLog['status'] = MailerLog::STATUS_PAUSED;
     MailerLog::updateMailerLog($mailerLog);
     $mailerLog = MailerLog::getMailerLog();
     expect($mailerLog['status'])->equals(MailerLog::STATUS_PAUSED);

mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php

@@ -371,8 +371,12 @@ class NewsletterTest extends \MailPoetTest {
       self::fail('Sending error exception was not thrown.');
     } catch (\Exception $e) {
       $mailerLog = MailerLog::getMailerLog();
-      expect($mailerLog['error']['operation'])->equals('queue_save');
+
-      expect($mailerLog['error']['error_message'])->equals('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.');
+      expect(is_array($mailerLog['error']));
+      if (is_array($mailerLog['error'])) {
+        expect($mailerLog['error']['operation'])->equals('queue_save');
+        expect($mailerLog['error']['error_message'])->equals('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.');
+      }
     }
   }
 
@@ -392,8 +396,11 @@ class NewsletterTest extends \MailPoetTest {
       self::fail('Sending error exception was not thrown.');
     } catch (\Exception $e) {
       $mailerLog = MailerLog::getMailerLog();
-      expect($mailerLog['error']['operation'])->equals('queue_save');
+      expect(is_array($mailerLog['error']));
-      expect($mailerLog['error']['error_message'])->equals('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.');
+      if (is_array($mailerLog['error'])) {
+        expect($mailerLog['error']['operation'])->equals('queue_save');
+        expect($mailerLog['error']['error_message'])->equals('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.');
+      }
     }
   }
 
@@ -426,8 +433,11 @@ class NewsletterTest extends \MailPoetTest {
       self::fail('Sending error exception was not thrown.');
     } catch (\Exception $e) {
       $mailerLog = MailerLog::getMailerLog();
-      expect($mailerLog['error']['operation'])->equals('queue_save');
+      expect(is_array($mailerLog['error']));
-      expect($mailerLog['error']['error_message'])->equals('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.');
+      if (is_array($mailerLog['error'])) {
+        expect($mailerLog['error']['operation'])->equals('queue_save');
+        expect($mailerLog['error']['error_message'])->equals('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.');
+      }
     }
   }
 

mailpoet/tests/integration/Mailer/MailerLogTest.php

@@ -27,15 +27,17 @@ class MailerLogTest extends \MailPoetTest {
   }
 
   public function testItGetsMailerLogWhenOneDoesNotExist() {
+    $resultExpectedGreaterThan = time() - 1;
     $mailerLog = MailerLog::getMailerLog();
     expect($mailerLog['sent'])->equals([]);
-    expect(strlen($mailerLog['started']))->greaterThan(5);
+    expect($mailerLog['started'])->greaterThan($resultExpectedGreaterThan);
   }
 
   public function testItCreatesMailer() {
+    $resultExpectedGreaterThan = time() - 1;
     $mailerLog = MailerLog::createMailerLog();
     expect($mailerLog['sent'])->equals([]);
-    expect(strlen($mailerLog['started']))->greaterThan(5);
+    expect($mailerLog['started'])->greaterThan($resultExpectedGreaterThan);
   }
 
   public function testItResetsMailerLog() {
@@ -53,10 +55,9 @@ class MailerLogTest extends \MailPoetTest {
 
   public function testItUpdatesMailerLog() {
     $started = time() - 10;
-    $mailerLog = [
+    $mailerLog = MailerLog::getMailerLog();
-      'sent' => [date('Y-m-d H:i:s', $started) => 1],
+    $mailerLog['sent'] = [date('Y-m-d H:i:s', $started) => 1];
-      'started' => $started,
+    $mailerLog['started'] = $started;
-    ];
     MailerLog::updateMailerLog($mailerLog);
     $updatedMailerLog = $this->settings->get(MailerLog::SETTING_NAME);
     expect($updatedMailerLog)->equals($mailerLog);
@@ -128,9 +129,10 @@ class MailerLogTest extends \MailPoetTest {
   }
 
   public function testItChecksWhenSendingIsPaused() {
-    $mailerLog = ['status' => MailerLog::STATUS_PAUSED];
+    $mailerLog = MailerLog::getMailerLog();
+    $mailerLog['status'] = MailerLog::STATUS_PAUSED;
     expect(MailerLog::isSendingPaused($mailerLog))->true();
-    $mailerLog = ['status' => false];
+    $mailerLog['status'] = null;
     expect(MailerLog::isSendingPaused($mailerLog))->false();
   }
 
@@ -155,7 +157,8 @@ class MailerLogTest extends \MailPoetTest {
 
   public function testItResumesSending() {
     // set status to "paused"
-    $mailerLog = ['status' => MailerLog::STATUS_PAUSED];
+    $mailerLog = MailerLog::getMailerLog();
+    $mailerLog['status'] = MailerLog::STATUS_PAUSED;
     MailerLog::updateMailerLog($mailerLog);
     $mailerLog = MailerLog::getMailerLog();
     expect($mailerLog['status'])->equals(MailerLog::STATUS_PAUSED);
@@ -166,11 +169,11 @@ class MailerLogTest extends \MailPoetTest {
   }
 
   public function testItPausesSending() {
-    $mailerLog = [
+    $mailerLog = MailerLog::getMailerLog();
-      'status' => null,
+    $mailerLog['status'] = null;
-      'retry_attempt' => MailerLog::RETRY_ATTEMPTS_LIMIT,
+    $mailerLog['retry_attempt'] = MailerLog::RETRY_ATTEMPTS_LIMIT;
-      'retry_at' => time() + 20,
+    $mailerLog['retry_at'] = time() + 20;
-    ];
+
     // status is set to PAUSED, retry attempt and retry at time are cleared
     MailerLog::pauseSending($mailerLog);
     $mailerLog = MailerLog::getMailerLog();
@@ -337,7 +340,11 @@ class MailerLogTest extends \MailPoetTest {
     $mailerLog = MailerLog::createMailerLog();
     $mailerLog['retry_attempt'] = 1;
     $mailerLog['retry_at'] = 1;
-    $mailerLog['error'] = 1;
+    $mailerLog['error'] = [
+      'operation' => 'operation',
+      'error_code' => 'error_code',
+      'error_message' => 'error_message',
+    ];
     $mailerLog['status'] = 'status';
     $mailerLog = MailerLog::clearSendingErrorLog($mailerLog);
     expect($mailerLog['retry_attempt'])->null();

mailpoet/tests/integration/Services/AuthorizedEmailsControllerTest.php

@@ -123,7 +123,10 @@ class AuthorizedEmailsControllerTest extends \MailPoetTest {
     $controller = $this->getController($authorizedEmailsFromApi = ['auth@email.com']);
     $controller->checkAuthorizedEmailAddresses();
     $error = MailerLog::getError();
-    expect($error['operation'])->equals(MailerError::OPERATION_SEND);
+    expect(is_array($error));
+    if (is_array($error)) {
+      expect($error['operation'])->equals(MailerError::OPERATION_SEND);
+    }
   }
 
   public function testItDoesNotResetMailerLogItErrorPersists() {
@@ -135,7 +138,10 @@ class AuthorizedEmailsControllerTest extends \MailPoetTest {
     $controller = $this->getController($authorizedEmailsFromApi = ['auth@email.com']);
     $controller->checkAuthorizedEmailAddresses();
     $error = MailerLog::getError();
-    expect($error['operation'])->equals(MailerError::OPERATION_AUTHORIZATION);
+    expect(is_array($error));
+    if (is_array($error)) {
+      expect($error['operation'])->equals(MailerError::OPERATION_AUTHORIZATION);
+    }
   }
 
   private function checkUnauthorizedInNewsletter($type, $status) {