Cavemanon/piratepoet
8
Fork 0
Code Pull Requests Actions Releases 16 Activity

Change data model to calculate sending limit reached

Browse Source 
Before this commit, we used to calculate whether the sending limit was reached
by simply counting, how many emails where sent and when we did send them the
last time. In case the last time we did send was before the sending frequency,
and we reached the limit we would reset the calculation.

This led to the following problem:

In case we had e.g. one email send at an outdated timestamp, we would first fill
the outdated timestamp and once we reached the limit, we would reset the counter
and start with the current timestamp. So it followed, we would send more emails
than the limit would allow for.

[MAILPOET-4122]
This commit is contained in:
David Remer
2022-03-03 09:46:03 +02:00
committed by Veljko V
parent 00b0723f96
commit 53da8bf999
2 changed files with 112 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
mailpoet/lib/Mailer/MailerLog.php
View File

@@ -17,12 +17,17 @@ class MailerLog {
if (!$mailerLog) {
$mailerLog = self::createMailerLog();
}
/**
* The old "sent" entry was just the number of emails.
* We need to update this entry to the new data structure.
*/
$mailerLog['sent'] = is_numeric($mailerLog['sent']) ? [self::sentEntriesDate(time() - 1) => $mailerLog['sent']] : (array)$mailerLog['sent'];
return $mailerLog;
}
public static function createMailerLog() {
$mailerLog = [
'sent' => null,
'sent' => [],
'started' => time(),
'status' => null,
'retry_attempt' => null,
@@ -39,6 +44,7 @@ class MailerLog {
}
public static function updateMailerLog($mailerLog) {
$mailerLog = self::removeOutdatedSentInformationFromMailerlog($mailerLog);
$settings = SettingsController::getInstance();
$settings->set(self::SETTING_NAME, $mailerLog);
return $mailerLog;
@@ -137,12 +143,19 @@ class MailerLog {
public static function incrementSentCount() {
$mailerLog = self::getMailerLog();
// do not increment count if sending limit is reached
if (self::isSendingLimitReached($mailerLog)) return;
if (self::isSendingLimitReached($mailerLog)) {
return;
}
// clear previous retry count, errors, etc.
if ($mailerLog['error']) {
$mailerLog = self::clearSendingErrorLog($mailerLog);
}
(int)$mailerLog['sent']++;
$time = self::sentEntriesDate();
if (!isset($mailerLog['sent'][$time])) {
$mailerLog['sent'][$time] = 0;
}
$mailerLog['sent'][$time]++;
return self::updateMailerLog($mailerLog);
}
@@ -159,21 +172,60 @@ class MailerLog {
// do not enforce sending limit for MailPoet's sending method
if ($mailerConfig['method'] === Mailer::METHOD_MAILPOET) return false;
$mailerLog = self::getMailerLog($mailerLog);
$elapsedTime = time() - (int)$mailerLog['started'];
if (empty($mailer['frequency'])) {
if (empty($mailerConfig['frequency'])) {
$defaultSettings = $settings->getAllDefaults();
$mailer['frequency'] = $defaultSettings['mta']['frequency'];
$mailerConfig['frequency'] = $defaultSettings['mta']['frequency'];
}
$frequencyInterval = (int)$mailerConfig['frequency']['interval'] * Mailer::SENDING_LIMIT_INTERVAL_MULTIPLIER;
$frequencyLimit = (int)$mailerConfig['frequency']['emails'];
$sent = self::sentSince($frequencyInterval, $mailerLog);
return $sent >= $frequencyLimit;
}
if ($mailerLog['sent'] >= $frequencyLimit) {
if ($elapsedTime <= $frequencyInterval) return true;
// reset mailer log as enough time has passed since the limit was reached
self::resetMailerLog();
}
return false;
public static function sentSince(int $sinceSeconds, array $mailerLog = null): int {
$sinceDate = date('Y-m-d H:i:s', time() - $sinceSeconds);
$mailerLog = self::getMailerLog($mailerLog);
return (int)array_sum(
array_filter(
(array)$mailerLog['sent'],
function($date) use ($sinceDate): bool {
return $sinceDate <= $date;
},
\ARRAY_FILTER_USE_KEY
)
);
}
/**
* Clears "sent" section of the mailer log from outdated entries.
*
* @param array|null $mailerLog
* @return array
*/
private static function removeOutdatedSentInformationFromMailerlog(array $mailerLog = null): array {
$settings = SettingsController::getInstance();
$mailerConfig = $settings->get(Mailer::MAILER_CONFIG_SETTING_NAME);
$frequencyInterval = (int)$mailerConfig['frequency']['interval'] * Mailer::SENDING_LIMIT_INTERVAL_MULTIPLIER;
$sinceDate = self::sentEntriesDate(time() - $frequencyInterval);
$mailerLog = self::getMailerLog($mailerLog);
$mailerLog['sent'] = array_filter(
(array)$mailerLog['sent'],
function($date) use ($sinceDate): bool {
return $sinceDate <= $date;
},
\ARRAY_FILTER_USE_KEY
);
return $mailerLog;
}
public static function sentEntriesDate(int $timestamp = null): string {
return date('Y-m-d H:i:s', $timestamp ?? time());
}
public static function isSendingPaused($mailerLog = false) {

70
mailpoet/tests/integration/Mailer/MailerLogTest.php
View File

@@ -19,7 +19,7 @@ class MailerLogTest extends \MailPoetTest {
public function testItGetsMailerLogWhenOneExists() {
$mailerLog = [
'sent' => 0,
'sent' => [],
'started' => time(),
];
$this->settings->set(MailerLog::SETTING_NAME, $mailerLog);
@@ -28,32 +28,34 @@ class MailerLogTest extends \MailPoetTest {
public function testItGetsMailerLogWhenOneDoesNotExist() {
$mailerLog = MailerLog::getMailerLog();
expect($mailerLog['sent'])->equals(0);
expect($mailerLog['sent'])->equals([]);
expect(strlen($mailerLog['started']))->greaterThan(5);
}
public function testItCreatesMailer() {
$mailerLog = MailerLog::createMailerLog();
expect($mailerLog['sent'])->equals(0);
expect($mailerLog['sent'])->equals([]);
expect(strlen($mailerLog['started']))->greaterThan(5);
}
public function testItResetsMailerLog() {
$started = time() - 10;
$mailerLog = [
'sent' => 1,
'started' => time() - 10,
'sent' => [date('Y-m-d H:i:s', $started) => 1],
'started' => $started,
];
$this->settings->set(MailerLog::SETTING_NAME, $mailerLog);
MailerLog::resetMailerLog();
$updatedMailerLog = $this->settings->get(MailerLog::SETTING_NAME);
expect($updatedMailerLog['sent'])->equals(0);
expect($updatedMailerLog['sent'])->equals([]);
expect($updatedMailerLog['started'])->greaterThan($mailerLog['started']);
}
public function testItUpdatesMailerLog() {
$started = time() - 10;
$mailerLog = [
'sent' => 1,
'started' => time() - 10,
'sent' => [date('Y-m-d H:i:s', $started) => 1],
'started' => $started,
];
MailerLog::updateMailerLog($mailerLog);
$updatedMailerLog = $this->settings->get(MailerLog::SETTING_NAME);
@@ -61,15 +63,40 @@ class MailerLogTest extends \MailPoetTest {
}
public function testItIncrementsSentCount() {
$started = time() - 10;
$mailerLog = [
'sent' => 1,
'started' => time(),
'sent' => [date('Y-m-d H:i:s', $started) => 1],
'started' => $started,
'error' => null,
];
$this->settings->set(MailerLog::SETTING_NAME, $mailerLog);
MailerLog::incrementSentCount();
$updatedMailerLog = $this->settings->get(MailerLog::SETTING_NAME);
expect($updatedMailerLog['sent'])->equals(2);
expect(array_sum($updatedMailerLog['sent']))->equals(2);
}
public function testItTruncatesOutdatedEntriesWhenIncrementingSentCount() {
$mailerConfig = [
'frequency' => [
'emails' => 12,
'interval' => 1,
],
];
$inCurrenTimeFrame = time() - 60;
$outdated = $inCurrenTimeFrame - 1;
$mailerLog = [
'sent' => [
date('Y-m-d H:i:s', $outdated) => 2,
date('Y-m-d H:i:s', $inCurrenTimeFrame) => 10,
],
'started' => $outdated,
'error' => null,
];
$this->settings->set(MailerLog::SETTING_NAME, $mailerLog);
$this->settings->set(Mailer::MAILER_CONFIG_SETTING_NAME, $mailerConfig);
MailerLog::incrementSentCount();
$updatedMailerLog = $this->settings->get(MailerLog::SETTING_NAME);
expect(array_sum($updatedMailerLog['sent']))->equals(11);
}
public function testItChecksWhenSendingLimitIsReached() {
@@ -82,17 +109,19 @@ class MailerLogTest extends \MailPoetTest {
$this->settings->set(Mailer::MAILER_CONFIG_SETTING_NAME, $mailerConfig);
// limit is not reached
$started = time() - 10;
$mailerLog = [
'sent' => 1,
'started' => time(),
'sent' => [date('Y-m-d H:i:s', $started) => 1],
'started' => $started,
];
$this->settings->set(MailerLog::SETTING_NAME, $mailerLog);
expect(MailerLog::isSendingLimitReached())->false();
// limit is reached
$started = time() - 10;
$mailerLog = [
'sent' => 2,
'started' => time(),
'sent' => [date('Y-m-d H:i:s', $started) => 2],
'started' => $started,
];
$this->settings->set(MailerLog::SETTING_NAME, $mailerLog);
expect(MailerLog::isSendingLimitReached())->true();
@@ -105,26 +134,23 @@ class MailerLogTest extends \MailPoetTest {
expect(MailerLog::isSendingPaused($mailerLog))->false();
}
public function testItResetsMailerAfterSendingLimitWaitPeriodIsOver() {
public function testItLimitReachedCalculationDoesNotIncludeOutdatedData() {
$mailerConfig = [
'frequency' => [
'emails' => 2,
'interval' => 1,
],
];
$started = time() - 61;
$mailerLog = [
'sent' => 2,
// (mailer config's interval * 60 seconds) + 1 second
'started' => time() - 61,
'sent' => [date('Y-m-d H:i:s', $started) => 2],
'started' => $started,
];
$this->settings->set(MailerLog::SETTING_NAME, $mailerLog);
$this->settings->set(Mailer::MAILER_CONFIG_SETTING_NAME, $mailerConfig);
// limit is not reached
expect(MailerLog::isSendingLimitReached())->false();
// mailer log is reset
$updatedMailerLog = $this->settings->get(MailerLog::SETTING_NAME);
expect($updatedMailerLog['sent'])->equals(0);
}
public function testItResumesSending() {