jenkins-bot has submitted this change and it was merged.
Change subject: Switch Signature of WatchedItemStore::addWatchBatch
......................................................................
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
---
M includes/WatchedItem.php
M includes/WatchedItemStore.php
M includes/user/User.php
M tests/phpunit/includes/WatchedItemStoreUnitTest.php
M tests/phpunit/includes/WatchedItemUnitTest.php
5 files changed, 95 insertions(+), 120 deletions(-)
Approvals:
WMDE-Fisch: Looks good to me, but someone else must approve
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/WatchedItem.php b/includes/WatchedItem.php
index 5b4a4fc..0495536 100644
--- a/includes/WatchedItem.php
+++ b/includes/WatchedItem.php
@@ -179,23 +179,31 @@
*/
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
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -615,30 +615,30 @@
* @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(),
@@ -646,10 +646,6 @@
'wl_notificationtimestamp' => null,
];
$this->uncache( $user, $target );
- }
-
- if ( !$rows ) {
- return false;
}
$dbw = $this->getConnection( DB_MASTER );
diff --git a/includes/user/User.php b/includes/user/User.php
index 027edf9..a272b37 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -3467,10 +3467,9 @@
*/
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
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -934,7 +934,7 @@
);
}
- public function testAddWatchBatch_nonAnonymousUser() {
+ public function testAddWatchBatchForUser_nonAnonymousUser() {
$mockDb = $this->getMockDb();
$mockDb->expects( $this->once() )
->method( 'insert' )
@@ -974,35 +974,44 @@
$mockUser = $this->getMockNonAnonUserWithId( 1 );
$this->assertTrue(
- $store->addWatchBatch(
- [
- [ $mockUser, new TitleValue( 0,
'Some_Page' ) ],
- [ $mockUser, new TitleValue( 1,
'Some_Page' ) ],
- ]
+ $store->addWatchBatchForUser(
+ $mockUser,
+ [ new TitleValue( 0, 'Some_Page' ), new
TitleValue( 1, 'Some_Page' ) ]
)
);
}
- public function testAddWatchBatch_anonymousUserCombinationsAreSkipped()
{
+ public function testAddWatchBatchForUser_anonymousUsersAreSkipped() {
$mockDb = $this->getMockDb();
- $mockDb->expects( $this->once() )
- ->method( 'insert' )
- ->with(
- 'watchlist',
- [
- [
- 'wl_user' => 1,
- 'wl_namespace' => 0,
- 'wl_title' => 'Some_Page',
- 'wl_notificationtimestamp' =>
null,
- ]
- ]
- );
+ $mockDb->expects( $this->never() )
+ ->method( 'insert' );
$mockCache = $this->getMockCache();
- $mockCache->expects( $this->once() )
- ->method( 'delete' )
- ->with( '0:Some_Page:1' );
+ $mockCache->expects( $this->never() )
+ ->method( 'delete' );
+
+ $store = new WatchedItemStore(
+ $this->getMockLoadBalancer( $mockDb ),
+ $mockCache
+ );
+
+ $this->assertFalse(
+ $store->addWatchBatchForUser(
+ $this->getAnonUser(),
+ [ new TitleValue( 0, 'Other_Page' ) ]
+ )
+ );
+ }
+
+ public function testAddWatchBatchReturnsTrue_whenGivenEmptyList() {
+ $user = $this->getMockNonAnonUserWithId( 1 );
+ $mockDb = $this->getMockDb();
+ $mockDb->expects( $this->never() )
+ ->method( 'insert' );
+
+ $mockCache = $this->getMockCache();
+ $mockCache->expects( $this->never() )
+ ->method( 'delete' );
$store = new WatchedItemStore(
$this->getMockLoadBalancer( $mockDb ),
@@ -1010,56 +1019,7 @@
);
$this->assertTrue(
- $store->addWatchBatch(
- [
- [ $this->getMockNonAnonUserWithId( 1 ),
new TitleValue( 0, 'Some_Page' ) ],
- [ $this->getAnonUser(), new TitleValue(
0, 'Other_Page' ) ],
- ]
- )
- );
- }
-
- public function
testAddWatchBatchReturnsFalse_whenOnlyGivenAnonymousUserCombinations() {
- $mockDb = $this->getMockDb();
- $mockDb->expects( $this->never() )
- ->method( 'insert' );
-
- $mockCache = $this->getMockCache();
- $mockCache->expects( $this->never() )
- ->method( 'delete' );
-
- $store = new WatchedItemStore(
- $this->getMockLoadBalancer( $mockDb ),
- $mockCache
- );
-
- $anonUser = $this->getAnonUser();
- $this->assertFalse(
- $store->addWatchBatch(
- [
- [ $anonUser, new TitleValue( 0,
'Some_Page' ) ],
- [ $anonUser, new TitleValue( 1,
'Other_Page' ) ],
- ]
- )
- );
- }
-
- public function testAddWatchBatchReturnsFalse_whenGivenEmptyList() {
- $mockDb = $this->getMockDb();
- $mockDb->expects( $this->never() )
- ->method( 'insert' );
-
- $mockCache = $this->getMockCache();
- $mockCache->expects( $this->never() )
- ->method( 'delete' );
-
- $store = new WatchedItemStore(
- $this->getMockLoadBalancer( $mockDb ),
- $mockCache
- );
-
- $this->assertFalse(
- $store->addWatchBatch( [] )
+ $store->addWatchBatchForUser( $user, [] )
);
}
diff --git a/tests/phpunit/includes/WatchedItemUnitTest.php
b/tests/phpunit/includes/WatchedItemUnitTest.php
index 58984cf..b4eaa76 100644
--- a/tests/phpunit/includes/WatchedItemUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemUnitTest.php
@@ -165,25 +165,37 @@
}
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 );
}
--
To view, visit https://gerrit.wikimedia.org/r/278947
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I90f9583b66bd3b5afcf07faefedb38a8a0149f6e
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: WMDE-Fisch <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
