Generalize a couple implementations of EchoGetDefaultNotifiedUsers

There are a variety of generic strategies you could define to choose
which users should be notified about an event, such as 'users watching
the title' or 'talk page owner' (User_talk only).

This adds a new way to generically implement these in Echo and expose them
to other extensions rather than having each extension implement these
generic strategies themselves.

This patch only converts one notification, edit-user-talk. The remaining
notifications will be converted in future patches. The first user of this
will be Flow for notifying all users watching a particular talk page in
I4e46a9c003fbdde274b20ac7aef8455eab4a5222

The users watching title implementation provided here is minimalist, a larger
refactor to accomidate pages with thousands of watchers is being handled
in I3d3fa9328f348bb48682d3658622952ce82d3925

Change-Id: I19bb6a794d22565f3bb5421de92426d390197796

Echo.php

@@ -89,6 +89,7 @@ $wgAutoloadClasses['EchoHTMLEmailDecorator'] = $dir . 'includes/EmailFormatter.p
 
 // Internal stuff
 $wgAutoloadClasses['EchoNotifier'] = $dir . 'Notifier.php';
+$wgAutoloadClasses['EchoUserLocator'] = $dir . 'includes/UserLocator.php';
 $wgAutoloadClasses['EchoNotificationController'] = $dir . 'controller/NotificationController.php';
 $wgAutoloadClasses['EchoDiscussionParser'] = $dir . 'includes/DiscussionParser.php';
 $wgAutoloadClasses['EchoDiffParser'] = $dir . 'includes/DiffParser.php';
