jenkins-bot has submitted this change and it was merged.
Change subject: Add WatchedItemStore::countVisitingWatchersMultiple
......................................................................
Add WatchedItemStore::countVisitingWatchersMultiple
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
---
M includes/WatchedItemStore.php
M includes/api/ApiQueryInfo.php
M tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
M tests/phpunit/includes/WatchedItemStoreUnitTest.php
4 files changed, 383 insertions(+), 48 deletions(-)
Approvals:
WMDE-Fisch: Looks good to me, but someone else must approve
Addshore: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php
index b3a06e2..4e2dfa5 100644
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -340,6 +340,100 @@
}
/**
+ * 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)
*
* @param User $user
diff --git a/includes/api/ApiQueryInfo.php b/includes/api/ApiQueryInfo.php
index f33da1e..2d382dd 100644
--- a/includes/api/ApiQueryInfo.php
+++ b/includes/api/ApiQueryInfo.php
@@ -461,7 +461,7 @@
}
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 @@
$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 @@
$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;
+
$timestamps[$row->page_namespace][$row->page_title] = $revTimestamp - $age;
}
-
- 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 )
. ')';
- }
+ $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 );
+ $titlesWithThresholds = array_merge(
+ $titlesWithThresholds,
+ array_map(
+ function( LinkTarget $target ) {
+ return [ $target, null ];
+ },
+ $this->missing
+ )
+ );
}
- // 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;
- }
+ $this->visitingwatchers =
WatchedItemStore::getDefaultInstance()->countVisitingWatchersMultiple(
+ $titlesWithThresholds,
+ !$canUnwatchedpages ? $unwatchedPageThreshold : null
+ );
}
public function getCacheMode( $params ) {
diff --git a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
index 91dd1aa..0dea461 100644
--- a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php
@@ -86,6 +86,12 @@
$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 @@
$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() {
diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
index 983a5fe..869b0d2 100644
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -316,6 +316,247 @@
$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 );
--
To view, visit https://gerrit.wikimedia.org/r/277488
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia9a534f5edb7af3cb7bf86be358dddb5d8c259cf
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: WMDE-leszek <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[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
