From 846d373c28f3d032aaa5cafdab3e63539b8deae7 Mon Sep 17 00:00:00 2001 From: addshore Date: Thu, 10 Mar 2016 12:04:31 +0000 Subject: [PATCH] Add countUnreadNotifications to WatchedItemStore This query / logic has been extracted from ApiQueryUserInfo. Unit & Integration tests have also been added. Relating to the task linked this is the last change needed in this ApiQueryUserInfo! Bug: T129482 Change-Id: I91aa109416c16cd1f257c9de46669e35d6fd34d7 --- includes/WatchedItemStore.php | 38 ++++++ includes/api/ApiQueryUserInfo.php | 18 +-- .../WatchedItemStoreIntegrationTest.php | 9 ++ .../includes/WatchedItemStoreUnitTest.php | 108 +++++++++++++++++- 4 files changed, 158 insertions(+), 15 deletions(-) diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php index c59beec4f0..1a9868e154 100644 --- a/includes/WatchedItemStore.php +++ b/includes/WatchedItemStore.php @@ -579,6 +579,44 @@ class WatchedItemStore { return $notificationTimestamp; } + /** + * @param User $user + * @param int $unreadLimit + * + * @return int|bool The number of unread notifications + * true if greater than or equal to $unreadLimit + */ + public function countUnreadNotifications( User $user, $unreadLimit = null ) { + $queryOptions = []; + if ( $unreadLimit !== null ) { + $unreadLimit = (int)$unreadLimit; + $queryOptions['LIMIT'] = $unreadLimit; + } + + $dbr = $this->loadBalancer->getConnection( DB_SLAVE, [ 'watchlist' ] ); + $rowCount = $dbr->selectRowCount( + 'watchlist', + '1', + [ + 'wl_user' => $user->getId(), + 'wl_notificationtimestamp IS NOT NULL', + ], + __METHOD__, + $queryOptions + ); + $this->loadBalancer->reuseConnection( $dbr ); + + if ( !isset( $unreadLimit ) ) { + return $rowCount; + } + + if ( $rowCount >= $unreadLimit ) { + return true; + } + + return $rowCount; + } + /** * Check if the given title already is watched by the user, and if so * add a watch for the new title. diff --git a/includes/api/ApiQueryUserInfo.php b/includes/api/ApiQueryUserInfo.php index 9e7e62e7be..0fc443ab21 100644 --- a/includes/api/ApiQueryUserInfo.php +++ b/includes/api/ApiQueryUserInfo.php @@ -225,23 +225,15 @@ class ApiQueryUserInfo extends ApiQueryBase { } if ( isset( $this->prop['unreadcount'] ) ) { - $dbr = $this->getQuery()->getNamedDB( 'watchlist', DB_SLAVE, 'watchlist' ); - - $count = $dbr->selectRowCount( - 'watchlist', - '1', - [ - 'wl_user' => $user->getId(), - 'wl_notificationtimestamp IS NOT NULL', - ], - __METHOD__, - [ 'LIMIT' => self::WL_UNREAD_LIMIT ] + $unreadNotifications = WatchedItemStore::getDefaultInstance()->countUnreadNotifications( + $user, + self::WL_UNREAD_LIMIT ); - if ( $count >= self::WL_UNREAD_LIMIT ) { + if ( $unreadNotifications === true ) { $vals['unreadcount'] = self::WL_UNREAD_LIMIT . '+'; } else { - $vals['unreadcount'] = $count; + $vals['unreadcount'] = $unreadNotifications; } } diff --git a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php index 2aa9027408..a34f8ee43b 100644 --- a/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php +++ b/tests/phpunit/includes/WatchedItemStoreIntegrationTest.php @@ -71,6 +71,7 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase { $store->addWatch( $user, $title ); $this->assertNull( $store->loadWatchedItem( $user, $title )->getNotificationTimestamp() ); $initialVisitingWatchers = $store->countVisitingWatchers( $title, '20150202020202' ); + $initialUnreadNotifications = $store->countUnreadNotifications( $user ); $store->updateNotificationTimestamp( $otherUser, $title, '20150202010101' ); $this->assertEquals( @@ -81,6 +82,14 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase { $initialVisitingWatchers - 1, $store->countVisitingWatchers( $title, '20150202020202' ) ); + $this->assertEquals( + $initialUnreadNotifications + 1, + $store->countUnreadNotifications( $user ) + ); + $this->assertSame( + true, + $store->countUnreadNotifications( $user, $initialUnreadNotifications + 1 ) + ); $this->assertTrue( $store->resetNotificationTimestamp( $user, $title ) ); $this->assertNull( $store->getWatchedItem( $user, $title )->getNotificationTimestamp() ); diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php index 988ffed01a..1dd819bf2a 100644 --- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php @@ -166,7 +166,7 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase { $this->assertEquals( $expected, $store->countWatchersMultiple( $titleValues ) ); } - public function provideMinimumWatchers() { + public function provideIntWithDbUnsafeVersion() { return [ [ 50 ], [ "50; DROP TABLE watchlist;\n--" ], @@ -174,7 +174,7 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase { } /** - * @dataProvider provideMinimumWatchers + * @dataProvider provideIntWithDbUnsafeVersion */ public function testCountWatchersMultiple_withMinimumWatchers( $minWatchers ) { $titleValues = [ @@ -276,6 +276,110 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase { $this->assertEquals( 7, $store->countVisitingWatchers( $titleValue, '111' ) ); } + public function testCountUnreadNotifications() { + $user = $this->getMockNonAnonUserWithId( 1 ); + + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->exactly( 1 ) ) + ->method( 'selectRowCount' ) + ->with( + 'watchlist', + '1', + [ + "wl_notificationtimestamp IS NOT NULL", + 'wl_user' => 1, + ], + $this->isType( 'string' ) + ) + ->will( $this->returnValue( 9 ) ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->never() )->method( 'set' ); + $mockCache->expects( $this->never() )->method( 'get' ); + $mockCache->expects( $this->never() )->method( 'delete' ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache + ); + + $this->assertEquals( 9, $store->countUnreadNotifications( $user ) ); + } + + /** + * @dataProvider provideIntWithDbUnsafeVersion + */ + public function testCountUnreadNotifications_withUnreadLimit_overLimit( $limit ) { + $user = $this->getMockNonAnonUserWithId( 1 ); + + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->exactly( 1 ) ) + ->method( 'selectRowCount' ) + ->with( + 'watchlist', + '1', + [ + "wl_notificationtimestamp IS NOT NULL", + 'wl_user' => 1, + ], + $this->isType( 'string' ), + [ 'LIMIT' => 50 ] + ) + ->will( $this->returnValue( 50 ) ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->never() )->method( 'set' ); + $mockCache->expects( $this->never() )->method( 'get' ); + $mockCache->expects( $this->never() )->method( 'delete' ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache + ); + + $this->assertSame( + true, + $store->countUnreadNotifications( $user, $limit ) + ); + } + + /** + * @dataProvider provideIntWithDbUnsafeVersion + */ + public function testCountUnreadNotifications_withUnreadLimit_underLimit( $limit ) { + $user = $this->getMockNonAnonUserWithId( 1 ); + + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->exactly( 1 ) ) + ->method( 'selectRowCount' ) + ->with( + 'watchlist', + '1', + [ + "wl_notificationtimestamp IS NOT NULL", + 'wl_user' => 1, + ], + $this->isType( 'string' ), + [ 'LIMIT' => 50 ] + ) + ->will( $this->returnValue( 9 ) ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->never() )->method( 'set' ); + $mockCache->expects( $this->never() )->method( 'get' ); + $mockCache->expects( $this->never() )->method( 'delete' ); + + $store = new WatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache + ); + + $this->assertEquals( + 9, + $store->countUnreadNotifications( $user, $limit ) + ); + } + public function testDuplicateEntry_nothingToDuplicate() { $mockDb = $this->getMockDb(); $mockDb->expects( $this->once() ) -- 2.20.1