aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--includes/WatchedItem.php30
-rw-r--r--includes/WatchedItemStore.php30
-rw-r--r--includes/user/User.php7
-rw-r--r--tests/phpunit/includes/WatchedItemStoreUnitTest.php64
-rw-r--r--tests/phpunit/includes/WatchedItemUnitTest.php42
5 files changed, 74 insertions, 99 deletions
diff --git a/includes/WatchedItem.php b/includes/WatchedItem.php
index 5b4a4fcfe1ad..0495536a2641 100644
--- 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 4eea54deecaf..c4340ad5e1f6 100644
--- 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 027edf929b13..a272b37d5f8a 100644
--- 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 afde88cddf4d..5f07dbf4301f 100644
--- 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 58984cf389d9..b4eaa765e9c3 100644
--- 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 );
}