Switch Signature of WatchedItemStore::addWatchBatch

Adding batches of watched items per users
makes much more sense.
Only the deprecated static WatchedItem method
needed the old silly way of passing in objects.

Change-Id: I90f9583b66bd3b5afcf07faefedb38a8a0149f6e

diff --git a/includes/WatchedItem.php b/includes/WatchedItem.php
index 5b4a4fc..0495536 100644 (file)
--- a/includes/WatchedItem.php
+++ b/includes/WatchedItem.php
@@ -179,23 +179,31 @@ class WatchedItem {
         */
        public static function batchAddWatch( array $items ) {
                // wfDeprecated( __METHOD__, '1.27' );
-               $userTargetCombinations = [];
+               if ( !$items ) {
+                       return false;
+               }
+
+               $targets = [];
+               $users = [];
                /** @var WatchedItem $watchedItem */
                foreach ( $items as $watchedItem ) {
-                       if ( $watchedItem->checkRights && !$watchedItem->getUser()->isAllowed( 'editmywatchlist' ) ) {
+                       $user = $watchedItem->getUser();
+                       if ( $watchedItem->checkRights && !$user->isAllowed( 'editmywatchlist' ) ) {
                                continue;
                        }
-                       $userTargetCombinations[] = [
-                               $watchedItem->getUser(),
-                               $watchedItem->getTitle()->getSubjectPage()
-                       ];
-                       $userTargetCombinations[] = [
-                               $watchedItem->getUser(),
-                               $watchedItem->getTitle()->getTalkPage()
-                       ];
+                       $userId = $user->getId();
+                       $users[$userId] = $user;
+                       $targets[$userId][] = $watchedItem->getTitle()->getSubjectPage();
+                       $targets[$userId][] = $watchedItem->getTitle()->getTalkPage();
                }
+
                $store = WatchedItemStore::getDefaultInstance();
-               return $store->addWatchBatch( $userTargetCombinations );
+               $success = true;
+               foreach ( $users as $userId => $user ) {
+                       $success &= $store->addWatchBatchForUser( $user, $targets[$userId] );
+               }
+
+               return $success;
        }
 
        /**

diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php
index 4eea54d..c4340ad 100644 (file)
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -615,30 +615,30 @@ class WatchedItemStore {
         * @param LinkTarget $target
         */
        public function addWatch( User $user, LinkTarget $target ) {
-               $this->addWatchBatch( [ [ $user, $target ] ] );
+               $this->addWatchBatchForUser( $user, [ $target ] );
        }
 
        /**
-        * @param array[] $userTargetCombinations array of arrays containing [0] => User [1] => LinkTarget
+        * @param User $user
+        * @param LinkTarget[] $targets
         *
         * @return bool success
         */
-       public function addWatchBatch( array $userTargetCombinations ) {
+       public function addWatchBatchForUser( User $user, array $targets ) {
                if ( $this->loadBalancer->getReadOnlyReason() !== false ) {
                        return false;
                }
+               // Only loggedin user can have a watchlist
+               if ( $user->isAnon() ) {
+                       return false;
+               }
+
+               if ( !$targets ) {
+                       return true;
+               }
 
                $rows = [];
-               foreach ( $userTargetCombinations as list( $user, $target ) ) {
-                       /**
-                        * @var User $user
-                        * @var LinkTarget $target
-                        */
-
-                       // Only loggedin user can have a watchlist
-                       if ( $user->isAnon() ) {
-                               continue;
-                       }
+               foreach ( $targets as $target ) {
                        $rows[] = [
                                'wl_user' => $user->getId(),
                                'wl_namespace' => $target->getNamespace(),
@@ -648,10 +648,6 @@ class WatchedItemStore {
                        $this->uncache( $user, $target );
                }
 
-               if ( !$rows ) {
-                       return false;
-               }
-
                $dbw = $this->getConnection( DB_MASTER );
                foreach ( array_chunk( $rows, 100 ) as $toInsert ) {
                        // Use INSERT IGNORE to avoid overwriting the notification timestamp

diff --git a/includes/user/User.php b/includes/user/User.php
index 027edf9..a272b37 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -3467,10 +3467,9 @@ class User implements IDBAccessObject {
         */
        public function addWatch( $title, $checkRights = self::CHECK_USER_RIGHTS ) {
                if ( !$checkRights || $this->isAllowed( 'editmywatchlist' ) ) {
-                       WatchedItemStore::getDefaultInstance()->addWatchBatch( [
-                               [ $this, $title->getSubjectPage() ],
-                               [ $this, $title->getTalkPage() ],
-                       ]
+                       WatchedItemStore::getDefaultInstance()->addWatchBatchForUser(
+                               $this,
+                               [ $title->getSubjectPage(), $title->getTalkPage() ]
                        );
                }
                $this->invalidateCache();

diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
index afde88c..5f07dbf 100644 (file)
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -934,7 +934,7 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                );
        }
 
-       public function testAddWatchBatch_nonAnonymousUser() {
+       public function testAddWatchBatchForUser_nonAnonymousUser() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->once() )
                        ->method( 'insert' )
@@ -974,52 +974,14 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                $mockUser = $this->getMockNonAnonUserWithId( 1 );
 
                $this->assertTrue(
-                       $store->addWatchBatch(
-                               [
-                                       [ $mockUser, new TitleValue( 0, 'Some_Page' ) ],
-                                       [ $mockUser, new TitleValue( 1, 'Some_Page' ) ],
-                               ]
-                       )
-               );
-       }
-
-       public function testAddWatchBatch_anonymousUserCombinationsAreSkipped() {
-               $mockDb = $this->getMockDb();
-               $mockDb->expects( $this->once() )
-                       ->method( 'insert' )
-                       ->with(
-                               'watchlist',
-                               [
-                                       [
-                                               'wl_user' => 1,
-                                               'wl_namespace' => 0,
-                                               'wl_title' => 'Some_Page',
-                                               'wl_notificationtimestamp' => null,
-                                       ]
-                               ]
-                       );
-
-               $mockCache = $this->getMockCache();
-               $mockCache->expects( $this->once() )
-                       ->method( 'delete' )
-                       ->with( '0:Some_Page:1' );
-
-               $store = new WatchedItemStore(
-                       $this->getMockLoadBalancer( $mockDb ),
-                       $mockCache
-               );
-
-               $this->assertTrue(
-                       $store->addWatchBatch(
-                               [
-                                       [ $this->getMockNonAnonUserWithId( 1 ), new TitleValue( 0, 'Some_Page' ) ],
-                                       [ $this->getAnonUser(), new TitleValue( 0, 'Other_Page' ) ],
-                               ]
+                       $store->addWatchBatchForUser(
+                               $mockUser,
+                               [ new TitleValue( 0, 'Some_Page' ), new TitleValue( 1, 'Some_Page' ) ]
                        )
                );
        }
 
-       public function testAddWatchBatchReturnsFalse_whenOnlyGivenAnonymousUserCombinations() {
+       public function testAddWatchBatchForUser_anonymousUsersAreSkipped() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->never() )
                        ->method( 'insert' );
@@ -1033,18 +995,16 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                        $mockCache
                );
 
-               $anonUser = $this->getAnonUser();
                $this->assertFalse(
-                       $store->addWatchBatch(
-                               [
-                                       [ $anonUser, new TitleValue( 0, 'Some_Page' ) ],
-                                       [ $anonUser, new TitleValue( 1, 'Other_Page' ) ],
-                               ]
+                       $store->addWatchBatchForUser(
+                               $this->getAnonUser(),
+                               [ new TitleValue( 0, 'Other_Page' ) ]
                        )
                );
        }
 
-       public function testAddWatchBatchReturnsFalse_whenGivenEmptyList() {
+       public function testAddWatchBatchReturnsTrue_whenGivenEmptyList() {
+               $user = $this->getMockNonAnonUserWithId( 1 );
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->never() )
                        ->method( 'insert' );
@@ -1058,8 +1018,8 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                        $mockCache
                );
 
-               $this->assertFalse(
-                       $store->addWatchBatch( [] )
+               $this->assertTrue(
+                       $store->addWatchBatchForUser( $user, [] )
                );
        }
 

diff --git a/tests/phpunit/includes/WatchedItemUnitTest.php b/tests/phpunit/includes/WatchedItemUnitTest.php
index 58984cf..b4eaa76 100644 (file)
--- a/tests/phpunit/includes/WatchedItemUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemUnitTest.php
@@ -165,25 +165,37 @@ class WatchedItemUnitTest extends PHPUnit_Framework_TestCase {
        }
 
        public function testBatchAddWatch() {
-               /** @var WatchedItem[] $items */
-               $items = [
-                       new WatchedItem( User::newFromId( 1 ), new TitleValue( 0, 'Title1' ), null ),
-                       new WatchedItem( User::newFromId( 3 ), Title::newFromText( 'Title2' ), '20150101010101' ),
-               ];
-
-               $userTargetCombinations = [];
-               foreach ( $items as $item ) {
-                       $userTargetCombinations[] = [ $item->getUser(), $item->getTitle()->getSubjectPage() ];
-                       $userTargetCombinations[] = [ $item->getUser(), $item->getTitle()->getTalkPage() ];
-               }
+               $itemOne = new WatchedItem( User::newFromId( 1 ), new TitleValue( 0, 'Title1' ), null );
+               $itemTwo = new WatchedItem(
+                       User::newFromId( 3 ),
+                       Title::newFromText( 'Title2' ),
+                       '20150101010101'
+               );
 
                $store = $this->getMockWatchedItemStore();
-               $store->expects( $this->once() )
-                       ->method( 'addWatchBatch' )
-                       ->with( $userTargetCombinations );
+               $store->expects( $this->exactly( 2 ) )
+                       ->method( 'addWatchBatchForUser' );
+               $store->expects( $this->at( 0 ) )
+                       ->method( 'addWatchBatchForUser' )
+                       ->with(
+                               $itemOne->getUser(),
+                               [
+                                       $itemOne->getTitle()->getSubjectPage(),
+                                       $itemOne->getTitle()->getTalkPage(),
+                               ]
+                       );
+               $store->expects( $this->at( 1 ) )
+                       ->method( 'addWatchBatchForUser' )
+                       ->with(
+                               $itemTwo->getUser(),
+                               [
+                                       $itemTwo->getTitle()->getSubjectPage(),
+                                       $itemTwo->getTitle()->getTalkPage(),
+                               ]
+                       );
                $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store );
 
-               WatchedItem::batchAddWatch( $items );
+               WatchedItem::batchAddWatch( [ $itemOne, $itemTwo ] );
 
                ScopedCallback::consume( $scopedOverride );
        }