Add WatchedItemStore::countVisitingWatchersMultiple
authorLeszek Manicki <leszek.manicki@wikimedia.de>
Tue, 15 Mar 2016 10:39:22 +0000 (11:39 +0100)
committerAddshore <addshorewiki@gmail.com>
Thu, 17 Mar 2016 16:17:38 +0000 (16:17 +0000)
This is for batch counting of visiting watchers, following the change
made in I2868c31fc09121de381d822e8f49194e3022bb42.
Query/logic has been extracted from ApiQueryInfo.

Bug: T129482
Change-Id: Ia9a534f5edb7af3cb7bf86be358dddb5d8c259cf

includes/WatchedItemStore.php
includes/api/ApiQueryInfo.php
tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
tests/phpunit/includes/WatchedItemStoreUnitTest.php

index b3a06e2..4e2dfa5 100644 (file)
@@ -339,6 +339,100 @@ class WatchedItemStore {
                return $watchCounts;
        }
 
+       /**
+        * Number of watchers of each page who have visited recent edits to that page
+        *
+        * @param array $targetsWithVisitThresholds array of pairs (LinkTarget $target, mixed $threshold),
+        *        $threshold is:
+        *        - a timestamp of the recent edit if $target exists (format accepted by wfTimestamp)
+        *        - null if $target doesn't exist
+        * @param int|null $minimumWatchers
+        * @return array multi-dimensional like $return[$namespaceId][$titleString] = $watchers,
+        *         where $watchers is an int:
+        *         - if the page exists, number of users watching who have visited the page recently
+        *         - if the page doesn't exist, number of users that have the page on their watchlist
+        *         - 0 means there are no visiting watchers or their number is below the minimumWatchers
+        *         option (if passed).
+        */
+       public function countVisitingWatchersMultiple(
+               array $targetsWithVisitThresholds,
+               $minimumWatchers = null
+       ) {
+               $dbr = $this->getConnection( DB_SLAVE );
+
+               $conds = $this->getVisitingWatchersCondition( $dbr, $targetsWithVisitThresholds );
+
+               $dbOptions = [ 'GROUP BY' => [ 'wl_namespace', 'wl_title' ] ];
+               if ( $minimumWatchers !== null ) {
+                       $dbOptions['HAVING'] = 'COUNT(*) >= ' . (int)$minimumWatchers;
+               }
+               $res = $dbr->select(
+                       'watchlist',
+                       [ 'wl_namespace', 'wl_title', 'watchers' => 'COUNT(*)' ],
+                       $conds,
+                       __METHOD__,
+                       $dbOptions
+               );
+
+               $this->reuseConnection( $dbr );
+
+               $watcherCounts = [];
+               foreach ( $targetsWithVisitThresholds as list( $target ) ) {
+                       /* @var LinkTarget $target */
+                       $watcherCounts[$target->getNamespace()][$target->getDBkey()] = 0;
+               }
+
+               foreach ( $res as $row ) {
+                       $watcherCounts[$row->wl_namespace][$row->wl_title] = (int)$row->watchers;
+               }
+
+               return $watcherCounts;
+       }
+
+       /**
+        * Generates condition for the query used in a batch count visiting watchers.
+        *
+        * @param IDatabase $db
+        * @param array $targetsWithVisitThresholds array of pairs (LinkTarget, last visit threshold)
+        * @return string
+        */
+       private function getVisitingWatchersCondition(
+               IDatabase $db,
+               array $targetsWithVisitThresholds
+       ) {
+               $missingTargets = [];
+               $namespaceConds = [];
+               foreach ( $targetsWithVisitThresholds as list( $target, $threshold ) ) {
+                       if ( $threshold === null ) {
+                               $missingTargets[] = $target;
+                               continue;
+                       }
+                       /* @var LinkTarget $target */
+                       $namespaceConds[$target->getNamespace()][] = $db->makeList( [
+                               'wl_title = ' . $db->addQuotes( $target->getDBkey() ),
+                               $db->makeList( [
+                                       'wl_notificationtimestamp >= ' . $db->addQuotes( $db->timestamp( $threshold ) ),
+                                       'wl_notificationtimestamp IS NULL'
+                               ], LIST_OR )
+                       ], LIST_AND );
+               }
+
+               $conds = [];
+               foreach ( $namespaceConds as $namespace => $pageConds ) {
+                       $conds[] = $db->makeList( [
+                               'wl_namespace = ' . $namespace,
+                               '(' . $db->makeList( $pageConds, LIST_OR ) . ')'
+                       ], LIST_AND );
+               }
+
+               if ( $missingTargets ) {
+                       $lb = new LinkBatch( $missingTargets );
+                       $conds[] = $lb->constructSet( 'wl', $db );
+               }
+
+               return $db->makeList( $conds, LIST_OR );
+       }
+
        /**
         * Get an item (may be cached)
         *
index f33da1e..2d382dd 100644 (file)
@@ -461,7 +461,7 @@ class ApiQueryInfo extends ApiQueryBase {
                }
 
                if ( $this->fld_visitingwatchers ) {
-                       if ( isset( $this->visitingwatchers[$ns][$dbkey] ) ) {
+                       if ( $this->visitingwatchers !== null && $this->visitingwatchers[$ns][$dbkey] !== 0 ) {
                                $pageInfo['visitingwatchers'] = $this->visitingwatchers[$ns][$dbkey];
                        } elseif ( $this->showZeroWatchers ) {
                                $pageInfo['visitingwatchers'] = 0;
@@ -831,14 +831,7 @@ class ApiQueryInfo extends ApiQueryBase {
 
                $this->showZeroWatchers = $canUnwatchedpages;
 
-               // Assemble a WHERE condition to find:
-               // * if the page exists, number of users watching who have
-               //   visited the page recently
-               // * if the page doesn't exist, number of users that have
-               //   the page on their watchlist
-               $whereStrings = [];
-
-               // For pages that exist
+               $titlesWithThresholds = [];
                if ( $this->titles ) {
                        $lb = new LinkBatch( $this->titles );
 
@@ -853,55 +846,38 @@ class ApiQueryInfo extends ApiQueryBase {
                        $this->addOption( 'GROUP BY', [ 'page_namespace', 'page_title' ] );
                        $timestampRes = $this->select( __METHOD__ );
 
-                       // Assemble SQL WHERE condition to find number of page watchers who also
-                       // visited a "recent" edit (last visited about 26 weeks before latest edit)
                        $age = $config->get( 'WatchersMaxAge' );
                        $timestamps = [];
                        foreach ( $timestampRes as $row ) {
                                $revTimestamp = wfTimestamp( TS_UNIX, (int)$row->rev_timestamp );
-                               $threshold = $db->timestamp( $revTimestamp - $age );
-                               $timestamps[$row->page_namespace][$row->page_title] = $threshold;
-                       }
-
-                       foreach ( $timestamps as $ns_key => $namespace ) {
-                               $pageStrings = [];
-                               foreach ( $namespace as $pg_key => $threshold ) {
-                                       $pageStrings[] = "wl_title = '$pg_key' AND" .
-                                               ' (wl_notificationtimestamp >= ' .
-                                               $db->addQuotes( $threshold ) .
-                                               ' OR wl_notificationtimestamp IS NULL)';
-                               }
-                               $whereStrings[] = "wl_namespace = '$ns_key' AND (" .
-                                       $db->makeList( $pageStrings, LIST_OR ) . ')';
+                               $timestamps[$row->page_namespace][$row->page_title] = $revTimestamp - $age;
                        }
+                       $titlesWithThresholds = array_map(
+                               function( LinkTarget $target ) use ( $timestamps ) {
+                                       return [
+                                               $target, $timestamps[$target->getNamespace()][$target->getDBkey()]
+                                       ];
+                               },
+                               $this->titles
+                       );
                }
 
-               // For nonexistant pages
                if ( $this->missing ) {
-                       $lb = new LinkBatch( $this->missing );
-                       $whereStrings[] = $lb->constructSet( 'wl', $db );
-               }
-
-               // Make the actual string and do the query
-               $whereString = $db->makeList( $whereStrings, LIST_OR );
-
-               $this->resetQueryParams();
-               $this->addTables( [ 'watchlist' ] );
-               $this->addFields( [
-                       'wl_namespace',
-                       'wl_title',
-                       'count' => 'COUNT(*)'
-               ] );
-               $this->addWhere( [ $whereString ] );
-               $this->addOption( 'GROUP BY', [ 'wl_namespace', 'wl_title' ] );
-               if ( !$canUnwatchedpages ) {
-                       $this->addOption( 'HAVING', "COUNT(*) >= $unwatchedPageThreshold" );
-               }
-
-               $res = $this->select( __METHOD__ );
-               foreach ( $res as $row ) {
-                       $this->visitingwatchers[$row->wl_namespace][$row->wl_title] = (int)$row->count;
-               }
+                       $titlesWithThresholds = array_merge(
+                               $titlesWithThresholds,
+                               array_map(
+                                       function( LinkTarget $target ) {
+                                               return [ $target, null ];
+                                       },
+                                       $this->missing
+                               )
+                       );
+               }
+
+               $this->visitingwatchers = WatchedItemStore::getDefaultInstance()->countVisitingWatchersMultiple(
+                       $titlesWithThresholds,
+                       !$canUnwatchedpages ? $unwatchedPageThreshold : null
+               );
        }
 
        public function getCacheMode( $params ) {
index 91dd1aa..0dea461 100644 (file)
@@ -85,6 +85,12 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase {
                        $initialVisitingWatchers - 1,
                        $store->countVisitingWatchers( $title, '20150202020202' )
                );
+               $this->assertEquals(
+                       $initialVisitingWatchers - 1,
+                       $store->countVisitingWatchersMultiple(
+                               [ [ $title, '20150202020202' ] ]
+                       )[$title->getNamespace()][$title->getDBkey()]
+               );
                $this->assertEquals(
                        $initialUnreadNotifications + 1,
                        $store->countUnreadNotifications( $user )
@@ -100,6 +106,24 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase {
                        $initialVisitingWatchers,
                        $store->countVisitingWatchers( $title, '20150202020202' )
                );
+               $this->assertEquals(
+                       $initialVisitingWatchers,
+                       $store->countVisitingWatchersMultiple(
+                               [ [ $title, '20150202020202' ] ]
+                       )[$title->getNamespace()][$title->getDBkey()]
+               );
+               $this->assertEquals(
+                       [ 0 => [ 'WatchedItemStoreIntegrationTestPage' => $initialVisitingWatchers ] ],
+                       $store->countVisitingWatchersMultiple(
+                               [ [ $title, '20150202020202' ] ], $initialVisitingWatchers
+                       )
+               );
+               $this->assertEquals(
+                       [ 0 => [ 'WatchedItemStoreIntegrationTestPage' => 0 ] ],
+                       $store->countVisitingWatchersMultiple(
+                               [ [ $title, '20150202020202' ] ], $initialVisitingWatchers + 1
+                       )
+               );
        }
 
        public function testDuplicateAllAssociatedEntries() {
index 983a5fe..869b0d2 100644 (file)
@@ -316,6 +316,247 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                $this->assertEquals( 7, $store->countVisitingWatchers( $titleValue, '111' ) );
        }
 
+       public function testCountVisitingWatchersMultiple() {
+               $titleValuesWithThresholds = [
+                       [ new TitleValue( 0, 'SomeDbKey' ), '111' ],
+                       [ new TitleValue( 0, 'OtherDbKey' ), '111' ],
+                       [ new TitleValue( 1, 'AnotherDbKey' ), '123' ],
+               ];
+
+               $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 = $this->getMockDb();
+               $mockDb->expects( $this->exactly( 2 * 3 ) )
+                       ->method( 'addQuotes' )
+                       ->will( $this->returnCallback( function( $value ) {
+                               return "'$value'";
+                       } ) );
+               $mockDb->expects( $this->exactly( 3 ) )
+                       ->method( 'timestamp' )
+                       ->will( $this->returnCallback( function( $value ) {
+                               return 'TS' . $value . 'TS';
+                       } ) );
+               $mockDb->expects( $this->any() )
+                       ->method( 'makeList' )
+                       ->with(
+                               $this->isType( 'array' ),
+                               $this->isType( 'int' )
+                       )
+                       ->will( $this->returnCallback( function( $a, $conj ) {
+                               $sqlConj = $conj === LIST_AND ? ' AND ' : ' OR ';
+                               return join( $sqlConj, array_map( function( $s ) {
+                                       return '(' . $s . ')';
+                               }, $a
+                               ) );
+                       } ) );
+               $mockDb->expects( $this->never() )
+                       ->method( 'makeWhereFrom2d' );
+
+               $expectedCond =
+                       '((wl_namespace = 0) AND (' .
+                       "(((wl_title = 'SomeDbKey') AND (" .
+                       "(wl_notificationtimestamp >= 'TS111TS') OR (wl_notificationtimestamp IS NULL)" .
+                       ')) OR (' .
+                       "(wl_title = 'OtherDbKey') AND (" .
+                       "(wl_notificationtimestamp >= 'TS111TS') OR (wl_notificationtimestamp IS NULL)" .
+                       '))))' .
+                       ') OR ((wl_namespace = 1) AND (' .
+                       "(((wl_title = 'AnotherDbKey') AND (".
+                       "(wl_notificationtimestamp >= 'TS123TS') OR (wl_notificationtimestamp IS NULL)" .
+                       ')))))';
+               $mockDb->expects( $this->once() )
+                       ->method( 'select' )
+                       ->with(
+                               'watchlist',
+                               [ 'wl_namespace', 'wl_title', 'watchers' => 'COUNT(*)' ],
+                               $expectedCond,
+                               $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->countVisitingWatchersMultiple( $titleValuesWithThresholds )
+               );
+       }
+
+       public function testCountVisitingWatchersMultiple_withMissingTargets() {
+               $titleValuesWithThresholds = [
+                       [ new TitleValue( 0, 'SomeDbKey' ), '111' ],
+                       [ new TitleValue( 0, 'OtherDbKey' ), '111' ],
+                       [ new TitleValue( 1, 'AnotherDbKey' ), '123' ],
+                       [ new TitleValue( 0, 'SomeNotExisitingDbKey' ), null ],
+                       [ new TitleValue( 0, 'OtherNotExisitingDbKey' ), null ],
+               ];
+
+               $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 ] ),
+                       $this->getFakeRow(
+                               [ 'wl_title' => 'SomeNotExisitingDbKey', 'wl_namespace' => 0, 'watchers' => 100 ]
+                       ),
+                       $this->getFakeRow(
+                               [ 'wl_title' => 'OtherNotExisitingDbKey', 'wl_namespace' => 0, 'watchers' => 200 ]
+                       ),
+               ];
+               $mockDb = $this->getMockDb();
+               $mockDb->expects( $this->exactly( 2 * 3 ) )
+                       ->method( 'addQuotes' )
+                       ->will( $this->returnCallback( function( $value ) {
+                               return "'$value'";
+                       } ) );
+               $mockDb->expects( $this->exactly( 3 ) )
+                       ->method( 'timestamp' )
+                       ->will( $this->returnCallback( function( $value ) {
+                               return 'TS' . $value . 'TS';
+                       } ) );
+               $mockDb->expects( $this->any() )
+                       ->method( 'makeList' )
+                       ->with(
+                               $this->isType( 'array' ),
+                               $this->isType( 'int' )
+                       )
+                       ->will( $this->returnCallback( function( $a, $conj ) {
+                               $sqlConj = $conj === LIST_AND ? ' AND ' : ' OR ';
+                               return join( $sqlConj, array_map( function( $s ) {
+                                       return '(' . $s . ')';
+                               }, $a
+                               ) );
+                       } ) );
+               $mockDb->expects( $this->once() )
+                       ->method( 'makeWhereFrom2d' )
+                       ->with(
+                               [ [ 'SomeNotExisitingDbKey' => 1, 'OtherNotExisitingDbKey' => 1 ] ],
+                               $this->isType( 'string' ),
+                               $this->isType( 'string' )
+                       )
+                       ->will( $this->returnValue( 'makeWhereFrom2d return value' ) );
+
+               $expectedCond =
+                       '((wl_namespace = 0) AND (' .
+                       "(((wl_title = 'SomeDbKey') AND (" .
+                       "(wl_notificationtimestamp >= 'TS111TS') OR (wl_notificationtimestamp IS NULL)" .
+                       ')) OR (' .
+                       "(wl_title = 'OtherDbKey') AND (" .
+                       "(wl_notificationtimestamp >= 'TS111TS') OR (wl_notificationtimestamp IS NULL)" .
+                       '))))' .
+                       ') OR ((wl_namespace = 1) AND (' .
+                       "(((wl_title = 'AnotherDbKey') AND (".
+                       "(wl_notificationtimestamp >= 'TS123TS') OR (wl_notificationtimestamp IS NULL)" .
+                       '))))' .
+                       ') OR ' .
+                       '(makeWhereFrom2d return value)';
+               $mockDb->expects( $this->once() )
+                       ->method( 'select' )
+                       ->with(
+                               'watchlist',
+                               [ 'wl_namespace', 'wl_title', 'watchers' => 'COUNT(*)' ],
+                               $expectedCond,
+                               $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,
+                               'SomeNotExisitingDbKey' => 100, 'OtherNotExisitingDbKey' => 200
+                       ],
+                       1 => [ 'AnotherDbKey' => 500 ],
+               ];
+               $this->assertEquals(
+                       $expected,
+                       $store->countVisitingWatchersMultiple( $titleValuesWithThresholds )
+               );
+       }
+
+       /**
+        * @dataProvider provideIntWithDbUnsafeVersion
+        */
+       public function testCountVisitingWatchersMultiple_withMinimumWatchers( $minWatchers ) {
+               $titleValuesWithThresholds = [
+                       [ new TitleValue( 0, 'SomeDbKey' ), '111' ],
+                       [ new TitleValue( 0, 'OtherDbKey' ), '111' ],
+                       [ new TitleValue( 1, 'AnotherDbKey' ), '123' ],
+               ];
+
+               $mockDb = $this->getMockDb();
+               $mockDb->expects( $this->any() )
+                       ->method( 'makeList' )
+                       ->will( $this->returnValue( 'makeList return value' ) );
+               $mockDb->expects( $this->once() )
+                       ->method( 'select' )
+                       ->with(
+                               'watchlist',
+                               [ 'wl_namespace', 'wl_title', 'watchers' => 'COUNT(*)' ],
+                               'makeList return value',
+                               $this->isType( 'string' ),
+                               [
+                                       'GROUP BY' => [ 'wl_namespace', 'wl_title' ],
+                                       'HAVING' => 'COUNT(*) >= 50',
+                               ]
+                       )
+                       ->will(
+                               $this->returnValue( [] )
+                       );
+
+               $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' => 0, 'OtherDbKey' => 0 ],
+                       1 => [ 'AnotherDbKey' => 0 ],
+               ];
+               $this->assertEquals(
+                       $expected,
+                       $store->countVisitingWatchersMultiple( $titleValuesWithThresholds, $minWatchers )
+               );
+       }
+
        public function testCountUnreadNotifications() {
                $user = $this->getMockNonAnonUserWithId( 1 );