@@ -442,6 +443,9 @@ $wgEchoNotifications = array(
 		'icon' => 'site',
 	),
 	'edit-user-talk' => array(
+		'user-locators' => array(
+			'EchoUserLocator::locateTalkPageOwner',
+		),
 		'primary-link' => array( 'message' => 'notification-link-text-view-message', 'destination' => 'section' ),
 		'secondary-link' => array( 'message' => 'notification-link-text-view-changes', 'destination' => 'diff' ),
 		'category' => 'edit-user-talk',

Hooks.php

@@ -134,19 +134,6 @@ class EchoHooks {
 	 */
 	public static function getDefaultNotifiedUsers( $event, &$users ) {
 		switch ( $event->getType() ) {
-			// Everyone deserves to know when something happens
-			// on their user talk page
-			case 'edit-user-talk':
-				if ( !$event->getTitle() || !$event->getTitle()->getNamespace() == NS_USER_TALK ) {
-					break;
-				}
-
-				$username = $event->getTitle()->getText();
-				$user = User::newFromName( $username );
-				if ( $user && $user->getId() ) {
-					$users[$user->getId()] = $user;
-				}
-				break;
 			case 'add-comment':
 			case 'add-talkpage-topic':
 				// Handled by EchoDiscussionParser

controller/NotificationController.php

@@ -87,7 +87,7 @@ class EchoNotificationController {
 					continue;
 				}
 				if ( $blacklisted && !self::isWhitelistedByUser( $event, $user ) ) {
-					continue;
+	 				continue;
 				}
 
 				wfRunHooks( 'EchoGetNotificationTypes', array( $user, $event, &$notifyTypes ) );
@@ -190,13 +190,27 @@ class EchoNotificationController {
 	/**
 	 * Retrieves an array of User objects to be notified for an EchoEvent.
 	 *
-	 * @param $event EchoEvent to retrieve users to be notified for.
-	 * @return Array of User objects
+	 * @param EchoEvent $event
+	 * @return array keys are user ids, values are User objects
 	 */
-	protected static function getUsersToNotifyForEvent( $event ) {
-		$users = $notifyList = array();
+	public static function getUsersToNotifyForEvent( EchoEvent $event ) {
+		$type = $event->getType();
+		// Key notifyList by user id to ensure there are no duplicated users
+		$notifyList = array();
+
+		$attributeManager = EchoAttributeManager::newFromGlobalVars();
+		foreach ( $attributeManager->getUserLocators( $type ) as $callable ) {
+			if ( is_callable( $callable ) ) {
+				$notifyList += call_user_func( $callable, $event );
+			} else {
+				wfDebugLog( __CLASS__, __FUNCTION__ . ": Invalid user-locator returned for $type" );
+			}
+		}
+
+		// hook for injecting more users.
+		// @deprecated
+		$users = array();
 		wfRunHooks( 'EchoGetDefaultNotifiedUsers', array( $event, &$users ) );
-		// Make sure there is no duplicated users
 		foreach ( $users as $user ) {
 			$notifyList[$user->getId()] = $user;
 		}
@@ -259,7 +273,7 @@ class EchoNotificationController {
 	/**
 	 * INTERNAL.  Must be public to be callable by the php error handling methods.
 	 *
-	 * Converts E_RECOVERABLE_ERROR, such as passing null to a method expecting 
+	 * Converts E_RECOVERABLE_ERROR, such as passing null to a method expecting
 	 * a non-null object, into exceptions.
 	 */
 	public static function formatterErrorHandler( $errno, $errstr, $errfile, $errline ) {

includes/AttributeManager.php

@@ -55,6 +55,21 @@ class EchoAttributeManager {
 		return self::$globalVarInstance[$wikiId];
 	}
 
+	/**
+	 * Get the user-locators related to the provided event type
+	 *
+	 * @param string $type
+	 * @return array
+	 */
+	public function getUserLocators( $type ) {
+		if ( isset( $this->notifications[$type]['user-locators'] ) ) {
+			return (array)$this->notifications[$type]['user-locators'];
+		} else {
+			wfDebugLog( 'Echo', __METHOD__ . ": No user-locators configured for $type" );
+			return array();
+		}
+	}
+
 	/**
 	 * Get the enabled events for a user, which excludes user-dismissed events
 	 * from the general enabled events

includes/UserLocator.php

@@ -0,0 +1,49 @@
+<?php
+
+class EchoUserLocator {
+
+	/**
+	 * The echo job queue must be enabled to prevent timeouts submitting to
+	 * heavily watched pages when this is used.
+	 */
+	public static function locateUsersWatchingTitle( EchoEvent $event, $batchSize = 500 ) {
+		$title = $event->getTitle();
+		if ( !$title ) {
+			return array();
+		}
+
+		$dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+		$res = $dbr->select(
+			array( 'watchlist' ),
+			array( 'wl_user' ),
+			array(
+				'wl_namespace' => $title->getNamespace(),
+				'wl_title' => $title->getDBkey(),
+			),
+			__METHOD__
+		);
+
+		$users = array();
+		if ( $res ) {
+			foreach ( $res as $row ) {
+				$users[$row->wl_user] = User::newFromId( $row->wl_user );
+			}
+		}
+
+		return $users;
+	}
+
+	public static function locateTalkPageOwner( EchoEvent $event ) {
+		$title = $event->getTitle();
+		if ( !$title || $title->getNamespace() !== NS_USER_TALK ) {
+			return array();
+		}
+
+		$user = User::newFromName( $title->getDBkey() );
+		if ( $user && !$user->isAnon() ) {
+			return array( $user->getId() => $user );
+		} else {
+			return array();
+		}
+	}
+}

tests/NotificationControllerTest.php

@@ -0,0 +1,67 @@
+<?php
+
+class NotificationControllerTest extends MediaWikiTestCase {
+
+	public function getUsersToNotifyForEventProvider() {
+		return array(
+			array(
+				'With no options no users are notified',
+				// expected result
+				array(),
+				// event user locator config
+				array(),
+			),
+
+			array(
+				'Does not error when given non-existant user-locator',
+				// expected result
+				array(),
+				// event user locator config
+				array( 'not-callable' ),
+			),
+
+			array(
+				'Calls selected locator and returns result',
+				// expected result
+				array( 123 ),
+				// event user locator config
+				function() { return array( 123 => 123 ); }
+			),
+
+			array(
+				'merges results of multiple locators',
+				// expected result
+				array( 123, 456 ),
+				// event user locator config
+				array(
+					function() { return array( 123 => 123 ); },
+					function() { return array( 456 => 456 ); },
+				),
+			),
+
+		);
+	}
+
+	/**
+	 * @dataProvider getUsersToNotifyForEventProvider
+	 */
+	public function testGetUsersToNotifyForEvent( $message, $expect, $locatorConfigForEventType ) {
+		$this->setMwGlobals( array(
+			'wgEchoNotifications' => array(
+				'unit-test' => array(
+					'user-locators' => $locatorConfigForEventType
+				),
+			),
+		) );
+
+		$event = $this->getMockBuilder( 'EchoEvent' )
+			->disableOriginalConstructor()
+			->getMock();
+		$event->expects( $this->any() )
+			->method( 'getType' )
+			->will( $this->returnValue( 'unit-test' ) );
+
+		$users = EchoNotificationController::getUsersToNotifyForEvent( $event );
+		$this->assertEquals( $expect, array_keys( $users ), $message );
+	}
+}

tests/includes/AttributeManagerTest.php

@@ -6,6 +6,61 @@ class EchoAttributeManagerTest extends MediaWikiTestCase {
 		$this->assertInstanceOf( 'EchoAttributeManager', EchoAttributeManager::newFromGlobalVars() );
 	}
 
+	public static function getUserLocatorsProvider() {
+		return array(
+			array(
+				'No errors when requesting unknown type',
+				// expected result
+				array(),
+				// event type
+				'foo',
+				// notification configuration
+				array(),
+			),
+
+			array(
+				'Returns selected notification configuration',
+				// expected result
+				array( 'woot!' ),
+				// event type
+				'magic',
+				// notification configuration
+				array(
+					'foo' => array(
+						'user-locators' => array( 'frown' ),
+					),
+					'magic' => array(
+						'user-locators' => array( 'woot!' ),
+					),
+				),
+			),
+
+			array(
+				'Accepts user-locators as string and returns array',
+				// expected result
+				array( 'sagen' ),
+				// event type
+				'challah',
+				// notification configuration
+				array(
+					'challah' => array(
+						'user-locators' => 'sagen',
+					),
+				),
+			),
+		);
+	}
+
+	/**
+	 * @dataProvider getUserLocatorsProvider
+	 */
+	public function testGetUserLocators( $message, $expect, $type, $notifications) {
+		$manager = new EchoAttributeManager( $notifications, array() );
+
+		$result = $manager->getUserLocators( $type );
+		$this->assertEquals( $expect, $result, $message );
+	}
+
 	public function testGetCategoryEligibility() {
 		$notif = array(
 			'event_one' => array (