From ac52f2af999ee55a95ba812d63d07b9f3e6e9a22 Mon Sep 17 00:00:00 2001 From: David Remer Date: Wed, 9 Mar 2022 08:55:43 +0200 Subject: [PATCH] Introduce phpstan-type for MailerLog data [MAILPOET-4122] --- mailpoet/lib/Mailer/MailerLog.php | 131 +++++++++++++++--- .../tasks/phpstan/phpstan-7-baseline.neon | 5 - .../tasks/phpstan/phpstan-8-baseline.neon | 5 - .../integration/API/JSON/v1/MailerTest.php | 3 +- .../SendingQueue/Tasks/NewsletterTest.php | 22 ++- .../integration/Mailer/MailerLogTest.php | 37 +++-- .../AuthorizedEmailsControllerTest.php | 10 +- 7 files changed, 158 insertions(+), 55 deletions(-) diff --git a/mailpoet/lib/Mailer/MailerLog.php b/mailpoet/lib/Mailer/MailerLog.php index 24af4cdde2..5cf1b920e7 100644 --- a/mailpoet/lib/Mailer/MailerLog.php +++ b/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, + * "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 - * @return array + * @param MailerLogData|null $mailerLog + * @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; } diff --git a/mailpoet/tasks/phpstan/phpstan-7-baseline.neon b/mailpoet/tasks/phpstan/phpstan-7-baseline.neon index e1973d7c88..e41d3f74bb 100644 --- a/mailpoet/tasks/phpstan/phpstan-7-baseline.neon +++ b/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 diff --git a/mailpoet/tasks/phpstan/phpstan-8-baseline.neon b/mailpoet/tasks/phpstan/phpstan-8-baseline.neon index 9bbfafff14..ab4034d354 100644 --- a/mailpoet/tasks/phpstan/phpstan-8-baseline.neon +++ b/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 diff --git a/mailpoet/tests/integration/API/JSON/v1/MailerTest.php b/mailpoet/tests/integration/API/JSON/v1/MailerTest.php index fb58e0db92..ff3fde4967 100644 --- a/mailpoet/tests/integration/API/JSON/v1/MailerTest.php +++ b/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); diff --git a/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php b/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php index c96d57b0ae..5477644e50 100644 --- a/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php +++ b/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($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.'); + } } } @@ -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($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.'); + } } } diff --git a/mailpoet/tests/integration/Mailer/MailerLogTest.php b/mailpoet/tests/integration/Mailer/MailerLogTest.php index b69d4a8872..8dff5f1639 100644 --- a/mailpoet/tests/integration/Mailer/MailerLogTest.php +++ b/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 = [ - 'sent' => [date('Y-m-d H:i:s', $started) => 1], - 'started' => $started, - ]; + $mailerLog = MailerLog::getMailerLog(); + $mailerLog['sent'] = [date('Y-m-d H:i:s', $started) => 1]; + $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 = [ - 'status' => null, - 'retry_attempt' => MailerLog::RETRY_ATTEMPTS_LIMIT, - 'retry_at' => time() + 20, - ]; + $mailerLog = MailerLog::getMailerLog(); + $mailerLog['status'] = null; + $mailerLog['retry_attempt'] = MailerLog::RETRY_ATTEMPTS_LIMIT; + $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(); diff --git a/mailpoet/tests/integration/Services/AuthorizedEmailsControllerTest.php b/mailpoet/tests/integration/Services/AuthorizedEmailsControllerTest.php index 4660c760c8..c68fd0b65d 100644 --- a/mailpoet/tests/integration/Services/AuthorizedEmailsControllerTest.php +++ b/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) {