diff --git a/lib/Cron/CronWorkerInterface.php b/lib/Cron/CronWorkerInterface.php index 741c22dc2c..121f415387 100644 --- a/lib/Cron/CronWorkerInterface.php +++ b/lib/Cron/CronWorkerInterface.php @@ -2,6 +2,7 @@ namespace MailPoet\Cron; +use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Models\ScheduledTask; interface CronWorkerInterface { @@ -20,11 +21,11 @@ interface CronWorkerInterface { public function init(); /** - * @param ScheduledTask $task + * @param ScheduledTaskEntity $task * @param float $timer * @return bool */ - public function prepareTaskStrategy(ScheduledTask $task, $timer); + public function prepareTaskStrategy(ScheduledTaskEntity $task, $timer); /** * @param ScheduledTask $task diff --git a/lib/Cron/CronWorkerRunner.php b/lib/Cron/CronWorkerRunner.php index 9c1a7c67ca..f04a276935 100644 --- a/lib/Cron/CronWorkerRunner.php +++ b/lib/Cron/CronWorkerRunner.php @@ -68,13 +68,13 @@ class CronWorkerRunner { $parisTask = null; foreach ($dueTasks as $task) { - $parisTask = $this->convertTaskClass($task); + $parisTask = ScheduledTask::getFromDoctrineEntity($task); if ($parisTask) { $this->prepareTask($worker, $parisTask); } } foreach ($runningTasks as $task) { - $parisTask = $this->convertTaskClass($task); + $parisTask = ScheduledTask::getFromDoctrineEntity($task); if ($parisTask) { $this->processTask($worker, $parisTask); } @@ -101,10 +101,13 @@ class CronWorkerRunner { // abort if execution limit is reached $this->cronHelper->enforceExecutionLimit($this->timer); - $prepareCompleted = $worker->prepareTaskStrategy($task, $this->timer); - if ($prepareCompleted) { - $task->status = null; - $task->save(); + $doctrineTask = $this->convertTaskClassToDoctrine($task); + if ($doctrineTask) { + $prepareCompleted = $worker->prepareTaskStrategy($doctrineTask, $this->timer); + if ($prepareCompleted) { + $task->status = null; + $task->save(); + } } } @@ -182,15 +185,15 @@ class CronWorkerRunner { $task->save(); } - // temporary function to convert an ScheduledTaskEntity object to ScheduledTask while we don't migrate the rest of + // temporary function to convert an ScheduledTask object to ScheduledTaskEntity while we don't migrate the rest of // the code in this class to use Doctrine entities - private function convertTaskClass(ScheduledTaskEntity $doctrineTask): ?ScheduledTask { - $parisTask = ScheduledTask::findOne($doctrineTask->getId()); + private function convertTaskClassToDoctrine(ScheduledTask $parisTask): ?ScheduledTaskEntity { + $doctrineTask = $this->scheduledTasksRepository->findOneById($parisTask->id); - if (!$parisTask instanceof ScheduledTask) { + if (!$doctrineTask instanceof ScheduledTaskEntity) { return null; } - return $parisTask; + return $doctrineTask; } } diff --git a/lib/Cron/Workers/Bounce.php b/lib/Cron/Workers/Bounce.php index 4d18bfaa93..ccfe9bf3ae 100644 --- a/lib/Cron/Workers/Bounce.php +++ b/lib/Cron/Workers/Bounce.php @@ -70,11 +70,11 @@ class Bounce extends SimpleWorker { return $this->bridge->isMailpoetSendingServiceEnabled(); } - public function prepareTaskStrategy(ScheduledTask $task, $timer) { + public function prepareTaskStrategy(ScheduledTaskEntity $task, $timer) { BounceTask::prepareSubscribers($task); - if (!ScheduledTaskSubscriber::getUnprocessedCount($task->id)) { - ScheduledTaskSubscriber::where('task_id', $task->id)->deleteMany(); + if (!ScheduledTaskSubscriber::getUnprocessedCount($task->getId())) { + ScheduledTaskSubscriber::where('task_id', $task->getId())->deleteMany(); return false; } return true; diff --git a/lib/Cron/Workers/SendingQueue/Migration.php b/lib/Cron/Workers/SendingQueue/Migration.php index 2cb86ecfce..244c8e247b 100644 --- a/lib/Cron/Workers/SendingQueue/Migration.php +++ b/lib/Cron/Workers/SendingQueue/Migration.php @@ -3,6 +3,7 @@ namespace MailPoet\Cron\Workers\SendingQueue; use MailPoet\Cron\Workers\SimpleWorker; +use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Mailer\MailerLog; use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTaskSubscriber; @@ -20,7 +21,7 @@ class Migration extends SimpleWorker { return empty($completedTasks); } - public function prepareTaskStrategy(ScheduledTask $task, $timer) { + public function prepareTaskStrategy(ScheduledTaskEntity $task, $timer) { $unmigratedColumns = $this->checkUnmigratedColumnsExist(); $unmigratedQueuesCount = 0; $unmigratedQueueSubscribers = []; @@ -35,9 +36,10 @@ class Migration extends SimpleWorker { && count($unmigratedQueueSubscribers) == 0) ) { // nothing to migrate, complete task - $task->processedAt = WPFunctions::get()->currentTime('mysql'); - $task->status = ScheduledTask::STATUS_COMPLETED; - $task->save(); + $task->setProcessedAt(Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'))); + $task->setStatus(ScheduledTask::STATUS_COMPLETED); + $this->scheduledTasksRepository->persist($task); + $this->scheduledTasksRepository->flush(); $this->resumeSending(); return false; } diff --git a/lib/Cron/Workers/SimpleWorker.php b/lib/Cron/Workers/SimpleWorker.php index 069f930d41..9c7093d87e 100644 --- a/lib/Cron/Workers/SimpleWorker.php +++ b/lib/Cron/Workers/SimpleWorker.php @@ -7,6 +7,7 @@ use MailPoet\Cron\CronWorkerInterface; use MailPoet\Cron\CronWorkerRunner; use MailPoet\Cron\CronWorkerScheduler; use MailPoet\DI\ContainerWrapper; +use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Models\ScheduledTask; use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\WP\Functions as WPFunctions; @@ -68,7 +69,7 @@ abstract class SimpleWorker implements CronWorkerInterface { public function init() { } - public function prepareTaskStrategy(ScheduledTask $task, $timer) { + public function prepareTaskStrategy(ScheduledTaskEntity $task, $timer) { return true; } diff --git a/lib/Models/ScheduledTask.php b/lib/Models/ScheduledTask.php index fb84f7c316..5e2e64273b 100644 --- a/lib/Models/ScheduledTask.php +++ b/lib/Models/ScheduledTask.php @@ -191,4 +191,16 @@ class ScheduledTask extends Model { return $query->findMany(); } + + // temporary function to convert an ScheduledTaskEntity object to ScheduledTask while we don't migrate the rest of + // the code in this class to use Doctrine entities + public static function getFromDoctrineEntity(ScheduledTaskEntity $doctrineTask): ?ScheduledTask { + $parisTask = self::findOne($doctrineTask->getId()); + + if (!$parisTask instanceof ScheduledTask) { + return null; + } + + return $parisTask; + } } diff --git a/lib/Tasks/Bounce.php b/lib/Tasks/Bounce.php index 8fb37e094e..ec166ceb92 100644 --- a/lib/Tasks/Bounce.php +++ b/lib/Tasks/Bounce.php @@ -2,12 +2,12 @@ namespace MailPoet\Tasks; -use MailPoet\Models\ScheduledTask; +use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Models\ScheduledTaskSubscriber; use MailPoet\Models\Subscriber; class Bounce { - public static function prepareSubscribers(ScheduledTask $task) { + public static function prepareSubscribers(ScheduledTaskEntity $task) { // Prepare subscribers on the DB side for performance reasons Subscriber::rawExecute( 'INSERT IGNORE INTO ' . MP_SCHEDULED_TASK_SUBSCRIBERS_TABLE . ' @@ -17,7 +17,7 @@ class Bounce { WHERE s.`deleted_at` IS NULL AND s.`status` IN (?, ?)', [ - $task->id, + $task->getId(), ScheduledTaskSubscriber::STATUS_UNPROCESSED, Subscriber::STATUS_SUBSCRIBED, Subscriber::STATUS_UNCONFIRMED, diff --git a/tests/integration/Cron/Workers/BounceTest.php b/tests/integration/Cron/Workers/BounceTest.php index 491fdf282f..f38d490dab 100644 --- a/tests/integration/Cron/Workers/BounceTest.php +++ b/tests/integration/Cron/Workers/BounceTest.php @@ -20,6 +20,7 @@ use MailPoet\Settings\SettingsController; use MailPoet\Settings\SettingsRepository; use MailPoet\Statistics\StatisticsBouncesRepository; use MailPoet\Subscribers\SubscribersRepository; +use MailPoet\Test\DataFactories\ScheduledTask as ScheduledTaskFactory; use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Carbon\Carbon; @@ -36,6 +37,9 @@ class BounceTest extends \MailPoetTest { /** @var SubscribersRepository */ private $subscribersRepository; + /** @var ScheduledTaskFactory */ + private $scheduledTaskFactory; + public function _before() { parent::_before(); $this->cleanup(); @@ -45,6 +49,7 @@ class BounceTest extends \MailPoetTest { 'good_address@example.com', ]; $this->subscribersRepository = $this->diContainer->get(SubscribersRepository::class); + $this->scheduledTaskFactory = new ScheduledTaskFactory(); foreach ($this->emails as $email) { $subscriber = new SubscriberEntity(); @@ -93,47 +98,47 @@ class BounceTest extends \MailPoetTest { // 1st run - subscribers will be processed $task = $this->createScheduledTask(); $this->worker->prepareTaskStrategy($task, microtime(true)); - expect(ScheduledTaskSubscriber::where('task_id', $task->id)->findMany())->notEmpty(); + expect(ScheduledTaskSubscriber::where('task_id', $task->getId())->findMany())->notEmpty(); // 2nd run - nothing more to process, ScheduledTaskSubscriber will be cleaned up $this->truncateEntity(SubscriberEntity::class); $task = $this->createScheduledTask(); $this->worker->prepareTaskStrategy($task, microtime(true)); - expect(ScheduledTaskSubscriber::where('task_id', $task->id)->findMany())->isEmpty(); + expect(ScheduledTaskSubscriber::where('task_id', $task->getId())->findMany())->isEmpty(); } public function testItPreparesTask() { $task = $this->createScheduledTask(); - expect(ScheduledTaskSubscriber::getUnprocessedCount($task->id))->isEmpty(); + expect(ScheduledTaskSubscriber::getUnprocessedCount($task->getId()))->isEmpty(); $result = $this->worker->prepareTaskStrategy($task, microtime(true)); expect($result)->true(); - expect(ScheduledTaskSubscriber::getUnprocessedCount($task->id))->notEmpty(); + expect(ScheduledTaskSubscriber::getUnprocessedCount($task->getId()))->notEmpty(); } public function testItDeletesAllSubscribersIfThereAreNoSubscribersToProcessWhenProcessingTask() { // prepare subscribers $task = $this->createScheduledTask(); $this->worker->prepareTaskStrategy($task, microtime(true)); - expect(ScheduledTaskSubscriber::where('task_id', $task->id)->findMany())->notEmpty(); + expect(ScheduledTaskSubscriber::where('task_id', $task->getId())->findMany())->notEmpty(); // process - no subscribers found, ScheduledTaskSubscriber will be cleaned up $this->truncateEntity(SubscriberEntity::class); - $task = $this->createScheduledTask(); - $this->worker->processTaskStrategy($task, microtime(true)); - expect(ScheduledTaskSubscriber::where('task_id', $task->id)->findMany())->isEmpty(); + $task = ScheduledTask::getFromDoctrineEntity($this->createScheduledTask()); + $this->worker->processTaskStrategy($task, microtime(true)); // @phpstan-ignore-line + expect(ScheduledTaskSubscriber::where('task_id', $task->id)->findMany())->isEmpty(); // @phpstan-ignore-line } public function testItProcessesTask() { $task = $this->createRunningTask(); $this->worker->prepareTaskStrategy($task, microtime(true)); - expect(ScheduledTaskSubscriber::getUnprocessedCount($task->id))->notEmpty(); - $this->worker->processTaskStrategy($task, microtime(true)); - expect(ScheduledTaskSubscriber::getProcessedCount($task->id))->notEmpty(); + expect(ScheduledTaskSubscriber::getUnprocessedCount($task->getId()))->notEmpty(); + $this->worker->processTaskStrategy(ScheduledTask::getFromDoctrineEntity($task), microtime(true)); // @phpstan-ignore-line + expect(ScheduledTaskSubscriber::getProcessedCount($task->getId()))->notEmpty(); } public function testItSetsSubscriberStatusAsBounced() { $task = $this->createRunningTask(); - $this->worker->processEmails($task, $this->emails); + $this->worker->processEmails(ScheduledTask::getFromDoctrineEntity($task), $this->emails); $subscribers = $this->subscribersRepository->findAll(); @@ -153,11 +158,12 @@ class BounceTest extends \MailPoetTest { $this->createScheduledTaskSubscriber($oldSendingTask, $subscriber); // create previous bounce task $previousBounceTask = $this->createRunningTask(); - $previousBounceTask->status = ScheduledTask::STATUS_COMPLETED; - $previousBounceTask->createdAt = Carbon::now()->subDays(6); - $previousBounceTask->scheduledAt = Carbon::now()->subDays(4); - $previousBounceTask->updatedAt = Carbon::now()->subDays(4); - $previousBounceTask->save(); + $previousBounceTask->setStatus(ScheduledTask::STATUS_COMPLETED); + $previousBounceTask->setCreatedAt(Carbon::now()->subDays(6)); + $previousBounceTask->setScheduledAt(Carbon::now()->subDays(4)); + $previousBounceTask->setUpdatedAt(Carbon::now()->subDays(4)); + $this->entityManager->persist($previousBounceTask); + $this->entityManager->flush(); // create data that should be used for the current bounce task run $newsletter = $this->createNewsletter(); $sendingTask = $this->createSendingTask() ; @@ -169,7 +175,7 @@ class BounceTest extends \MailPoetTest { $this->entityManager->flush(); $this->entityManager->clear(); // run the code - $this->worker->processEmails($this->createRunningTask(), $this->emails); + $this->worker->processEmails(ScheduledTask::getFromDoctrineEntity($this->createRunningTask()), $this->emails); // test it $statisticsRepository = $this->diContainer->get(StatisticsBouncesRepository::class); $statistics = $statisticsRepository->findAll(); @@ -187,22 +193,20 @@ class BounceTest extends \MailPoetTest { ); } - private function createScheduledTask() { - $task = ScheduledTask::create(); - $task->type = 'bounce'; - $task->status = ScheduledTask::STATUS_SCHEDULED; - $task->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); - $task->save(); - return $task; + private function createScheduledTask(): ScheduledTaskEntity { + return $this->scheduledTaskFactory->create( + 'bounce', + ScheduledTaskEntity::STATUS_SCHEDULED, + Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')) + ); } - private function createRunningTask() { - $task = ScheduledTask::create(); - $task->type = 'bounce'; - $task->status = null; - $task->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); - $task->save(); - return $task; + private function createRunningTask(): ScheduledTaskEntity { + return $this->scheduledTaskFactory->create( + 'bounce', + null, + Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')) + ); } private function createNewsletter(): NewsletterEntity { diff --git a/tests/integration/Cron/Workers/SendingQueue/MigrationTest.php b/tests/integration/Cron/Workers/SendingQueue/MigrationTest.php index b945a37ff7..ec5d276d73 100644 --- a/tests/integration/Cron/Workers/SendingQueue/MigrationTest.php +++ b/tests/integration/Cron/Workers/SendingQueue/MigrationTest.php @@ -4,6 +4,7 @@ namespace MailPoet\Test\Cron\Workers; use Codeception\Stub; use MailPoet\Cron\Workers\SendingQueue\Migration; +use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Mailer\MailerLog; use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTaskSubscriber; @@ -11,6 +12,7 @@ use MailPoet\Models\SendingQueue; use MailPoet\Models\Subscriber; use MailPoet\Settings\SettingsRepository; use MailPoet\Tasks\Sending as SendingTask; +use MailPoet\Test\DataFactories\ScheduledTask as ScheduledTaskFactory; use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Carbon\Carbon; use MailPoetVendor\Idiorm\ORM; @@ -23,6 +25,8 @@ class MigrationTest extends \MailPoetTest { public $queueRunning; public $subscriberProcessed; public $subscriberToProcess; + /** @var ScheduledTaskFactory */ + private $scheduledTaskFactory; /** @var Migration */ private $worker; @@ -48,6 +52,7 @@ class MigrationTest extends \MailPoetTest { $this->queueCompleted = $this->createSendingQueue(SendingQueue::STATUS_COMPLETED); $this->queueScheduled = $this->createSendingQueue(SendingQueue::STATUS_SCHEDULED); + $this->scheduledTaskFactory = new ScheduledTaskFactory(); $this->worker = new Migration(); } @@ -82,7 +87,7 @@ class MigrationTest extends \MailPoetTest { SendingQueue::deleteMany(); $task = $this->createScheduledTask(); $this->worker->prepareTaskStrategy($task, microtime(true)); - $task = ScheduledTask::findOne($task->id); + $task = ScheduledTask::findOne($task->getId()); assert($task instanceof ScheduledTask); expect($task->status)->equals(ScheduledTask::STATUS_COMPLETED); } @@ -137,12 +142,11 @@ class MigrationTest extends \MailPoetTest { } private function createScheduledTask() { - $task = ScheduledTask::create(); - $task->type = Migration::TASK_TYPE; - $task->status = ScheduledTask::STATUS_SCHEDULED; - $task->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); - $task->save(); - return $task; + return $this->scheduledTaskFactory->create( + Migration::TASK_TYPE, + ScheduledTaskEntity::STATUS_SCHEDULED, + Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')) + ); } private function createRunningTask() {