- Updates cron's sending queue worker to use model objects

- Adds new method to render newsletter to the newsletter model
- Adds new transient object to newsletter model that will hold temporary
  values (i.e., rendered body) when working with the model

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;
+  public $timer;
-  const BATCH_SIZE = 50;
 
   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(
+      $subscriber_batches =
-        $queue->subscribers['to_process'],
+        SubscribersTask::splitSubscribersIntoBatches($queue->subscribers['to_process']);
-        self::BATCH_SIZE
-      );
       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();
+          ->findMany();
-        $found_subscribers_ids = Helpers::arrayColumn($found_subscribers, 'id');
+        $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'])) {
-        if(!count($queue->subscribers['to_process'])) {
+            $this->updateQueue($queue);
-          $this->updateQueue($queue);
+            $this->newsletter_task->markNewsletterAsSent($newsletter);
-          continue;
+            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'],
+        'newsletter_id' => $newsletter->id,
-        'subscriber_id' => $subscriber['id'],
+        'subscriber_id' => $subscriber->id,
         'queue_id' => $queue->id
       );
       if($processing_method === 'individual') {

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) {
+  static function process($newsletter, $queue) {
-    list($newsletter, $links) = self::hashAndReplaceLinks($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);
   }
-}
+}
-

lib/Cron/Workers/SendingQueue/Tasks/Mailer.php

@@ -13,18 +13,18 @@ class Mailer {
     $this->mailer = $this->configureMailer();
   }
 
