diff --git a/lib/Cron/CronWorkerRunner.php b/lib/Cron/CronWorkerRunner.php index 36aedaec6e..8e76485dc3 100644 --- a/lib/Cron/CronWorkerRunner.php +++ b/lib/Cron/CronWorkerRunner.php @@ -65,8 +65,6 @@ class CronWorkerRunner { } try { - $parisTask = null; - foreach ($dueTasks as $task) { $parisTask = ScheduledTask::getFromDoctrineEntity($task); if ($parisTask) { @@ -80,8 +78,8 @@ class CronWorkerRunner { } } } catch (\Exception $e) { - if ($parisTask && $e->getCode() !== CronHelper::DAEMON_EXECUTION_LIMIT_REACHED) { - $parisTask->rescheduleProgressively(); + if ($task && $e->getCode() !== CronHelper::DAEMON_EXECUTION_LIMIT_REACHED) { + $this->cronWorkerScheduler->rescheduleProgressively($task); } throw $e; } diff --git a/lib/Cron/CronWorkerScheduler.php b/lib/Cron/CronWorkerScheduler.php index b653d82ead..b69c66d4db 100644 --- a/lib/Cron/CronWorkerScheduler.php +++ b/lib/Cron/CronWorkerScheduler.php @@ -61,4 +61,17 @@ class CronWorkerScheduler { $this->scheduledTaskRepository->persist($task); $this->scheduledTaskRepository->flush(); } + + public function rescheduleProgressively(ScheduledTaskEntity $task): int { + $scheduledAt = Carbon::createFromTimestamp($this->wp->currentTime('timestamp')); + $rescheduleCount = $task->getRescheduleCount(); + $timeout = (int)min(ScheduledTaskEntity::BASIC_RESCHEDULE_TIMEOUT * pow(2, $rescheduleCount), ScheduledTaskEntity::MAX_RESCHEDULE_TIMEOUT); + $task->setScheduledAt($scheduledAt->addMinutes($timeout)); + $task->setRescheduleCount($rescheduleCount + 1); + $task->setStatus(ScheduledTaskEntity::STATUS_SCHEDULED); + $this->scheduledTaskRepository->persist($task); + $this->scheduledTaskRepository->flush(); + + return $timeout; + } } diff --git a/lib/Cron/Workers/KeyCheck/KeyCheckWorker.php b/lib/Cron/Workers/KeyCheck/KeyCheckWorker.php index c1dc27d5ef..bd60ea1460 100644 --- a/lib/Cron/Workers/KeyCheck/KeyCheckWorker.php +++ b/lib/Cron/Workers/KeyCheck/KeyCheckWorker.php @@ -2,15 +2,27 @@ namespace MailPoet\Cron\Workers\KeyCheck; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Cron\Workers\SimpleWorker; use MailPoet\Entities\ScheduledTaskEntity; -use MailPoet\Models\ScheduledTask; use MailPoet\Services\Bridge; +use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Carbon\Carbon; abstract class KeyCheckWorker extends SimpleWorker { public $bridge; + /** @var CronWorkerScheduler */ + protected $cronWorkerScheduler; + + public function __construct( + CronWorkerScheduler $cronWorkerScheduler, + WPFunctions $wp = null + ) { + parent::__construct($wp); + $this->cronWorkerScheduler = $cronWorkerScheduler; + } + public function init() { if (!$this->bridge) { $this->bridge = new Bridge(); @@ -25,10 +37,7 @@ abstract class KeyCheckWorker extends SimpleWorker { } if (empty($result['code']) || $result['code'] == Bridge::CHECK_ERROR_UNAVAILABLE) { - $parisTask = ScheduledTask::getFromDoctrineEntity($task); - if ($parisTask) { - $parisTask->rescheduleProgressively(); - } + $this->cronWorkerScheduler->rescheduleProgressively($task); return false; } diff --git a/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php b/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php index 18236a496d..df13f0935a 100644 --- a/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php +++ b/lib/Cron/Workers/KeyCheck/PremiumKeyCheck.php @@ -2,6 +2,7 @@ namespace MailPoet\Cron\Workers\KeyCheck; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Services\Bridge; use MailPoet\Settings\SettingsController; @@ -12,10 +13,11 @@ class PremiumKeyCheck extends KeyCheckWorker { private $settings; public function __construct( - SettingsController $settings + SettingsController $settings, + CronWorkerScheduler $cronWorkerScheduler ) { $this->settings = $settings; - parent::__construct(); + parent::__construct($cronWorkerScheduler); } public function checkProcessingRequirements() { diff --git a/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php b/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php index 997b3b6f60..f73eb93951 100644 --- a/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php +++ b/lib/Cron/Workers/KeyCheck/SendingServiceKeyCheck.php @@ -3,6 +3,7 @@ namespace MailPoet\Cron\Workers\KeyCheck; use MailPoet\Config\ServicesChecker; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Mailer\Mailer; use MailPoet\Mailer\MailerLog; use MailPoet\Services\Bridge; @@ -20,11 +21,12 @@ class SendingServiceKeyCheck extends KeyCheckWorker { public function __construct( SettingsController $settings, - ServicesChecker $servicesChecker + ServicesChecker $servicesChecker, + CronWorkerScheduler $cronWorkerScheduler ) { $this->settings = $settings; $this->servicesChecker = $servicesChecker; - parent::__construct(); + parent::__construct($cronWorkerScheduler); } public function checkProcessingRequirements() { diff --git a/lib/Cron/Workers/Scheduler.php b/lib/Cron/Workers/Scheduler.php index 106f359c1d..30cdbb0cd0 100644 --- a/lib/Cron/Workers/Scheduler.php +++ b/lib/Cron/Workers/Scheduler.php @@ -3,7 +3,9 @@ namespace MailPoet\Cron\Workers; use MailPoet\Cron\CronHelper; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Entities\NewsletterEntity; +use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Logging\LoggerFactory; use MailPoet\Models\Newsletter; use MailPoet\Models\ScheduledTask; @@ -13,6 +15,7 @@ use MailPoet\Models\SubscriberSegment; use MailPoet\Newsletter\Scheduler\PostNotificationScheduler; use MailPoet\Newsletter\Scheduler\Scheduler as NewsletterScheduler; use MailPoet\Newsletter\Scheduler\WelcomeScheduler; +use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Segments\SubscribersFinder; use MailPoet\Tasks\Sending as SendingTask; @@ -28,14 +31,24 @@ class Scheduler { /** @var CronHelper */ private $cronHelper; + /** @var CronWorkerScheduler */ + private $cronWorkerScheduler; + + /** @var ScheduledTasksRepository */ + private $scheduledTasksRepository; + public function __construct( SubscribersFinder $subscribersFinder, LoggerFactory $loggerFactory, - CronHelper $cronHelper + CronHelper $cronHelper, + CronWorkerScheduler $cronWorkerScheduler, + ScheduledTasksRepository $scheduledTasksRepository ) { $this->cronHelper = $cronHelper; $this->subscribersFinder = $subscribersFinder; $this->loggerFactory = $loggerFactory; + $this->cronWorkerScheduler = $cronWorkerScheduler; + $this->scheduledTasksRepository = $scheduledTasksRepository; } public function process($timer = false) { @@ -219,7 +232,12 @@ class Scheduler { public function verifySubscriber($subscriber, $queue) { if ($subscriber->status === Subscriber::STATUS_UNCONFIRMED) { // reschedule delivery - $queue->rescheduleProgressively(); + $task = $this->scheduledTasksRepository->findOneById($queue->task()->id); + + if ($task instanceof ScheduledTaskEntity) { + $this->cronWorkerScheduler->rescheduleProgressively($task); + } + return false; } else if ($subscriber->status === Subscriber::STATUS_UNSUBSCRIBED) { $queue->delete(); diff --git a/lib/Entities/ScheduledTaskEntity.php b/lib/Entities/ScheduledTaskEntity.php index 8a483e4bc0..d8f6594a02 100644 --- a/lib/Entities/ScheduledTaskEntity.php +++ b/lib/Entities/ScheduledTaskEntity.php @@ -24,6 +24,8 @@ class ScheduledTaskEntity { const PRIORITY_HIGH = 1; const PRIORITY_MEDIUM = 5; const PRIORITY_LOW = 10; + const BASIC_RESCHEDULE_TIMEOUT = 5; // minutes + const MAX_RESCHEDULE_TIMEOUT = 1440; // minutes use AutoincrementedIdTrait; use CreatedAtTrait; diff --git a/lib/Models/ScheduledTask.php b/lib/Models/ScheduledTask.php index 5e2e64273b..f273c16af7 100644 --- a/lib/Models/ScheduledTask.php +++ b/lib/Models/ScheduledTask.php @@ -30,8 +30,8 @@ class ScheduledTask extends Model { const PRIORITY_MEDIUM = ScheduledTaskEntity::PRIORITY_MEDIUM; const PRIORITY_LOW = ScheduledTaskEntity::PRIORITY_LOW; - const BASIC_RESCHEDULE_TIMEOUT = 5; //minutes - const MAX_RESCHEDULE_TIMEOUT = 1440; //minutes + const BASIC_RESCHEDULE_TIMEOUT = ScheduledTaskEntity::BASIC_RESCHEDULE_TIMEOUT; + const MAX_RESCHEDULE_TIMEOUT = ScheduledTaskEntity::MAX_RESCHEDULE_TIMEOUT; private $wp; @@ -130,16 +130,6 @@ class ScheduledTask extends Model { return null; } - public function rescheduleProgressively() { - $scheduledAt = Carbon::createFromTimestamp($this->wp->currentTime('timestamp')); - $timeout = (int)min(self::BASIC_RESCHEDULE_TIMEOUT * pow(2, $this->rescheduleCount), self::MAX_RESCHEDULE_TIMEOUT); - $this->scheduledAt = $scheduledAt->addMinutes($timeout); - $this->rescheduleCount++; - $this->status = ScheduledTask::STATUS_SCHEDULED; - $this->save(); - return $timeout; - } - public static function touchAllByIds(array $ids) { ScheduledTask::rawExecute( 'UPDATE `' . ScheduledTask::$_table . '`' . diff --git a/tests/integration/Cron/CronWorkerSchedulerTest.php b/tests/integration/Cron/CronWorkerSchedulerTest.php index 0b612c6e6b..5cb2e488f5 100644 --- a/tests/integration/Cron/CronWorkerSchedulerTest.php +++ b/tests/integration/Cron/CronWorkerSchedulerTest.php @@ -4,6 +4,7 @@ namespace MailPoet\Test\Cron; use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Entities\ScheduledTaskEntity; +use MailPoet\Test\DataFactories\ScheduledTask as ScheduledTaskFactory; use MailPoetVendor\Carbon\Carbon; require_once __DIR__ . '/Workers/SimpleWorkerMockImplementation.php'; @@ -12,8 +13,12 @@ class CronWorkerSchedulerTest extends \MailPoetTest { /** @var CronWorkerScheduler */ private $cronWorkerScheduler; + /** @var ScheduledTaskFactory */ + private $scheduledTaskFactory; + public function _before() { $this->cronWorkerScheduler = $this->diContainer->get(CronWorkerScheduler::class); + $this->scheduledTaskFactory = new ScheduledTaskFactory(); $this->truncateEntity(ScheduledTaskEntity::class); } @@ -87,6 +92,23 @@ class CronWorkerSchedulerTest extends \MailPoetTest { expect($tasks[0]->getScheduledAt())->greaterThan(Carbon::now()); } + public function testItCanRescheduleTasksProgressively() { + $task = $this->scheduledTaskFactory->create('test', null, new Carbon()); + $scheduledAt = $task->getScheduledAt(); + + $timeout = $this->cronWorkerScheduler->rescheduleProgressively($task); + expect($timeout)->equals(ScheduledTaskEntity::BASIC_RESCHEDULE_TIMEOUT); + expect($scheduledAt < $task->getScheduledAt())->true(); + expect($task->getStatus())->equals(ScheduledTaskEntity::STATUS_SCHEDULED); + + $timeout = $this->cronWorkerScheduler->rescheduleProgressively($task); + expect($timeout)->equals(ScheduledTaskEntity::BASIC_RESCHEDULE_TIMEOUT * 2); + + $task->setRescheduleCount(123456); // too many + $timeout = $this->cronWorkerScheduler->rescheduleProgressively($task); + expect($timeout)->equals(ScheduledTaskEntity::MAX_RESCHEDULE_TIMEOUT); + } + public function _after() { $this->truncateEntity(ScheduledTaskEntity::class); } diff --git a/tests/integration/Cron/Workers/KeyCheck/KeyCheckWorkerTest.php b/tests/integration/Cron/Workers/KeyCheck/KeyCheckWorkerTest.php index 5c087b0a07..e02c762ad1 100644 --- a/tests/integration/Cron/Workers/KeyCheck/KeyCheckWorkerTest.php +++ b/tests/integration/Cron/Workers/KeyCheck/KeyCheckWorkerTest.php @@ -3,9 +3,9 @@ namespace MailPoet\Test\Cron\Workers\KeyCheck; use Codeception\Stub; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Cron\Workers\KeyCheck\KeyCheckWorkerMockImplementation as MockKeyCheckWorker; use MailPoet\Entities\ScheduledTaskEntity; -use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Services\Bridge; use MailPoet\Settings\SettingsRepository; use MailPoet\Test\DataFactories\ScheduledTask as ScheduledTaskFactory; @@ -20,14 +20,11 @@ class KeyCheckWorkerTest extends \MailPoetTest { /** @var ScheduledTaskFactory */ private $scheduledTaskFactory; - /** @var ScheduledTasksRepository */ - private $scheduledTasksRepository; - public function _before() { parent::_before(); $this->scheduledTaskFactory = new ScheduledTaskFactory(); - $this->worker = new MockKeyCheckWorker(); - $this->scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class); + $cronWorkerScheduler = $this->diContainer->get(CronWorkerScheduler::class); + $this->worker = new MockKeyCheckWorker($cronWorkerScheduler); } public function testItCanInitializeBridgeAPI() { @@ -42,37 +39,37 @@ class KeyCheckWorkerTest extends \MailPoetTest { } public function testItReschedulesCheckOnException() { + $cronWorkerSchedulerMock = $this->createMock(CronWorkerScheduler::class); + $cronWorkerSchedulerMock->expects($this->once())->method('rescheduleProgressively'); + $worker = Stub::make( $this->worker, [ 'checkKey' => function () { throw new \Exception; }, + 'cronWorkerScheduler' => $cronWorkerSchedulerMock, ], $this ); + $currentTime = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); $task = $this->createRunningTask($currentTime); + $result = $worker->processTaskStrategy($task, microtime(true)); - // need to clear Doctrine cache and get the entity again while ScheduledTask::rescheduleProgressively() is not migrated to Doctrine - $this->entityManager->clear(); - $task = $this->scheduledTasksRepository->findOneById($task->getId()); - - assert($task instanceof ScheduledTaskEntity); - assert($task->getScheduledAt() instanceof \DateTimeInterface); - $newScheduledAtTime = $currentTime->addMinutes(5)->format('Y-m-d H:i:s'); - $scheduledAt = $task->getScheduledAt()->format('Y-m-d H:i:s'); $this->assertFalse($result); - $this->assertSame($newScheduledAtTime, $scheduledAt); - $this->assertSame(1, $task->getRescheduleCount()); } public function testItReschedulesCheckOnError() { + $cronWorkerSchedulerMock = $this->createMock(CronWorkerScheduler::class); + $cronWorkerSchedulerMock->expects($this->once())->method('rescheduleProgressively'); + $worker = Stub::make( $this->worker, [ 'checkKey' => ['code' => Bridge::CHECK_ERROR_UNAVAILABLE], + 'cronWorkerScheduler' => $cronWorkerSchedulerMock, ], $this ); @@ -82,17 +79,7 @@ class KeyCheckWorkerTest extends \MailPoetTest { $result = $worker->processTaskStrategy($task, microtime(true)); - // need to clear Doctrine cache and get the entity again while ScheduledTask::rescheduleProgressively() is not migrated to Doctrine - $this->entityManager->clear(); - $task = $this->scheduledTasksRepository->findOneById($task->getId()); - - assert($task instanceof ScheduledTaskEntity); - assert($task->getScheduledAt() instanceof \DateTimeInterface); - $newScheduledAtTime = $currentTime->addMinutes(5)->format('Y-m-d H:i:s'); - $scheduledAt = $task->getScheduledAt()->format('Y-m-d H:i:s'); $this->assertFalse($result); - $this->assertSame($newScheduledAtTime, $scheduledAt); - $this->assertSame(1, $task->getRescheduleCount()); } public function testItNextRunIsNextDay(): void { diff --git a/tests/integration/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php b/tests/integration/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php index 920ad20ad9..fbd72ff1fb 100644 --- a/tests/integration/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php +++ b/tests/integration/Cron/Workers/KeyCheck/PremiumKeyCheckTest.php @@ -3,6 +3,7 @@ namespace MailPoet\Test\Cron\Workers\KeyCheck; use Codeception\Util\Stub; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Cron\Workers\KeyCheck\PremiumKeyCheck; use MailPoet\Services\Bridge; use MailPoet\Settings\SettingsController; @@ -20,7 +21,8 @@ class PremiumKeyCheckTest extends \MailPoetTest { parent::_before(); $this->settings = SettingsController::getInstance(); $this->premiumKey = '123457890abcdef'; - $this->worker = new PremiumKeyCheck($this->settings); + $cronWorkerScheduler = $this->diContainer->get(CronWorkerScheduler::class); + $this->worker = new PremiumKeyCheck($this->settings, $cronWorkerScheduler); } public function testItRequiresPremiumKeyToBeSpecified() { diff --git a/tests/integration/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php b/tests/integration/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php index 0f2bb4487c..bd2f8df3f8 100644 --- a/tests/integration/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php +++ b/tests/integration/Cron/Workers/KeyCheck/SendingServiceKeyCheckTest.php @@ -5,6 +5,7 @@ namespace MailPoet\Test\Cron\Workers\KeyCheck; use Codeception\Stub; use Codeception\Stub\Expected; use MailPoet\Config\ServicesChecker; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Cron\Workers\KeyCheck\SendingServiceKeyCheck; use MailPoet\Mailer\Mailer; use MailPoet\Mailer\MailerLog; @@ -23,7 +24,8 @@ class SendingServiceKeyCheckTest extends \MailPoetTest { $this->mssKey = 'some_key'; $this->worker = new SendingServiceKeyCheck( $this->diContainer->get(SettingsController::class), - $this->diContainer->get(ServicesChecker::class) + $this->diContainer->get(ServicesChecker::class), + $this->diContainer->get(CronWorkerScheduler::class) ); } @@ -64,7 +66,8 @@ class SendingServiceKeyCheckTest extends \MailPoetTest { $worker = new SendingServiceKeyCheck( $this->diContainer->get(SettingsController::class), - $servicesChecker + $servicesChecker, + $this->diContainer->get(CronWorkerScheduler::class) ); $bridge = $this->make(new Bridge, [ diff --git a/tests/integration/Cron/Workers/SchedulerTest.php b/tests/integration/Cron/Workers/SchedulerTest.php index 9c032dc6fa..0866602a19 100644 --- a/tests/integration/Cron/Workers/SchedulerTest.php +++ b/tests/integration/Cron/Workers/SchedulerTest.php @@ -5,6 +5,7 @@ namespace MailPoet\Test\Cron\Workers; use Codeception\Stub; use Codeception\Stub\Expected; use MailPoet\Cron\CronHelper; +use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Cron\Workers\Scheduler; use MailPoet\Logging\LoggerFactory; use MailPoet\Models\Newsletter; @@ -18,6 +19,7 @@ use MailPoet\Models\SendingQueue; use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberSegment; use MailPoet\Newsletter\Scheduler\WelcomeScheduler; +use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Segments\SubscribersFinder; use MailPoet\Settings\SettingsRepository; use MailPoet\Tasks\Sending as SendingTask; @@ -34,16 +36,24 @@ class SchedulerTest extends \MailPoetTest { /** @var SubscribersFinder */ private $subscribersFinder; + /** @var CronWorkerScheduler */ + private $cronWorkerScheduler; + + /** @var ScheduledTasksRepository */ + private $scheduledTasksRepository; + public function _before() { parent::_before(); $this->loggerFactory = LoggerFactory::getInstance(); $this->cronHelper = $this->diContainer->get(CronHelper::class); $this->subscribersFinder = $this->diContainer->get(SubscribersFinder::class); + $this->cronWorkerScheduler = $this->diContainer->get(CronWorkerScheduler::class); + $this->scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class); } public function testItThrowsExceptionWhenExecutionLimitIsReached() { try { - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->process(microtime(true) - $this->cronHelper->getDaemonExecutionLimit()); self::fail('Maximum execution time limit exception was not thrown.'); } catch (\Exception $e) { @@ -73,7 +83,7 @@ class SchedulerTest extends \MailPoetTest { expect($notificationHistory)->isEmpty(); // create notification history and ensure that it exists - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->createNotificationHistory($newsletter->id); $notificationHistory = Newsletter::where('type', Newsletter::TYPE_NOTIFICATION_HISTORY) ->where('parent_id', $newsletter->id) @@ -90,7 +100,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // queue and associated newsletter should be deleted when interval type is set to "immediately" expect(SendingQueue::findMany())->notEmpty(); @@ -108,7 +118,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // queue's next run date should change when interval type is set to anything // other than "immediately" @@ -142,7 +152,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return false and delete queue when subscriber is not a WP user $result = $scheduler->verifyWPSubscriber($subscriber->id, $newsletter, $queue); @@ -166,7 +176,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return false and delete queue when subscriber role is different from the one // specified for the welcome email @@ -189,7 +199,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return true when user exists and WP role matches the one specified for the welcome email $result = $scheduler->verifyWPSubscriber($subscriber->id, $newsletter, $queue); @@ -211,7 +221,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // true when user exists and has any role $result = $scheduler->verifyWPSubscriber($subscriber->id, $newsletter, $queue); @@ -225,7 +235,7 @@ class SchedulerTest extends \MailPoetTest { $queue->setSubscribers([]); // delete queue when the list of subscribers to process is blank - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $result = $scheduler->processWelcomeNewsletter($newsletter, $queue); expect($result)->false(); expect(SendingQueue::findMany())->count(0); @@ -299,7 +309,7 @@ class SchedulerTest extends \MailPoetTest { } public function testItFailsMailpoetSubscriberVerificationWhenSubscriberDoesNotExist() { - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $newsletter = $this->_createNewsletter(); $queue = $this->_createQueue($newsletter->id); @@ -320,7 +330,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return false $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -352,7 +362,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return false $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -384,7 +394,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return false $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -408,7 +418,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return true after successful verification $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -431,7 +441,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return true expect($scheduler->processScheduledStandardNewsletter($newsletter, $queue))->true(); @@ -471,7 +481,7 @@ class SchedulerTest extends \MailPoetTest { $newsletterSegment = $this->_createNewsletterSegment($newsletter->id, $segment->id); // delete or reschedule queue when there are no subscribers in segments - $scheduler = $this->construct(Scheduler::class, [$this->subscribersFinder, $this->loggerFactory, $this->cronHelper], [ + $scheduler = $this->construct(Scheduler::class, [$this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository], [ 'deleteQueueOrUpdateNextRunDate' => Expected::exactly(1, function() { return false; }), @@ -495,7 +505,7 @@ class SchedulerTest extends \MailPoetTest { $newsletter = Newsletter::filter('filterWithOptions', Newsletter::TYPE_NOTIFICATION) ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); - $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); // return true expect($scheduler->processPostNotificationNewsletter($newsletter, $queue))->true(); @@ -520,7 +530,7 @@ class SchedulerTest extends \MailPoetTest { } public function testItFailsToProcessWhenScheduledQueuesNotFound() { - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); expect($scheduler->process())->false(); } @@ -528,7 +538,7 @@ class SchedulerTest extends \MailPoetTest { $queue = $this->_createQueue(1); $queue->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); $queue->save(); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->process(); expect(SendingQueue::findMany())->count(0); } @@ -540,7 +550,7 @@ class SchedulerTest extends \MailPoetTest { $queue = $this->_createQueue($newsletter->id); $queue->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); $queue->save(); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->process(); expect(SendingQueue::findMany())->count(0); } @@ -632,7 +642,7 @@ class SchedulerTest extends \MailPoetTest { $newsletter = $this->_createNewsletter(Newsletter::TYPE_STANDARD, Newsletter::STATUS_DRAFT); $queue = $this->_createQueue($newsletter->id); $finder = $this->makeEmpty(SubscribersFinder::class); - $scheduler = new Scheduler($finder, $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($finder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->processScheduledStandardNewsletter($newsletter, $queue); $refetchedTask = ScheduledTask::where('id', $task->id)->findOne(); @@ -671,7 +681,7 @@ class SchedulerTest extends \MailPoetTest { expect($task->getSubscribers())->equals([$subscriber->id]); // task should have its status set to null (i.e., sending) - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->process(); $task = SendingTask::getByNewsletterId($newsletter->id); expect($task->status)->null(); @@ -695,7 +705,7 @@ class SchedulerTest extends \MailPoetTest { expect($task->getSubscribers())->equals([$subscriber->id]); // task should be deleted - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->process(); $task = SendingTask::getByNewsletterId($newsletter->id); expect($task)->false(); @@ -723,7 +733,7 @@ class SchedulerTest extends \MailPoetTest { expect($task->newsletterId)->equals($newsletter->id); // task should have its status set to null (i.e., sending) - $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->process(); $task = SendingTask::getByNewsletterId($newsletter->id); expect($task->status)->null(); @@ -740,7 +750,7 @@ class SchedulerTest extends \MailPoetTest { $queue->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); $queue->updatedAt = $originalUpdated; $queue->save(); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); $scheduler->process(); $newQueue = ScheduledTask::findOne($queue->taskId); assert($newQueue instanceof ScheduledTask); diff --git a/tests/integration/Models/ScheduledTaskTest.php b/tests/integration/Models/ScheduledTaskTest.php index 302b77674c..4b224f494d 100644 --- a/tests/integration/Models/ScheduledTaskTest.php +++ b/tests/integration/Models/ScheduledTaskTest.php @@ -163,24 +163,6 @@ class ScheduledTaskTest extends \MailPoetTest { expect($task->meta)->equals($meta); } - public function testItCanRescheduleTasksProgressively() { - $task = $this->task; - $task->status = null; - $scheduledAt = $task->scheduledAt; - - $timeout = $task->rescheduleProgressively(); - expect($timeout)->equals(ScheduledTask::BASIC_RESCHEDULE_TIMEOUT); - expect($scheduledAt < $task->scheduledAt)->true(); - expect($task->status)->equals(ScheduledTask::STATUS_SCHEDULED); - - $timeout = $task->rescheduleProgressively(); - expect($timeout)->equals(ScheduledTask::BASIC_RESCHEDULE_TIMEOUT * 2); - - $task->rescheduleCount = 123456; // too many - $timeout = $task->rescheduleProgressively(); - expect($timeout)->equals(ScheduledTask::MAX_RESCHEDULE_TIMEOUT); - } - public function testItCanGetFutureScheduledTasks() { // scheduled (in future) ScheduledTask::createOrUpdate([