From c1b4b19de384d4e28f1dbdcc0fd59212a6742618 Mon Sep 17 00:00:00 2001 From: Leszek Manicki Date: Fri, 17 Jun 2016 13:11:05 +0200 Subject: [PATCH] Refactor database-related code in ApiQueryWatchlistRaw Database queries used to get user's watchlist items in API are quite complex due to number of options oferred by the API. Generating the query is moved to WatchedItemQueryService. ApiQueryWatchlistRaw no longer contains database-related code. Simple user watchlist item lookups should use WatchedItemStore. ApiQueryWatchlistRaw tests have been introduced in I9c07aa237607143985f0efe20ed0065d2bde27e4 Bug: T132566 Change-Id: I875a92074b52c00ac11db1fa05615abbf5262ab1 --- includes/WatchedItemQueryService.php | 184 ++++++++++- includes/api/ApiQueryWatchlistRaw.php | 85 ++--- .../WatchedItemQueryServiceUnitTest.php | 299 ++++++++++++++++++ .../ApiQueryWatchlistRawIntegrationTest.php | 4 +- 4 files changed, 508 insertions(+), 64 deletions(-) diff --git a/includes/WatchedItemQueryService.php b/includes/WatchedItemQueryService.php index 14d6aac77f..3dcd30f006 100644 --- a/includes/WatchedItemQueryService.php +++ b/includes/WatchedItemQueryService.php @@ -1,5 +1,6 @@ getConnection(); - $fields = $this->getFields( $options ); - $conds = $this->getConds( $db, $user, $options ); - $dbOptions = $this->getDbOptions( $options ); - $joinConds = $this->getJoinConds( $options ); + $fields = $this->getWatchedItemsWithRCInfoQueryFields( $options ); + $conds = $this->getWatchedItemsWithRCInfoQueryConds( $db, $user, $options ); + $dbOptions = $this->getWatchedItemsWithRCInfoQueryDbOptions( $options ); + $joinConds = $this->getWatchedItemsWithRCInfoQueryJoinConds( $options ); $res = $db->select( $tables, @@ -192,6 +198,81 @@ class WatchedItemQueryService { return $items; } + /** + * For simple listing of user's watchlist items, see WatchedItemStore::getWatchedItemsForUser + * + * @param User $user + * @param array $options Allowed keys: + * 'sort' => string optional sorting by namespace ID and title + * one of the self::SORT_* constants + * 'namespaceIds' => int[] optional namespace IDs to filter by (defaults to all namespaces) + * 'limit' => int maximum number of items to return + * 'filter' => string optional filter, one of the self::FILTER_* contants + * 'from' => LinkTarget requires 'sort' key, only return items starting from + * those related to the link target + * 'until' => LinkTarget requires 'sort' key, only return items until + * those related to the link target + * 'startFrom' => LinkTarget requires 'sort' key, only return items starting from + * those related to the link target, allows to skip some link targets + * specified using the form option + * @return WatchedItem[] + */ + public function getWatchedItemsForUser( User $user, array $options = [] ) { + if ( $user->isAnon() ) { + // TODO: should this just return an empty array or rather complain loud at this point + // as e.g. ApiBase::getWatchlistUser does? + return []; + } + + $options += [ 'namespaceIds' => [] ]; + + Assert::parameter( + !isset( $options['sort'] ) || in_array( $options['sort'], [ self::SORT_ASC, self::SORT_DESC ] ), + '$options[\'sort\']', + 'must be SORT_ASC or SORT_DESC' + ); + Assert::parameter( + !isset( $options['filter'] ) || in_array( + $options['filter'], [ self::FILTER_CHANGED, self::FILTER_NOT_CHANGED ] + ), + '$options[\'filter\']', + 'must be FILTER_CHANGED or FILTER_NOT_CHANGED' + ); + Assert::parameter( + !isset( $options['from'] ) && !isset( $options['until'] ) && !isset( $options['startFrom'] ) + || isset( $options['sort'] ), + '$options[\'sort\']', + 'must be provided if any of "from", "until", "startFrom" options is provided' + ); + + $db = $this->getConnection(); + + $conds = $this->getWatchedItemsForUserQueryConds( $db, $user, $options ); + $dbOptions = $this->getWatchedItemsForUserQueryDbOptions( $options ); + + $res = $db->select( + 'watchlist', + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], + $conds, + __METHOD__, + $dbOptions + ); + + $this->reuseConnection( $db ); + + $watchedItems = []; + foreach ( $res as $row ) { + // todo these could all be cached at some point? + $watchedItems[] = new WatchedItem( + $user, + new TitleValue( (int)$row->wl_namespace, $row->wl_title ), + $row->wl_notificationtimestamp + ); + } + + return $watchedItems; + } + private function getRecentChangeFieldsFromRow( stdClass $row ) { // This can be simplified to single array_filter call filtering by key value, // once we stop supporting PHP 5.5 @@ -205,7 +286,7 @@ class WatchedItemQueryService { return array_intersect_key( $allFields, array_flip( $rcKeys ) ); } - private function getFields( array $options ) { + private function getWatchedItemsWithRCInfoQueryFields( array $options ) { $fields = [ 'rc_id', 'rc_namespace', @@ -255,7 +336,11 @@ class WatchedItemQueryService { return $fields; } - private function getConds( DatabaseBase $db, User $user, array $options ) { + private function getWatchedItemsWithRCInfoQueryConds( + DatabaseBase $db, + User $user, + array $options + ) { $watchlistOwnerId = $this->getWatchlistOwnerId( $user, $options ); $conds = [ 'wl_user' => $watchlistOwnerId ]; @@ -274,7 +359,10 @@ class WatchedItemQueryService { $conds['rc_type'] = array_map( 'intval', $options['rcTypes'] ); } - $conds = array_merge( $conds, $this->getFilterConds( $user, $options ) ); + $conds = array_merge( + $conds, + $this->getWatchedItemsWithRCInfoQueryFilterConds( $user, $options ) + ); $conds = array_merge( $conds, $this->getStartEndConds( $db, $options ) ); @@ -316,7 +404,7 @@ class WatchedItemQueryService { return $user->getId(); } - private function getFilterConds( User $user, array $options ) { + private function getWatchedItemsWithRCInfoQueryFilterConds( User $user, array $options ) { $conds = []; if ( in_array( self::FILTER_MINOR, $options['filters'] ) ) { @@ -441,7 +529,62 @@ class WatchedItemQueryService { ); } - private function getDbOptions( array $options ) { + private function getWatchedItemsForUserQueryConds( DatabaseBase $db, User $user, array $options ) { + $conds = [ 'wl_user' => $user->getId() ]; + if ( $options['namespaceIds'] ) { + $conds['wl_namespace'] = array_map( 'intval', $options['namespaceIds'] ); + } + if ( isset( $options['filter'] ) ) { + $filter = $options['filter']; + if ( $filter === self::FILTER_CHANGED ) { + $conds[] = 'wl_notificationtimestamp IS NOT NULL'; + } else { + $conds[] = 'wl_notificationtimestamp IS NULL'; + } + } + + if ( isset( $options['from'] ) ) { + $op = $options['sort'] === self::SORT_ASC ? '>' : '<'; + $conds[] = $this->getFromUntilTargetConds( $db, $options['from'], $op ); + } + if ( isset( $options['until'] ) ) { + $op = $options['sort'] === self::SORT_ASC ? '<' : '>'; + $conds[] = $this->getFromUntilTargetConds( $db, $options['until'], $op ); + } + if ( isset( $options['startFrom'] ) ) { + $op = $options['sort'] === self::SORT_ASC ? '>' : '<'; + $conds[] = $this->getFromUntilTargetConds( $db, $options['startFrom'], $op ); + } + + return $conds; + } + + /** + * Creates a query condition part for getting only items before or after the given link target + * (while ordering using $sort mode) + * + * @param DatabaseBase $db + * @param LinkTarget $target + * @param string $op comparison operator to use in the conditions + * @return string + */ + private function getFromUntilTargetConds( DatabaseBase $db, LinkTarget $target, $op ) { + return $db->makeList( + [ + "wl_namespace $op " . $target->getNamespace(), + $db->makeList( + [ + 'wl_namespace = ' . $target->getNamespace(), + "wl_title $op= " . $db->addQuotes( $target->getDBkey() ) + ], + LIST_AND + ) + ], + LIST_OR + ); + } + + private function getWatchedItemsWithRCInfoQueryDbOptions( array $options ) { $dbOptions = []; if ( array_key_exists( 'dir', $options ) ) { @@ -456,7 +599,24 @@ class WatchedItemQueryService { return $dbOptions; } - private function getJoinConds( array $options ) { + private function getWatchedItemsForUserQueryDbOptions( array $options ) { + $dbOptions = []; + if ( array_key_exists( 'sort', $options ) ) { + $dbOptions['ORDER BY'] = [ + "wl_namespace {$options['sort']}", + "wl_title {$options['sort']}" + ]; + if ( count( $options['namespaceIds'] ) === 1 ) { + $dbOptions['ORDER BY'] = "wl_title {$options['sort']}"; + } + } + if ( array_key_exists( 'limit', $options ) ) { + $dbOptions['LIMIT'] = (int)$options['limit']; + } + return $dbOptions; + } + + private function getWatchedItemsWithRCInfoQueryJoinConds( array $options ) { $joinConds = [ 'watchlist' => [ 'INNER JOIN', [ diff --git a/includes/api/ApiQueryWatchlistRaw.php b/includes/api/ApiQueryWatchlistRaw.php index 64b97fe828..806861e800 100644 --- a/includes/api/ApiQueryWatchlistRaw.php +++ b/includes/api/ApiQueryWatchlistRaw.php @@ -24,6 +24,8 @@ * @file */ +use MediaWiki\MediaWikiServices; + /** * This query action allows clients to retrieve a list of pages * on the logged-in user's watchlist. @@ -49,95 +51,78 @@ class ApiQueryWatchlistRaw extends ApiQueryGeneratorBase { * @return void */ private function run( $resultPageSet = null ) { - $this->selectNamedDB( 'watchlist', DB_SLAVE, 'watchlist' ); - $params = $this->extractRequestParams(); $user = $this->getWatchlistUser( $params ); $prop = array_flip( (array)$params['prop'] ); $show = array_flip( (array)$params['show'] ); - if ( isset( $show['changed'] ) && isset( $show['!changed'] ) ) { + if ( isset( $show[WatchedItemQueryService::FILTER_CHANGED] ) + && isset( $show[WatchedItemQueryService::FILTER_NOT_CHANGED] ) + ) { $this->dieUsageMsg( 'show' ); } - $this->addTables( 'watchlist' ); - $this->addFields( [ 'wl_namespace', 'wl_title' ] ); - $this->addFieldsIf( 'wl_notificationtimestamp', isset( $prop['changed'] ) ); - $this->addWhereFld( 'wl_user', $user->getId() ); - $this->addWhereFld( 'wl_namespace', $params['namespace'] ); - $this->addWhereIf( 'wl_notificationtimestamp IS NOT NULL', isset( $show['changed'] ) ); - $this->addWhereIf( 'wl_notificationtimestamp IS NULL', isset( $show['!changed'] ) ); + $options = []; + if ( $params['namespace'] ) { + $options['namespaceIds'] = $params['namespace']; + } + if ( isset( $show[WatchedItemQueryService::FILTER_CHANGED] ) ) { + $options['filter'] = WatchedItemQueryService::FILTER_CHANGED; + } + if ( isset( $show[WatchedItemQueryService::FILTER_NOT_CHANGED] ) ) { + $options['filter'] = WatchedItemQueryService::FILTER_NOT_CHANGED; + } if ( isset( $params['continue'] ) ) { $cont = explode( '|', $params['continue'] ); $this->dieContinueUsageIf( count( $cont ) != 2 ); $ns = intval( $cont[0] ); $this->dieContinueUsageIf( strval( $ns ) !== $cont[0] ); - $title = $this->getDB()->addQuotes( $cont[1] ); - $op = $params['dir'] == 'ascending' ? '>' : '<'; - $this->addWhere( - "wl_namespace $op $ns OR " . - "(wl_namespace = $ns AND " . - "wl_title $op= $title)" - ); + $title = $cont[1]; + $options['startFrom'] = new TitleValue( $ns, $title ); } if ( isset( $params['fromtitle'] ) ) { list( $ns, $title ) = $this->prefixedTitlePartToKey( $params['fromtitle'] ); - $title = $this->getDB()->addQuotes( $title ); - $op = $params['dir'] == 'ascending' ? '>' : '<'; - $this->addWhere( - "wl_namespace $op $ns OR " . - "(wl_namespace = $ns AND " . - "wl_title $op= $title)" - ); + $options['from'] = new TitleValue( $ns, $title ); } if ( isset( $params['totitle'] ) ) { list( $ns, $title ) = $this->prefixedTitlePartToKey( $params['totitle'] ); - $title = $this->getDB()->addQuotes( $title ); - $op = $params['dir'] == 'ascending' ? '<' : '>'; // Reversed from above! - $this->addWhere( - "wl_namespace $op $ns OR " . - "(wl_namespace = $ns AND " . - "wl_title $op= $title)" - ); + $options['until'] = new TitleValue( $ns, $title ); } - $sort = ( $params['dir'] == 'descending' ? ' DESC' : '' ); - // Don't ORDER BY wl_namespace if it's constant in the WHERE clause - if ( count( $params['namespace'] ) == 1 ) { - $this->addOption( 'ORDER BY', 'wl_title' . $sort ); - } else { - $this->addOption( 'ORDER BY', [ - 'wl_namespace' . $sort, - 'wl_title' . $sort - ] ); + $options['sort'] = WatchedItemStore::SORT_ASC; + if ( $params['dir'] === 'descending' ) { + $options['sort'] = WatchedItemStore::SORT_DESC; } - $this->addOption( 'LIMIT', $params['limit'] + 1 ); - $res = $this->select( __METHOD__ ); + $options['limit'] = $params['limit'] + 1; $titles = []; $count = 0; - foreach ( $res as $row ) { + $items = MediaWikiServices::getInstance()->getWatchedItemQueryService() + ->getWatchedItemsForUser( $user, $options ); + foreach ( $items as $item ) { + $ns = $item->getLinkTarget()->getNamespace(); + $dbKey = $item->getLinkTarget()->getDBkey(); if ( ++$count > $params['limit'] ) { // We've reached the one extra which shows that there are // additional pages to be had. Stop here... - $this->setContinueEnumParameter( 'continue', $row->wl_namespace . '|' . $row->wl_title ); + $this->setContinueEnumParameter( 'continue', $ns . '|' . $dbKey ); break; } - $t = Title::makeTitle( $row->wl_namespace, $row->wl_title ); + $t = Title::makeTitle( $ns, $dbKey ); if ( is_null( $resultPageSet ) ) { $vals = []; ApiQueryBase::addTitleInfo( $vals, $t ); - if ( isset( $prop['changed'] ) && !is_null( $row->wl_notificationtimestamp ) ) { - $vals['changed'] = wfTimestamp( TS_ISO_8601, $row->wl_notificationtimestamp ); + if ( isset( $prop['changed'] ) && !is_null( $item->getNotificationTimestamp() ) ) { + $vals['changed'] = wfTimestamp( TS_ISO_8601, $item->getNotificationTimestamp() ); } $fit = $this->getResult()->addValue( $this->getModuleName(), null, $vals ); if ( !$fit ) { - $this->setContinueEnumParameter( 'continue', $row->wl_namespace . '|' . $row->wl_title ); + $this->setContinueEnumParameter( 'continue', $ns . '|' . $dbKey ); break; } } else { @@ -177,8 +162,8 @@ class ApiQueryWatchlistRaw extends ApiQueryGeneratorBase { 'show' => [ ApiBase::PARAM_ISMULTI => true, ApiBase::PARAM_TYPE => [ - 'changed', - '!changed', + WatchedItemQueryService::FILTER_CHANGED, + WatchedItemQueryService::FILTER_NOT_CHANGED ] ], 'owner' => [ diff --git a/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php b/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php index b63a1f4896..1d232fee8e 100644 --- a/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php +++ b/tests/phpunit/includes/WatchedItemQueryServiceUnitTest.php @@ -141,6 +141,14 @@ class WatchedItemQueryServiceUnitTest extends PHPUnit_Framework_TestCase { return $mock; } + private function getMockAnonUser() { + $mock = $this->getMock( User::class ); + $mock->expects( $this->any() ) + ->method( 'isAnon' ) + ->will( $this->returnValue( true ) ); + return $mock; + } + private function getFakeRow( array $rowValues ) { $fakeRow = new stdClass(); foreach ( $rowValues as $valueName => $value ) { @@ -1010,4 +1018,295 @@ class WatchedItemQueryServiceUnitTest extends PHPUnit_Framework_TestCase { ); } + public function testGetWatchedItemsForUser() { + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'select' ) + ->with( + 'watchlist', + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], + [ 'wl_user' => 1 ] + ) + ->will( $this->returnValue( [ + $this->getFakeRow( [ + 'wl_namespace' => 0, + 'wl_title' => 'Foo1', + 'wl_notificationtimestamp' => '20151212010101', + ] ), + $this->getFakeRow( [ + 'wl_namespace' => 1, + 'wl_title' => 'Foo2', + 'wl_notificationtimestamp' => null, + ] ), + ] ) ); + + $queryService = new WatchedItemQueryService( $this->getMockLoadBalancer( $mockDb ) ); + $user = $this->getMockNonAnonUserWithId( 1 ); + + $items = $queryService->getWatchedItemsForUser( $user ); + + $this->assertInternalType( 'array', $items ); + $this->assertCount( 2, $items ); + $this->assertContainsOnlyInstancesOf( WatchedItem::class, $items ); + $this->assertEquals( + new WatchedItem( $user, new TitleValue( 0, 'Foo1' ), '20151212010101' ), + $items[0] + ); + $this->assertEquals( + new WatchedItem( $user, new TitleValue( 1, 'Foo2' ), null ), + $items[1] + ); + } + + public function provideGetWatchedItemsForUserOptions() { + return [ + [ + [ 'namespaceIds' => [ 0, 1 ], ], + [ 'wl_namespace' => [ 0, 1 ], ], + [] + ], + [ + [ 'sort' => WatchedItemQueryService::SORT_ASC, ], + [], + [ 'ORDER BY' => [ 'wl_namespace ASC', 'wl_title ASC' ] ] + ], + [ + [ + 'namespaceIds' => [ 0 ], + 'sort' => WatchedItemQueryService::SORT_ASC, + ], + [ 'wl_namespace' => [ 0 ], ], + [ 'ORDER BY' => 'wl_title ASC' ] + ], + [ + [ 'limit' => 10 ], + [], + [ 'LIMIT' => 10 ] + ], + [ + [ + 'namespaceIds' => [ 0, "1; DROP TABLE watchlist;\n--" ], + 'limit' => "10; DROP TABLE watchlist;\n--", + ], + [ 'wl_namespace' => [ 0, 1 ], ], + [ 'LIMIT' => 10 ] + ], + [ + [ 'filter' => WatchedItemQueryService::FILTER_CHANGED ], + [ 'wl_notificationtimestamp IS NOT NULL' ], + [] + ], + [ + [ 'filter' => WatchedItemQueryService::FILTER_NOT_CHANGED ], + [ 'wl_notificationtimestamp IS NULL' ], + [] + ], + [ + [ 'sort' => WatchedItemQueryService::SORT_DESC, ], + [], + [ 'ORDER BY' => [ 'wl_namespace DESC', 'wl_title DESC' ] ] + ], + [ + [ + 'namespaceIds' => [ 0 ], + 'sort' => WatchedItemQueryService::SORT_DESC, + ], + [ 'wl_namespace' => [ 0 ], ], + [ 'ORDER BY' => 'wl_title DESC' ] + ], + ]; + } + + /** + * @dataProvider provideGetWatchedItemsForUserOptions + */ + public function testGetWatchedItemsForUser_optionsAndEmptyResult( + array $options, + array $expectedConds, + array $expectedDbOptions + ) { + $mockDb = $this->getMockDb(); + $user = $this->getMockNonAnonUserWithId( 1 ); + + $expectedConds = array_merge( [ 'wl_user' => 1 ], $expectedConds ); + $mockDb->expects( $this->once() ) + ->method( 'select' ) + ->with( + 'watchlist', + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], + $expectedConds, + $this->isType( 'string' ), + $expectedDbOptions + ) + ->will( $this->returnValue( [] ) ); + + $queryService = new WatchedItemQueryService( $this->getMockLoadBalancer( $mockDb ) ); + + $items = $queryService->getWatchedItemsForUser( $user, $options ); + $this->assertEmpty( $items ); + } + + public function provideGetWatchedItemsForUser_fromUntilStartFromOptions() { + return [ + [ + [ + 'from' => new TitleValue( 0, 'SomeDbKey' ), + 'sort' => WatchedItemQueryService::SORT_ASC + ], + [ "(wl_namespace > 0) OR ((wl_namespace = 0) AND (wl_title >= 'SomeDbKey'))", ], + [ 'ORDER BY' => [ 'wl_namespace ASC', 'wl_title ASC' ] ] + ], + [ + [ + 'from' => new TitleValue( 0, 'SomeDbKey' ), + 'sort' => WatchedItemQueryService::SORT_DESC, + ], + [ "(wl_namespace < 0) OR ((wl_namespace = 0) AND (wl_title <= 'SomeDbKey'))", ], + [ 'ORDER BY' => [ 'wl_namespace DESC', 'wl_title DESC' ] ] + ], + [ + [ + 'until' => new TitleValue( 0, 'SomeDbKey' ), + 'sort' => WatchedItemQueryService::SORT_ASC + ], + [ "(wl_namespace < 0) OR ((wl_namespace = 0) AND (wl_title <= 'SomeDbKey'))", ], + [ 'ORDER BY' => [ 'wl_namespace ASC', 'wl_title ASC' ] ] + ], + [ + [ + 'until' => new TitleValue( 0, 'SomeDbKey' ), + 'sort' => WatchedItemQueryService::SORT_DESC + ], + [ "(wl_namespace > 0) OR ((wl_namespace = 0) AND (wl_title >= 'SomeDbKey'))", ], + [ 'ORDER BY' => [ 'wl_namespace DESC', 'wl_title DESC' ] ] + ], + [ + [ + 'from' => new TitleValue( 0, 'AnotherDbKey' ), + 'until' => new TitleValue( 0, 'SomeOtherDbKey' ), + 'startFrom' => new TitleValue( 0, 'SomeDbKey' ), + 'sort' => WatchedItemQueryService::SORT_ASC + ], + [ + "(wl_namespace > 0) OR ((wl_namespace = 0) AND (wl_title >= 'AnotherDbKey'))", + "(wl_namespace < 0) OR ((wl_namespace = 0) AND (wl_title <= 'SomeOtherDbKey'))", + "(wl_namespace > 0) OR ((wl_namespace = 0) AND (wl_title >= 'SomeDbKey'))", + ], + [ 'ORDER BY' => [ 'wl_namespace ASC', 'wl_title ASC' ] ] + ], + [ + [ + 'from' => new TitleValue( 0, 'SomeOtherDbKey' ), + 'until' => new TitleValue( 0, 'AnotherDbKey' ), + 'startFrom' => new TitleValue( 0, 'SomeDbKey' ), + 'sort' => WatchedItemQueryService::SORT_DESC + ], + [ + "(wl_namespace < 0) OR ((wl_namespace = 0) AND (wl_title <= 'SomeOtherDbKey'))", + "(wl_namespace > 0) OR ((wl_namespace = 0) AND (wl_title >= 'AnotherDbKey'))", + "(wl_namespace < 0) OR ((wl_namespace = 0) AND (wl_title <= 'SomeDbKey'))", + ], + [ 'ORDER BY' => [ 'wl_namespace DESC', 'wl_title DESC' ] ] + ], + ]; + } + + /** + * @dataProvider provideGetWatchedItemsForUser_fromUntilStartFromOptions + */ + public function testGetWatchedItemsForUser_fromUntilStartFromOptions( + array $options, + array $expectedConds, + array $expectedDbOptions + ) { + $user = $this->getMockNonAnonUserWithId( 1 ); + + $expectedConds = array_merge( [ 'wl_user' => 1 ], $expectedConds ); + + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->any() ) + ->method( 'addQuotes' ) + ->will( $this->returnCallback( function( $value ) { + return "'$value'"; + } ) ); + $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( 'select' ) + ->with( + 'watchlist', + [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ], + $expectedConds, + $this->isType( 'string' ), + $expectedDbOptions + ) + ->will( $this->returnValue( [] ) ); + + $queryService = new WatchedItemQueryService( $this->getMockLoadBalancer( $mockDb ) ); + + $items = $queryService->getWatchedItemsForUser( $user, $options ); + $this->assertEmpty( $items ); + } + + public function getWatchedItemsForUserInvalidOptionsProvider() { + return [ + [ + [ 'sort' => 'foo' ], + 'Bad value for parameter $options[\'sort\']' + ], + [ + [ 'filter' => 'foo' ], + 'Bad value for parameter $options[\'filter\']' + ], + [ + [ 'from' => new TitleValue( 0, 'SomeDbKey' ), ], + 'Bad value for parameter $options[\'sort\']: must be provided' + ], + [ + [ 'until' => new TitleValue( 0, 'SomeDbKey' ), ], + 'Bad value for parameter $options[\'sort\']: must be provided' + ], + [ + [ 'startFrom' => new TitleValue( 0, 'SomeDbKey' ), ], + 'Bad value for parameter $options[\'sort\']: must be provided' + ], + ]; + } + + /** + * @dataProvider getWatchedItemsForUserInvalidOptionsProvider + */ + public function testGetWatchedItemsForUser_invalidOptionThrowsException( + array $options, + $expectedInExceptionMessage + ) { + $queryService = new WatchedItemQueryService( $this->getMockLoadBalancer( $this->getMockDb() ) ); + + $this->setExpectedException( InvalidArgumentException::class, $expectedInExceptionMessage ); + $queryService->getWatchedItemsForUser( $this->getMockNonAnonUserWithId( 1 ), $options ); + } + + public function testGetWatchedItemsForUser_userNotAllowedToViewWatchlist() { + $mockDb = $this->getMockDb(); + + $mockDb->expects( $this->never() ) + ->method( $this->anything() ); + + $queryService = new WatchedItemQueryService( $this->getMockLoadBalancer( $mockDb ) ); + + $items = $queryService->getWatchedItemsForUser( $this->getMockAnonUser() ); + $this->assertEmpty( $items ); + } + } diff --git a/tests/phpunit/includes/api/ApiQueryWatchlistRawIntegrationTest.php b/tests/phpunit/includes/api/ApiQueryWatchlistRawIntegrationTest.php index 85bcf5fd1f..582c0769a2 100644 --- a/tests/phpunit/includes/api/ApiQueryWatchlistRawIntegrationTest.php +++ b/tests/phpunit/includes/api/ApiQueryWatchlistRawIntegrationTest.php @@ -138,10 +138,10 @@ class ApiQueryWatchlistRawIntegrationTest extends ApiTestCase { ); $resultChanged = $this->doListWatchlistRawRequest( - [ 'wrprop' => 'changed', 'wrshow' => 'changed' ] + [ 'wrprop' => 'changed', 'wrshow' => WatchedItemQueryService::FILTER_CHANGED ] ); $resultNotChanged = $this->doListWatchlistRawRequest( - [ 'wrprop' => 'changed', 'wrshow' => '!changed' ] + [ 'wrprop' => 'changed', 'wrshow' => WatchedItemQueryService::FILTER_NOT_CHANGED ] ); $this->assertEquals( -- 2.20.1