-  function configureMailer(array $newsletter = null) {
+  function configureMailer($newsletter = null) {
-    $sender['address'] = (!empty($newsletter['sender_address'])) ?
+    $sender['address'] = (!empty($newsletter->sender_address)) ?
-      $newsletter['sender_address'] :
+      $newsletter->sender_address :
       false;
-    $sender['name'] = (!empty($newsletter['sender_name'])) ?
+    $sender['name'] = (!empty($newsletter->sender_name)) ?
-      $newsletter['sender_name'] :
+      $newsletter->sender_name :
       false;
-    $reply_to['address'] = (!empty($newsletter['reply_to_address'])) ?
+    $reply_to['address'] = (!empty($newsletter->reply_to_address)) ?
-      $newsletter['reply_to_address'] :
+      $newsletter->reply_to_address :
       false;
-    $reply_to['name'] = (!empty($newsletter['reply_to_name'])) ?
+    $reply_to['name'] = (!empty($newsletter->reply_to_name)) ?
-      $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) {
+  function prepareSubscriberForSending($subscriber) {
-    return $this->mailer->transformSubscriber($subscriber);
+    return $this->mailer->formatSubscriberNameAndEmailAddress($subscriber);
   }
 
   function send($prepared_newsletters, $prepared_subscribers) {

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) {
+  function getAndPreProcess($queue) {
-    $newsletter = NewsletterModel::findOne($newsletter_id);
+    $newsletter = $queue->getNewsletter();
-    return ($newsletter) ? $newsletter->asArray() : false;
-  }
-
-  function getAndPreProcess(array $queue) {
-    $newsletter = $this->get($queue['newsletter_id']);
     if(!$newsletter) {
       return false;
     }
     // if the newsletter was previously rendered, return it
     // otherwise, process/render it
-    if(!is_null($queue['newsletter_rendered_body'])) {
+    if(!is_null($queue->newsletter_rendered_body)) {
-      $newsletter['rendered_body'] = json_decode($queue['newsletter_rendered_body'], true);
+      $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
+      // hook to the newsletter post-processing filter and add tracking image
-      if(!$this->tracking_image_inserted) {
+      $this->tracking_image_inserted = OpenTracking::addTrackingImage();
-        $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');
+    $newsletter_contains_posts = strpos($newsletter->_transient->rendered_body['html'], 'data-post-id');
-    if($newsletter['type'] === 'notification' && !$newsletter_contains_posts) {
+    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) {
+  function prepareNewsletterForSending($newsletter, $subscriber, $queue) {
-    $renderer = new Renderer($newsletter);
-    $newsletter['rendered_body'] = $renderer->render();
-    return $newsletter;
-  }
-
-  function prepareNewsletterForSending(
-    array $newsletter, array $subscriber, array $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->subject,
-        $newsletter['rendered_body']['html'],
+        $newsletter->_transient->rendered_body['html'],
-        $newsletter['rendered_body']['text']
+        $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'],
+        $subscriber->id,
-        $queue['id'],
+        $queue->id,
         $prepared_newsletter
       );
     }
@@ -103,8 +87,7 @@ class Newsletter {
     );
   }
 
-  function markNewsletterAsSent($newsletter_id) {
+  function markNewsletterAsSent($newsletter) {
-    $newsletter = NewsletterModel::findOne($newsletter_id);
     // if it's a standard newsletter, update its status
     if($newsletter->type === NewsletterModel::TYPE_STANDARD) {
       $newsletter->setStatus(NewsletterModel::STATUS_SENT);

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) {
+  static function extractAndSave($newsletter) {
-    if(empty($newsletter['rendered_body']['html']) || empty($newsletter['id'])) {
+    if(empty($newsletter->_transient->rendered_body['html']) || empty($newsletter->id)) {
-      return;
+      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_id = ($newsletter->type === NewsletterModel::TYPE_NOTIFICATION_HISTORY) ?
-      $newsletter['parent_id'] :
+      $newsletter->parent_id :
-      $newsletter['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;
   }
 }

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);
   }
-}
+}
-

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,
+    $found_subscribers_ids,
-    array $subscribers_to_process_ids,
+    $subscribers_to_process_ids,
-    array $queue_subscribers
+    $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(
+    $queue_subscribers['to_process'] = array_values(
-      $subscibers_to_exclude,
+      array_diff(
-      $queue_subscribers['to_process']
+        $queue_subscribers['to_process'],
+        $subscibers_to_exclude
+      )
     );
     return $queue_subscribers;
   }
 
-  static function updateFailedList(
+  static function updateFailedList($failed_subscribers, $queue_subscribers) {
-    array $failed_subscribers, array $queue_subscribers
-  ) {
     $queue_subscribers['failed'] = array_merge(
       $queue_subscribers['failed'],
       $failed_subscribers
     );
-    $queue_subscribers['to_process'] = array_diff(
+    $queue_subscribers['to_process'] = array_values(
-      $queue_subscribers['to_process'],
+      array_diff(
-      $failed_subscribers
+        $queue_subscribers['to_process'],
+        $failed_subscribers
+      )
     );
     return $queue_subscribers;
   }
 
-  static function updateProcessedList(
+  static function updateProcessedList($processed_subscribers, $queue_subscribers) {
-    array $processed_subscribers, array $queue_subscribers
-  ) {
     $queue_subscribers['processed'] = array_merge(
       $queue_subscribers['processed'],
       $processed_subscribers
     );
-    $queue_subscribers['to_process'] = array_diff(
+    $queue_subscribers['to_process'] = array_values(
-      $queue_subscribers['to_process'],
+      array_diff(
-      $processed_subscribers
+        $queue_subscribers['to_process'],
+        $processed_subscribers
+      )
     );
     return $queue_subscribers;
   }

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->sender = $this->getSenderNameAndAddress($sender);
-    $this->reply_to = $this->getReplyTo($reply_to);
+    $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['name'] = (!empty($reply_to['name'])) ?
-        $reply_to = array(
+        $reply_to['name'] :
-          'name' => $this->sender['from_name'],
+        $this->sender['from_name'];
-          'address' => $this->sender['from_email']
+      $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'] : '';

lib/Models/Newsletter.php

@@ -1,12 +1,13 @@
 <?php
 namespace MailPoet\Models;
+use MailPoet\Newsletter\Renderer\Renderer;
 use MailPoet\Util\Helpers;
 
 if(!defined('ABSPATH')) exit;
 
 class Newsletter extends Model {
   public static $_table = MP_NEWSLETTERS_TABLE;
-
+  public $_transient;
   const TYPE_STANDARD = 'standard';
   const TYPE_WELCOME = 'welcome';
   const TYPE_NOTIFICATION = 'notification';
@@ -22,7 +23,7 @@ class Newsletter extends Model {
 
   function __construct() {
     parent::__construct();
-
+    $this->_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(

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);
   }

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 <test@email.com>');
-    expect($mailer->transformSubscriber(
+    expect($mailer->formatSubscriberNameAndEmailAddress(
       array(
         'last_name' => 'Last',
         'email' => 'test@email.com'
       ))
     )->equals('Last <test@email.com>');
-    expect($mailer->transformSubscriber(
+    expect($mailer->formatSubscriberNameAndEmailAddress(
       array(
         'first_name' => 'First',
         'last_name' => 'Last',