From d44ed993a2f53b1eab5ad11d2a065c23aea07bc9 Mon Sep 17 00:00:00 2001 From: bsitu Date: Tue, 5 Mar 2013 16:04:48 -0800 Subject: [PATCH] Add email bundling function to Echo notification * This patch needs intensive testing on Redis delayed job queue * This patch is -2 mainly for redis/phpredis are not ready on test/test2/mediawiki To test this locally, you need to: * set up Redis and phpredis locally * add the following to localSettings.php $wgJobTypeConf['MWEchoNotificationEmailBundleJob'] = array( 'class' => 'JobQueueRedis', 'redisServer' => '127.0.0.1', 'redisConfig' => array( 'connectTimeout' => 1 ), 'claimTTL' => 3600, 'checkDelay' => true ); * set $wgMainCacheType to CACHE_DB or memcache * set $wgEchoBundleEmailInterval to smaller number for testing purpose, 0 to disable email bundling Change-Id: I9313e7f6ed3e13478cec294b5b8408fe8e941faf --- Echo.i18n.php | 45 ++-- Echo.php | 21 +- Hooks.php | 2 + Notifier.php | 64 +++-- db_patches/patch-email_batch-new-field.sql | 7 + echo.sql | 5 +- formatters/BasicFormatter.php | 40 ++- formatters/PageLinkFormatter.php | 5 +- includes/DbEchoBackend.php | 60 +++-- includes/DbEmailBatch.php | 25 +- includes/DbEmailBundler.php | 51 ++++ includes/EchoBackend.php | 3 +- includes/EmailBatch.php | 28 +- includes/EmailBundler.php | 286 +++++++++++++++++++++ jobs/NotificationEmailBundleJob.php | 24 ++ 15 files changed, 588 insertions(+), 78 deletions(-) create mode 100644 db_patches/patch-email_batch-new-field.sql create mode 100644 includes/DbEmailBundler.php create mode 100644 includes/EmailBundler.php create mode 100644 jobs/NotificationEmailBundleJob.php diff --git a/Echo.i18n.php b/Echo.i18n.php index d4cdb9371..aaaa5daa2 100644 --- a/Echo.i18n.php +++ b/Echo.i18n.php @@ -73,7 +73,7 @@ $messages['en'] = array( 'notification-reverted2' => 'Your {{PLURAL:$4|edit on [[$2]] has|edits on [[$2]] have}} been {{GENDER:$1|reverted}} by [[User:$1|$1]] $3', 'notification-reverted-flyout2' => 'Your {{PLURAL:$4|edit on $2 has|edits on $2 have}} been {{GENDER:$1|reverted}} by $1 $3', 'notification-edit-talk-page-email-subject2' => 'You have a new talkpage message', - 'notification-edit-talk-page-email-body2' => '{{SITENAME}} user $1 {{GENDER:$1|posted}} on your talk page: + 'notification-edit-talk-page-email-body2' => '$1 $3 @@ -84,7 +84,7 @@ $2 $4', 'notification-edit-talk-page-email-batch-body2' => '$1 {{GENDER:$1|posted}} on your talk page', 'notification-page-linked-email-subject' => 'A page you started was linked on {{SITENAME}}', - 'notification-page-linked-email-body' => '$2 was {{GENDER:$1|linked}} from $4 + 'notification-page-linked-email-body' => '$1 See all links to this page: @@ -152,6 +152,8 @@ $1', // Bundle 'notification-edit-talk-page-bundle' => '$1 and $3 {{PLURAL:$4|other|others}} {{GENDER:$1|posted}} on your [[User talk:$2|talk page]].', 'notification-page-linked-bundle' => '$2 was {{GENDER:$1|linked}} from $3 and $4 other {{PLURAL:$5|page|pages}}. [[Special:WhatLinksHere/$2|See all links to this page]]', + 'notification-edit-user-talk-email-batch-bundle-body' => '$1 and $2 {{PLURAL:$3|other|others}} {{GENDER:$1|posted}} on your talk page', + 'notification-page-linked-email-batch-bundle-body' => '$2 was {{GENDER:$1|linked}} from $3 and $4 other {{PLURAL:$5|page|pages}}', // Email batch 'echo-email-batch-separator' => '________________________________________________', # only translate this message to other languages if you have to change it @@ -345,7 +347,7 @@ See also: {{Related|Notification-reverted}}", 'notification-edit-talk-page-email-subject2' => 'E-mail subject.', 'notification-edit-talk-page-email-body2' => 'E-mail notification. Parameters: -* $1 is a username +* $1 is the email intro, could be {{msg-mw|notification-edit-talk-page-email-batch-body2}} or {{msg-mw|notification-edit-talk-page-email-batch-bundle-body}} * $2 is a link to a change * $3 is the edit summary. * $4 is the e-mail footer, {{msg-mw|echo-email-footer-default}}', @@ -358,10 +360,9 @@ See also: * {{msg-mw|Notification-page-linked-email-batch-body}} * {{msg-mw|Notification-page-linked-email-body}}', 'notification-page-linked-email-body' => 'E-mail notification. Parameters: -* $1 is the username of the person who linked the page, plain text. Can be used for GENDER. +* $1 is the email intro, could be {{msg-mw|notification-page-linked-email-batch-body}} or {{msg-mw|notification-page-linked-email-batch-bundle-body}} * $2 is the page being linked. * $3 is the e-mail footer, {{msg-mw|echo-email-footer-default}}. -* $4 is the page linked from See also: * {{msg-mw|Notification-page-linked}} * {{msg-mw|Notification-page-linked-flyout}} @@ -443,17 +444,6 @@ The header text for each notification section which is grouped by date * $1 is the month, it could be {{msg-mw|january-gen}}, {{msg-mw|february-gen}}, {{msg-mw|march-gen}}, {{msg-mw|april-gen}}, {{msg-mw|may-gen}}, {{msg-mw|june-gen}}, {{msg-mw|july-gen}}, {{msg-mw|august-gen}}, {{msg-mw|september-gen}}, {{msg-mw|october-gen}}, {{msg-mw|november-gen}}, {{msg-mw|december-gen}} * $2 is the date of a month, eg 21', 'echo-load-more-error' => 'Error message for errors in loading more notifications', - 'notification-edit-talk-page-bundle' => 'Bundled message for edit-user-talk notification. Parameters: -* $1 - the username who performs the action, which can be used for gender support -* $2 - the username -* $3 - the count of other action performers, could be number or {{msg-mw|echo-notification-count}}, eg, 7 others or 99+ others -* $4 - a number used for plural support', - 'notification-page-linked-bundle' => 'Bundled message for page-linked notification. Parameters: -* $1 - the username who performs the action, which can be used for gender support -* $2 - the page title -* $3 - the page linked from -* $4 - the count of other action performers, could be number or {{msg-mw|echo-notification-count}}, eg, 7 others or 99+ others -* $5 - a number used for plural support', 'echo-email-batch-separator' => '{{optional}} Email batch content separator', 'echo-email-batch-bullet' => '{{optional}}', @@ -492,6 +482,29 @@ See also: ** {{msg-mw|Echo-category-title-mention}} ** {{msg-mw|Echo-category-title-other}} ** {{msg-mw|Echo-category-title-system}}', + + // Bundle + 'notification-edit-talk-page-bundle' => 'Bundled message for edit-user-talk notification. Parameters: +* $1 is the username who performs the action, which can be used for gender support +* $2 is the username +* $3 is the count of other action performers, could be number or {{msg:-mew|echo-notification-count}}, eg, 7 others or 99+ others +* $4 is a number used for plural support', + 'notification-page-linked-bundle' => 'Bundled message for page-linked notification. Parameters: +* $1 is the username who performs the action, which can be used for gender support +* $2 is the page title +* $3 is the page linked from +* $4 is the count of other action performers, could be number or {{msg:-mew|echo-notification-count}}, eg, 7 others or 99+ others +* $5 is a number used for plural support', + 'notification-edit-user-talk-email-batch-bundle-body' => 'Bundled message for edit-user-talk email digest notification. Parameters: +* $1 is the username who performs the action, which can be used for gender support +* $2 is the count of other action performers, could be number or {{msg:-mew|echo-notification-count}} +* $3 is a number used for plural support', + 'notification-page-linked-email-batch-bundle-body' => 'Bundled message for page-linked email digest notification. Parameters: +* $1 is the username who performs the action, which can be used for gender support +* $2 is the link-to page title +* $3 is the link-from page title +* $4 is the cout of other link-from page title, can be number or {{msg:-mew|echo-notification-count}} +* $5 is a number used for plural support', ); /** Afrikaans (Afrikaans) diff --git a/Echo.php b/Echo.php index 02c7af1f3..60868e9ff 100644 --- a/Echo.php +++ b/Echo.php @@ -49,6 +49,8 @@ $wgAutoloadClasses['EchoEvent'] = $dir . 'model/Event.php'; $wgAutoloadClasses['EchoNotification'] = $dir . 'model/Notification.php'; $wgAutoloadClasses['MWEchoEmailBatch'] = $dir . 'includes/EmailBatch.php'; $wgAutoloadClasses['MWDbEchoEmailBatch'] = $dir . 'includes/DbEmailBatch.php'; +$wgAutoloadClasses['MWEchoEmailBundler'] = $dir . 'includes/EmailBundler.php'; +$wgAutoloadClasses['MWDbEchoEmailBundler'] = $dir . 'includes/DbEmailBundler.php'; // Formatters $wgAutoloadClasses['EchoNotificationFormatter'] = $dir . 'formatters/NotificationFormatter.php'; @@ -66,6 +68,8 @@ $wgAutoloadClasses['EchoDiscussionParser'] = $dir . 'includes/DiscussionParser.p // Job queue $wgAutoloadClasses['EchoNotificationJob'] = $dir . 'jobs/NotificationJob.php'; $wgJobClasses['EchoNotificationJob'] = 'EchoNotificationJob'; +$wgAutoloadClasses['MWEchoNotificationEmailBundleJob'] = $dir . 'jobs/NotificationEmailBundleJob.php'; +$wgJobClasses['MWEchoNotificationEmailBundleJob'] = 'MWEchoNotificationEmailBundleJob'; // API $wgAutoloadClasses['ApiEchoNotifications'] = $dir . 'api/ApiEchoNotifications.php'; @@ -220,6 +224,11 @@ $wgEchoCluster = false; // The max number showed in bundled message, eg, and 99+ others $wgEchoMaxNotificationCount = 99; +// The time interval between each bundle email in seconds +// set a small number for test wikis, should set this to 0 to disable email bundling +// if there is no delay queue support +$wgEchoBundleEmailInterval = 0; + // Define which output formats are available for each notification category $wgEchoDefaultNotificationTypes = array( 'all' => array( @@ -298,9 +307,11 @@ $wgEchoNotifications = array( 'flyout-params' => array( 'agent', 'user' ), 'email-subject-message' => 'notification-edit-talk-page-email-subject2', 'email-body-message' => 'notification-edit-talk-page-email-body2', - 'email-body-params' => array( 'agent', 'titlelink', 'summary', 'email-footer' ), + 'email-body-params' => array( 'email-intro', 'titlelink', 'summary', 'email-footer' ), 'email-body-batch-message' => 'notification-edit-talk-page-email-batch-body2', - 'email-body-batch-params' => array( 'agent', 'difflink', 'summary' ), + 'email-body-batch-params' => array( 'agent' ), + 'email-body-batch-bundle-message' => 'notification-edit-user-talk-email-batch-bundle-body', + 'email-body-batch-bundle-params' => array( 'agent', 'agent-other-display', 'agent-other-count' ), 'icon' => 'chat', ), 'reverted' => array( @@ -323,7 +334,7 @@ $wgEchoNotifications = array( 'page-linked' => array( 'category' => 'article-linked', 'group' => 'positive', - 'bundle' => array( 'web' => true, 'email' => false ), + 'bundle' => array( 'web' => true, 'email' => true ), 'formatter-class' => 'EchoPageLinkFormatter', 'title-message' => 'notification-page-linked', 'title-params' => array( 'agent', 'title', 'link-from-page' ), @@ -335,9 +346,11 @@ $wgEchoNotifications = array( 'email-subject-message' => 'notification-page-linked-email-subject', 'email-subject-params' => array(), 'email-body-message' => 'notification-page-linked-email-body', - 'email-body-params' => array( 'agent', 'title', 'email-footer', 'link-from-page' ), + 'email-body-params' => array( 'email-intro', 'title', 'email-footer', 'link-from-page' ), 'email-body-batch-message' => 'notification-page-linked-email-batch-body', 'email-body-batch-params' => array( 'agent', 'title', 'link-from-page' ), + 'email-body-batch-bundle-message' => 'notification-page-linked-email-batch-bundle-body', + 'email-body-batch-bundle-params' => array( 'agent', 'title', 'link-from-page', 'link-from-page-other-display', 'link-from-page-other-count' ), 'icon' => 'linked', ), 'mention' => array( diff --git a/Hooks.php b/Hooks.php index 5b8b2b61a..961622899 100644 --- a/Hooks.php +++ b/Hooks.php @@ -82,6 +82,8 @@ class EchoHooks { $updater->addExtensionIndex( 'echo_event', 'event_type', "$dir/db_patches/patch-alter-type_page-index.sql" ); $updater->dropTable( 'echo_subscription' ); $updater->dropExtensionField( 'echo_event', 'event_timestamp', "$dir/db_patches/patch-drop-echo_event-event_timestamp.sql" ); + $updater->addExtensionField( 'echo_email_batch', 'eeb_event_hash', + "$dir/db_patches/patch-email_batch-new-field.sql" ); return true; } diff --git a/Notifier.php b/Notifier.php index e05de9a70..5177d0a20 100644 --- a/Notifier.php +++ b/Notifier.php @@ -62,29 +62,63 @@ class EchoNotifier { // No valid email address return false; } - // See if the user wants to receive emails for this category of event + + // See if the user wants to receive emails for this category if ( $user->getOption( 'echo-subscriptions-email-' . $event->getCategory() ) ) { - global $wgEchoEnableEmailBatch, $wgPasswordSender, $wgPasswordSenderName; - - // batched email notification + global $wgEchoEnableEmailBatch, $wgEchoNotifications, $wgPasswordSender, $wgPasswordSenderName, $wgEchoBundleEmailInterval; + + $priority = EchoNotificationController::getNotificationPriority( $event->getType() ); + + $bundleString = $bundleHash = ''; + + // We should have bundling for email digest as long as either web or email bundling is on, for example, talk page + // email bundling is off, but if a user decides to receive email digest, we should bundle those messages + if ( !empty( $wgEchoNotifications[$event->getType()]['bundle']['web'] ) || !empty( $wgEchoNotifications[$event->getType()]['bundle']['email'] ) ) { + wfRunHooks( 'EchoGetBundleRules', array( $event, &$bundleString ) ); + } + if ( $bundleString ) { + $bundleHash = md5( $bundleString ); + } + + // email digest notification ( weekly or daily ) if ( $wgEchoEnableEmailBatch && $user->getOption( 'echo-email-frequency' ) > 0 ) { - $priority = EchoNotificationController::getNotificationPriority( $event->getType() ); - MWEchoEmailBatch::addToQueue( $user->getId(), $event->getId(), $priority ); + // always create a unique event hash for those events don't support bundling + // this is mainly for group by + if ( !$bundleHash ) { + $bundleHash = md5( $event->getType() . '-' . $event->getId() ); + } + MWEchoEmailBatch::addToQueue( $user->getId(), $event->getId(), $priority, $bundleHash ); return true; } // no email notification if ( $user->getOption( 'echo-email-frequency' ) < 0 ) { return false; } - - // instant email notification - $adminAddress = new MailAddress( $wgPasswordSender, $wgPasswordSenderName ); - $address = new MailAddress( $user ); - $email = EchoNotificationController::formatNotification( $event, $user, 'email' ); - $subject = $email['subject']; - $body = $email['body']; - - UserMailer::send( $address, $adminAddress, $subject, $body ); + + $addedToQueue = false; + + // only send bundle email if email bundling is on + if ( $wgEchoBundleEmailInterval && $bundleHash && !empty( $wgEchoNotifications[$event->getType()]['bundle']['email'] ) ) { + $bundler = MWEchoEmailBundler::newFromUserHash( $user, $bundleHash ); + if ( $bundler ) { + $addedToQueue = $bundler->addToEmailBatch( $event->getId(), $priority ); + } + } + + // send single notification if the email wasn't added to queue for bundling + if ( !$addedToQueue ) { + // instant email notification + $adminAddress = new MailAddress( $wgPasswordSender, $wgPasswordSenderName ); + $address = new MailAddress( $user ); + // Since we are sending a single email, should set the bundle hash to null + // if it is set with a value from somewhere else + $event->setBundleHash( null ); + $email = EchoNotificationController::formatNotification( $event, $user, 'email', 'email' ); + $subject = $email['subject']; + $body = $email['body']; + + UserMailer::send( $address, $adminAddress, $subject, $body ); + } } return true; diff --git a/db_patches/patch-email_batch-new-field.sql b/db_patches/patch-email_batch-new-field.sql new file mode 100644 index 000000000..f58e33af0 --- /dev/null +++ b/db_patches/patch-email_batch-new-field.sql @@ -0,0 +1,7 @@ +ALTER TABLE /*_*/echo_email_batch ADD COLUMN eeb_event_hash varchar(32) binary not null; + +DROP INDEX /*i*/echo_email_batch_user_priority_event ON /*_*/echo_email_batch; + +CREATE INDEX /*i*/echo_email_batch_user_hash_priority ON /*_*/echo_email_batch (eeb_user_id, eeb_event_hash, eeb_event_priority); + + diff --git a/echo.sql b/echo.sql index 7868ab130..eb90f68a2 100644 --- a/echo.sql +++ b/echo.sql @@ -34,8 +34,9 @@ CREATE TABLE /*_*/echo_email_batch ( eeb_id int unsigned not null primary key auto_increment, eeb_user_id int unsigned not null, eeb_event_priority tinyint unsigned not null default 10, -- event priority - eeb_event_id int unsigned not null + eeb_event_id int unsigned not null, + eeb_event_hash varchar(32) binary not null ) /*$wgDBTableOptions*/; CREATE UNIQUE INDEX /*i*/echo_email_batch_user_event ON /*_*/echo_email_batch (eeb_user_id,eeb_event_id); -CREATE UNIQUE INDEX /*i*/echo_email_batch_user_priority_event ON /*_*/echo_email_batch (eeb_user_id,eeb_event_priority,eeb_event_id); +CREATE INDEX /*i*/echo_email_batch_user_hash_priority ON /*_*/echo_email_batch (eeb_user_id, eeb_event_hash, eeb_event_priority); diff --git a/formatters/BasicFormatter.php b/formatters/BasicFormatter.php index 5ce27e7e1..c50539cb8 100644 --- a/formatters/BasicFormatter.php +++ b/formatters/BasicFormatter.php @@ -99,12 +99,15 @@ class EchoBasicFormatter extends EchoNotificationFormatter { 'batch-body' => array( 'message' => $params['email-body-batch-message'], 'params' => $params['email-body-batch-params'] + ), + 'batch-bundle-body' => array( + 'message' => $params['email-body-batch-bundle-message'], + 'params' => $params['email-body-batch-bundle-params'] ) ); // Notification icon for the event type $this->icon = $params['icon']; - } /** @@ -125,6 +128,8 @@ class EchoBasicFormatter extends EchoNotificationFormatter { 'email-body-params' => array( 'text-notification' ), 'email-body-batch-message' => 'echo-email-batch-body-default', 'email-body-batch-params' => array(), + 'email-body-batch-bundle-message' => '', + 'email-body-batch-bundle-params' => array(), 'icon' => 'placeholder' ); } @@ -141,7 +146,7 @@ class EchoBasicFormatter extends EchoNotificationFormatter { global $wgEchoNotificationCategories; // Use the bundle message if use-bundle is true and there is a bundle message - $this->generateBundleData( $event, $user ); + $this->generateBundleData( $event, $user, $type ); if ( $this->bundleData['use-bundle'] && isset( $this->bundleTitle['message'] ) ) { $this->title = $this->flyoutTitle = $this->bundleTitle; } @@ -245,7 +250,13 @@ class EchoBasicFormatter extends EchoNotificationFormatter { $body = preg_replace( "/\n{3,}/", "\n\n", $this->formatFragment( $this->email['body'], $event, $user )->text() ); - $batchBody = preg_replace( "/\n{3,}/", "\n\n", $this->formatFragment( $this->email['batch-body'], $event, $user )->text() ); + if ( $this->bundleData['use-bundle'] && $this->email['batch-bundle-body'] ) { + $bodyKey = $this->email['batch-bundle-body']; + } else { + $bodyKey = $this->email['batch-body']; + } + + $batchBody = preg_replace( "/\n{3,}/", "\n\n", $this->formatFragment( $bodyKey, $event, $user )->text() ); return array( 'subject' => $subject, 'body' => $body, 'batch-body' => $batchBody ); } @@ -333,9 +344,10 @@ class EchoBasicFormatter extends EchoNotificationFormatter { * Get raw bundle data for an event so it can be manipulated * @param $event EchoEvent * @param $user User + * @param $type string Notification distribution type: web/email * @return ResultWrapper|bool */ - protected function getRawBundleData( $event, $user ) { + protected function getRawBundleData( $event, $user, $type ) { global $wgEchoBackend; // We should keep bundling for events as long as it has bundle @@ -345,7 +357,7 @@ class EchoBasicFormatter extends EchoNotificationFormatter { return false; } - return $wgEchoBackend->getRawBundleData( $user, $event->getBundleHash() ); + return $wgEchoBackend->getRawBundleData( $user, $event->getBundleHash(), $type ); } /** @@ -354,11 +366,12 @@ class EchoBasicFormatter extends EchoNotificationFormatter { * this function to use a differnt group iterator such as title, namespace * @param $event EchoEvent * @param $user User + * @param $type string Notification distribution type */ - protected function generateBundleData( $event, $user ) { + protected function generateBundleData( $event, $user, $type ) { global $wgEchoMaxNotificationCount; - $data = $this->getRawBundleData( $event, $user ); + $data = $this->getRawBundleData( $event, $user, $type ); if ( !$data ) { return; @@ -468,6 +481,19 @@ class EchoBasicFormatter extends EchoNotificationFormatter { $this->setOutputFormat( $oldOutputFormat ); $message->params( $textNotification ); + } elseif ( $param === 'email-intro' ) { + if ( $this->bundleData['use-bundle'] && isset( $this->email['batch-bundle-body']['message'] ) ) { + $detail = array( + 'message' => $this->email['batch-bundle-body']['message'], + 'params' => $this->email['batch-bundle-body']['params'] + ); + } else { + $detail = array( + 'message' => $this->email['batch-body']['message'], + 'params' => $this->email['batch-body']['params'] + ); + } + $message->params( $this->formatFragment( $detail, $event, $user )->text() ); } elseif ( $param === 'email-footer' ) { global $wgEchoEmailFooterAddress; $message->params( diff --git a/formatters/PageLinkFormatter.php b/formatters/PageLinkFormatter.php index 00b802114..3145b23f0 100644 --- a/formatters/PageLinkFormatter.php +++ b/formatters/PageLinkFormatter.php @@ -11,11 +11,12 @@ class EchoPageLinkFormatter extends EchoBasicFormatter { * link from Page B and X other pages * @param $event EchoEvent * @param $user User + * @param $type string Notification disytribution type */ - protected function generateBundleData( $event, $user ) { + protected function generateBundleData( $event, $user, $type ) { global $wgEchoMaxNotificationCount; - $data = $this->getRawBundleData( $event, $user ); + $data = $this->getRawBundleData( $event, $user, $type ); if ( !$data ) { return; diff --git a/includes/DbEchoBackend.php b/includes/DbEchoBackend.php index ec6fc307a..32b249661 100644 --- a/includes/DbEchoBackend.php +++ b/includes/DbEchoBackend.php @@ -88,32 +88,54 @@ class MWDbEchoBackend extends MWEchoBackend { /** * @param $user User * @param $bundleHash string the bundle hash + * @param $type string * @return ResultWrapper|bool */ - public function getRawBundleData( $user, $bundleHash ) { + public function getRawBundleData( $user, $bundleHash, $type = 'web' ) { // We only display 99+ if the number is over 100, we can do limit 250, this should be sufficient // to return 99 distinct group iterators, avoid select count( distinct ) for the folliwng: // 1. it will not scale for large volume data // 2. notification may have random grouping iterator // 2. agent may be anonymous, can't do distinct over two columens: event_agent_id and event_agent_ip - $res = $this->dbr->select( - array( 'echo_notification', 'echo_event' ), - array( - 'event_agent_id', - 'event_agent_ip', - 'event_extra', - 'event_page_namespace', - 'event_page_title' - ), - array( - 'notification_event=event_id', - 'notification_user' => $user->getId(), - 'notification_bundle_base' => 0, - 'notification_bundle_display_hash' => $bundleHash - ), - __METHOD__, - array( 'ORDER BY' => 'notification_timestamp DESC', 'LIMIT' => 250 ) - ); + if ( $type == 'web' ) { + $res = $this->dbr->select( + array( 'echo_notification', 'echo_event' ), + array( + 'event_agent_id', + 'event_agent_ip', + 'event_extra', + 'event_page_namespace', + 'event_page_title' + ), + array( + 'notification_event=event_id', + 'notification_user' => $user->getId(), + 'notification_bundle_base' => 0, + 'notification_bundle_display_hash' => $bundleHash + ), + __METHOD__, + array( 'ORDER BY' => 'notification_timestamp DESC', 'LIMIT' => 250 ) + ); + // this would be email for now + } else { + $res = $this->dbr->select( + array( 'echo_email_batch', 'echo_event' ), + array( + 'event_agent_id', + 'event_agent_ip', + 'event_extra', + 'event_page_namespace', + 'event_page_title' + ), + array( + 'eeb_event_id=event_id', + 'eeb_user_id' => $user->getId(), + 'eeb_event_hash' => $bundleHash + ), + __METHOD__, + array( 'ORDER BY' => 'eeb_event_id DESC', 'LIMIT' => 250 ) + ); + } return $res; } diff --git a/includes/DbEmailBatch.php b/includes/DbEmailBatch.php index b39106d30..f693344e4 100644 --- a/includes/DbEmailBatch.php +++ b/includes/DbEmailBatch.php @@ -39,6 +39,10 @@ class MWDbEchoEmailBatch extends MWEchoEmailBatch { $validEvents = array_keys( $wgEchoNotifications ); + // Per the tech discussion in the design meeting (03/22/2013), since this is + // processed by a cron job, it's okay to use GROUP BY over more complex + // composite index, favor insert performance, storage space over read + // performance in this case if ( $validEvents ) { $dbr = MWEchoDbFactory::getDB( DB_SLAVE ); @@ -48,6 +52,7 @@ class MWDbEchoEmailBatch extends MWEchoEmailBatch { 'event_type' => $validEvents ); + // See setLastEvent() for more detail for this variable if ( $this->lastEvent ) { $conds[] = 'eeb_event_id <= ' . intval( $this->lastEvent ); } @@ -57,11 +62,19 @@ class MWDbEchoEmailBatch extends MWEchoEmailBatch { array( '*' ), $conds, __METHOD__, - array( 'ORDER BY' => 'eeb_event_priority, eeb_event_id', 'LIMIT' => self::$displaySize + 1 ) + array( + 'ORDER BY' => 'eeb_event_priority', + 'LIMIT' => self::$displaySize + 1, + 'GROUP BY' => 'eeb_event_hash' + ) ); - foreach( $res as $row ) { - $events[$row->eeb_id] = $row; + foreach ( $res as $row ) { + // records in the queue inserted before email bundling code + // have no hash, in this case, we just ignore them + if ( $row->eeb_event_hash ) { + $events[$row->eeb_id] = $row; + } } } @@ -93,8 +106,9 @@ class MWDbEchoEmailBatch extends MWEchoEmailBatch { * @param $userId int * @param $eventId int * @param $priority int + * @param $hash string */ - public static function actuallyAddToQueue( $userId, $eventId, $priority ) { + public static function actuallyAddToQueue( $userId, $eventId, $priority, $hash ) { if ( !$userId || !$eventId ) { return; } @@ -105,7 +119,8 @@ class MWDbEchoEmailBatch extends MWEchoEmailBatch { array( 'eeb_user_id' => $userId, 'eeb_event_id' => $eventId, - 'eeb_event_priority' => $priority + 'eeb_event_priority' => $priority, + 'eeb_event_hash' => $hash ), __METHOD__, array( 'IGNORE' ) diff --git a/includes/DbEmailBundler.php b/includes/DbEmailBundler.php new file mode 100644 index 000000000..b3dbb8b72 --- /dev/null +++ b/includes/DbEmailBundler.php @@ -0,0 +1,51 @@ +selectRow( + array( 'echo_email_batch' ), + array( 'eeb_event_id' ), + array( + 'eeb_user_id' => $this->mUser->getId(), + 'eeb_event_hash' => $this->bundleHash + ), + __METHOD__, + array( 'ORDER BY' => 'eeb_event_priority DESC, eeb_id DESC', 'LIMIT' => 1 ) + ); + if ( !$res ) { + return false; + } + $this->baseEvent = EchoEvent::newFromId( $res->eeb_event_id ); + return true; + } + + /** + * Clear processed events from the queue + */ + protected function clearProcessedEvent() { + if ( !$this->baseEvent ) { + return; + } + $conds = array( 'eeb_user_id' => $this->mUser->getId(), 'eeb_event_hash' => $this->bundleHash ); + + $conds[] = 'eeb_event_id <= ' . intval( $this->baseEvent->getId() ); + + $dbw = MWEchoDbFactory::getDB( DB_MASTER ); + $dbw->delete( + 'echo_email_batch', + $conds, + __METHOD__, + array() + ); + } + +} diff --git a/includes/EchoBackend.php b/includes/EchoBackend.php index 8cfb72d8f..a3dc9c383 100644 --- a/includes/EchoBackend.php +++ b/includes/EchoBackend.php @@ -75,9 +75,10 @@ abstract class MWEchoBackend { * Get the bundle data for user/hash * @param $user User * @param $bundleHash string The hash used to identify a set of bundle-able events + * @param $type string 'web'/'email' * @return ResultWrapper|bool */ - abstract public function getRawBundleData( $user, $bundleHash ); + abstract public function getRawBundleData( $user, $bundleHash, $type = 'web' ); /** * Get the last bundle stat - read_timestamp & bundle_display_hash diff --git a/includes/EmailBatch.php b/includes/EmailBatch.php index 7ec03bca6..8caf9b5ce 100644 --- a/includes/EmailBatch.php +++ b/includes/EmailBatch.php @@ -15,9 +15,9 @@ abstract class MWEchoEmailBatch { // the event count, this count is supported up to self::$displaySize + 1 protected $count = 0; - // number of events to include in an email, we couldn't include + // number of bundle events to include in an email, we couldn't include // all events in a batch email - protected static $displaySize = 10; + protected static $displaySize = 20; /** * @param $user User @@ -51,6 +51,17 @@ abstract class MWEchoEmailBatch { return false; } + // @Todo - There may be some items idling in the queue, eg, a bundle job is lost + // and there is not never another message with the same hash or a user switches from + // digest to instant. We should check the first item in the queue, if it doesn't + // have either web or email bundling or created long ago, then clear it, this will + // prevent idling item queuing up. + + // user has instant email delivery + if ( $userEmailSetting == 0 ) { + return false; + } + $userLastBatch = $user->getOption( 'echo-email-last-batch' ); // send email batch, if @@ -106,7 +117,7 @@ abstract class MWEchoEmailBatch { break; } $event = EchoEvent::newFromRow( $row ); - $this->appendContent( $event ); + $this->appendContent( $event, $row->eeb_event_hash ); } $this->sendEmail(); @@ -144,11 +155,12 @@ abstract class MWEchoEmailBatch { /** * Add individual event template to the big email content */ - protected function appendContent( $event ) { + protected function appendContent( $event, $hash ) { // get the category for this event $category = $event->getCategory(); + $event->setBundleHash( $hash ); + $email = EchoNotificationController::formatNotification( $event, $this->mUser, 'email', 'email' ); - $email = EchoNotificationController::formatNotification( $event, $this->mUser, 'email' ); if ( !isset( $this->content[$category] ) ) { $this->content[$category] = array(); } @@ -238,6 +250,7 @@ abstract class MWEchoEmailBatch { $adminAddress = new MailAddress( $wgPasswordSender, $wgPasswordSenderName ); $address = new MailAddress( $this->mUser ); + // @Todo Push the email to job queue or just send it out directly? UserMailer::send( $address, $adminAddress, $subject, $body ); } @@ -246,15 +259,16 @@ abstract class MWEchoEmailBatch { * @param $userId int * @param $eventId int * @param $priority int + * @param $hash string */ - public static function addToQueue( $userId, $eventId, $priority ) { + public static function addToQueue( $userId, $eventId, $priority, $hash ) { $batchClassName = self::getEmailBatchClass(); if ( !method_exists( $batchClassName, 'actuallyAddToQueue' ) ) { throw new MWException( "$batchClassName must implement method actuallyAddToQueue()" ); } - $batchClassName::actuallyAddToQueue( $userId, $eventId, $priority ); + $batchClassName::actuallyAddToQueue( $userId, $eventId, $priority, $hash ); } /** diff --git a/includes/EmailBundler.php b/includes/EmailBundler.php new file mode 100644 index 000000000..ad03938f7 --- /dev/null +++ b/includes/EmailBundler.php @@ -0,0 +1,286 @@ +mUser = $user; + $this->bundleHash = $hash; + $this->emailInterval = $wgEchoBundleEmailInterval; + + if ( $this->emailInterval < 0 ) { + $this->emailInterval = 0; + } + } + + /** + * Get the name of the email batch class + * @return string + * @throws MWException + */ + private static function getEmailBundlerClass() { + global $wgEchoBackendName; + + $className = 'MW' . $wgEchoBackendName . 'EchoEmailBundler'; + + if ( !class_exists( $className ) ) { + throw new MWException( "$wgEchoBackendName email batch is not supported!" ); + } + + return $className; + } + + /** + * Factory method + */ + public static function newFromUserHash( User $user, $hash ) { + if ( !$user->getId() ) { + return false; + } + if ( !$hash || !preg_match( '/^[a-f0-9]{32}$/', $hash ) ) { + return false; + } + $className = self::getEmailBundlerClass(); + return new $className( $user, $hash ); + } + + /** + * Check if a new notification should be added to the batch queue + * true - added to the queue for bundling email + * false - not added, the client should send single email + * @return bool + */ + public function addToEmailBatch( $eventId, $eventPriority ) { + $this->retrieveLastEmailTimestamp(); + $this->retrieveBaseEvent(); + + // send instant single notification email if there is no base event in the batch queue + // and the email is ready to send, otherwiase, add the email to batch and schedule + // a delayed job + if ( !$this->baseEvent && $this->shouldSendEmailNow() ) { + $this->timestamp = wfTimestampNow(); + $this->updateEmailMetadata(); + return false; + } else { + // add to email batch queue + MWEchoEmailBatch::addToQueue( + $this->mUser->getId(), + $eventId, + $eventPriority, + $this->bundleHash + ); + + // always push the job to job queue in case the previous job + // was lost, job queue will ignore duplicate + $this->pushToJobQueue( $this->getDelayTime() ); + return true; + } + } + + /** + * Get the time diff since last email + */ + protected function timeSinceLastEmail() { + // if there is no timestamp, next email should be sent right away + // set the time diff longer than the email interval + if ( !$this->timestamp ) { + return $this->emailInterval + 600; + } + + static $now; + + if ( !$now ) { + $now = wfTimestamp( TS_UNIX ); + } + + return $now - wfTimestamp( TS_UNIX, $this->timestamp ); + } + + /** + * Check if an email should be sent right away + * @return bool + */ + protected function shouldSendEmailNow() { + if ( $this->timeSinceLastEmail() > $this->emailInterval ) { + return true; + } else { + return false; + } + } + + /** + * Get the delay time + * @return int + */ + protected function getDelayTime() { + $delay = $this->emailInterval - $this->timeSinceLastEmail(); + if ( $delay <= 0 ) { + $delay = 0; + } + return $delay; + } + + /** + * Get the timestamp of last email + */ + protected function retrieveLastEmailTimestamp() { + global $wgMemc; + + $data = $wgMemc->get( $this->getMemcacheKey() ); + if ( $data !== false ) { + $this->timestamp = $data['timestamp']; + } + } + + /** + * Get the memcache key + * @return string + */ + protected function getMemcacheKey() { + return wfMemcKey( 'echo', 'email_bundle_status', $this->mUser->getId(), $this->bundleHash ); + } + + /** + * Retrieve the base event for email bundling + * @return bool + */ + abstract protected function retrieveBaseEvent(); + + /** + * Push the latest bundle data to the queue + * @param $delay int To delay the job in $delay seconds + */ + public function pushToJobQueue( $delay = 0 ) { + $title = Title::newMainPage(); + $job = new MWEchoNotificationEmailBundleJob( + $title, + array( + 'user_id' => $this->mUser->getId(), + 'bundle_hash' => $this->bundleHash, + 'jobReleaseTimestamp' => wfTimestamp( TS_MW, wfTimestamp( TS_UNIX ) + $delay ) + ) + ); + JobQueueGroup::singleton()->push( $job ); + } + + /** + * Main function for processinig bundle email + */ + public function processBundleEmail() { + $this->retrieveLastEmailTimestamp(); + + // if there is nothing in the queue, do not schedule a job or + // update timestamp so next email would be just an instant email + if ( $this->retrieveBaseEvent() ) { + $this->timestamp = wfTimestampNow(); + $this->updateEmailMetadata(); + $this->sendEmail(); + // for testing purpose, comment out the line below so events are kept + $this->clearProcessedEvent(); + $this->pushToJobQueue( $this->emailInterval ); + } + } + + /** + * Send the bundle email + */ + protected function sendEmail() { + $content = $this->generateEmailContent(); + + // Error has occurred + // @Todo more error handling + if ( !isset( $content['subject'] ) || !isset( $content['body'] ) ) { + return; + } + + global $wgPasswordSender, $wgPasswordSenderName; + + $adminAddress = new MailAddress( $wgPasswordSender, $wgPasswordSenderName ); + $address = new MailAddress( $this->mUser ); + + // Schedule a email job or just send the email directly? + UserMailer::send( $address, $adminAddress, $content['subject'], $content['body'] ); + } + + /** + * Generate the content for bundle email + * @return string + */ + protected function generateEmailContent() { + if ( !$this->baseEvent ) { + return ''; + } + $this->baseEvent->setBundleHash( $this->bundleHash ); + return EchoNotificationController::formatNotification( $this->baseEvent, $this->mUser, 'email', 'email' ); + } + + /** + * Update bundle email metadata for user/hash pair + */ + protected function updateEmailMetadata() { + global $wgMemc; + + $key = $this->getMemcacheKey(); + + // Delete existing data + $wgMemc->delete( $key ); + // Store new data and make it expire in 7 days + $wgMemc->set( + $key, + array( + 'timestamp' => $this->timestamp + ), + 3600 * 24 * 7 + ); + } + + /** + * clear processed event in the queue + */ + abstract protected function clearProcessedEvent(); + +} diff --git a/jobs/NotificationEmailBundleJob.php b/jobs/NotificationEmailBundleJob.php new file mode 100644 index 000000000..22deebe29 --- /dev/null +++ b/jobs/NotificationEmailBundleJob.php @@ -0,0 +1,24 @@ +removeDuplicates = true; + } + + function run() { + $user = User::newFromId( $this->params['user_id'] ); + if ( $user ) { + $bundle = MWEchoEmailBundler::newFromUserHash( $user, $this->params['bundle_hash'] ); + if ( $bundle ) { + $bundle->processBundleEmail(); + } + } else { + //@Todo: delete notifications for this user_id + } + return true; + } +}