From a683b11b23b8caaaf5ef881759034864bdb64f99 Mon Sep 17 00:00:00 2001 From: addshore Date: Fri, 5 Feb 2016 13:14:13 +0100 Subject: [PATCH] Move EmailNotification::updateWatchlistTimestamp to WatchedItemStore Flow is the only thing remaining that uses this deprecated method. Change-Id: Iaa4e1e34cb3f2a91c163565fb0107c500e3852d7 --- RELEASE-NOTES-1.27 | 1 + includes/WatchedItemStore.php | 48 ++++++++++ includes/mail/EmailNotification.php | 58 ++++-------- .../WatchedItemStoreIntegrationTest.php | 11 ++- .../includes/WatchedItemStoreUnitTest.php | 89 +++++++++++++++++++ 5 files changed, 165 insertions(+), 42 deletions(-) diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index b66d2d188d..a7706e7a95 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -334,6 +334,7 @@ changes to languages because of Phabricator reports. ** WatchedItem::removeWatch was deprecated. ** WatchedItem::isWatched was deprecated. ** WatchedItem::duplicateEntries was deprecated. +** EmailNotification::updateWatchlistTimestamp was deprecated. ** User::getWatchedItem was removed. == Compatibility == diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php index 150fa1aae8..1aed8e01f2 100644 --- a/includes/WatchedItemStore.php +++ b/includes/WatchedItemStore.php @@ -311,6 +311,54 @@ class WatchedItemStore { return $success; } + /** + * @param User $editor The editor that triggered the update. Their notification + * timestamp will not be updated(they have already seen it) + * @param LinkTarget $target The target to update timestamps for + * @param string $timestamp Set the update timestamp to this value + * + * @return int[] Array of user IDs the timestamp has been updated for + */ + public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) { + $dbw = $this->loadBalancer->getConnection( DB_MASTER, [ 'watchlist' ] ); + $res = $dbw->select( [ 'watchlist' ], + [ 'wl_user' ], + [ + 'wl_user != ' . intval( $editor->getId() ), + 'wl_namespace' => $target->getNamespace(), + 'wl_title' => $target->getDBkey(), + 'wl_notificationtimestamp IS NULL', + ], __METHOD__ + ); + + $watchers = []; + foreach ( $res as $row ) { + $watchers[] = intval( $row->wl_user ); + } + + if ( $watchers ) { + // Update wl_notificationtimestamp for all watching users except the editor + $fname = __METHOD__; + $dbw->onTransactionIdle( + function () use ( $dbw, $timestamp, $watchers, $target, $fname ) { + $dbw->update( 'watchlist', + [ /* SET */ + 'wl_notificationtimestamp' => $dbw->timestamp( $timestamp ) + ], [ /* WHERE */ + 'wl_user' => $watchers, + 'wl_namespace' => $target->getNamespace(), + 'wl_title' => $target->getDBkey(), + ], $fname + ); + } + ); + } + + $this->loadBalancer->reuseConnection( $dbw ); + + return $watchers; + } + /** * Reset the notification timestamp of this entry * diff --git a/includes/mail/EmailNotification.php b/includes/mail/EmailNotification.php index 30907e636c..4f8f6b3da1 100644 --- a/includes/mail/EmailNotification.php +++ b/includes/mail/EmailNotification.php @@ -72,10 +72,13 @@ class EmailNotification { protected $editor; /** + * @deprecated since 1.27 use WatchedItemStore::updateNotificationTimestamp directly + * * @param User $editor The editor that triggered the update. Their notification * timestamp will not be updated(they have already seen it) * @param LinkTarget $linkTarget The link target of the title to update timestamps for * @param string $timestamp Set the update timestamp to this value + * * @return int[] Array of user IDs */ public static function updateWatchlistTimestamp( @@ -83,47 +86,16 @@ class EmailNotification { LinkTarget $linkTarget, $timestamp ) { - global $wgEnotifWatchlist, $wgShowUpdatedMarker; - - if ( !$wgEnotifWatchlist && !$wgShowUpdatedMarker ) { + // wfDeprecated( __METHOD__, '1.27' ); + $config = RequestContext::getMain()->getConfig(); + if ( !$config->get( 'EnotifWatchlist' ) && !$config->get( 'ShowUpdatedMarker' ) ) { return []; } - - $dbw = wfGetDB( DB_MASTER ); - $res = $dbw->select( [ 'watchlist' ], - [ 'wl_user' ], - [ - 'wl_user != ' . intval( $editor->getID() ), - 'wl_namespace' => $linkTarget->getNamespace(), - 'wl_title' => $linkTarget->getDBkey(), - 'wl_notificationtimestamp IS NULL', - ], __METHOD__ + return WatchedItemStore::getDefaultInstance()->updateNotificationTimestamp( + $editor, + $linkTarget, + $timestamp ); - - $watchers = []; - foreach ( $res as $row ) { - $watchers[] = intval( $row->wl_user ); - } - - if ( $watchers ) { - // Update wl_notificationtimestamp for all watching users except the editor - $fname = __METHOD__; - $dbw->onTransactionIdle( - function () use ( $dbw, $timestamp, $watchers, $linkTarget, $fname ) { - $dbw->update( 'watchlist', - [ /* SET */ - 'wl_notificationtimestamp' => $dbw->timestamp( $timestamp ) - ], [ /* WHERE */ - 'wl_user' => $watchers, - 'wl_namespace' => $linkTarget->getNamespace(), - 'wl_title' => $linkTarget->getDBkey(), - ], $fname - ); - } - ); - } - - return $watchers; } /** @@ -149,7 +121,15 @@ class EmailNotification { } // update wl_notificationtimestamp for watchers - $watchers = self::updateWatchlistTimestamp( $editor, $title, $timestamp ); + $config = RequestContext::getMain()->getConfig(); + $watchers = []; + if ( $config->get( 'EnotifWatchlist' ) || $config->get( 'ShowUpdatedMarker' ) ) { + $watchers = WatchedItemStore::getDefaultInstance()->updateNotificationTimestamp( + $editor, + $title, + $timestamp + ); + } $sendEmail = true; // $watchers deals with $wgEnotifWatchlist. diff --git a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php index 5e74544494..9341fb89b6 100644 --- a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php +++ b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php @@ -42,15 +42,20 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase { ); } - public function testResetNotificationTimestamp() { + public function testUpdateAndResetNotificationTimestamp() { $user = $this->getUser(); $otherUser = ( new TestUser( 'WatchedItemStoreIntegrationTestUser_otherUser' ) )->getUser(); $title = Title::newFromText( 'WatchedItemStoreIntegrationTestPage' ); $store = WatchedItemStore::getDefaultInstance(); $store->addWatch( $user, $title ); - EmailNotification::updateWatchlistTimestamp( $otherUser, $title, '20150202010101' ); + $this->assertNull( $store->loadWatchedItem( $user, $title )->getNotificationTimestamp() ); + + $store->updateNotificationTimestamp( $otherUser, $title, '20150202010101' ); + $this->assertEquals( + '20150202010101', + $store->loadWatchedItem( $user, $title )->getNotificationTimestamp() + ); - $this->assertNotNull( $store->loadWatchedItem( $user, $title )->getNotificationTimestamp() ); $this->assertTrue( $store->resetNotificationTimestamp( $user, $title ) ); $this->assertNull( $store->loadWatchedItem( $user, $title )->getNotificationTimestamp() ); } diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php index 57cadc571e..709b4b419f 100644 --- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php @@ -1144,4 +1144,93 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase { $this->assertEquals( 1, $getTimestampCallCounter ); } + public function testUpdateNotificationTimestamp_watchersExist() { + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'select' ) + ->with( + [ 'watchlist' ], + [ 'wl_user' ], + [ + 'wl_user != 1', + 'wl_namespace' => 0, + 'wl_title' => 'SomeDbKey', + 'wl_notificationtimestamp IS NULL' + ] + ) + ->will( + $this->returnValue( [ + $this->getFakeRow( [ 'wl_user' => '2' ] ), + $this->getFakeRow( [ 'wl_user' => '3' ] ) + ] ) + ); + $mockDb->expects( $this->once() ) + ->method( 'onTransactionIdle' ) + ->with( $this->isType( 'callable' ) ) + ->will( $this->returnCallback( function( $callable ) { + $callable(); + } ) ); + $mockDb->expects( $this->once() ) + ->method( 'update' ) + ->with( + 'watchlist', + [ 'wl_notificationtimestamp' => null ], + [ + 'wl_user' => [ 2, 3 ], + 'wl_namespace' => 0, + 'wl_title' => 'SomeDbKey', + ] + ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + new HashBagOStuff( [ 'maxKeys' => 100 ] ) + ); + + $this->assertEquals( + [ 2, 3 ], + $store->updateNotificationTimestamp( + $this->getMockNonAnonUserWithId( 1 ), + new TitleValue( 0, 'SomeDbKey' ), + '20151212010101' + ) + ); + } + + public function testUpdateNotificationTimestamp_noWatchers() { + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'select' ) + ->with( + [ 'watchlist' ], + [ 'wl_user' ], + [ + 'wl_user != 1', + 'wl_namespace' => 0, + 'wl_title' => 'SomeDbKey', + 'wl_notificationtimestamp IS NULL' + ] + ) + ->will( + $this->returnValue( [] ) + ); + $mockDb->expects( $this->never() ) + ->method( 'onTransactionIdle' ); + $mockDb->expects( $this->never() ) + ->method( 'update' ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + new HashBagOStuff( [ 'maxKeys' => 100 ] ) + ); + + $watchers = $store->updateNotificationTimestamp( + $this->getMockNonAnonUserWithId( 1 ), + new TitleValue( 0, 'SomeDbKey' ), + '20151212010101' + ); + $this->assertInternalType( 'array', $watchers ); + $this->assertEmpty( $watchers ); + } + } -- 2.20.1