diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index 55fdbc889a..076a897f51 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -9,15 +9,13 @@ use MailPoet\Mailer\MailerLog; use MailPoet\Models\SendingQueue as SendingQueueModel; use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel; use MailPoet\Models\Subscriber as SubscriberModel; -use MailPoet\Util\Helpers; if(!defined('ABSPATH')) exit; class SendingQueue { public $mailer_task; public $newsletter_task; - private $timer; - const BATCH_SIZE = 50; + public $timer; function __construct($timer = false) { $this->mailer_task = new MailerTask(); @@ -32,27 +30,29 @@ class SendingQueue { // abort if sending limit is reached MailerLog::enforceSendingLimit(); // get and pre-process newsletter (render, replace shortcodes/links, etc.) - $newsletter = $this->newsletter_task->getAndPreProcess($queue->asArray()); + $newsletter = $this->newsletter_task->getAndPreProcess($queue); if(!$newsletter) { $queue->delete(); continue; } - // configure mailer - $this->mailer_task->configureMailer($newsletter); if(is_null($queue->newsletter_rendered_body)) { - $queue->newsletter_rendered_body = json_encode($newsletter['rendered_body']); + $queue->newsletter_rendered_body = json_encode($newsletter->_transient->rendered_body); $queue->save(); } + // configure mailer + $this->mailer_task->configureMailer($newsletter); // get subscribers $queue->subscribers = $queue->getSubscribers(); - $subscriber_batches = array_chunk( - $queue->subscribers['to_process'], - self::BATCH_SIZE - ); + $subscriber_batches = + SubscribersTask::splitSubscribersIntoBatches($queue->subscribers['to_process']); foreach($subscriber_batches as $subscribers_to_process_ids) { + // abort if execution limit is reached + CronHelper::enforceExecutionLimit($this->timer); $found_subscribers = SubscriberModel::whereIn('id', $subscribers_to_process_ids) - ->findArray(); - $found_subscribers_ids = Helpers::arrayColumn($found_subscribers, 'id'); + ->findMany(); + $found_subscribers_ids = array_map(function($subscriber) { + return $subscriber->id; + }, $found_subscribers); // if some subscribers weren't found, remove them from the processing list if(count($found_subscribers_ids) !== count($subscribers_to_process_ids)) { $queue->subscribers = SubscribersTask::updateToProcessList( @@ -60,10 +60,11 @@ class SendingQueue { $subscribers_to_process_ids, $queue->subscribers ); - } - if(!count($queue->subscribers['to_process'])) { - $this->updateQueue($queue); - continue; + if(!count($queue->subscribers['to_process'])) { + $this->updateQueue($queue); + $this->newsletter_task->markNewsletterAsSent($newsletter); + continue; + } } $queue = $this->processQueue( $queue, @@ -71,10 +72,8 @@ class SendingQueue { $found_subscribers ); if($queue->status === SendingQueueModel::STATUS_COMPLETED) { - $this->newsletter_task->markNewsletterAsSent($queue->newsletter_id); + $this->newsletter_task->markNewsletterAsSent($newsletter); } - // abort if execution limit is reached - CronHelper::enforceExecutionLimit($this->timer); } } } @@ -92,7 +91,7 @@ class SendingQueue { $this->newsletter_task->prepareNewsletterForSending( $newsletter, $subscriber, - $queue->asArray() + $queue ); if(!$queue->newsletter_rendered_subject) { $queue->newsletter_rendered_subject = $prepared_newsletters[0]['subject']; @@ -101,11 +100,11 @@ class SendingQueue { $prepared_subscribers[] = $this->mailer_task->prepareSubscriberForSending( $subscriber ); - $prepared_subscribers_ids[] = $subscriber['id']; + $prepared_subscribers_ids[] = $subscriber->id; // keep track of values for statistics purposes $statistics[] = array( - 'newsletter_id' => $newsletter['id'], - 'subscriber_id' => $subscriber['id'], + 'newsletter_id' => $newsletter->id, + 'subscriber_id' => $subscriber->id, 'queue_id' => $queue->id ); if($processing_method === 'individual') { diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Links.php b/lib/Cron/Workers/SendingQueue/Tasks/Links.php index 8da6229264..7cd66ca334 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Links.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Links.php @@ -7,27 +7,28 @@ use MailPoet\Util\Helpers; if(!defined('ABSPATH')) exit; class Links { - static function process(array $newsletter, array $queue) { - list($newsletter, $links) = self::hashAndReplaceLinks($newsletter, $queue); + static function process($newsletter, $queue) { + list($rendered_body, $links) = + self::hashAndReplaceLinks($newsletter->_transient->rendered_body); self::saveLinks($links, $newsletter, $queue); + $newsletter->_transient->rendered_body = $rendered_body; return $newsletter; } - static function hashAndReplaceLinks(array $newsletter, array $queue) { + static function hashAndReplaceLinks($newsletter_rendered_body) { // join HTML and TEXT rendered body into a text string - $content = Helpers::joinObject($newsletter['rendered_body']); + $content = Helpers::joinObject($newsletter_rendered_body); list($content, $links) = NewsletterLinks::process($content); // split the processed body with hashed links back to HTML and TEXT - list($newsletter['rendered_body']['html'], $newsletter['rendered_body']['text']) + list($newsletter_rendered_body['html'], $newsletter_rendered_body['text']) = Helpers::splitObject($content); return array( - $newsletter, + $newsletter_rendered_body, $links ); } static function saveLinks($links, $newsletter, $queue) { - return NewsletterLinks::save($links, $newsletter['id'], $queue['id']); + return NewsletterLinks::save($links, $newsletter->id, $queue->id); } -} - +} \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php index c2a105166d..5882c48222 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php @@ -13,18 +13,18 @@ class Mailer { $this->mailer = $this->configureMailer(); } - function configureMailer(array $newsletter = null) { - $sender['address'] = (!empty($newsletter['sender_address'])) ? - $newsletter['sender_address'] : + function configureMailer($newsletter = null) { + $sender['address'] = (!empty($newsletter->sender_address)) ? + $newsletter->sender_address : false; - $sender['name'] = (!empty($newsletter['sender_name'])) ? - $newsletter['sender_name'] : + $sender['name'] = (!empty($newsletter->sender_name)) ? + $newsletter->sender_name : false; - $reply_to['address'] = (!empty($newsletter['reply_to_address'])) ? - $newsletter['reply_to_address'] : + $reply_to['address'] = (!empty($newsletter->reply_to_address)) ? + $newsletter->reply_to_address : false; - $reply_to['name'] = (!empty($newsletter['reply_to_name'])) ? - $newsletter['reply_to_name'] : + $reply_to['name'] = (!empty($newsletter->reply_to_name)) ? + $newsletter->reply_to_name : false; if(!$sender['address']) { $sender = false; @@ -50,8 +50,8 @@ class Mailer { 'individual'; } - function prepareSubscriberForSending(array $subscriber) { - return $this->mailer->transformSubscriber($subscriber); + function prepareSubscriberForSending($subscriber) { + return $this->mailer->formatSubscriberNameAndEmailAddress($subscriber); } function send($prepared_newsletters, $prepared_subscribers) { diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php index 7c38d6deae..7fa17dd919 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php @@ -19,42 +19,34 @@ class Newsletter { function __construct() { $this->tracking_enabled = (boolean)Setting::getValue('tracking.enabled'); - $this->tracking_image_inserted = false; } - function get($newsletter_id) { - $newsletter = NewsletterModel::findOne($newsletter_id); - return ($newsletter) ? $newsletter->asArray() : false; - } - - function getAndPreProcess(array $queue) { - $newsletter = $this->get($queue['newsletter_id']); + function getAndPreProcess($queue) { + $newsletter = $queue->getNewsletter(); if(!$newsletter) { return false; } // if the newsletter was previously rendered, return it // otherwise, process/render it - if(!is_null($queue['newsletter_rendered_body'])) { - $newsletter['rendered_body'] = json_decode($queue['newsletter_rendered_body'], true); + if(!is_null($queue->newsletter_rendered_body)) { + $newsletter->_transient->rendered_body = $queue->getRenderedNewsletterBody(); return $newsletter; } // if tracking is enabled, do additional processing if($this->tracking_enabled) { - // hook once to the newsletter post-processing filter and add tracking image - if(!$this->tracking_image_inserted) { - $this->tracking_image_inserted = OpenTracking::addTrackingImage(); - } + // hook to the newsletter post-processing filter and add tracking image + $this->tracking_image_inserted = OpenTracking::addTrackingImage(); // render newsletter - $newsletter = $this->render($newsletter); + $newsletter->_transient->rendered_body = $newsletter->render(); // hash and save all links $newsletter = LinksTask::process($newsletter, $queue); } else { // render newsletter - $newsletter = $this->render($newsletter); + $newsletter->_transient->rendered_body = $newsletter->render(); } // check if this is a post notification and if it contains posts - $newsletter_contains_posts = strpos($newsletter['rendered_body']['html'], 'data-post-id'); - if($newsletter['type'] === 'notification' && !$newsletter_contains_posts) { + $newsletter_contains_posts = strpos($newsletter->_transient->rendered_body['html'], 'data-post-id'); + if($newsletter->type === 'notification' && !$newsletter_contains_posts) { return false; } // extract and save newsletter posts @@ -62,22 +54,14 @@ class Newsletter { return $newsletter; } - function render(array $newsletter) { - $renderer = new Renderer($newsletter); - $newsletter['rendered_body'] = $renderer->render(); - return $newsletter; - } - - function prepareNewsletterForSending( - array $newsletter, array $subscriber, array $queue - ) { + function prepareNewsletterForSending($newsletter, $subscriber, $queue) { // shortcodes and links will be replaced in the subject, html and text body // to speed the processing, join content into a continuous string $prepared_newsletter = Helpers::joinObject( array( - $newsletter['subject'], - $newsletter['rendered_body']['html'], - $newsletter['rendered_body']['text'] + $newsletter->subject, + $newsletter->_transient->rendered_body['html'], + $newsletter->_transient->rendered_body['text'] ) ); $prepared_newsletter = ShortcodesTask::process( @@ -88,8 +72,8 @@ class Newsletter { ); if($this->tracking_enabled) { $prepared_newsletter = NewsletterLinks::replaceSubscriberData( - $subscriber['id'], - $queue['id'], + $subscriber->id, + $queue->id, $prepared_newsletter ); } @@ -103,8 +87,7 @@ class Newsletter { ); } - function markNewsletterAsSent($newsletter_id) { - $newsletter = NewsletterModel::findOne($newsletter_id); + function markNewsletterAsSent($newsletter) { // if it's a standard newsletter, update its status if($newsletter->type === NewsletterModel::TYPE_STANDARD) { $newsletter->setStatus(NewsletterModel::STATUS_SENT); diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Posts.php b/lib/Cron/Workers/SendingQueue/Tasks/Posts.php index 6cbd102c71..9f8c6419fc 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Posts.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Posts.php @@ -7,26 +7,27 @@ use MailPoet\Models\NewsletterPost; if(!defined('ABSPATH')) exit; class Posts { - static function extractAndSave(array $newsletter) { - if(empty($newsletter['rendered_body']['html']) || empty($newsletter['id'])) { - return; + static function extractAndSave($newsletter) { + if(empty($newsletter->_transient->rendered_body['html']) || empty($newsletter->id)) { + return false; } preg_match_all( '/data-post-id="(\d+)"/ism', - $newsletter['rendered_body']['html'], + $newsletter->_transient->rendered_body['html'], $matched_posts_ids); $matched_posts_ids = $matched_posts_ids[1]; if(!count($matched_posts_ids)) { - return $newsletter; + return false; } - $newsletter_id = ($newsletter['type'] === NewsletterModel::TYPE_NOTIFICATION_HISTORY) ? - $newsletter['parent_id'] : - $newsletter['id']; + $newsletter_id = ($newsletter->type === NewsletterModel::TYPE_NOTIFICATION_HISTORY) ? + $newsletter->parent_id : + $newsletter->id; foreach($matched_posts_ids as $post_id) { $newletter_post = NewsletterPost::create(); $newletter_post->newsletter_id = $newsletter_id; $newletter_post->post_id = $post_id; $newletter_post->save(); } + return true; } } \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php b/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php index 7871a3c114..e342de6d1c 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php @@ -6,9 +6,8 @@ use MailPoet\Newsletter\Shortcodes\Shortcodes as NewsletterShortcodes; if(!defined('ABSPATH')) exit; class Shortcodes { - static function process($content, array $newsletter, array $subscriber, array $queue) { + static function process($content, $newsletter, $subscriber, $queue) { $shortcodes = new NewsletterShortcodes($newsletter, $subscriber, $queue); return $shortcodes->replace($content); } -} - +} \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Subscribers.php b/lib/Cron/Workers/SendingQueue/Tasks/Subscribers.php index 76f010aa5e..c33fb30dae 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Subscribers.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Subscribers.php @@ -4,48 +4,59 @@ namespace MailPoet\Cron\Workers\SendingQueue\Tasks; if(!defined('ABSPATH')) exit; class Subscribers { + const BATCH_SIZE = 50; + + static function splitSubscribersIntoBatches(array $subscribers) { + return array_chunk( + $subscribers, + self::BATCH_SIZE + ); + } + static function updateToProcessList( - array $found_subscribers_ids, - array $subscribers_to_process_ids, - array $queue_subscribers + $found_subscribers_ids, + $subscribers_to_process_ids, + $queue_subscribers ) { - // compare existing subscriber to the ones that queued for processing + // compare existing subscribers to the ones that are queued for processing $subscibers_to_exclude = array_diff( $subscribers_to_process_ids, $found_subscribers_ids ); // remove nonexistent subscribers from the processing list - $queue_subscribers['to_process'] = array_diff( - $subscibers_to_exclude, - $queue_subscribers['to_process'] + $queue_subscribers['to_process'] = array_values( + array_diff( + $queue_subscribers['to_process'], + $subscibers_to_exclude + ) ); return $queue_subscribers; } - static function updateFailedList( - array $failed_subscribers, array $queue_subscribers - ) { + static function updateFailedList($failed_subscribers, $queue_subscribers) { $queue_subscribers['failed'] = array_merge( $queue_subscribers['failed'], $failed_subscribers ); - $queue_subscribers['to_process'] = array_diff( - $queue_subscribers['to_process'], - $failed_subscribers + $queue_subscribers['to_process'] = array_values( + array_diff( + $queue_subscribers['to_process'], + $failed_subscribers + ) ); return $queue_subscribers; } - static function updateProcessedList( - array $processed_subscribers, array $queue_subscribers - ) { + static function updateProcessedList($processed_subscribers, $queue_subscribers) { $queue_subscribers['processed'] = array_merge( $queue_subscribers['processed'], $processed_subscribers ); - $queue_subscribers['to_process'] = array_diff( - $queue_subscribers['to_process'], - $processed_subscribers + $queue_subscribers['to_process'] = array_values( + array_diff( + $queue_subscribers['to_process'], + $processed_subscribers + ) ); return $queue_subscribers; } diff --git a/lib/Mailer/Mailer.php b/lib/Mailer/Mailer.php index d66c4339c8..0a868d20de 100644 --- a/lib/Mailer/Mailer.php +++ b/lib/Mailer/Mailer.php @@ -24,13 +24,13 @@ class Mailer { function __construct($mailer = false, $sender = false, $reply_to = false) { $this->mailer_config = self::getMailerConfig($mailer); - $this->sender = $this->getSender($sender); - $this->reply_to = $this->getReplyTo($reply_to); + $this->sender = $this->getSenderNameAndAddress($sender); + $this->reply_to = $this->getReplyToNameAndAddress($reply_to); $this->mailer_instance = $this->buildMailer(); } function send($newsletter, $subscriber) { - $subscriber = $this->transformSubscriber($subscriber); + $subscriber = $this->formatSubscriberNameAndEmailAddress($subscriber); return $this->mailer_instance->send($newsletter, $subscriber); } @@ -115,7 +115,7 @@ class Mailer { return $mailer; } - function getSender($sender = false) { + function getSenderNameAndAddress($sender = false) { if(empty($sender)) { $sender = Setting::getValue('sender', array()); if(empty($sender['address'])) throw new \Exception(__('Sender name and email are not configured')); @@ -127,15 +127,15 @@ class Mailer { ); } - function getReplyTo($reply_to = false) { + function getReplyToNameAndAddress($reply_to = array()) { if(!$reply_to) { $reply_to = Setting::getValue('reply_to', null); - if(!$reply_to) { - $reply_to = array( - 'name' => $this->sender['from_name'], - 'address' => $this->sender['from_email'] - ); - } + $reply_to['name'] = (!empty($reply_to['name'])) ? + $reply_to['name'] : + $this->sender['from_name']; + $reply_to['address'] = (!empty($reply_to['address'])) ? + $reply_to['address'] : + $this->sender['from_email']; } if(empty($reply_to['address'])) { $reply_to['address'] = $this->sender['from_email']; @@ -147,7 +147,8 @@ class Mailer { ); } - function transformSubscriber($subscriber) { + function formatSubscriberNameAndEmailAddress($subscriber) { + $subscriber = (is_object($subscriber)) ? $subscriber->asArray() : $subscriber; if(!is_array($subscriber)) return $subscriber; if(isset($subscriber['address'])) $subscriber['email'] = $subscriber['address']; $first_name = (isset($subscriber['first_name'])) ? $subscriber['first_name'] : ''; diff --git a/lib/Models/Newsletter.php b/lib/Models/Newsletter.php index 70a0470d88..71405f2480 100644 --- a/lib/Models/Newsletter.php +++ b/lib/Models/Newsletter.php @@ -1,12 +1,13 @@ _transient = new \stdClass(); $this->addValidations('type', array( 'required' => __('Please specify a type') )); @@ -246,6 +247,11 @@ class Newsletter extends Model { return $this; } + function render() { + $renderer = new Renderer($this); + return $renderer->render(); + } + function getStatistics() { $statistics_query = SendingQueue::tableAlias('queues') ->selectExpr( diff --git a/lib/Models/SendingQueue.php b/lib/Models/SendingQueue.php index eab387d0f3..e3308820cb 100644 --- a/lib/Models/SendingQueue.php +++ b/lib/Models/SendingQueue.php @@ -59,6 +59,10 @@ class SendingQueue extends Model { return $subscribers; } + function getNewsletter() { + return Newsletter::findOne($this->newsletter_id); + } + function getRenderedNewsletterBody() { return json_decode($this->newsletter_rendered_body, true); } diff --git a/tests/unit/Mailer/MailerTest.php b/tests/unit/Mailer/MailerTest.php index eeee942bad..caabafd291 100644 --- a/tests/unit/Mailer/MailerTest.php +++ b/tests/unit/Mailer/MailerTest.php @@ -116,32 +116,32 @@ class MailerTest extends MailPoetTest { function testItSetsReplyToAddressWhenOnlyNameIsAvailable() { $reply_to = array('name' => 'test'); $mailer = new Mailer($this->mailer, $this->sender, $reply_to); - $reply_to = $mailer->getReplyTo(); + $reply_to = $mailer->getReplyToNameAndAddress(); expect($reply_to['reply_to_email'])->equals($this->sender['address']); } function testItCanTransformSubscriber() { $mailer = new Mailer($this->mailer, $this->sender, $this->reply_to); - expect($mailer->transformSubscriber('test@email.com')) + expect($mailer->formatSubscriberNameAndEmailAddress('test@email.com')) ->equals('test@email.com'); - expect($mailer->transformSubscriber( + expect($mailer->formatSubscriberNameAndEmailAddress( array( 'email' => 'test@email.com' )) )->equals('test@email.com'); - expect($mailer->transformSubscriber( + expect($mailer->formatSubscriberNameAndEmailAddress( array( 'first_name' => 'First', 'email' => 'test@email.com' )) )->equals('First '); - expect($mailer->transformSubscriber( + expect($mailer->formatSubscriberNameAndEmailAddress( array( 'last_name' => 'Last', 'email' => 'test@email.com' )) )->equals('Last '); - expect($mailer->transformSubscriber( + expect($mailer->formatSubscriberNameAndEmailAddress( array( 'first_name' => 'First', 'last_name' => 'Last',