From a12d0ff4bcb82db8d1330f6dee5d34bec2b1bc38 Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Wed, 30 Jan 2019 14:18:13 +0100 Subject: [PATCH] Refactor newly added cron workers to use new settings [MAILPOET-1757] --- .../Workers/StatsNotifications/Scheduler.php | 12 ++++++++-- .../Workers/StatsNotifications/Worker.php | 9 ++++++-- lib/Cron/Workers/WorkersFactory.php | 2 +- .../Workers/SendingQueue/SendingQueueTest.php | 4 ++-- .../StatsNotifications/SchedulerTest.php | 22 ++++++++++++------- .../Workers/StatsNotifications/WorkerTest.php | 10 +++++++-- 6 files changed, 42 insertions(+), 17 deletions(-) diff --git a/lib/Cron/Workers/StatsNotifications/Scheduler.php b/lib/Cron/Workers/StatsNotifications/Scheduler.php index d140a61ad3..f75ed05e10 100644 --- a/lib/Cron/Workers/StatsNotifications/Scheduler.php +++ b/lib/Cron/Workers/StatsNotifications/Scheduler.php @@ -7,6 +7,7 @@ use MailPoet\Models\Newsletter; use MailPoet\Models\ScheduledTask; use MailPoet\Models\Setting; use MailPoet\Models\StatsNotification; +use MailPoet\Settings\SettingsController; class Scheduler { @@ -16,6 +17,13 @@ class Scheduler { */ const HOURS_TO_SEND_AFTER_NEWSLETTER = 24; + /** @var SettingsController */ + private $settings; + + function __construct(SettingsController $settings) { + $this->settings = $settings; + } + function schedule(Newsletter $newsletter) { if(!$this->shouldSchedule($newsletter)) { return false; @@ -47,7 +55,7 @@ class Scheduler { } private function isDisabled() { - $settings = Setting::getValue(Worker::SETTINGS_KEY); + $settings = $this->settings->get(Worker::SETTINGS_KEY); if(!is_array($settings)) { return true; } @@ -60,7 +68,7 @@ class Scheduler { if(empty(trim($settings['address']))) { return true; } - if(!(bool)Setting::getValue('tracking.enabled')) { + if(!(bool)$this->settings->get('tracking.enabled')) { return true; } return !(bool)$settings['enabled']; diff --git a/lib/Cron/Workers/StatsNotifications/Worker.php b/lib/Cron/Workers/StatsNotifications/Worker.php index 3a216dfff7..f5c859f7e3 100644 --- a/lib/Cron/Workers/StatsNotifications/Worker.php +++ b/lib/Cron/Workers/StatsNotifications/Worker.php @@ -10,6 +10,7 @@ use MailPoet\Models\Newsletter; use MailPoet\Models\NewsletterLink; use MailPoet\Models\ScheduledTask; use MailPoet\Models\Setting; +use MailPoet\Settings\SettingsController; use MailPoet\Tasks\Sending; class Worker { @@ -28,15 +29,19 @@ class Worker { /** @var \MailPoet\Mailer\Mailer */ private $mailer; - function __construct(Mailer $mailer, Renderer $renderer, $timer = false) { + /** @var SettingsController */ + private $settings; + + function __construct(Mailer $mailer, Renderer $renderer, SettingsController $settings, $timer = false) { $this->timer = $timer ?: microtime(true); $this->renderer = $renderer; $this->mailer = $mailer; + $this->settings = $settings; } /** @throws \Exception */ function process() { - $settings = Setting::getValue(self::SETTINGS_KEY); + $settings = $this->settings->get(self::SETTINGS_KEY); $this->mailer->sender = $this->mailer->getSenderNameAndAddress($this->constructSenderEmail()); foreach(self::getDueTasks() as $task) { try { diff --git a/lib/Cron/Workers/WorkersFactory.php b/lib/Cron/Workers/WorkersFactory.php index f74e31308b..de6d6ecd5b 100644 --- a/lib/Cron/Workers/WorkersFactory.php +++ b/lib/Cron/Workers/WorkersFactory.php @@ -59,7 +59,7 @@ class WorkersFactory { } function createStatsNotificationsWorker($timer) { - return new StatsNotificationsWorker($this->mailer, $this->renderer, $timer); + return new StatsNotificationsWorker($this->mailer, $this->renderer, $this->settings, $timer); } /** @return SendingServiceKeyCheckWorker */ diff --git a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php b/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php index a8a7facfa7..53b5592642 100644 --- a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php +++ b/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php @@ -83,9 +83,9 @@ class SendingQueueTest extends \MailPoetTest { $this->newsletter_link->hash = 'abcde'; $this->newsletter_link->save(); $this->sending_error_handler = new SendingErrorHandler(); - $this->stats_notifications_worker = new StatsNotificationsScheduler(); - $this->sending_queue_worker = new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker); $this->settings = new SettingsController(); + $this->stats_notifications_worker = new StatsNotificationsScheduler($this->settings); + $this->sending_queue_worker = new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker); } private function getDirectUnsubscribeURL() { diff --git a/tests/integration/Cron/Workers/StatsNotifications/SchedulerTest.php b/tests/integration/Cron/Workers/StatsNotifications/SchedulerTest.php index a4fc1af259..52d4394523 100644 --- a/tests/integration/Cron/Workers/StatsNotifications/SchedulerTest.php +++ b/tests/integration/Cron/Workers/StatsNotifications/SchedulerTest.php @@ -6,19 +6,25 @@ use MailPoet\Models\Newsletter; use MailPoet\Models\ScheduledTask; use MailPoet\Models\Setting; use MailPoet\Models\StatsNotification; +use MailPoet\Settings\SettingsController; class SchedulerTest extends \MailPoetTest { /** @var Scheduler */ private $stats_notifications; + /** @var SettingsController */ + private $settings; + function _before() { - $this->stats_notifications = new Scheduler(); - Setting::setValue(Worker::SETTINGS_KEY, [ + parent::_before(); + $this->settings = new SettingsController(); + $this->stats_notifications = new Scheduler($this->settings); + $this->settings->set(Worker::SETTINGS_KEY, [ 'enabled' => true, 'address' => 'email@example.com' ]); - Setting::setValue('tracking.enabled', true); + $this->settings->set('tracking.enabled', true); } function testShouldSchedule() { @@ -32,7 +38,7 @@ class SchedulerTest extends \MailPoetTest { } function testShouldNotScheduleIfTrackingIsDisabled() { - Setting::setValue('tracking.enabled', false); + $this->settings->set('tracking.enabled', false); $newsletter_id = 13; $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); $this->stats_notifications->schedule($newsletter); @@ -42,7 +48,7 @@ class SchedulerTest extends \MailPoetTest { function testShouldNotScheduleIfDisabled() { $newsletter_id = 6; - Setting::setValue(Worker::SETTINGS_KEY, [ + $this->settings->set(Worker::SETTINGS_KEY, [ 'enabled' => false, 'address' => 'email@example.com' ]); @@ -54,7 +60,7 @@ class SchedulerTest extends \MailPoetTest { function testShouldNotScheduleIfSettingsMissing() { $newsletter_id = 7; - Setting::setValue(Worker::SETTINGS_KEY, []); + $this->settings->set(Worker::SETTINGS_KEY, []); $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); $this->stats_notifications->schedule($newsletter); $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); @@ -63,7 +69,7 @@ class SchedulerTest extends \MailPoetTest { function testShouldNotScheduleIfEmailIsMissing() { $newsletter_id = 8; - Setting::setValue(Worker::SETTINGS_KEY, [ + $this->settings->set(Worker::SETTINGS_KEY, [ 'enabled' => true, ]); $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); @@ -74,7 +80,7 @@ class SchedulerTest extends \MailPoetTest { function testShouldNotScheduleIfEmailIsEmpty() { $newsletter_id = 9; - Setting::setValue(Worker::SETTINGS_KEY, [ + $this->settings->set(Worker::SETTINGS_KEY, [ 'enabled' => true, 'address' => ' ' ]); diff --git a/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php b/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php index 7adfb0f3ff..efb5c16319 100644 --- a/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php +++ b/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php @@ -13,6 +13,7 @@ use MailPoet\Models\StatisticsClicks; use MailPoet\Models\StatisticsOpens; use MailPoet\Models\StatisticsUnsubscribes; use MailPoet\Models\StatsNotification; +use MailPoet\Settings\SettingsController; use PHPUnit\Framework\MockObject\MockObject; class WorkerTest extends \MailPoetTest { @@ -26,15 +27,20 @@ class WorkerTest extends \MailPoetTest { /** @var MockObject */ private $renderer; + /** @var SettingsController */ + private $settings; + function _before() { + parent::_before(); \ORM::raw_execute('TRUNCATE ' . Newsletter::$_table); \ORM::raw_execute('TRUNCATE ' . ScheduledTask::$_table); \ORM::raw_execute('TRUNCATE ' . SendingQueue::$_table); \ORM::raw_execute('TRUNCATE ' . StatsNotification::$_table); $this->mailer = $this->createMock(Mailer::class); $this->renderer = $this->createMock(Renderer::class); - $this->stats_notifications = new Worker($this->mailer, $this->renderer); - Setting::setValue(Worker::SETTINGS_KEY, [ + $this->settings = new SettingsController(); + $this->stats_notifications = new Worker($this->mailer, $this->renderer, $this->settings); + $this->settings->set(Worker::SETTINGS_KEY, [ 'enabled' => true, 'address' => 'email@example.com' ]);