Move counting of watchers to WatchedItemStore

Also adds tests

Bug: T129479
Bug: T129482
Change-Id: I5a465773599cce9f8c9e94847cede6d12282c827

diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php
index fc3af5f..86e4925 100644 (file)
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -45,7 +45,14 @@ class WatchedItemStore {
         */
        private static $instance;
 
-       public function __construct( LoadBalancer $loadBalancer, HashBagOStuff $cache ) {
+       /**
+        * @param LoadBalancer $loadBalancer
+        * @param HashBagOStuff $cache
+        */
+       public function __construct(
+               LoadBalancer $loadBalancer,
+               HashBagOStuff $cache
+       ) {
                $this->loadBalancer = $loadBalancer;
                $this->cache = $cache;
                $this->deferredUpdatesAddCallableUpdateCallback = [ 'DeferredUpdates', 'addCallableUpdate' ];
@@ -177,6 +184,68 @@ class WatchedItemStore {
                ];
        }
 
+       /**
+        * @param LinkTarget $target
+        *
+        * @return int
+        */
+       public function countWatchers( LinkTarget $target ) {
+               $dbr = $this->loadBalancer->getConnection( DB_SLAVE, [ 'watchlist' ] );
+               $return = (int)$dbr->selectField(
+                       'watchlist',
+                       'COUNT(*)',
+                       [
+                               'wl_namespace' => $target->getNamespace(),
+                               'wl_title' => $target->getDBkey(),
+                       ],
+                       __METHOD__
+               );
+               $this->loadBalancer->reuseConnection( $dbr );
+
+               return $return;
+       }
+
+       /**
+        * @param LinkTarget[] $targets
+        * @param array $options Allowed keys:
+        *        'minimumWatchers' => int
+        *
+        * @return array multi dimensional like $return[$namespaceId][$titleString] = int $watchers
+        *         All targets will be present in the result. 0 either means no watchers or the number
+        *         of watchers was below the minimumWatchers option if passed.
+        */
+       public function countWatchersMultiple( array $targets, array $options = [] ) {
+               $dbOptions = [ 'GROUP BY' => [ 'wl_namespace', 'wl_title' ] ];
+
+               $dbr = $this->loadBalancer->getConnection( DB_SLAVE, [ 'watchlist' ] );
+
+               if ( array_key_exists( 'minimumWatchers', $options ) ) {
+                       $dbOptions['HAVING'] = 'COUNT(*) >= ' . (int)$options['minimumWatchers'];
+               }
+
+               $lb = new LinkBatch( $targets );
+               $res = $dbr->select(
+                       'watchlist',
+                       [ 'wl_title', 'wl_namespace', 'watchers' => 'COUNT(*)' ],
+                       [ $lb->constructSet( 'wl', $dbr ) ],
+                       __METHOD__,
+                       $dbOptions
+               );
+
+               $this->loadBalancer->reuseConnection( $dbr );
+
+               $watchCounts = [];
+               foreach ( $targets as $linkTarget ) {
+                       $watchCounts[$linkTarget->getNamespace()][$linkTarget->getDBkey()] = 0;
+               }
+
+               foreach ( $res as $row ) {
+                       $watchCounts[$row->wl_namespace][$row->wl_title] = (int)$row->watchers;
+               }
+
+               return $watchCounts;
+       }
+
        /**
         * Get an item (may be cached)
         *

diff --git a/includes/actions/InfoAction.php b/includes/actions/InfoAction.php
index 87d269a..60829b1 100644 (file)
--- a/includes/actions/InfoAction.php
+++ b/includes/actions/InfoAction.php
@@ -678,18 +678,7 @@ class InfoAction extends FormlessAction {
                                $setOpts += Database::getCacheSetOptions( $dbr, $dbrWatchlist );
 
                                $result = [];
-
-                               // Number of page watchers
-                               $watchers = (int)$dbrWatchlist->selectField(
-                                       'watchlist',
-                                       'COUNT(*)',
-                                       [
-                                               'wl_namespace' => $title->getNamespace(),
-                                               'wl_title' => $title->getDBkey(),
-                                       ],
-                                       $fname
-                               );
-                               $result['watchers'] = $watchers;
+                               $result['watchers'] = WatchedItemStore::getDefaultInstance()->countWatchers( $title );
 
                                if ( $config->get( 'ShowUpdatedMarker' ) ) {
                                        // Threshold: last visited about 26 weeks before latest edit

diff --git a/includes/api/ApiQueryInfo.php b/includes/api/ApiQueryInfo.php
index ea7818c..a3c98ed 100644 (file)
--- a/includes/api/ApiQueryInfo.php
+++ b/includes/api/ApiQueryInfo.php
@@ -799,28 +799,17 @@ class ApiQueryInfo extends ApiQueryBase {
                        return;
                }
 
-               $this->watchers = [];
                $this->showZeroWatchers = $canUnwatchedpages;
-               $db = $this->getDB();
-
-               $lb = new LinkBatch( $this->everything );
 
-               $this->resetQueryParams();
-               $this->addTables( [ 'watchlist' ] );
-               $this->addFields( [ 'wl_title', 'wl_namespace', 'count' => 'COUNT(*)' ] );
-               $this->addWhere( [
-                       $lb->constructSet( 'wl', $db )
-               ] );
-               $this->addOption( 'GROUP BY', [ 'wl_namespace', 'wl_title' ] );
+               $countOptions = [];
                if ( !$canUnwatchedpages ) {
-                       $this->addOption( 'HAVING', "COUNT(*) >= $unwatchedPageThreshold" );
+                       $countOptions['minimumWatchers'] = $unwatchedPageThreshold;
                }
 
-               $res = $this->select( __METHOD__ );
-
-               foreach ( $res as $row ) {
-                       $this->watchers[$row->wl_namespace][$row->wl_title] = (int)$row->count;
-               }
+               $this->watchers = WatchedItemStore::getDefaultInstance()->countWatchersMultiple(
+                       $this->everything,
+                       $countOptions
+               );
        }
 
        /**

diff --git a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
index e2ab512..3e5f261 100644 (file)
--- a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
@@ -25,21 +25,42 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase {
                $store = WatchedItemStore::getDefaultInstance();
                // Cleanup after previous tests
                $store->removeWatch( $user, $title );
+               $initialWatchers = $store->countWatchers( $title );
 
                $this->assertFalse(
                        $store->isWatched( $user, $title ),
                        'Page should not initially be watched'
                );
+
                $store->addWatch( $user, $title );
                $this->assertTrue(
                        $store->isWatched( $user, $title ),
                        'Page should be watched'
                );
+               $this->assertEquals( $initialWatchers + 1, $store->countWatchers( $title ) );
+               $this->assertEquals(
+                       $initialWatchers + 1,
+                       $store->countWatchersMultiple( [ $title ] )[$title->getNamespace()][$title->getDBkey()]
+               );
+               $this->assertEquals(
+                       [ 0 => [ 'WatchedItemStoreIntegrationTestPage' => $initialWatchers + 1 ] ],
+                       $store->countWatchersMultiple( [ $title ], [ 'minimumWatchers' => $initialWatchers + 1 ] )
+               );
+               $this->assertEquals(
+                       [ 0 => [ 'WatchedItemStoreIntegrationTestPage' => 0 ] ],
+                       $store->countWatchersMultiple( [ $title ], [ 'minimumWatchers' => $initialWatchers + 2 ] )
+               );
+
                $store->removeWatch( $user, $title );
                $this->assertFalse(
                        $store->isWatched( $user, $title ),
                        'Page should be unwatched'
                );
+               $this->assertEquals( $initialWatchers, $store->countWatchers( $title ) );
+               $this->assertEquals(
+                       $initialWatchers,
+                       $store->countWatchersMultiple( [ $title ] )[$title->getNamespace()][$title->getDBkey()]
+               );
        }
 
        public function testUpdateAndResetNotificationTimestamp() {

diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
index 1b1a319..c8621fc 100644 (file)
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -81,6 +81,160 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                $this->assertSame( $instanceOne, $instanceTwo );
        }
 
+       public function testCountWatchers() {
+               $titleValue = new TitleValue( 0, 'SomeDbKey' );
+
+               $mockDb = $this->getMockDb();
+               $mockDb->expects( $this->exactly( 1 ) )
+                       ->method( 'selectField' )
+                       ->with(
+                               'watchlist',
+                               'COUNT(*)',
+                               [
+                                       'wl_namespace' => $titleValue->getNamespace(),
+                                       'wl_title' => $titleValue->getDBkey(),
+                               ],
+                               $this->isType( 'string' )
+                       )
+                       ->will( $this->returnValue( 7 ) );
+
+               $mockCache = $this->getMockCache();
+               $mockCache->expects( $this->never() )->method( 'get' );
+               $mockCache->expects( $this->never() )->method( 'set' );
+               $mockCache->expects( $this->never() )->method( 'delete' );
+
+               $store = new WatchedItemStore(
+                       $this->getMockLoadBalancer( $mockDb ),
+                       $mockCache
+               );
+
+               $this->assertEquals( 7, $store->countWatchers( $titleValue ) );
+       }
+
+       public function testCountWatchersMultiple() {
+               $titleValues = [
+                       new TitleValue( 0, 'SomeDbKey' ),
+                       new TitleValue( 0, 'OtherDbKey' ),
+                       new TitleValue( 1, 'AnotherDbKey' ),
+               ];
+
+               $mockDb = $this->getMockDb();
+
+               $dbResult = [
+                       $this->getFakeRow( [ 'wl_title' => 'SomeDbKey', 'wl_namespace' => 0, 'watchers' => 100 ] ),
+                       $this->getFakeRow( [ 'wl_title' => 'OtherDbKey', 'wl_namespace' => 0, 'watchers' => 300 ] ),
+                       $this->getFakeRow( [ 'wl_title' => 'AnotherDbKey', 'wl_namespace' => 1, 'watchers' => 500 ]
+                       ),
+               ];
+               $mockDb->expects( $this->once() )
+                       ->method( 'makeWhereFrom2d' )
+                       ->with(
+                               [ [ 'SomeDbKey' => 1, 'OtherDbKey' => 1 ], [ 'AnotherDbKey' => 1 ] ],
+                               $this->isType( 'string' ),
+                               $this->isType( 'string' )
+                               )
+                       ->will( $this->returnValue( 'makeWhereFrom2d return value' ) );
+               $mockDb->expects( $this->once() )
+                       ->method( 'select' )
+                       ->with(
+                               'watchlist',
+                               [ 'wl_title', 'wl_namespace', 'watchers' => 'COUNT(*)' ],
+                               [ 'makeWhereFrom2d return value' ],
+                               $this->isType( 'string' ),
+                               [
+                                       'GROUP BY' => [ 'wl_namespace', 'wl_title' ],
+                               ]
+                       )
+                       ->will(
+                               $this->returnValue( $dbResult )
+                       );
+
+               $mockCache = $this->getMockCache();
+               $mockCache->expects( $this->never() )->method( 'get' );
+               $mockCache->expects( $this->never() )->method( 'set' );
+               $mockCache->expects( $this->never() )->method( 'delete' );
+
+               $store = new WatchedItemStore(
+                       $this->getMockLoadBalancer( $mockDb ),
+                       $mockCache
+               );
+
+               $expected = [
+                       0 => [ 'SomeDbKey' => 100, 'OtherDbKey' => 300 ],
+                       1 => [ 'AnotherDbKey' => 500 ],
+               ];
+               $this->assertEquals( $expected, $store->countWatchersMultiple( $titleValues ) );
+       }
+
+       public function provideMinimumWatchers() {
+               return [
+                       [ 50 ],
+                       [ "50; DROP TABLE watchlist;\n--" ],
+               ];
+       }
+
+       /**
+        * @dataProvider provideMinimumWatchers
+        */
+       public function testCountWatchersMultiple_withMinimumWatchers( $minWatchers ) {
+               $titleValues = [
+                       new TitleValue( 0, 'SomeDbKey' ),
+                       new TitleValue( 0, 'OtherDbKey' ),
+                       new TitleValue( 1, 'AnotherDbKey' ),
+               ];
+
+               $mockDb = $this->getMockDb();
+
+               $dbResult = [
+                       $this->getFakeRow( [ 'wl_title' => 'SomeDbKey', 'wl_namespace' => 0, 'watchers' => 100 ] ),
+                       $this->getFakeRow( [ 'wl_title' => 'OtherDbKey', 'wl_namespace' => 0, 'watchers' => 300 ] ),
+                       $this->getFakeRow( [ 'wl_title' => 'AnotherDbKey', 'wl_namespace' => 1, 'watchers' => 500 ]
+                       ),
+               ];
+               $mockDb->expects( $this->once() )
+                       ->method( 'makeWhereFrom2d' )
+                       ->with(
+                               [ [ 'SomeDbKey' => 1, 'OtherDbKey' => 1 ], [ 'AnotherDbKey' => 1 ] ],
+                               $this->isType( 'string' ),
+                               $this->isType( 'string' )
+                       )
+                       ->will( $this->returnValue( 'makeWhereFrom2d return value' ) );
+               $mockDb->expects( $this->once() )
+                       ->method( 'select' )
+                       ->with(
+                               'watchlist',
+                               [ 'wl_title', 'wl_namespace', 'watchers' => 'COUNT(*)' ],
+                               [ 'makeWhereFrom2d return value' ],
+                               $this->isType( 'string' ),
+                               [
+                                       'GROUP BY' => [ 'wl_namespace', 'wl_title' ],
+                                       'HAVING' => 'COUNT(*) >= 50',
+                               ]
+                       )
+                       ->will(
+                               $this->returnValue( $dbResult )
+                       );
+
+               $mockCache = $this->getMockCache();
+               $mockCache->expects( $this->never() )->method( 'get' );
+               $mockCache->expects( $this->never() )->method( 'set' );
+               $mockCache->expects( $this->never() )->method( 'delete' );
+
+               $store = new WatchedItemStore(
+                       $this->getMockLoadBalancer( $mockDb ),
+                       $mockCache
+               );
+
+               $expected = [
+                       0 => [ 'SomeDbKey' => 100, 'OtherDbKey' => 300 ],
+                       1 => [ 'AnotherDbKey' => 500 ],
+               ];
+               $this->assertEquals(
+                       $expected,
+                       $store->countWatchersMultiple( $titleValues, [ 'minimumWatchers' => $minWatchers ] )
+               );
+       }
+
        public function testDuplicateEntry_nothingToDuplicate() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->once() )