From 6daecd64668120d82b807e43eaf321b259a9c08b Mon Sep 17 00:00:00 2001 From: Vlad Date: Thu, 16 Jun 2016 15:14:10 -0400 Subject: [PATCH 01/12] - Fixes URL extraction (undefined index notice) - Updates link replacement in text body - Updates links saving logic --- lib/Newsletter/Links/Links.php | 14 ++++++++------ lib/Util/Helpers.php | 9 +++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/Newsletter/Links/Links.php b/lib/Newsletter/Links/Links.php index ea72c569c8..b2a548ad93 100644 --- a/lib/Newsletter/Links/Links.php +++ b/lib/Newsletter/Links/Links.php @@ -3,6 +3,7 @@ namespace MailPoet\Newsletter\Links; use MailPoet\Models\NewsletterLink; use MailPoet\Newsletter\Shortcodes\Shortcodes; +use MailPoet\Util\Helpers; use MailPoet\Util\Security; class Links { @@ -39,16 +40,16 @@ class Links { }, $shortcodes); // extract urls with href="url" format preg_match_all($regex, $content, $matched_urls); - $matched_urls_count = count($matched_urls[1]); + $matched_urls_count = count($matched_urls[0]); if($matched_urls_count) { - for($index = 0; $index <= $matched_urls_count; $index++) { + for($index = 0; $index < $matched_urls_count; $index++) { $extracted_links[] = array( 'html' => $matched_urls[0][$index], 'link' => $matched_urls[2][$index] ); } } - return $extracted_links; + return Helpers::arrayUnique($extracted_links); } static function process($content) { @@ -79,9 +80,9 @@ class Links { // third, replace text version URL with tracked link: [description](url) // regex is used to avoid replacing description URLs that are wrapped in round brackets // i.e., (http://google.com) => [(http://google.com)](http://tracked_link) - $regex_escaped_tracked_link = preg_quote($tracked_link, '/'); + $regex_escaped_extracted_link = preg_quote($extracted_link['link'], '/'); $content = preg_replace( - '/(\[' . $regex_escaped_tracked_link . '\])(\(' . $regex_escaped_tracked_link . '\))/', + '/\[(' . $regex_escaped_extracted_link . ')\](\(' . $regex_escaped_extracted_link . '\))/', '[$1](' . $tracked_link . ')', $content ); @@ -117,8 +118,9 @@ class Links { return $content; } - static function save($links, $newsletter_id, $queue_id) { + static function save(array $links, $newsletter_id, $queue_id) { foreach($links as $link) { + if(empty($link['hash'] || empty($link['url']))) continue; $newsletter_link = NewsletterLink::create(); $newsletter_link->newsletter_id = $newsletter_id; $newsletter_link->queue_id = $queue_id; diff --git a/lib/Util/Helpers.php b/lib/Util/Helpers.php index 5ae0770275..1d68e54767 100644 --- a/lib/Util/Helpers.php +++ b/lib/Util/Helpers.php @@ -113,4 +113,13 @@ class Helpers { $func = create_function('$c', 'return "_" . strtolower($c[1]);'); return preg_replace_callback('/([A-Z])/', $func, $str); } + + static function arrayUnique($arr) { + return array_map( + 'unserialize', + array_unique( + array_map('serialize', $arr) + ) + ); + } } \ No newline at end of file From 999a0b3edec7813a4b70a36b069ee7a861dcefe5 Mon Sep 17 00:00:00 2001 From: Vlad Date: Fri, 17 Jun 2016 07:27:06 -0400 Subject: [PATCH 02/12] - Refactors sending queue worker by breaking it into smaller tasks - Adds arrayUnique method to Helpers for multidimensional arrays --- lib/Cron/Workers/SendingQueue.php | 407 ------------------ .../Workers/SendingQueue/SendingQueue.php | 181 ++++++++ lib/Cron/Workers/SendingQueue/Tasks/Links.php | 33 ++ .../Workers/SendingQueue/Tasks/Mailer.php | 95 ++++ .../Workers/SendingQueue/Tasks/Newsletter.php | 96 +++++ lib/Cron/Workers/SendingQueue/Tasks/Posts.php | 29 ++ .../Workers/SendingQueue/Tasks/Shortcodes.php | 14 + .../Workers/SendingQueue/Tasks/Statistics.php | 30 ++ .../SendingQueue/Tasks/Subscribers.php | 41 ++ lib/Models/StatisticsNewsletters.php | 3 + .../Renderer/PostProcess/OpenTracking.php | 10 +- lib/Newsletter/Shortcodes/Shortcodes.php | 24 +- lib/Util/Helpers.php | 10 + 13 files changed, 556 insertions(+), 417 deletions(-) delete mode 100644 lib/Cron/Workers/SendingQueue.php create mode 100644 lib/Cron/Workers/SendingQueue/SendingQueue.php create mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Links.php create mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Mailer.php create mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php create mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Posts.php create mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php create mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Statistics.php create mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Subscribers.php diff --git a/lib/Cron/Workers/SendingQueue.php b/lib/Cron/Workers/SendingQueue.php deleted file mode 100644 index 2125be99cb..0000000000 --- a/lib/Cron/Workers/SendingQueue.php +++ /dev/null @@ -1,407 +0,0 @@ -mta_config = $this->getMailerConfig(); - $this->mta_log = $this->getMailerLog(); - $this->processing_method = ($this->mta_config['method'] === 'MailPoet') ? - 'processBulkSubscribers' : - 'processIndividualSubscriber'; - $this->timer = ($timer) ? $timer : microtime(true); - CronHelper::checkExecutionTimer($this->timer); - } - - function process() { - foreach($this->getQueues() as $queue) { - $newsletter = Newsletter::findOne($queue->newsletter_id); - if(!$newsletter) { - $queue->delete(); - continue; - } - $newsletter = $newsletter->asArray(); - $newsletter['body'] = $this->getOrRenderNewsletterBody($queue, $newsletter); - if($newsletter['type'] === 'notification' && - strpos($newsletter['body']['html'], 'data-post-id') === false - ){ - $queue->delete(); - continue; - } - $queue->subscribers = (object) unserialize($queue->subscribers); - if(!isset($queue->subscribers->processed)) { - $queue->subscribers->processed = array(); - } - if(!isset($queue->subscribers->failed)) { - $queue->subscribers->failed = array(); - } - $mailer = $this->configureMailer($newsletter); - foreach(array_chunk($queue->subscribers->to_process, self::BATCH_SIZE) as - $subscribers_ids) { - $subscribers = Subscriber::whereIn('id', $subscribers_ids) - ->findArray(); - if(count($subscribers_ids) !== count($subscribers)) { - $queue->subscribers->to_process = $this->recalculateSubscriberCount( - Helpers::arrayColumn($subscribers, 'id'), - $subscribers_ids, - $queue->subscribers->to_process - ); - } - if(!count($queue->subscribers->to_process)) { - $this->updateQueue($queue); - continue; - } - $queue->subscribers = call_user_func_array( - array( - $this, - $this->processing_method - ), - array( - $mailer, - $newsletter, - $subscribers, - $queue - ) - ); - } - } - } - - function getOrRenderNewsletterBody($queue, $newsletter) { - // check if newsletter has been rendered, in which case return its contents - // or render and save for future reuse - if($queue->newsletter_rendered_body === null) { - if((boolean) Setting::getValue('tracking.enabled')) { - // insert tracking code - add_filter('mailpoet_rendering_post_process', function($template) { - return OpenTracking::process($template); - }); - // render newsletter - $rendered_newsletter = $this->renderNewsletter($newsletter); - // process link shortcodes, extract and save links in the database - $processed_newsletter = $this->processLinksAndShortcodes( - $this->joinObject($rendered_newsletter), - $newsletter['id'], - $queue->id - ); - list($newsletter['body']['html'], $newsletter['body']['text']) = - $this->splitObject($processed_newsletter); - } - else { - // render newsletter - $newsletter['body'] = $this->renderNewsletter($newsletter); - } - $this->extractAndSaveNewsletterPosts( - $newsletter['id'], - $newsletter['body']['html'] - ); - $queue->newsletter_rendered_body = json_encode($newsletter['body']); - $queue->save(); - } else { - $newsletter['body'] = json_decode($queue->newsletter_rendered_body); - } - return (array) $newsletter['body']; - } - - function processBulkSubscribers($mailer, $newsletter, $subscribers, $queue) { - foreach($subscribers as $subscriber) { - $processed_newsletters[] = - $this->processNewsletterBeforeSending($newsletter, $subscriber, $queue); - if(!$queue->newsletter_rendered_subject) { - $queue->newsletter_rendered_subject = $processed_newsletters[0]['subject']; - } - $transformed_subscribers[] = - $mailer->transformSubscriber($subscriber); - } - $result = $this->sendNewsletter( - $mailer, - $processed_newsletters, - $transformed_subscribers - ); - $subscribers_ids = Helpers::arrayColumn($subscribers, 'id'); - if(!$result) { - $queue->subscribers->failed = array_merge( - $queue->subscribers->failed, - $subscribers_ids - ); - } else { - $newsletter_statistics = - array_map(function($data) use ($newsletter, $subscribers_ids, $queue) { - return array( - $newsletter['id'], - $subscribers_ids[$data], - $queue->id - ); - }, range(0, count($transformed_subscribers) - 1)); - $newsletter_statistics = Helpers::flattenArray($newsletter_statistics); - $this->updateMailerLog(); - $this->updateNewsletterStatistics($newsletter_statistics); - $queue->subscribers->processed = array_merge( - $queue->subscribers->processed, - $subscribers_ids - ); - } - $this->updateQueue($queue); - $this->checkSendingLimit(); - CronHelper::checkExecutionTimer($this->timer); - return $queue->subscribers; - } - - function processIndividualSubscriber($mailer, $newsletter, $subscribers, $queue) { - foreach($subscribers as $subscriber) { - $this->checkSendingLimit(); - $processed_newsletter = $this->processNewsletterBeforeSending($newsletter, $subscriber, $queue); - if(!$queue->newsletter_rendered_subject) { - $queue->newsletter_rendered_subject = $processed_newsletter['subject']; - } - $transformed_subscriber = $mailer->transformSubscriber($subscriber); - $result = $this->sendNewsletter( - $mailer, - $processed_newsletter, - $transformed_subscriber - ); - if(!$result) { - $queue->subscribers->failed[] = $subscriber['id']; - } else { - $queue->subscribers->processed[] = $subscriber['id']; - $newsletter_statistics = array( - $newsletter['id'], - $subscriber['id'], - $queue->id - ); - $this->updateMailerLog(); - $this->updateNewsletterStatistics($newsletter_statistics); - } - $this->updateQueue($queue); - CronHelper::checkExecutionTimer($this->timer); - } - return $queue->subscribers; - } - - function updateNewsletterStatistics($data) { - return StatisticsNewsletters::createMultiple($data); - } - - function renderNewsletter($newsletter) { - $renderer = new Renderer($newsletter); - return $renderer->render(); - } - - function processLinksAndShortcodes($content, $newsletter_id, $queue_id) { - // process only link shortcodes - $shortcodes = new Shortcodes($newsletter = false, $subscriber = false, $queue_id); - $content = $shortcodes->replace( - $content, - $categories = array('link') - ); - // extract and save links and link shortcodes - list($content, $processed_links) = - Links::process( - $content, - $links = false, - $process_link_shortcodes = true, - $queue_id - ); - Links::save($processed_links, $newsletter_id, $queue_id); - return $content; - } - - function processNewsletterBeforeSending($newsletter, $subscriber = false, $queue) { - $data_for_shortcodes = array( - $newsletter['subject'], - $newsletter['body']['html'], - $newsletter['body']['text'] - ); - $processed_newsletter = $this->replaceShortcodes( - $newsletter, - $subscriber, - $queue, - $this->joinObject($data_for_shortcodes) - ); - if((boolean) Setting::getValue('tracking.enabled')) { - $processed_newsletter = Links::replaceSubscriberData( - $newsletter['id'], - $subscriber['id'], - $queue->id, - $processed_newsletter - ); - } - list($newsletter['subject'], - $newsletter['body']['html'], - $newsletter['body']['text'] - ) = $this->splitObject($processed_newsletter); - return $newsletter; - } - - function replaceShortcodes($newsletter, $subscriber, $queue, $body) { - $shortcodes = new Shortcodes( - $newsletter, - $subscriber, - $queue - ); - return $shortcodes->replace($body); - } - - function sendNewsletter($mailer, $newsletter, $subscriber) { - return $mailer->mailer_instance->send( - $newsletter, - $subscriber - ); - } - - function configureMailer($newsletter) { - $sender['address'] = (!empty($newsletter['sender_address'])) ? - $newsletter['sender_address'] : - false; - $sender['name'] = (!empty($newsletter['sender_name'])) ? - $newsletter['sender_name'] : - false; - $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'] : - false; - if(!$sender['address']) { - $sender = false; - } - if(!$reply_to['address']) { - $reply_to = false; - } - $mailer = new Mailer($method = false, $sender, $reply_to); - return $mailer; - } - - function getQueues() { - return SendingQueueModel::orderByDesc('priority') - ->whereNull('deleted_at') - ->whereNull('status') - ->findResultSet(); - } - - function updateQueue($queue) { - $queue = clone($queue); - $queue->subscribers->to_process = array_diff( - $queue->subscribers->to_process, - array_merge( - $queue->subscribers->processed, - $queue->subscribers->failed - ) - ); - $queue->subscribers->to_process = array_values( - $queue->subscribers->to_process - ); - $queue->count_processed = - count($queue->subscribers->processed) + count($queue->subscribers->failed); - $queue->count_to_process = count($queue->subscribers->to_process); - $queue->count_failed = count($queue->subscribers->failed); - $queue->count_total = - $queue->count_processed + $queue->count_to_process; - if(!$queue->count_to_process) { - $queue->processed_at = current_time('mysql'); - $queue->status = SendingQueueModel::STATUS_COMPLETED; - - // set newsletter status to sent - $newsletter = Newsletter::findOne($queue->newsletter_id); - // if it's a standard newsletter, update its status - if($newsletter->type === Newsletter::TYPE_STANDARD) { - $newsletter->setStatus(Newsletter::STATUS_SENT); - } else if($newsletter->type === Newsletter::TYPE_NOTIFICATION) { - // TODO: Check with Vlad - } - } - $queue->subscribers = serialize((array) $queue->subscribers); - $queue->save(); - } - - function updateMailerLog() { - $this->mta_log['sent']++; - return Setting::setValue('mta_log', $this->mta_log); - } - - function getMailerConfig() { - $mta_config = Setting::getValue('mta'); - if(!$mta_config) { - throw new \Exception(__('Mailer is not configured.')); - } - return $mta_config; - } - - function getMailerLog() { - $mta_log = Setting::getValue('mta_log'); - if(!$mta_log) { - $mta_log = array( - 'sent' => 0, - 'started' => time() - ); - Setting::setValue('mta_log', $mta_log); - } - return $mta_log; - } - - function checkSendingLimit() { - $frequency_interval = (int)$this->mta_config['frequency']['interval'] * 60; - $frequency_limit = (int)$this->mta_config['frequency']['emails']; - $elapsed_time = time() - (int)$this->mta_log['started']; - if($this->mta_log['sent'] === $frequency_limit && - $elapsed_time <= $frequency_interval - ) { - throw new \Exception(__('Sending frequency limit has been reached.')); - } - if($elapsed_time > $frequency_interval) { - $this->mta_log = array( - 'sent' => 0, - 'started' => time() - ); - Setting::setValue('mta_log', $this->mta_log); - } - return; - } - - function recalculateSubscriberCount( - $found_subscriber, $existing_subscribers, $subscribers_to_process) { - $subscibers_to_exclude = array_diff($existing_subscribers, $found_subscriber); - return array_diff($subscribers_to_process, $subscibers_to_exclude); - } - - function extractAndSaveNewsletterPosts($newletter_id, $content) { - preg_match_all('/data-post-id="(\d+)"/ism', $content, $posts); - $posts = $posts[1]; - foreach($posts as $post) { - $newletter_post = NewsletterPost::create(); - $newletter_post->newsletter_id = $newletter_id; - $newletter_post->post_id = $post; - $newletter_post->save(); - } - } - - private function joinObject($object = array()) { - return implode(self::DIVIDER, $object); - } - - private function splitObject($object = array()) { - return explode(self::DIVIDER, $object); - } -} \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php new file mode 100644 index 0000000000..c4a63f08b4 --- /dev/null +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -0,0 +1,181 @@ +mta_config = MailerTask::getMailerConfig(); + $this->mta_log = MailerTask::getMailerLog(); + $this->timer = ($timer) ? $timer : microtime(true); + CronHelper::checkExecutionTimer($this->timer); + } + + function process() { + foreach($this->getQueues() as $queue) { + $newsletter = NewsletterTask::getAndPreProcess($queue->asArray()); + if(!$newsletter) { + $queue->delete(); + continue; + } + if(is_null($queue->newsletter_rendered_body)) { + $queue->newsletter_rendered_body = json_encode($newsletter['rendered_body']); + $queue->save(); + } + $queue->subscribers = SubscribersTask::get($queue->asArray()); + // configure mailer with newsletter data (from/reply-to) + $mailer = MailerTask::configureMailer($newsletter); + $processing_method = MailerTask::getProcessingMethod($this->mta_config); + foreach(array_chunk($queue->subscribers['to_process'], self::BATCH_SIZE) as + $subscribers_to_process_ids) { + $subscribers = Subscriber::whereIn('id', $subscribers_to_process_ids) + ->findArray(); + // if some subscribers weren't found, remove them from the processing list + if(count($subscribers) !== count($subscribers_to_process_ids)) { + $queue->subscribers['to_process'] = Subscribers::updateCount( + Helpers::arrayColumn($subscribers, 'id'), + $subscribers_to_process_ids, + $queue->subscribers['to_process'] + ); + } + if(!count($queue->subscribers['to_process'])) { + $this->updateQueue($queue); + continue; + } + $queue->subscribers = call_user_func_array( + array($this, $processing_method), + array($mailer, $newsletter, $subscribers, $queue) + ); + } + } + } + + function processBulkSubscribers($mailer, $newsletter, $subscribers, $queue) { + $subscriber_log = array(); + $subscribers_ids = Helpers::arrayColumn($subscribers, 'id'); + foreach($subscribers as $subscriber) { + // render shortcodes and replace subscriber data in tracked links + $prepared_newsletters[] = + NewsletterTask::prepareNewsletterForSending( + $newsletter, + $subscriber, + $queue->asArray() + ); + if(!$queue->newsletter_rendered_subject) { + $queue->newsletter_rendered_subject = $prepared_newsletters[0]['subject']; + } + // format subscriber name/address according to mailer settings + $prepared_subscribers[] = MailerTask::prepareSubscriberForSending( + $mailer, + $subscriber + ); + } + // send + $result = MailerTask::send($mailer, $prepared_newsletters, $prepared_subscribers); + if(!$result) { + // record failed subscribers + $subscriber_log['failed'] = SubscribersTask::updateFailedList( + $queue->subscribers['failed'], + $subscribers_ids + ); + } else { + StatisticsTask::updateBulkNewsletterStatistics( + $subscribers_ids, + $newsletter['id'], + $queue->id + ); + MailerTask::updateMailerLog($this->mta_log); + $subscriber_log['processed'] = array_merge( + $queue->subscribers['processed'], + $subscribers_ids + ); + } + // TODO + //$queue = $this->updateQueue($queue, $subscriber_log); + MailerTask::checkSendingLimit($this->mta_config, $this->mta_log); + CronHelper::checkExecutionTimer($this->timer); + return $queue->subscribers; + } + + /* function processIndividualSubscriber($mailer, $newsletter, $subscribers, $queue) { + foreach($subscribers as $subscriber) { + $this->checkSendingLimit(); + $processed_newsletter = $this->prepareNewsletterForSending($newsletter, $subscriber, $queue); + if(!$queue->newsletter_rendered_subject) { + $queue->newsletter_rendered_subject = $processed_newsletter['subject']; + } + $transformed_subscriber = $mailer->transformSubscriber($subscriber); + $result = $this->sendNewsletter( + $mailer, + $processed_newsletter, + $transformed_subscriber + ); + if(!$result) { + $queue->subscribers['failed'][] = $subscriber['id']; + } else { + $queue->subscribers['processed'][] = $subscriber['id']; + $newsletter_statistics = array( + $newsletter['id'], + $subscriber['id'], + $queue->id + ); + $this->updateMailerLog(); + $this->updateNewsletterStatistics($newsletter_statistics); + } + $this->updateQueue($queue); + CronHelper::checkExecutionTimer($this->timer); + } + return $queue->subscribers; + }*/ + + function getQueues() { + return SendingQueueModel::orderByDesc('priority') + ->whereNull('deleted_at') + ->whereNull('status') + ->findResultSet(); + } + + function updateQueue($queue) { + // TODO + return; + $queue = clone($queue); + $queue->subscribers['to_process'] = array_diff( + $queue->subscribers['to_process'], + array_merge( + $queue->subscribers['processed'], + $queue->subscribers['failed'] + ) + ); + $queue->subscribers['to_process'] = array_values( + $queue->subscribers['to_process'] + ); + $queue->count_processed = + count($queue->subscribers['processed']) + count($queue->subscribers['failed']); + $queue->count_to_process = count($queue->subscribers['to_process']); + $queue->count_failed = count($queue->subscribers['failed']); + $queue->count_total = + $queue->count_processed + $queue->count_to_process; + if(!$queue->count_to_process) { + $queue->processed_at = current_time('mysql'); + $queue->status = self::STATUS_COMPLETED; + } + $queue->subscribers = serialize((array) $queue->subscribers); + $queue->save(); + } +} \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Links.php b/lib/Cron/Workers/SendingQueue/Tasks/Links.php new file mode 100644 index 0000000000..8da6229264 --- /dev/null +++ b/lib/Cron/Workers/SendingQueue/Tasks/Links.php @@ -0,0 +1,33 @@ + 0, + 'started' => time() + ); + Setting::setValue('mta_log', $mta_log); + } + return $mta_log; + } + + static function getProcessingMethod($mta_config) { + return ($mta_config['method'] === 'MailPoet') ? + 'processBulkSubscribers' : + 'processIndividualSubscriber'; + } + + static function prepareSubscriberForSending($mailer, $subscriber) { + return ($mailer instanceof \MailPoet\Mailer\Mailer) ? + $mailer->transformSubscriber($subscriber) : + false; + } + + static function send($mailer, $newsletter, $subscriber) { + return ($mailer instanceof \MailPoet\Mailer\Mailer) ? + $mailer->mailer_instance->send($newsletter, $subscriber) : + false; + } + + static function checkSendingLimit($mta_config, $mta_log) { + $frequency_interval = (int) $mta_config['frequency']['interval'] * 60; + $frequency_limit = (int) $mta_config['frequency']['emails']; + $elapsed_time = time() - (int) $mta_log['started']; + if($mta_log['sent'] === $frequency_limit && + $elapsed_time <= $frequency_interval + ) { + throw new \Exception(__('Sending frequency limit has been reached.')); + } + if($elapsed_time > $frequency_interval) { + $mta_log = array( + 'sent' => 0, + 'started' => time() + ); + Setting::setValue('mta_log', $mta_log); + } + return; + } +} \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php new file mode 100644 index 0000000000..92e827bad3 --- /dev/null +++ b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php @@ -0,0 +1,96 @@ +asArray() : false; + } + + static function getAndPreProcess(array $queue) { + $newsletter = self::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'])) { + $newsletter['rendered_body'] = json_decode($queue['newsletter_rendered_body'], true); + return $newsletter; + } + // if tracking is enabled, do additional processing + if((boolean) Setting::getValue('tracking.enabled')) { + // add tracking image + OpenTracking::addTrackingImage(); + // render newsletter + $newsletter = self::render($newsletter); + // hash and save all links + $newsletter = LinksTask::process($newsletter, $queue); + } else { + // render newsletter + $newsletter = self::render($newsletter); + } + // 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) { + return false; + } + // save all posts + $newsletter = PostsTask::extractAndSave($newsletter); + return $newsletter; + } + + static function render($newsletter) { + $renderer = new Renderer($newsletter); + $newsletter['rendered_body'] = $renderer->render(); + return $newsletter; + } + + static 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['rendered_body']['html'], + $newsletter['rendered_body']['text'] + ) + ); + $prepared_newsletter = ShortcodesTask::process( + $prepared_newsletter, + $newsletter, + $subscriber, + $queue + ); + if((boolean) Setting::getValue('tracking.enabled')) { + $prepared_newsletter = NewsletterLinks::replaceSubscriberData( + $newsletter['id'], + $subscriber['id'], + $queue['id'], + $prepared_newsletter + ); + } + $prepared_newsletter = Helpers::splitObject($prepared_newsletter); + return array( + 'subject' => $prepared_newsletter[0], + 'body' => array( + 'html' => $prepared_newsletter[1], + 'text' => $prepared_newsletter[2] + ) + ); + } +} \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Posts.php b/lib/Cron/Workers/SendingQueue/Tasks/Posts.php new file mode 100644 index 0000000000..c5bd23cee6 --- /dev/null +++ b/lib/Cron/Workers/SendingQueue/Tasks/Posts.php @@ -0,0 +1,29 @@ +newsletter_id = $newsletter['id']; + $newletter_post->post_id = $post; + $newletter_post->save(); + } + return $newsletter; + } +} \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php b/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php new file mode 100644 index 0000000000..7871a3c114 --- /dev/null +++ b/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php @@ -0,0 +1,14 @@ +replace($content); + } +} + diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php b/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php new file mode 100644 index 0000000000..ceb2f368ee --- /dev/null +++ b/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php @@ -0,0 +1,30 @@ +parseStr($template); $template = $DOM->query('body'); - $open_tracking_link = sprintf( + $open_tracking_image = sprintf( '', home_url(), esc_attr('?mailpoet&endpoint=track&action=open&data=' . Links::DATA_TAG) ); - $template->html($template->html() . $open_tracking_link); + $template->html($template->html() . $open_tracking_image); return $DOM->__toString(); } + + static function addTrackingImage() { + add_filter('mailpoet_rendering_post_process', function ($template) { + return OpenTracking::process($template); + }); + } } \ No newline at end of file diff --git a/lib/Newsletter/Shortcodes/Shortcodes.php b/lib/Newsletter/Shortcodes/Shortcodes.php index 5e551365f0..06d24288b8 100644 --- a/lib/Newsletter/Shortcodes/Shortcodes.php +++ b/lib/Newsletter/Shortcodes/Shortcodes.php @@ -5,6 +5,7 @@ class Shortcodes { public $newsletter; public $subscriber; public $queue; + const SHORTCODE_CATEGORY_NAMESPACE = __NAMESPACE__ . '\\Categories\\'; function __construct( $newsletter = false, @@ -22,14 +23,17 @@ class Shortcodes { $queue; } - function extract($content, $categories= false) { + function extract($content, $categories = false) { $categories = (is_array($categories)) ? implode('|', $categories) : false; $regex = sprintf( '/\[%s:.*?\]/ism', ($categories) ? '(?:' . $categories . ')' : '(?:\w+)' ); preg_match_all($regex, $content, $shortcodes); - return array_unique($shortcodes[0]); + $shortcodes = $shortcodes[0]; + return (count($shortcodes)) ? + array_unique($shortcodes) : + false; } function match($shortcode) { @@ -43,18 +47,19 @@ class Shortcodes { function process($shortcodes, $content = false) { $processed_shortcodes = array_map( - function($shortcode) use($content) { + function($shortcode) use ($content) { $shortcode_details = $this->match($shortcode); - $shortcode_category = isset($shortcode_details['category']) ? + $shortcode_category = !empty($shortcode_details['category']) ? ucfirst($shortcode_details['category']) : false; - $shortcode_action = isset($shortcode_details['action']) ? + $shortcode_action = !empty($shortcode_details['action']) ? $shortcode_details['action'] : false; $shortcode_class = - __NAMESPACE__ . '\\Categories\\' . $shortcode_category; - $shortcode_default_value = isset($shortcode_details['default']) - ? $shortcode_details['default'] : false; + self::SHORTCODE_CATEGORY_NAMESPACE . $shortcode_category; + $shortcode_default_value = !empty($shortcode_details['default']) ? + $shortcode_details['default'] : + false; if(!class_exists($shortcode_class)) { $custom_shortcode = apply_filters( 'mailpoet_newsletter_shortcode', @@ -82,6 +87,9 @@ class Shortcodes { function replace($content, $categories = false) { $shortcodes = $this->extract($content, $categories); + if(!$shortcodes) { + return $content; + } $processed_shortcodes = $this->process($shortcodes, $content); $shortcodes = array_intersect_key($shortcodes, $processed_shortcodes); return str_replace($shortcodes, $processed_shortcodes, $content); diff --git a/lib/Util/Helpers.php b/lib/Util/Helpers.php index 1d68e54767..a9db03e29b 100644 --- a/lib/Util/Helpers.php +++ b/lib/Util/Helpers.php @@ -2,6 +2,8 @@ namespace MailPoet\Util; class Helpers { + const DIVIDER = '***MailPoet***'; + static function getMaxPostSize($bytes = false) { $maxPostSize = ini_get('post_max_size'); if(!$bytes) return $maxPostSize; @@ -122,4 +124,12 @@ class Helpers { ) ); } + + static function joinObject($object = array()) { + return implode(self::DIVIDER, $object); + } + + static function splitObject($object = array()) { + return explode(self::DIVIDER, $object); + } } \ No newline at end of file From c83ab0886f5bbf6848f6b67d33eae0771a3d40e4 Mon Sep 17 00:00:00 2001 From: Vlad Date: Fri, 17 Jun 2016 15:00:46 -0400 Subject: [PATCH 03/12] - Rebases master --- .../Workers/SendingQueue/SendingQueue.php | 170 ++++++++++-------- lib/Cron/Workers/SendingQueue/Tasks/Posts.php | 11 +- .../Workers/SendingQueue/Tasks/Statistics.php | 12 +- .../SendingQueue/Tasks/Subscribers.php | 40 ++++- .../Renderer/PostProcess/OpenTracking.php | 5 + 5 files changed, 144 insertions(+), 94 deletions(-) diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index c4a63f08b4..f794838c70 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -6,8 +6,8 @@ use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Statistics as StatisticsTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Subscribers as SubscribersTask; +use MailPoet\Models\Newsletter as NewsletterModel; use MailPoet\Models\SendingQueue as SendingQueueModel; -use MailPoet\Models\Setting; use MailPoet\Models\Subscriber; use MailPoet\Util\Helpers; @@ -15,20 +15,21 @@ if(!defined('ABSPATH')) exit; class SendingQueue { public $mta_config; - public $mta_log; private $timer; const BATCH_SIZE = 50; - const STATUS_COMPLETED = 'completed'; function __construct($timer = false) { $this->mta_config = MailerTask::getMailerConfig(); - $this->mta_log = MailerTask::getMailerLog(); $this->timer = ($timer) ? $timer : microtime(true); CronHelper::checkExecutionTimer($this->timer); } function process() { + return; + $mta_log = MailerTask::getMailerLog(); + MailerTask::checkSendingLimit($this->mta_config, $mta_log); foreach($this->getQueues() as $queue) { + // get and pre-process newsletter (render, replace shortcodes/links, etc.) $newsletter = NewsletterTask::getAndPreProcess($queue->asArray()); if(!$newsletter) { $queue->delete(); @@ -36,22 +37,26 @@ class SendingQueue { } if(is_null($queue->newsletter_rendered_body)) { $queue->newsletter_rendered_body = json_encode($newsletter['rendered_body']); - $queue->save(); + //$queue->save(); } + // get subscribers $queue->subscribers = SubscribersTask::get($queue->asArray()); // configure mailer with newsletter data (from/reply-to) $mailer = MailerTask::configureMailer($newsletter); + // determine if processing is done in bulk or individually $processing_method = MailerTask::getProcessingMethod($this->mta_config); - foreach(array_chunk($queue->subscribers['to_process'], self::BATCH_SIZE) as - $subscribers_to_process_ids) { - $subscribers = Subscriber::whereIn('id', $subscribers_to_process_ids) - ->findArray(); + foreach(array_chunk($queue->subscribers['to_process'], self::BATCH_SIZE) + as $subscribers_to_process_ids + ) { + $found_subscribers = Subscriber::whereIn('id', $subscribers_to_process_ids) + ->findArray(); + $found_subscribers_ids = Helpers::arrayColumn($found_subscribers, 'id'); // if some subscribers weren't found, remove them from the processing list - if(count($subscribers) !== count($subscribers_to_process_ids)) { - $queue->subscribers['to_process'] = Subscribers::updateCount( - Helpers::arrayColumn($subscribers, 'id'), + if(count($found_subscribers_ids) !== count($subscribers_to_process_ids)) { + $queue->subscribers = SubscribersTask::updateToProcessList( + $found_subscribers_ids, $subscribers_to_process_ids, - $queue->subscribers['to_process'] + $queue->subscribers ); } if(!count($queue->subscribers['to_process'])) { @@ -59,15 +64,24 @@ class SendingQueue { continue; } $queue->subscribers = call_user_func_array( - array($this, $processing_method), - array($mailer, $newsletter, $subscribers, $queue) + array( + $this, + $processing_method + ), + array( + $mailer, + $mta_log, + $newsletter, + $found_subscribers, + $queue + ) ); } } } - function processBulkSubscribers($mailer, $newsletter, $subscribers, $queue) { - $subscriber_log = array(); + // TODO: merge processBulkSubscribers with processIndividualSubscriber + function processBulkSubscribers($mailer, $mta_log, $newsletter, $subscribers, $queue) { $subscribers_ids = Helpers::arrayColumn($subscribers, 'id'); foreach($subscribers as $subscriber) { // render shortcodes and replace subscriber data in tracked links @@ -87,62 +101,80 @@ class SendingQueue { ); } // send - $result = MailerTask::send($mailer, $prepared_newsletters, $prepared_subscribers); - if(!$result) { - // record failed subscribers - $subscriber_log['failed'] = SubscribersTask::updateFailedList( - $queue->subscribers['failed'], - $subscribers_ids + $send_result = MailerTask::send($mailer, $prepared_newsletters, $prepared_subscribers); + if(!$send_result) { + // update failed/to process list + $queue->subscribers = SubscribersTask::updateFailedList( + $subscribers_ids, + $queue->subscribers ); } else { - StatisticsTask::updateBulkNewsletterStatistics( + // update processed/to process list + $queue->subscribers = SubscribersTask::updateProcessedList( + $subscribers_ids, + $queue->subscribers + ); + // log statistics + StatisticsTask::processAndLogBulkNewsletterStatistics( $subscribers_ids, $newsletter['id'], $queue->id ); - MailerTask::updateMailerLog($this->mta_log); - $subscriber_log['processed'] = array_merge( - $queue->subscribers['processed'], - $subscribers_ids - ); + // keep track of sent items + $mta_log = MailerTask::updateMailerLog($mta_log); } - // TODO - //$queue = $this->updateQueue($queue, $subscriber_log); - MailerTask::checkSendingLimit($this->mta_config, $this->mta_log); + $this->updateQueue($queue); + MailerTask::checkSendingLimit($this->mta_config, $mta_log); CronHelper::checkExecutionTimer($this->timer); - return $queue->subscribers; } - /* function processIndividualSubscriber($mailer, $newsletter, $subscribers, $queue) { + function processIndividualSubscriber($mailer, $mta_log, $newsletter, $subscribers, $queue) { + $subscribers_ids = Helpers::arrayColumn($subscribers, 'id'); foreach($subscribers as $subscriber) { - $this->checkSendingLimit(); - $processed_newsletter = $this->prepareNewsletterForSending($newsletter, $subscriber, $queue); - if(!$queue->newsletter_rendered_subject) { - $queue->newsletter_rendered_subject = $processed_newsletter['subject']; - } - $transformed_subscriber = $mailer->transformSubscriber($subscriber); - $result = $this->sendNewsletter( - $mailer, - $processed_newsletter, - $transformed_subscriber - ); - if(!$result) { - $queue->subscribers['failed'][] = $subscriber['id']; - } else { - $queue->subscribers['processed'][] = $subscriber['id']; - $newsletter_statistics = array( - $newsletter['id'], - $subscriber['id'], - $queue->id + // render shortcodes and replace subscriber data in tracked links + $prepared_newsletter = + NewsletterTask::prepareNewsletterForSending( + $newsletter, + $subscriber, + $queue->asArray() ); - $this->updateMailerLog(); - $this->updateNewsletterStatistics($newsletter_statistics); + if(!$queue->newsletter_rendered_subject) { + $queue->newsletter_rendered_subject = $prepared_newsletter['subject']; } - $this->updateQueue($queue); + // format subscriber name/address according to mailer settings + $prepared_subscriber = MailerTask::prepareSubscriberForSending( + $mailer, + $subscriber + ); + $send_result = MailerTask::send($mailer, $prepared_newsletter, $prepared_subscriber); + if(!$send_result) { + // update failed/to process list + $queue->subscribers = SubscribersTask::updateFailedList( + $subscribers_ids, + $queue->subscribers + ); + } else { + // update processed/to process list + $queue->subscribers = SubscribersTask::updateProcessedList( + $subscribers_ids, + $queue->subscribers + ); + // log statistics + StatisticsTask::logStatistics( + array( + $newsletter['id'], + $subscriber['id'], + $queue->id + ) + ); + // keep track of sent items + $mta_log = MailerTask::updateMailerLog($mta_log); + } + $queue = $this->updateQueue($queue); + MailerTask::checkSendingLimit($this->mta_config, $mta_log); CronHelper::checkExecutionTimer($this->timer); } - return $queue->subscribers; - }*/ + } function getQueues() { return SendingQueueModel::orderByDesc('priority') @@ -152,19 +184,6 @@ class SendingQueue { } function updateQueue($queue) { - // TODO - return; - $queue = clone($queue); - $queue->subscribers['to_process'] = array_diff( - $queue->subscribers['to_process'], - array_merge( - $queue->subscribers['processed'], - $queue->subscribers['failed'] - ) - ); - $queue->subscribers['to_process'] = array_values( - $queue->subscribers['to_process'] - ); $queue->count_processed = count($queue->subscribers['processed']) + count($queue->subscribers['failed']); $queue->count_to_process = count($queue->subscribers['to_process']); @@ -173,9 +192,16 @@ class SendingQueue { $queue->count_processed + $queue->count_to_process; if(!$queue->count_to_process) { $queue->processed_at = current_time('mysql'); - $queue->status = self::STATUS_COMPLETED; + $queue->status = SendingQueueModel::STATUS_COMPLETED; + // set newsletter status to sent + $newsletter = NewsletterModel::findOne($queue->newsletter_id); + // if it's a standard newsletter, update its status + if($newsletter->type === NewsletterModel::TYPE_STANDARD) { + $newsletter->setStatus(NewsletterModel::STATUS_SENT); + } } $queue->subscribers = serialize((array) $queue->subscribers); $queue->save(); + return $queue; } } \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Posts.php b/lib/Cron/Workers/SendingQueue/Tasks/Posts.php index c5bd23cee6..c91da44aaa 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Posts.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Posts.php @@ -13,17 +13,16 @@ class Posts { preg_match_all( '/data-post-id="(\d+)"/ism', $newsletter['rendered_body']['html'], - $matached_posts); - $matached_posts = $matached_posts[1]; - if(!count($matached_posts)) { + $matched_posts_ids); + $matched_posts_ids = $matched_posts_ids[1]; + if(!count($matched_posts_ids)) { return $newsletter; } - foreach($matached_posts as $post) { + foreach($matched_posts_ids as $post_id) { $newletter_post = NewsletterPost::create(); $newletter_post->newsletter_id = $newsletter['id']; - $newletter_post->post_id = $post; + $newletter_post->post_id = $post_id; $newletter_post->save(); } - return $newsletter; } } \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php b/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php index ceb2f368ee..92e6b6d8ad 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php @@ -7,7 +7,7 @@ use MailPoet\Util\Helpers; if(!defined('ABSPATH')) exit; class Statistics { - static function updateBulkNewsletterStatistics( + static function processAndLogBulkNewsletterStatistics( array $processed_subscribers_ids, $newsletter_id, $queue_id ) { $newsletter_statistics = array(); @@ -19,12 +19,10 @@ class Statistics { ); } $newsletter_statistics = Helpers::flattenArray($newsletter_statistics); + return self::logStatistics($newsletter_statistics); + } + + static function logStatistics($newsletter_statistics) { return StatisticsNewsletters::createMultiple($newsletter_statistics); } - - static function updateBulkNewsletterStatistics( - array $processed_subscribers_ids, $newsletter_id, $queue_id - ) { - - } } \ 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 0d58694d2f..6f9c74cb67 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Subscribers.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Subscribers.php @@ -16,26 +16,48 @@ class Subscribers { } static function updateToProcessList( - array $existing_subscribers_ids, + array $found_subscribers_ids, array $subscribers_to_process_ids, - array $queue_subscribers_to_process_ids + array $queue_subscribers ) { // compare existing subscriber to the ones that queued for processing $subscibers_to_exclude = array_diff( $subscribers_to_process_ids, - $existing_subscribers_ids + $found_subscribers_ids ); // remove nonexistent subscribers from the processing list - return array_diff( - $queue_subscribers_to_process_ids, - $subscibers_to_exclude + $queue_subscribers['to_process'] = array_diff( + $subscibers_to_exclude, + $queue_subscribers['to_process'] ); + return $queue_subscribers; } - static function updateFailedList(array $subscribers, array $failed_subscribers) { - return array_merge( - $subscribers, + static function updateFailedList( + array $failed_subscribers, array $queue_subscribers + ) { + $queue_subscribers['failed'] = array_merge( + $queue_subscribers['failed'], $failed_subscribers ); + $queue_subscribers['to_process'] = array_diff( + $failed_subscribers, + $queue_subscribers['to_process'] + ); + return $queue_subscribers; + } + + static function updateProcessedList( + array $processed_subscribers, array $queue_subscribers + ) { + $queue_subscribers['processed'] = array_merge( + $queue_subscribers['processed'], + $processed_subscribers + ); + $queue_subscribers['to_process'] = array_diff( + $processed_subscribers, + $queue_subscribers['to_process'] + ); + return $queue_subscribers; } } \ No newline at end of file diff --git a/lib/Newsletter/Renderer/PostProcess/OpenTracking.php b/lib/Newsletter/Renderer/PostProcess/OpenTracking.php index a5f256aa2f..780939969e 100644 --- a/lib/Newsletter/Renderer/PostProcess/OpenTracking.php +++ b/lib/Newsletter/Renderer/PostProcess/OpenTracking.php @@ -4,6 +4,8 @@ namespace MailPoet\Newsletter\Renderer\PostProcess; use MailPoet\Newsletter\Links\Links; class OpenTracking { + const OPEN_TRACKING_FILTER_NAME = 'mailpoet_rendering_post_process'; + static function process($template) { $DOM = new \pQuery(); $DOM = $DOM->parseStr($template); @@ -18,6 +20,9 @@ class OpenTracking { } static function addTrackingImage() { + if(array_key_exists(self::OPEN_TRACKING_FILTER_NAME, $GLOBALS['wp_filter'])) { + return; + }; add_filter('mailpoet_rendering_post_process', function ($template) { return OpenTracking::process($template); }); From 22dfb372ec70738ef3f4e999b0fe1952d507d222 Mon Sep 17 00:00:00 2001 From: Vlad Date: Mon, 20 Jun 2016 10:34:41 -0400 Subject: [PATCH 04/12] - Updates bulk insert logic --- lib/Models/StatisticsNewsletters.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/Models/StatisticsNewsletters.php b/lib/Models/StatisticsNewsletters.php index 026ed4e1e9..1d838c9029 100644 --- a/lib/Models/StatisticsNewsletters.php +++ b/lib/Models/StatisticsNewsletters.php @@ -10,18 +10,27 @@ class StatisticsNewsletters extends Model { parent::__construct(); } - static function createMultiple($data) { - if (count($data) % 3 !== 0) { - return false; + static function createMultiple(array $data) { + $values = array(); + foreach($data as $value) { + if(!empty($value['newsletter_id']) && + !empty($value['subscriber_id']) && + !empty($value['queue_id']) + ) { + $values[] = $value['newsletter_id']; + $values[] = $value['subscriber_id']; + $values[] = $value['queue_id']; + } } + if (!count($values)) return false; return self::rawExecute( 'INSERT INTO `' . self::$_table . '` ' . '(newsletter_id, subscriber_id, queue_id) ' . 'VALUES ' . rtrim( - str_repeat('(?,?,?), ', count($data)/3), + str_repeat('(?,?,?), ', count($values) / 3), ', ' ), - $data + $values ); } } \ No newline at end of file From e807aad8141f0e068f14aa5b4de7d64dcd078899 Mon Sep 17 00:00:00 2001 From: Vlad Date: Mon, 20 Jun 2016 10:35:10 -0400 Subject: [PATCH 05/12] - Updates array flatten function for multidimensional arrays - Removes custom array unique method for multidimensional arrays --- lib/Util/Helpers.php | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/lib/Util/Helpers.php b/lib/Util/Helpers.php index a9db03e29b..c4058c6fde 100644 --- a/lib/Util/Helpers.php +++ b/lib/Util/Helpers.php @@ -7,26 +7,28 @@ class Helpers { static function getMaxPostSize($bytes = false) { $maxPostSize = ini_get('post_max_size'); if(!$bytes) return $maxPostSize; - switch (substr($maxPostSize, -1)) { + switch(substr($maxPostSize, -1)) { case 'M': case 'm': - return (int)$maxPostSize * 1048576; + return (int) $maxPostSize * 1048576; case 'K': case 'k': - return (int)$maxPostSize * 1024; + return (int) $maxPostSize * 1024; case 'G': case 'g': - return (int)$maxPostSize * 1073741824; + return (int) $maxPostSize * 1073741824; default: return $maxPostSize; } } - static function flattenArray($array) { - if(!$array) return; - $flattened_array = array(); - array_walk_recursive($array, function ($a) use (&$flattened_array) { $flattened_array[] = $a; }); - return $flattened_array; + static function flattenMultidimensionalArray($array) { + if(!$array) return $array; + return iterator_to_array( + new \RecursiveIteratorIterator( + new \RecursiveArrayIterator($array) + ) + ); } /* @@ -71,13 +73,13 @@ class Helpers { $paramsIndexKey = null; if(isset($params[2])) { if(is_float($params[2]) || is_int($params[2])) { - $paramsIndexKey = (int)$params[2]; + $paramsIndexKey = (int) $params[2]; } else { $paramsIndexKey = (string) $params[2]; } } $resultArray = array(); - foreach ($paramsInput as $row) { + foreach($paramsInput as $row) { $key = $value = null; $keySet = $valueSet = false; if($paramsIndexKey !== null && array_key_exists($paramsIndexKey, $row)) { @@ -116,15 +118,6 @@ class Helpers { return preg_replace_callback('/([A-Z])/', $func, $str); } - static function arrayUnique($arr) { - return array_map( - 'unserialize', - array_unique( - array_map('serialize', $arr) - ) - ); - } - static function joinObject($object = array()) { return implode(self::DIVIDER, $object); } From f32d6bb331ff3e86dcb81a45dc72506f0275632a Mon Sep 17 00:00:00 2001 From: Vlad Date: Mon, 20 Jun 2016 23:12:32 -0400 Subject: [PATCH 06/12] - Joins bulk and individual processing into one method - Refactors code as per code review comments --- lib/Cron/Daemon.php | 28 +-- lib/Cron/Supervisor.php | 1 - .../Workers/SendingQueue/SendingQueue.php | 165 ++++++++---------- .../Workers/SendingQueue/Tasks/Mailer.php | 53 +++--- .../Workers/SendingQueue/Tasks/Newsletter.php | 32 ++-- .../Workers/SendingQueue/Tasks/Statistics.php | 28 --- .../SendingQueue/Tasks/Subscribers.php | 12 +- lib/Newsletter/Links/Links.php | 2 +- .../Renderer/PostProcess/OpenTracking.php | 9 +- lib/Newsletter/Renderer/Renderer.php | 3 +- 10 files changed, 155 insertions(+), 178 deletions(-) delete mode 100644 lib/Cron/Workers/SendingQueue/Tasks/Statistics.php diff --git a/lib/Cron/Daemon.php b/lib/Cron/Daemon.php index 0113d9f761..b86ac388b1 100644 --- a/lib/Cron/Daemon.php +++ b/lib/Cron/Daemon.php @@ -1,8 +1,8 @@ abortIfStopped($daemon); try { - $scheduler = new Scheduler(); - $scheduler->process($this->timer); - $queue = new SendingQueue(); - $queue->process($this->timer); + $scheduler = new SchedulerWorker($this->timer); + $scheduler->process(); + $queue = new SendingQueueWorker($this->timer); + $queue->process(); } catch(\Exception $e) { - // continue processing, no need to catch errors + // continue processing, no need to handle errors } $elapsed_time = microtime(true) - $this->timer; if($elapsed_time < CronHelper::DAEMON_EXECUTION_LIMIT) { sleep(CronHelper::DAEMON_EXECUTION_LIMIT - $elapsed_time); } - // after each execution, re-read daemon data in case it was deleted or + // after each execution, re-read daemon data in case its status was changed // its status has changed $daemon = CronHelper::getDaemon(); if(!$daemon || $daemon['token'] !== $this->data['token']) { - exit; + self::terminate(); } $daemon['counter']++; $this->abortIfStopped($daemon); @@ -69,11 +69,13 @@ class Daemon { } function abortIfStopped($daemon) { - if($daemon['status'] === self::STATUS_STOPPED) exit; + if($daemon['status'] === self::STATUS_STOPPED) { + self::terminate(); + } if($daemon['status'] === self::STATUS_STOPPING) { $daemon['status'] = self::STATUS_STOPPED; CronHelper::saveDaemon($daemon); - exit; + self::terminate(); } } @@ -83,6 +85,10 @@ class Daemon { function callSelf() { CronHelper::accessDaemon($this->token, self::REQUEST_TIMEOUT); + self::terminate(); + } + + function terminate() { exit; } } \ No newline at end of file diff --git a/lib/Cron/Supervisor.php b/lib/Cron/Supervisor.php index f89f51dfa0..a598283e85 100644 --- a/lib/Cron/Supervisor.php +++ b/lib/Cron/Supervisor.php @@ -26,7 +26,6 @@ class Supervisor { $daemon['status'] === Daemon::STATUS_STOPPED ) { return $this->formatDaemonStatusMessage($daemon['status']); - } $elapsed_time = time() - (int)$daemon['updated_at']; // if it's been less than 40 seconds since last execution and we're not diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index f794838c70..2d2b7aa561 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -4,51 +4,48 @@ namespace MailPoet\Cron\Workers\SendingQueue; use MailPoet\Cron\CronHelper; use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterTask; -use MailPoet\Cron\Workers\SendingQueue\Tasks\Statistics as StatisticsTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Subscribers as SubscribersTask; use MailPoet\Models\Newsletter as NewsletterModel; use MailPoet\Models\SendingQueue as SendingQueueModel; -use MailPoet\Models\Subscriber; +use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel; +use MailPoet\Models\Subscriber as SubscriberModel; use MailPoet\Util\Helpers; if(!defined('ABSPATH')) exit; class SendingQueue { - public $mta_config; + public $mailer_task; + public $newsletter_task; private $timer; const BATCH_SIZE = 50; function __construct($timer = false) { - $this->mta_config = MailerTask::getMailerConfig(); + $this->mailer_task = new MailerTask(); + $this->newsletter_task = new NewsletterTask(); $this->timer = ($timer) ? $timer : microtime(true); - CronHelper::checkExecutionTimer($this->timer); } function process() { - return; - $mta_log = MailerTask::getMailerLog(); - MailerTask::checkSendingLimit($this->mta_config, $mta_log); + $this->mailer_task->checkSendingLimit(); foreach($this->getQueues() as $queue) { // get and pre-process newsletter (render, replace shortcodes/links, etc.) - $newsletter = NewsletterTask::getAndPreProcess($queue->asArray()); + $newsletter = $this->newsletter_task->getAndPreProcess($queue->asArray()); if(!$newsletter) { $queue->delete(); continue; } if(is_null($queue->newsletter_rendered_body)) { $queue->newsletter_rendered_body = json_encode($newsletter['rendered_body']); - //$queue->save(); + $queue->save(); } // get subscribers - $queue->subscribers = SubscribersTask::get($queue->asArray()); + $queue->subscribers = SubscribersTask::get($queue->subscribers); // configure mailer with newsletter data (from/reply-to) - $mailer = MailerTask::configureMailer($newsletter); - // determine if processing is done in bulk or individually - $processing_method = MailerTask::getProcessingMethod($this->mta_config); + $mailer = $this->mailer_task->configureMailer($newsletter); foreach(array_chunk($queue->subscribers['to_process'], self::BATCH_SIZE) as $subscribers_to_process_ids ) { - $found_subscribers = Subscriber::whereIn('id', $subscribers_to_process_ids) + $found_subscribers = SubscriberModel::whereIn('id', $subscribers_to_process_ids) ->findArray(); $found_subscribers_ids = Helpers::arrayColumn($found_subscribers, 'id'); // if some subscribers weren't found, remove them from the processing list @@ -63,30 +60,26 @@ class SendingQueue { $this->updateQueue($queue); continue; } - $queue->subscribers = call_user_func_array( - array( - $this, - $processing_method - ), - array( - $mailer, - $mta_log, - $newsletter, - $found_subscribers, - $queue - ) + $queue = $this->processQueue( + $queue, + $mailer, + $newsletter, + $found_subscribers ); } } } - // TODO: merge processBulkSubscribers with processIndividualSubscriber - function processBulkSubscribers($mailer, $mta_log, $newsletter, $subscribers, $queue) { - $subscribers_ids = Helpers::arrayColumn($subscribers, 'id'); + function processQueue($queue, $mailer, $newsletter, $subscribers) { + // determine if processing is done in bulk or individually + $processing_method = $this->mailer_task->getProcessingMethod(); + $prepared_newsletters = array(); + $prepared_subscribers = array(); + $statistics = array(); foreach($subscribers as $subscriber) { // render shortcodes and replace subscriber data in tracked links $prepared_newsletters[] = - NewsletterTask::prepareNewsletterForSending( + $this->newsletter_task->prepareNewsletterForSending( $newsletter, $subscriber, $queue->asArray() @@ -95,13 +88,53 @@ class SendingQueue { $queue->newsletter_rendered_subject = $prepared_newsletters[0]['subject']; } // format subscriber name/address according to mailer settings - $prepared_subscribers[] = MailerTask::prepareSubscriberForSending( + $prepared_subscribers[] = $this->mailer_task->prepareSubscriberForSending( $mailer, $subscriber ); + $prepared_subscribers_ids[] = $subscriber['id']; + // keep track of values for statistics purposes + $statistics[] = array( + 'newsletter_id' => $newsletter['id'], + 'subscriber_id' => $subscriber['id'], + 'queue_id' => $queue->id + ); + if($processing_method === 'individual') { + $queue = $this->sendNewsletters( + $queue, + $mailer, + $prepared_subscribers_ids, + $prepared_newsletters[0], + $prepared_subscribers[0], + $statistics + ); + $prepared_newsletters = array(); + $prepared_subscribers = array(); + $statistics = array(); + } } - // send - $send_result = MailerTask::send($mailer, $prepared_newsletters, $prepared_subscribers); + if($processing_method === 'bulk') { + $queue = $this->sendNewsletters( + $queue, + $mailer, + $prepared_subscribers_ids, + $prepared_newsletters, + $prepared_subscribers, + $statistics + ); + } + return $queue; + } + + function sendNewsletters( + $queue, $mailer, $subscribers_ids, $newsletters, $subscribers, $statistics + ) { + // send newsletter + $send_result = $this->mailer_task->send( + $mailer, + $newsletters, + $subscribers + ); if(!$send_result) { // update failed/to process list $queue->subscribers = SubscribersTask::updateFailedList( @@ -115,65 +148,17 @@ class SendingQueue { $queue->subscribers ); // log statistics - StatisticsTask::processAndLogBulkNewsletterStatistics( - $subscribers_ids, - $newsletter['id'], - $queue->id - ); + StatisticsNewslettersModel::createMultiple($statistics); // keep track of sent items - $mta_log = MailerTask::updateMailerLog($mta_log); + $this->mailer_task->updateMailerLog(); + $subscribers_to_process_count = count($queue->subscribers['to_process']); + } + $queue = $this->updateQueue($queue); + if ($subscribers_to_process_count) { + $this->mailer_task->checkSendingLimit(); } - $this->updateQueue($queue); - MailerTask::checkSendingLimit($this->mta_config, $mta_log); CronHelper::checkExecutionTimer($this->timer); - } - - function processIndividualSubscriber($mailer, $mta_log, $newsletter, $subscribers, $queue) { - $subscribers_ids = Helpers::arrayColumn($subscribers, 'id'); - foreach($subscribers as $subscriber) { - // render shortcodes and replace subscriber data in tracked links - $prepared_newsletter = - NewsletterTask::prepareNewsletterForSending( - $newsletter, - $subscriber, - $queue->asArray() - ); - if(!$queue->newsletter_rendered_subject) { - $queue->newsletter_rendered_subject = $prepared_newsletter['subject']; - } - // format subscriber name/address according to mailer settings - $prepared_subscriber = MailerTask::prepareSubscriberForSending( - $mailer, - $subscriber - ); - $send_result = MailerTask::send($mailer, $prepared_newsletter, $prepared_subscriber); - if(!$send_result) { - // update failed/to process list - $queue->subscribers = SubscribersTask::updateFailedList( - $subscribers_ids, - $queue->subscribers - ); - } else { - // update processed/to process list - $queue->subscribers = SubscribersTask::updateProcessedList( - $subscribers_ids, - $queue->subscribers - ); - // log statistics - StatisticsTask::logStatistics( - array( - $newsletter['id'], - $subscriber['id'], - $queue->id - ) - ); - // keep track of sent items - $mta_log = MailerTask::updateMailerLog($mta_log); - } - $queue = $this->updateQueue($queue); - MailerTask::checkSendingLimit($this->mta_config, $mta_log); - CronHelper::checkExecutionTimer($this->timer); - } + return $queue; } function getQueues() { diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php index 9262167fca..b87eb13df9 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php @@ -7,7 +7,15 @@ use MailPoet\Models\Setting; if(!defined('ABSPATH')) exit; class Mailer { - static function configureMailer(array $newsletter) { + public $mta_config; + public $mta_log; + + function __construct() { + $this->mta_config = $this->getMailerConfig(); + $this->mta_log = $this->getMailerLog(); + } + + function configureMailer(array $newsletter) { $sender['address'] = (!empty($newsletter['sender_address'])) ? $newsletter['sender_address'] : false; @@ -30,7 +38,7 @@ class Mailer { return $mailer; } - static function getMailerConfig() { + function getMailerConfig() { $mta_config = Setting::getValue('mta'); if(!$mta_config) { throw new \Exception(__('Mailer is not configured.')); @@ -38,13 +46,7 @@ class Mailer { return $mta_config; } - static function updateMailerLog($mta_log) { - $mta_log['sent']++; - Setting::setValue('mta_log', $mta_log); - return $mta_log; - } - - static function getMailerLog() { + function getMailerLog() { $mta_log = Setting::getValue('mta_log'); if(!$mta_log) { $mta_log = array( @@ -56,40 +58,45 @@ class Mailer { return $mta_log; } - static function getProcessingMethod($mta_config) { - return ($mta_config['method'] === 'MailPoet') ? - 'processBulkSubscribers' : - 'processIndividualSubscriber'; + function updateMailerLog() { + $this->mta_log['sent']++; + Setting::setValue('mta_log', $this->mta_log); } - static function prepareSubscriberForSending($mailer, $subscriber) { + function getProcessingMethod() { + return ($this->mta_config['method'] === 'MailPoet') ? + 'bulk' : + 'individual'; + } + + function prepareSubscriberForSending($mailer, $subscriber) { return ($mailer instanceof \MailPoet\Mailer\Mailer) ? $mailer->transformSubscriber($subscriber) : false; } - static function send($mailer, $newsletter, $subscriber) { + function send($mailer, $newsletter, $subscriber) { return ($mailer instanceof \MailPoet\Mailer\Mailer) ? $mailer->mailer_instance->send($newsletter, $subscriber) : false; } - static function checkSendingLimit($mta_config, $mta_log) { - $frequency_interval = (int) $mta_config['frequency']['interval'] * 60; - $frequency_limit = (int) $mta_config['frequency']['emails']; - $elapsed_time = time() - (int) $mta_log['started']; - if($mta_log['sent'] === $frequency_limit && + function checkSendingLimit() { + if($this->mta_config['method'] === 'MailPoet') return; + $frequency_interval = (int) $this->mta_config['frequency']['interval'] * 60; + $frequency_limit = (int) $this->mta_config['frequency']['emails']; + $elapsed_time = time() - (int) $this->mta_log['started']; + if($this->mta_log['sent'] === $frequency_limit && $elapsed_time <= $frequency_interval ) { throw new \Exception(__('Sending frequency limit has been reached.')); } if($elapsed_time > $frequency_interval) { - $mta_log = array( + $this->mta_log = array( 'sent' => 0, 'started' => time() ); - Setting::setValue('mta_log', $mta_log); + Setting::setValue('mta_log', $this->mta_log); } - return; } } \ No newline at end of file diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php index 92e827bad3..da22e508b0 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php @@ -14,13 +14,21 @@ use MailPoet\Util\Helpers; if(!defined('ABSPATH')) exit; class Newsletter { - static function get($newsletter_id) { + public $tracking_enabled; + public $tracking_image_inserted; + + 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; } - static function getAndPreProcess(array $queue) { - $newsletter = self::get($queue['newsletter_id']); + function getAndPreProcess(array $queue) { + $newsletter = $this->get($queue['newsletter_id']); if(!$newsletter) { return false; } @@ -31,16 +39,18 @@ class Newsletter { return $newsletter; } // if tracking is enabled, do additional processing - if((boolean) Setting::getValue('tracking.enabled')) { - // add tracking image - OpenTracking::addTrackingImage(); + 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(); + } // render newsletter - $newsletter = self::render($newsletter); + $newsletter = $this->render($newsletter); // hash and save all links $newsletter = LinksTask::process($newsletter, $queue); } else { // render newsletter - $newsletter = self::render($newsletter); + $newsletter = $this->render($newsletter); } // check if this is a post notification and if it contains posts $newsletter_contains_posts = strpos($newsletter['rendered_body']['html'], 'data-post-id'); @@ -52,13 +62,13 @@ class Newsletter { return $newsletter; } - static function render($newsletter) { + function render($newsletter) { $renderer = new Renderer($newsletter); $newsletter['rendered_body'] = $renderer->render(); return $newsletter; } - static function prepareNewsletterForSending( + function prepareNewsletterForSending( array $newsletter, array $subscriber, array $queue ) { // shortcodes and links will be replaced in the subject, html and text body @@ -76,7 +86,7 @@ class Newsletter { $subscriber, $queue ); - if((boolean) Setting::getValue('tracking.enabled')) { + if($this->tracking_enabled) { $prepared_newsletter = NewsletterLinks::replaceSubscriberData( $newsletter['id'], $subscriber['id'], diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php b/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php deleted file mode 100644 index 92e6b6d8ad..0000000000 --- a/lib/Cron/Workers/SendingQueue/Tasks/Statistics.php +++ /dev/null @@ -1,28 +0,0 @@ -parseStr($template); @@ -20,11 +19,9 @@ class OpenTracking { } static function addTrackingImage() { - if(array_key_exists(self::OPEN_TRACKING_FILTER_NAME, $GLOBALS['wp_filter'])) { - return; - }; - add_filter('mailpoet_rendering_post_process', function ($template) { + add_filter(Renderer::POST_PROCESS_FILTER, function ($template) { return OpenTracking::process($template); }); + return true; } } \ No newline at end of file diff --git a/lib/Newsletter/Renderer/Renderer.php b/lib/Newsletter/Renderer/Renderer.php index 397454cbef..66d4954612 100644 --- a/lib/Newsletter/Renderer/Renderer.php +++ b/lib/Newsletter/Renderer/Renderer.php @@ -10,6 +10,7 @@ class Renderer { public $CSS_inliner; public $newsletter; const NEWSLETTER_TEMPLATE = 'Template.html'; + const POST_PROCESS_FILTER = 'mailpoet_rendering_post_process'; function __construct(array $newsletter) { $this->newsletter = $newsletter; @@ -102,7 +103,7 @@ class Renderer { str_replace('&', '&', $template->html()) ); $template = apply_filters( - 'mailpoet_rendering_post_process', + self::POST_PROCESS_FILTER, $DOM->__toString() ); return $template; From ce6327c3d556b6e1c36dd4139c3223dbc80ec80f Mon Sep 17 00:00:00 2001 From: Vlad Date: Mon, 20 Jun 2016 23:35:47 -0400 Subject: [PATCH 07/12] - Re-adds the old multidimensional array flatten method --- lib/Util/Helpers.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/Util/Helpers.php b/lib/Util/Helpers.php index c4058c6fde..31f97767d1 100644 --- a/lib/Util/Helpers.php +++ b/lib/Util/Helpers.php @@ -22,13 +22,11 @@ class Helpers { } } - static function flattenMultidimensionalArray($array) { - if(!$array) return $array; - return iterator_to_array( - new \RecursiveIteratorIterator( - new \RecursiveArrayIterator($array) - ) - ); + static function flattenArray($array) { + if(!$array) return; + $flattened_array = array(); + array_walk_recursive($array, function ($a) use (&$flattened_array) { $flattened_array[] = $a; }); + return $flattened_array; } /* From efc9bac7603edf60a144cc2d0232711a651dddff Mon Sep 17 00:00:00 2001 From: Vlad Date: Mon, 20 Jun 2016 23:36:30 -0400 Subject: [PATCH 08/12] - Updates unit tests --- tests/unit/Models/NewsletterTest.php | 7 +++++-- tests/unit/Models/SubscriberSegmentTest.php | 4 ++++ tests/unit/Models/SubscriberTest.php | 6 +++++- tests/unit/Statistics/Track/ClicksTest.php | 2 ++ tests/unit/Statistics/Track/OpensTest.php | 2 ++ tests/unit/Statistics/Track/UnsubscribesTest.php | 2 ++ 6 files changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/unit/Models/NewsletterTest.php b/tests/unit/Models/NewsletterTest.php index 7778b5598b..cc5123f228 100644 --- a/tests/unit/Models/NewsletterTest.php +++ b/tests/unit/Models/NewsletterTest.php @@ -113,7 +113,9 @@ class NewsletterTest extends MailPoetTest { $sending_queue->save(); $subscriber = Subscriber::createOrUpdate(array( - 'email' => 'john.doe@mailpoet.com' + 'email' => 'john.doe@mailpoet.com', + 'first_name' => 'John', + 'last_name' => 'Doe' )); $opens = StatisticsOpens::create(); @@ -123,7 +125,7 @@ class NewsletterTest extends MailPoetTest { $opens->save(); $newsletter->queue = $newsletter->getQueue()->asArray(); - $statistics = $newsletter->getStatistics(); + $statistics = $newsletter->getStatistics( $sending_queue->id); expect($statistics->opened)->equals(1); expect($statistics->clicked)->equals(0); expect($statistics->unsubscribed)->equals(0); @@ -250,5 +252,6 @@ class NewsletterTest extends MailPoetTest { ORM::raw_execute('TRUNCATE ' . Segment::$_table); ORM::raw_execute('TRUNCATE ' . NewsletterSegment::$_table); ORM::raw_execute('TRUNCATE ' . SendingQueue::$_table); + ORM::raw_execute('TRUNCATE ' . StatisticsOpens::$_table); } } diff --git a/tests/unit/Models/SubscriberSegmentTest.php b/tests/unit/Models/SubscriberSegmentTest.php index dfbb3e24cd..db2d8492d0 100644 --- a/tests/unit/Models/SubscriberSegmentTest.php +++ b/tests/unit/Models/SubscriberSegmentTest.php @@ -8,6 +8,8 @@ class SubscriberSegmentTest extends MailPoetTest { function _before() { $this->subscriber = Subscriber::createOrUpdate(array( 'email' => 'john.doe@mailpoet.com', + 'first_name' => 'John', + 'last_name' => 'Doe', 'status' => Subscriber::STATUS_SUBSCRIBED )); $this->segment_1 = Segment::createOrUpdate(array('name' => 'Segment 1')); @@ -133,6 +135,8 @@ class SubscriberSegmentTest extends MailPoetTest { // create a second subscriber $subscriber_2 = Subscriber::createOrUpdate(array( 'email' => 'jane.doe@mailpoet.com', + 'first_name' => 'Jane', + 'last_name' => 'Doe', 'status' => Subscriber::STATUS_SUBSCRIBED )); // subscribe her to segments diff --git a/tests/unit/Models/SubscriberTest.php b/tests/unit/Models/SubscriberTest.php index f65d91e44e..c84dcca8e4 100644 --- a/tests/unit/Models/SubscriberTest.php +++ b/tests/unit/Models/SubscriberTest.php @@ -188,7 +188,7 @@ class SubscriberTest extends MailPoetTest { $values[0]['first_name'] = 'John'; Subscriber::updateMultiple($columns, $values); $subscribers = Subscriber::findArray(); - expect($subscribers[0]['first_name'])->equals($values[0]['first_name']); + expect($subscribers[0]['first_name'])->equals($values[0]['first_name']); } function testItCanSubscribe() { @@ -316,6 +316,8 @@ class SubscriberTest extends MailPoetTest { function testItCannotTrashAWPUser() { $wp_subscriber = Subscriber::createOrUpdate(array( 'email' => 'some.wp.user@mailpoet.com', + 'first_name' => 'Some', + 'last_name' => 'WP User', 'wp_user_id' => 1 )); expect($wp_subscriber->trash())->equals(false); @@ -328,6 +330,8 @@ class SubscriberTest extends MailPoetTest { function testItCannotDeleteAWPUser() { $wp_subscriber = Subscriber::createOrUpdate(array( 'email' => 'some.wp.user@mailpoet.com', + 'first_name' => 'Some', + 'last_name' => 'WP User', 'wp_user_id' => 1 )); expect($wp_subscriber->delete())->equals(false); diff --git a/tests/unit/Statistics/Track/ClicksTest.php b/tests/unit/Statistics/Track/ClicksTest.php index 7a6dce26a3..a456d94366 100644 --- a/tests/unit/Statistics/Track/ClicksTest.php +++ b/tests/unit/Statistics/Track/ClicksTest.php @@ -18,6 +18,8 @@ class ClicksTest extends MailPoetTest { // create subscriber $subscriber = Subscriber::create(); $subscriber->email = 'test@example.com'; + $subscriber->first_name = 'First'; + $subscriber->last_name = 'Last'; $this->subscriber = $subscriber->save(); // create queue $queue = SendingQueue::create(); diff --git a/tests/unit/Statistics/Track/OpensTest.php b/tests/unit/Statistics/Track/OpensTest.php index 05eff7445a..6bd7c2b8d0 100644 --- a/tests/unit/Statistics/Track/OpensTest.php +++ b/tests/unit/Statistics/Track/OpensTest.php @@ -16,6 +16,8 @@ class OpensTest extends MailPoetTest { // create subscriber $subscriber = Subscriber::create(); $subscriber->email = 'test@example.com'; + $subscriber->first_name = 'First'; + $subscriber->last_name = 'Last'; $this->subscriber = $subscriber->save(); // create queue $queue = SendingQueue::create(); diff --git a/tests/unit/Statistics/Track/UnsubscribesTest.php b/tests/unit/Statistics/Track/UnsubscribesTest.php index 0002c6fbfb..25eef268b2 100644 --- a/tests/unit/Statistics/Track/UnsubscribesTest.php +++ b/tests/unit/Statistics/Track/UnsubscribesTest.php @@ -18,6 +18,8 @@ class UnsubscribesTest extends MailPoetTest { // create subscriber $subscriber = Subscriber::create(); $subscriber->email = 'test@example.com'; + $subscriber->first_name = 'First'; + $subscriber->last_name = 'Last'; $this->subscriber = $subscriber->save(); // create queue $queue = SendingQueue::create(); From e5f3fabcda216e7325d0a08e00a731ae3ebcf44b Mon Sep 17 00:00:00 2001 From: Vlad Date: Tue, 21 Jun 2016 10:14:19 -0400 Subject: [PATCH 09/12] - Moves mailer logic into Mailer Task class --- .../Workers/SendingQueue/SendingQueue.php | 27 +++++++++---------- .../Workers/SendingQueue/Tasks/Mailer.php | 26 +++++++++++------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index 2d2b7aa561..2f14d9e758 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -21,7 +21,7 @@ class SendingQueue { function __construct($timer = false) { $this->mailer_task = new MailerTask(); - $this->newsletter_task = new NewsletterTask(); + $this->newsletter_task = new NewsletterTask($this->mailer_task); $this->timer = ($timer) ? $timer : microtime(true); } @@ -40,8 +40,6 @@ class SendingQueue { } // get subscribers $queue->subscribers = SubscribersTask::get($queue->subscribers); - // configure mailer with newsletter data (from/reply-to) - $mailer = $this->mailer_task->configureMailer($newsletter); foreach(array_chunk($queue->subscribers['to_process'], self::BATCH_SIZE) as $subscribers_to_process_ids ) { @@ -62,7 +60,6 @@ class SendingQueue { } $queue = $this->processQueue( $queue, - $mailer, $newsletter, $found_subscribers ); @@ -70,7 +67,7 @@ class SendingQueue { } } - function processQueue($queue, $mailer, $newsletter, $subscribers) { + function processQueue($queue, $newsletter, $subscribers) { // determine if processing is done in bulk or individually $processing_method = $this->mailer_task->getProcessingMethod(); $prepared_newsletters = array(); @@ -89,7 +86,6 @@ class SendingQueue { } // format subscriber name/address according to mailer settings $prepared_subscribers[] = $this->mailer_task->prepareSubscriberForSending( - $mailer, $subscriber ); $prepared_subscribers_ids[] = $subscriber['id']; @@ -102,7 +98,7 @@ class SendingQueue { if($processing_method === 'individual') { $queue = $this->sendNewsletters( $queue, - $mailer, + $newsletter, $prepared_subscribers_ids, $prepared_newsletters[0], $prepared_subscribers[0], @@ -116,7 +112,7 @@ class SendingQueue { if($processing_method === 'bulk') { $queue = $this->sendNewsletters( $queue, - $mailer, + $newsletter, $prepared_subscribers_ids, $prepared_newsletters, $prepared_subscribers, @@ -127,24 +123,25 @@ class SendingQueue { } function sendNewsletters( - $queue, $mailer, $subscribers_ids, $newsletters, $subscribers, $statistics + $queue, $newsletter_object, $prepared_subscribers_ids, $prepared_newsletters, + $prepared_subscribers, $statistics ) { // send newsletter $send_result = $this->mailer_task->send( - $mailer, - $newsletters, - $subscribers + $newsletter_object, + $prepared_newsletters, + $prepared_subscribers ); if(!$send_result) { // update failed/to process list $queue->subscribers = SubscribersTask::updateFailedList( - $subscribers_ids, + $prepared_subscribers_ids, $queue->subscribers ); } else { // update processed/to process list $queue->subscribers = SubscribersTask::updateProcessedList( - $subscribers_ids, + $prepared_subscribers_ids, $queue->subscribers ); // log statistics @@ -154,7 +151,7 @@ class SendingQueue { $subscribers_to_process_count = count($queue->subscribers['to_process']); } $queue = $this->updateQueue($queue); - if ($subscribers_to_process_count) { + if($subscribers_to_process_count) { $this->mailer_task->checkSendingLimit(); } CronHelper::checkExecutionTimer($this->timer); diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php index b87eb13df9..61bbc26777 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php @@ -13,9 +13,10 @@ class Mailer { function __construct() { $this->mta_config = $this->getMailerConfig(); $this->mta_log = $this->getMailerLog(); + $this->mailer = $this->configureMailer(); } - function configureMailer(array $newsletter) { + function configureMailer(array $newsletter = null, $mailer = false) { $sender['address'] = (!empty($newsletter['sender_address'])) ? $newsletter['sender_address'] : false; @@ -34,7 +35,13 @@ class Mailer { if(!$reply_to['address']) { $reply_to = false; } - $mailer = new MailerFactory($method = false, $sender, $reply_to); + if (!$mailer) { + $mailer = new MailerFactory($method = false, $sender, $reply_to); + } + else { + $mailer->mailer_instance->sender = $mailer->getSender($sender); + $mailer->mailer_instance->reply_to = $mailer->getReplyTo($reply_to); + } return $mailer; } @@ -69,16 +76,15 @@ class Mailer { 'individual'; } - function prepareSubscriberForSending($mailer, $subscriber) { - return ($mailer instanceof \MailPoet\Mailer\Mailer) ? - $mailer->transformSubscriber($subscriber) : - false; + function prepareSubscriberForSending(array $subscriber) { + return $this->mailer->transformSubscriber($subscriber); } - function send($mailer, $newsletter, $subscriber) { - return ($mailer instanceof \MailPoet\Mailer\Mailer) ? - $mailer->mailer_instance->send($newsletter, $subscriber) : - false; + function send(array $newsletter_object, $prepared_newsletters, + $prepared_subscribers + ) { + $mailer = $this->configureMailer($newsletter_object, $this->mailer); + return $mailer->mailer_instance->send($prepared_newsletters, $prepared_subscribers); } function checkSendingLimit() { From f524ffcb2878769b0c4ed61e907726f961935ef4 Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 22 Jun 2016 11:15:40 -0400 Subject: [PATCH 10/12] - Updates mailer task to store mailer instance --- .../Workers/SendingQueue/SendingQueue.php | 9 +++---- .../Workers/SendingQueue/Tasks/Mailer.php | 25 +++++++------------ 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index 2f14d9e758..d2ddf7ac17 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -21,7 +21,7 @@ class SendingQueue { function __construct($timer = false) { $this->mailer_task = new MailerTask(); - $this->newsletter_task = new NewsletterTask($this->mailer_task); + $this->newsletter_task = new NewsletterTask(); $this->timer = ($timer) ? $timer : microtime(true); } @@ -34,6 +34,8 @@ class SendingQueue { $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->save(); @@ -98,7 +100,6 @@ class SendingQueue { if($processing_method === 'individual') { $queue = $this->sendNewsletters( $queue, - $newsletter, $prepared_subscribers_ids, $prepared_newsletters[0], $prepared_subscribers[0], @@ -112,7 +113,6 @@ class SendingQueue { if($processing_method === 'bulk') { $queue = $this->sendNewsletters( $queue, - $newsletter, $prepared_subscribers_ids, $prepared_newsletters, $prepared_subscribers, @@ -123,12 +123,11 @@ class SendingQueue { } function sendNewsletters( - $queue, $newsletter_object, $prepared_subscribers_ids, $prepared_newsletters, + $queue, $prepared_subscribers_ids, $prepared_newsletters, $prepared_subscribers, $statistics ) { // send newsletter $send_result = $this->mailer_task->send( - $newsletter_object, $prepared_newsletters, $prepared_subscribers ); diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php index 61bbc26777..dbafb33bd4 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php @@ -9,14 +9,14 @@ if(!defined('ABSPATH')) exit; class Mailer { public $mta_config; public $mta_log; + public $mailer = null; function __construct() { $this->mta_config = $this->getMailerConfig(); $this->mta_log = $this->getMailerLog(); - $this->mailer = $this->configureMailer(); } - function configureMailer(array $newsletter = null, $mailer = false) { + function configureMailer(array $newsletter = null) { $sender['address'] = (!empty($newsletter['sender_address'])) ? $newsletter['sender_address'] : false; @@ -35,14 +35,8 @@ class Mailer { if(!$reply_to['address']) { $reply_to = false; } - if (!$mailer) { - $mailer = new MailerFactory($method = false, $sender, $reply_to); - } - else { - $mailer->mailer_instance->sender = $mailer->getSender($sender); - $mailer->mailer_instance->reply_to = $mailer->getReplyTo($reply_to); - } - return $mailer; + $this->mailer = new MailerFactory($method = false, $sender, $reply_to); + return $this->mailer; } function getMailerConfig() { @@ -77,14 +71,13 @@ class Mailer { } function prepareSubscriberForSending(array $subscriber) { - return $this->mailer->transformSubscriber($subscriber); + return $this->mailer->transformSubscriber($subscriber); } - function send(array $newsletter_object, $prepared_newsletters, - $prepared_subscribers - ) { - $mailer = $this->configureMailer($newsletter_object, $this->mailer); - return $mailer->mailer_instance->send($prepared_newsletters, $prepared_subscribers); + function send($prepared_newsletters, $prepared_subscribers) { + return ($this->mailer) ? + $this->mailer->mailer_instance->send($prepared_newsletters, $prepared_subscribers) : + false; } function checkSendingLimit() { From 66d329f6304648308a1e2736e30301a192e5b435 Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 22 Jun 2016 11:21:11 -0400 Subject: [PATCH 11/12] - Configures mailer inside the mailer task class --- lib/Cron/Workers/SendingQueue/Tasks/Mailer.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php index dbafb33bd4..c07ada3aea 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php @@ -9,11 +9,12 @@ if(!defined('ABSPATH')) exit; class Mailer { public $mta_config; public $mta_log; - public $mailer = null; + public $mailer; function __construct() { $this->mta_config = $this->getMailerConfig(); $this->mta_log = $this->getMailerLog(); + $this->mailer = $this->configureMailer(); } function configureMailer(array $newsletter = null) { @@ -75,9 +76,10 @@ class Mailer { } function send($prepared_newsletters, $prepared_subscribers) { - return ($this->mailer) ? - $this->mailer->mailer_instance->send($prepared_newsletters, $prepared_subscribers) : - false; + return $this->mailer->mailer_instance->send( + $prepared_newsletters, + $prepared_subscribers + ); } function checkSendingLimit() { From d4143137497852a320a661a360f3e8e6d7ba3281 Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 22 Jun 2016 13:35:48 -0400 Subject: [PATCH 12/12] - Fixes const definition for PHP 5.5 --- lib/Newsletter/Shortcodes/Shortcodes.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Newsletter/Shortcodes/Shortcodes.php b/lib/Newsletter/Shortcodes/Shortcodes.php index 06d24288b8..ab0661dcc7 100644 --- a/lib/Newsletter/Shortcodes/Shortcodes.php +++ b/lib/Newsletter/Shortcodes/Shortcodes.php @@ -5,7 +5,7 @@ class Shortcodes { public $newsletter; public $subscriber; public $queue; - const SHORTCODE_CATEGORY_NAMESPACE = __NAMESPACE__ . '\\Categories\\'; + const SHORTCODE_CATEGORY_NAMESPACE = 'MailPoet\Newsletter\Shortcodes\Categories\\'; function __construct( $newsletter = false,