From 8a082cbf5cbc4b8734bc961f7e96e0952bffca9a Mon Sep 17 00:00:00 2001
From: Sam Reed <reedy@users.mediawiki.org>
Date: Thu, 22 Sep 2011 12:14:21 +0000
Subject: [PATCH] * (bug 31081) $wgEnotifUseJobQ causes many unnecessary jobs
 to be queued

Do some of the cheap checks before spawning attempting to send emails via any method
---
 RELEASE-NOTES-1.19               |  1 +
 includes/UserMailer.php          | 77 +++++++++++++++++++++++---------
 includes/job/EnotifNotifyJob.php |  2 +-
 3 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19
index ca273b99d8..9f8bf988e0 100644
--- a/RELEASE-NOTES-1.19
+++ b/RELEASE-NOTES-1.19
@@ -96,6 +96,7 @@ production.
   really small, and somewhat inconsistent with each other.
 * Per page edit-notices now work in namespaces without subpages enabled.
 * (bug 30245) Use the correct way to construct a log page title
+* (bug 31081) $wgEnotifUseJobQ causes many unnecessary jobs to be queued
 
 === API changes in 1.19 ===
 * (bug 19838) siprop=interwikimap can now use the interwiki cache.
diff --git a/includes/UserMailer.php b/includes/UserMailer.php
index 171f2a13db..d95a5c5652 100644
--- a/includes/UserMailer.php
+++ b/includes/UserMailer.php
@@ -364,7 +364,8 @@ class EmailNotification {
 	 * @param $oldid (default: false)
 	 */
 	public function notifyOnPageChange( $editor, $title, $timestamp, $summary, $minorEdit, $oldid = false ) {
-		global $wgEnotifUseJobQ, $wgEnotifWatchlist, $wgShowUpdatedMarker;
+		global $wgEnotifUseJobQ, $wgEnotifWatchlist, $wgShowUpdatedMarker, $wgEnotifMinorEdits,
+			$wgUsersNotifiedOnAllChanges, $wgEnotifUserTalk;
 
 		if ( $title->getNamespace() < 0 ) {
 			return;
@@ -403,6 +404,24 @@ class EmailNotification {
 			}
 		}
 
+		$sendEmail = true;
+		// If nobody is watching the page, and there are no users notified on all changes
+		// don't bother creating a job/trying to send emails
+		// $watchers deals with $wgEnotifWatchlist
+		if ( !count( $watchers ) && !count( $wgUsersNotifiedOnAllChanges ) ) {
+			$sendEmail = false;
+			// Only send notification for non minor edits, unless $wgEnotifMinorEdits
+			if ( !$minorEdit || ( $wgEnotifMinorEdits && !$editor->isAllowed( 'nominornewtalk' ) ) ) {
+				$isUserTalkPage = ( $title->getNamespace() == NS_USER_TALK );
+				if ( $wgEnotifUserTalk && $isUserTalkPage && $this->canSendUserTalkEmail( $editor, $title, $minorEdit ) ) {
+					$sendEmail = true;
+				}
+			}
+		}
+
+		if ( !$sendEmail ) {
+			return;
+		}
 		if ( $wgEnotifUseJobQ ) {
 			$params = array(
 				'editor' => $editor->getName(),
@@ -418,7 +437,6 @@ class EmailNotification {
 		} else {
 			$this->actuallyNotifyOnPageChange( $editor, $title, $timestamp, $summary, $minorEdit, $oldid, $watchers );
 		}
-
 	}
 
 	/**
@@ -459,25 +477,11 @@ class EmailNotification {
 		$userTalkId = false;
 
 		if ( !$minorEdit || ( $wgEnotifMinorEdits && !$editor->isAllowed( 'nominornewtalk' ) ) ) {
-			if ( $wgEnotifUserTalk && $isUserTalkPage ) {
+
+			if ( $wgEnotifUserTalk && $isUserTalkPage && $this->canSendUserTalkEmail( $editor, $title, $minorEdit ) ) {
 				$targetUser = User::newFromName( $title->getText() );
-				if ( !$targetUser || $targetUser->isAnon() ) {
-					wfDebug( __METHOD__ . ": user talk page edited, but user does not exist\n" );
-				} elseif ( $targetUser->getId() == $editor->getId() ) {
-					wfDebug( __METHOD__ . ": user edited their own talk page, no notification sent\n" );
-				} elseif ( $targetUser->getOption( 'enotifusertalkpages' ) &&
-					( !$minorEdit || $targetUser->getOption( 'enotifminoredits' ) ) )
-				{
-					if ( $targetUser->isEmailConfirmed() ) {
-						wfDebug( __METHOD__ . ": sending talk page update notification\n" );
-						$this->compose( $targetUser );
-						$userTalkId = $targetUser->getId();
-					} else {
-						wfDebug( __METHOD__ . ": talk page owner doesn't have validated email\n" );
-					}
-				} else {
-					wfDebug( __METHOD__ . ": talk page owner doesn't want notifications\n" );
-				}
+				$this->compose( $targetUser );
+				$userTalkId = $targetUser->getId();
 			}
 
 			if ( $wgEnotifWatchlist ) {
@@ -505,6 +509,39 @@ class EmailNotification {
 		wfProfileOut( __METHOD__ );
 	}
 
+	/**
+	 * @param $editor User
+	 * @param $title Title bool
+	 * @param $minorEdit
+	 * @return bool
+	 */
+	private function canSendUserTalkEmail( $editor, $title, $minorEdit ) {
+		global $wgEnotifUserTalk;
+		$isUserTalkPage = ( $title->getNamespace() == NS_USER_TALK );
+
+		if ( $wgEnotifUserTalk && $isUserTalkPage ) {
+			$targetUser = User::newFromName( $title->getText() );
+
+			if ( !$targetUser || $targetUser->isAnon() ) {
+				wfDebug( __METHOD__ . ": user talk page edited, but user does not exist\n" );
+			} elseif ( $targetUser->getId() == $editor->getId() ) {
+				wfDebug( __METHOD__ . ": user edited their own talk page, no notification sent\n" );
+			} elseif ( $targetUser->getOption( 'enotifusertalkpages' ) &&
+				( !$minorEdit || $targetUser->getOption( 'enotifminoredits' ) ) )
+			{
+				if ( $targetUser->isEmailConfirmed() ) {
+					wfDebug( __METHOD__ . ": sending talk page update notification\n" );
+					return true;
+				} else {
+					wfDebug( __METHOD__ . ": talk page owner doesn't have validated email\n" );
+				}
+			} else {
+				wfDebug( __METHOD__ . ": talk page owner doesn't want notifications\n" );
+			}
+		}
+	    return false;
+	}
+
 	/**
 	 * Generate the generic "this page has been changed" e-mail text.
 	 */
diff --git a/includes/job/EnotifNotifyJob.php b/includes/job/EnotifNotifyJob.php
index 5d2a08ead5..8545043f0c 100644
--- a/includes/job/EnotifNotifyJob.php
+++ b/includes/job/EnotifNotifyJob.php
@@ -20,7 +20,7 @@ class EnotifNotifyJob extends Job {
 	function run() {
 		$enotif = new EmailNotification();
 		// Get the user from ID (rename safe). Anons are 0, so defer to name.
-		if( isset($this->params['editorID']) && $this->params['editorID'] ) {
+		if( isset( $this->params['editorID'] ) && $this->params['editorID'] ) {
 			$editor = User::newFromId( $this->params['editorID'] );
 		// B/C, only the name might be given.
 		} else {
-- 
2.20.1