WMDE-leszek has uploaded a new change for review. https://gerrit.wikimedia.org/r/278283
Change subject: Use WatchedItemStore in ApiQueryInfo::getWatchedInfo ...................................................................... Use WatchedItemStore in ApiQueryInfo::getWatchedInfo Adds a method for getting WatchedItem objects for a batch of LinkTargets. Bug: T129482 Change-Id: I1f84212e7879a84b34bb3b53859069fcea282bba --- M includes/WatchedItemStore.php M includes/api/ApiQueryInfo.php M tests/phpunit/includes/WatchedItemStoreIntegrationTest.php M tests/phpunit/includes/WatchedItemStoreUnitTest.php 4 files changed, 287 insertions(+), 17 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/83/278283/1 diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php index 4e2dfa5..deeaf79 100644 --- a/includes/WatchedItemStore.php +++ b/includes/WatchedItemStore.php @@ -491,6 +491,61 @@ } /** + * Get items for targets + * + * @param User $user + * @param LinkTarget[] $targets + * + * @return WatchedItem[] + */ + public function getWatchedItems( User $user, array $targets ) { + if ( $user->isAnon() ) { + return []; + } + + $watchedItems = []; + $targetsToLoad = []; + foreach ( $targets as $target ) { + $cachedItem = $this->getCached( $user, $target ); + if ( $cachedItem ) { + $watchedItems[] = $cachedItem; + } else { + $targetsToLoad[] = $target; + } + } + + if ( !$targetsToLoad ) { + return $watchedItems; + } + + $dbr = $this->getConnection( DB_SLAVE ); + + $lb = new LinkBatch( $targetsToLoad ); + $res = $dbr->select( + 'watchlist', + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], + [ + $lb->constructSet( 'wl', $dbr ), + 'wl_user' => $user->getId(), + ], + __METHOD__ + ); + $this->reuseConnection( $dbr ); + + foreach ( $res as $row ) { + $item = new WatchedItem( + $user, + new TitleValue( (int)$row->wl_namespace, $row->wl_title ), + $row->wl_notificationtimestamp + ); + $this->cache( $item ); + $watchedItems[] = $item; + } + + return $watchedItems; + } + + /** * Must be called separately for Subject & Talk namespaces * * @param User $user diff --git a/includes/api/ApiQueryInfo.php b/includes/api/ApiQueryInfo.php index 2d382dd..62cdcc9 100644 --- a/includes/api/ApiQueryInfo.php +++ b/includes/api/ApiQueryInfo.php @@ -758,28 +758,17 @@ $this->watched = []; $this->notificationtimestamps = []; - $db = $this->getDB(); - $lb = new LinkBatch( $this->everything ); + $items = WatchedItemStore::getDefaultInstance()->getWatchedItems( $user, $this->everything ); - $this->resetQueryParams(); - $this->addTables( [ 'watchlist' ] ); - $this->addFields( [ 'wl_title', 'wl_namespace' ] ); - $this->addFieldsIf( 'wl_notificationtimestamp', $this->fld_notificationtimestamp ); - $this->addWhere( [ - $lb->constructSet( 'wl', $db ), - 'wl_user' => $user->getId() - ] ); - - $res = $this->select( __METHOD__ ); - - foreach ( $res as $row ) { + foreach ( $items as $item ) { + $target = $item->getLinkTarget(); if ( $this->fld_watched ) { - $this->watched[$row->wl_namespace][$row->wl_title] = true; + $this->watched[$target->getNamespace()][$target->getDBkey()] = true; } if ( $this->fld_notificationtimestamp ) { - $this->notificationtimestamps[$row->wl_namespace][$row->wl_title] = - $row->wl_notificationtimestamp; + $this->notificationtimestamps[$target->getNamespace()][$target->getDBkey()] = + $item->getNotificationTimestamp(); } } } diff --git a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php index 0dea461..3268bf7 100644 --- a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php +++ b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php @@ -52,6 +52,11 @@ [ 0 => [ 'WatchedItemStoreIntegrationTestPage' => 0 ] ], $store->countWatchersMultiple( [ $title ], [ 'minimumWatchers' => $initialWatchers + 2 ] ) ); + $watchedItems = $store->getWatchedItems( $user, [ $title ] ); + $this->assertCount( 1, $watchedItems ); + $this->assertEquals( $user, $watchedItems[0]->getUser() ); + $this->assertEquals( $title->getDBkey(), $watchedItems[0]->getLinkTarget()->getDBkey() ); + $this->assertEquals( $title->getNamespace(), $watchedItems[0]->getLinkTarget()->getNamespace() ); $store->removeWatch( $user, $title ); $this->assertFalse( @@ -64,6 +69,8 @@ $initialWatchers, $store->countWatchersMultiple( [ $title ] )[$title->getNamespace()][$title->getDBkey()] ); + $watchedItems = $store->getWatchedItems( $user, [ $title ] ); + $this->assertEmpty( $watchedItems ); } public function testUpdateAndResetNotificationTimestamp() { diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php index 869b0d2..6557869 100644 --- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php @@ -1380,6 +1380,225 @@ ); } + public function testGetWatchedItems() { + $targets = [ + new TitleValue( 0, 'SomeDbKey' ), + new TitleValue( 1, 'AnotherDbKey' ), + ]; + + $mockDb = $this->getMockDb(); + $dbResult = [ + $this->getFakeRow( [ + 'wl_namespace' => 0, + 'wl_title' => 'SomeDbKey', + 'wl_notificationtimestamp' => '20151212010101' + ] ), + $this->getFakeRow( + [ 'wl_namespace' => 1, 'wl_title' => 'AnotherDbKey', 'wl_notificationtimestamp' => null ] + ), + ]; + + $mockDb->expects( $this->once() ) + ->method( 'makeWhereFrom2d' ) + ->with( + [ [ 'SomeDbKey' => 1 ], [ 'AnotherDbKey' => 1 ] ], + $this->isType( 'string' ), + $this->isType( 'string' ) + ) + ->will( $this->returnValue( 'makeWhereFrom2d return value' ) ); + $mockDb->expects( $this->once() ) + ->method( 'select' ) + ->with( + 'watchlist', + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], + [ + 'makeWhereFrom2d return value', + 'wl_user' => 1 + ], + $this->isType( 'string' ) + ) + ->will( $this->returnValue( $dbResult ) ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->exactly( 2 ) ) + ->method( 'get' ) + ->withConsecutive( + [ '0:SomeDbKey:1' ], + [ '1:AnotherDbKey:1' ] + ) + ->will( $this->returnValue( null ) ); + $mockCache->expects( $this->exactly( 2 ) ) + ->method( 'set' ) + ->withConsecutive( + [ '0:SomeDbKey:1' ], + [ '1:AnotherDbKey:1' ] + ); + $mockCache->expects( $this->never() )->method( 'delete' ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache + ); + $user = $this->getMockNonAnonUserWithId( 1 ); + + $watchedItems = $store->getWatchedItems( + $user, + $targets + ); + + $this->assertInternalType( 'array', $watchedItems ); + $this->assertCount( 2, $watchedItems ); + + $this->assertEquals( + new WatchedItem( $user, $targets[0], '20151212010101' ), + $watchedItems[0] + ); + + $this->assertEquals( + new WatchedItem( $user, $targets[1], null ), + $watchedItems[1] + ); + } + + public function testGetWatchedItems_cachedItem() { + $targets = [ + new TitleValue( 0, 'SomeDbKey' ), + new TitleValue( 1, 'AnotherDbKey' ), + ]; + + $user = $this->getMockNonAnonUserWithId( 1 ); + $cachedItem = new WatchedItem( $user, $targets[0], '20151212010101' ); + + $mockDb = $this->getMockDb(); + + $mockDb->expects( $this->once() ) + ->method( 'makeWhereFrom2d' ) + ->with( + [ 1 => [ 'AnotherDbKey' => 1 ] ], + $this->isType( 'string' ), + $this->isType( 'string' ) + ) + ->will( $this->returnValue( 'makeWhereFrom2d return value' ) ); + $mockDb->expects( $this->once() ) + ->method( 'select' ) + ->with( + 'watchlist', + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], + [ + 'makeWhereFrom2d return value', + 'wl_user' => 1 + ], + $this->isType( 'string' ) + ) + ->will( $this->returnValue( [ + $this->getFakeRow( + [ 'wl_namespace' => 1, 'wl_title' => 'AnotherDbKey', 'wl_notificationtimestamp' => null ] + ) + ] ) ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->at( 1 ) ) + ->method( 'get' ) + ->with( '0:SomeDbKey:1' ) + ->will( $this->returnValue( $cachedItem ) ); + $mockCache->expects( $this->at( 3 ) ) + ->method( 'get' ) + ->with( '1:AnotherDbKey:1' ) + ->will( $this->returnValue( null ) ); + $mockCache->expects( $this->once() ) + ->method( 'set' ) + ->with( '1:AnotherDbKey:1' ); + $mockCache->expects( $this->never() )->method( 'delete' ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache + ); + + $watchedItems = $store->getWatchedItems( + $user, + $targets + ); + + $this->assertInternalType( 'array', $watchedItems ); + $this->assertCount( 2, $watchedItems ); + + $this->assertEquals( + $cachedItem, + $watchedItems[0] + ); + + $this->assertEquals( + new WatchedItem( $user, $targets[1], null ), + $watchedItems[1] + ); + } + + public function testGetWatchedItems_allItemsCached() { + $targets = [ + new TitleValue( 0, 'SomeDbKey' ), + new TitleValue( 1, 'AnotherDbKey' ), + ]; + + $user = $this->getMockNonAnonUserWithId( 1 ); + $cachedItems = [ + new WatchedItem( $user, $targets[0], '20151212010101' ), + new WatchedItem( $user, $targets[1], null ), + ]; + + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->never() )->method( $this->anything() ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->at( 1 ) ) + ->method( 'get' ) + ->with( '0:SomeDbKey:1' ) + ->will( $this->returnValue( $cachedItems[0] ) ); + $mockCache->expects( $this->at( 3 ) ) + ->method( 'get' ) + ->with( '1:AnotherDbKey:1' ) + ->will( $this->returnValue( $cachedItems[1] ) ); + $mockCache->expects( $this->never() )->method( 'set' ); + $mockCache->expects( $this->never() )->method( 'delete' ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache + ); + + $watchedItems = $store->getWatchedItems( + $user, + $targets + ); + + $this->assertEquals( $cachedItems, $watchedItems ); + } + + public function testGetWatchedItems_anonymousUser() { + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->never() )->method( $this->anything() ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->never() )->method( $this->anything() ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache + ); + + $watchedItems = $store->getWatchedItems( + $this->getAnonUser(), + [ + new TitleValue( 0, 'SomeDbKey' ), + new TitleValue( 0, 'OtherDbKey' ), + ] + ); + $this->assertInternalType( 'array', $watchedItems ); + $this->assertEmpty( + $watchedItems + ); + } + public function testIsWatchedItem_existingItem() { $mockDb = $this->getMockDb(); $mockDb->expects( $this->once() ) -- To view, visit https://gerrit.wikimedia.org/r/278283 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1f84212e7879a84b34bb3b53859069fcea282bba Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: WMDE-leszek <leszek.mani...@wikimedia.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits