From 02d6dc2e4f610c5939fde30d37bbc1eca622a447 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 5 Sep 2019 14:24:28 -0400 Subject: [PATCH] API: Use ApiBlockInfoTrait in ApiQueryUsers and AllUsers For efficient bulk querying, this means that ApiQueryBase::showHiddenUsersAddBlockInfo() needs to return everything needed by DatabaseBlock::newFromRow(). Since we're rewriting it anyway, we may as well also move ApiQueryBase::showHiddenUsersAddBlockInfo() out into a trait of its own. Bug: T232021 Change-Id: I9c5b17a232ecbfbffefc7e40608cf5684ce8a644 --- RELEASE-NOTES-1.34 | 7 +- autoload.php | 1 + includes/api/ApiQueryAllUsers.php | 15 ++- includes/api/ApiQueryBase.php | 62 +++++------- includes/api/ApiQueryBlockInfoTrait.php | 94 +++++++++++++++++++ includes/api/ApiQueryUsers.php | 13 +-- .../api/ApiQueryBlockInfoTraitTest.php | 77 +++++++++++++++ 7 files changed, 211 insertions(+), 58 deletions(-) create mode 100644 includes/api/ApiQueryBlockInfoTrait.php create mode 100644 tests/phpunit/includes/api/ApiQueryBlockInfoTraitTest.php diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 173a5da20e..af87e2b671 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -176,6 +176,9 @@ $wgPasswordPolicy['policies']['default']['PasswordNotInLargeBlacklist'] = false; set. Fields that are supported even if the file is missing include: 'canonicaltitle', ''archivename' (deleted files only), 'descriptionurl', 'descriptionshorturl'. +* The 'blockexpiry' result property in list=users and list=allusers will now be + returned in the same format used by the rest of the API: ISO 8601 for + expiring blocks, and "infinite" for non-expiring blocks. === Action API internal changes in 1.34 === * The exception thrown in ApiModuleManager::getModule has been changed @@ -183,7 +186,7 @@ $wgPasswordPolicy['policies']['default']['PasswordNotInLargeBlacklist'] = false; ApiModuleManager::getModule now also throws InvalidArgumentExceptions when ObjectFactory is presented with an invalid spec or incorrectly constructed objects. -* … +* Added ApiQueryBlockInfoTrait. === Languages updated in 1.34 === MediaWiki supports over 350 languages. Many localisations are updated regularly. @@ -587,6 +590,8 @@ because of Phabricator reports. * Global variable $wgSysopEmailBans is deprecated; to allow sysops to ban users from sending emails, use $wgGroupPermissions['sysop']['blockemail'] = true; +* ApiQueryBase::showHiddenUsersAddBlockInfo() is deprecated. Use + ApiQueryBlockInfoTrait instead. === Other changes in 1.34 === * … diff --git a/autoload.php b/autoload.php index 48d5b30d6d..7ff29ce803 100644 --- a/autoload.php +++ b/autoload.php @@ -89,6 +89,7 @@ $wgAutoloadLocalClasses = [ 'ApiQueryBacklinks' => __DIR__ . '/includes/api/ApiQueryBacklinks.php', 'ApiQueryBacklinksprop' => __DIR__ . '/includes/api/ApiQueryBacklinksprop.php', 'ApiQueryBase' => __DIR__ . '/includes/api/ApiQueryBase.php', + 'ApiQueryBlockInfoTrait' => __DIR__ . '/includes/api/ApiQueryBlockInfoTrait.php', 'ApiQueryBlocks' => __DIR__ . '/includes/api/ApiQueryBlocks.php', 'ApiQueryCategories' => __DIR__ . '/includes/api/ApiQueryCategories.php', 'ApiQueryCategoryInfo' => __DIR__ . '/includes/api/ApiQueryCategoryInfo.php', diff --git a/includes/api/ApiQueryAllUsers.php b/includes/api/ApiQueryAllUsers.php index a7d4fb90a0..0ea6af3247 100644 --- a/includes/api/ApiQueryAllUsers.php +++ b/includes/api/ApiQueryAllUsers.php @@ -20,12 +20,16 @@ * @file */ +use MediaWiki\Block\DatabaseBlock; + /** * Query module to enumerate all registered users. * * @ingroup API */ class ApiQueryAllUsers extends ApiQueryBase { + use ApiQueryBlockInfoTrait; + public function __construct( ApiQuery $query, $moduleName ) { parent::__construct( $query, $moduleName, 'au' ); } @@ -153,7 +157,7 @@ class ApiQueryAllUsers extends ApiQueryBase { $this->addWhere( 'user_editcount > 0' ); } - $this->showHiddenUsersAddBlockInfo( $fld_blockinfo ); + $this->addBlockInfoToQuery( $fld_blockinfo ); if ( $fld_groups || $fld_rights ) { $this->addFields( [ 'groups' => @@ -263,13 +267,8 @@ class ApiQueryAllUsers extends ApiQueryBase { ); } - if ( $fld_blockinfo && !is_null( $row->ipb_by_text ) ) { - $data['blockid'] = (int)$row->ipb_id; - $data['blockedby'] = $row->ipb_by_text; - $data['blockedbyid'] = (int)$row->ipb_by; - $data['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $row->ipb_timestamp ); - $data['blockreason'] = $commentStore->getComment( 'ipb_reason', $row )->text; - $data['blockexpiry'] = $row->ipb_expiry; + if ( $fld_blockinfo && !is_null( $row->ipb_id ) ) { + $data += $this->getBlockDetails( DatabaseBlock::newFromRow( $row ) ); } if ( $row->ipb_deleted ) { $data['hidden'] = true; diff --git a/includes/api/ApiQueryBase.php b/includes/api/ApiQueryBase.php index 10db848d94..8d9cb48c1a 100644 --- a/includes/api/ApiQueryBase.php +++ b/includes/api/ApiQueryBase.php @@ -31,6 +31,7 @@ use Wikimedia\Rdbms\IResultWrapper; * @ingroup API */ abstract class ApiQueryBase extends ApiBase { + use ApiQueryBlockInfoTrait; private $mQueryModule, $mDb, $tables, $where, $fields, $options, $join_conds; @@ -424,47 +425,6 @@ abstract class ApiQueryBase extends ApiBase { return Hooks::run( 'ApiQueryBaseProcessRow', [ $this, $row, &$data, &$hookData ] ); } - /** - * Filters hidden users (where the user doesn't have the right to view them) - * Also adds relevant block information - * - * @param bool $showBlockInfo - * @return void - */ - public function showHiddenUsersAddBlockInfo( $showBlockInfo ) { - $db = $this->getDB(); - - $tables = [ 'ipblocks' ]; - $fields = [ 'ipb_deleted' ]; - $joinConds = [ - 'blk' => [ 'LEFT JOIN', [ - 'ipb_user=user_id', - 'ipb_expiry > ' . $db->addQuotes( $db->timestamp() ), - ] ], - ]; - - if ( $showBlockInfo ) { - $actorQuery = ActorMigration::newMigration()->getJoin( 'ipb_by' ); - $commentQuery = CommentStore::getStore()->getJoin( 'ipb_reason' ); - $tables += $actorQuery['tables'] + $commentQuery['tables']; - $joinConds += $actorQuery['joins'] + $commentQuery['joins']; - $fields = array_merge( $fields, [ - 'ipb_id', - 'ipb_expiry', - 'ipb_timestamp' - ], $actorQuery['fields'], $commentQuery['fields'] ); - } - - $this->addTables( [ 'blk' => $tables ] ); - $this->addFields( $fields ); - $this->addJoinConds( $joinConds ); - - // Don't show hidden names - if ( !$this->getPermissionManager()->userHasRight( $this->getUser(), 'hideuser' ) ) { - $this->addWhere( 'ipb_deleted = 0 OR ipb_deleted IS NULL' ); - } - } - /** @} */ /************************************************************************//** @@ -610,4 +570,24 @@ abstract class ApiQueryBase extends ApiBase { } /** @} */ + + /************************************************************************//** + * @name Deprecated methods + * @{ + */ + + /** + * Filters hidden users (where the user doesn't have the right to view them) + * Also adds relevant block information + * + * @deprecated since 1.34, use ApiQueryBlockInfoTrait instead + * @param bool $showBlockInfo + * @return void + */ + public function showHiddenUsersAddBlockInfo( $showBlockInfo ) { + wfDeprecated( __METHOD__, '1.34' ); + return $this->addBlockInfoToQuery( $showBlockInfo ); + } + + /** @} */ } diff --git a/includes/api/ApiQueryBlockInfoTrait.php b/includes/api/ApiQueryBlockInfoTrait.php new file mode 100644 index 0000000000..a3be35650e --- /dev/null +++ b/includes/api/ApiQueryBlockInfoTrait.php @@ -0,0 +1,94 @@ +getDB(); + + if ( $showBlockInfo ) { + $queryInfo = DatabaseBlock::getQueryInfo(); + } else { + $queryInfo = [ + 'tables' => [ 'ipblocks' ], + 'fields' => [ 'ipb_deleted' ], + 'joins' => [], + ]; + } + + $this->addTables( [ 'blk' => $queryInfo['tables'] ] ); + $this->addFields( $queryInfo['fields'] ); + $this->addJoinConds( $queryInfo['joins'] ); + $this->addJoinConds( [ + 'blk' => [ 'LEFT JOIN', [ + 'ipb_user=user_id', + 'ipb_expiry > ' . $db->addQuotes( $db->timestamp() ), + ] ], + ] ); + + // Don't show hidden names + if ( !$this->getPermissionManager()->userHasRight( $this->getUser(), 'hideuser' ) ) { + $this->addWhere( 'ipb_deleted = 0 OR ipb_deleted IS NULL' ); + } + } + + /** + * @name Methods required from ApiQueryBase + * @{ + */ + + /** @see ApiBase::getDB */ + abstract protected function getDB(); + + /** @see ApiBase::getPermissionManager */ + abstract protected function getPermissionManager(): PermissionManager; + + /** @see IContextSource::getUser */ + abstract public function getUser(); + + /** @see ApiQueryBase::addTables */ + abstract protected function addTables( $tables, $alias = null ); + + /** @see ApiQueryBase::addFields */ + abstract protected function addFields( $fields ); + + /** @see ApiQueryBase::addWhere */ + abstract protected function addWhere( $conds ); + + /** @see ApiQueryBase::addJoinConds */ + abstract protected function addJoinConds( $conds ); + + /**@}*/ + +} diff --git a/includes/api/ApiQueryUsers.php b/includes/api/ApiQueryUsers.php index ce51a672ba..0171a37733 100644 --- a/includes/api/ApiQueryUsers.php +++ b/includes/api/ApiQueryUsers.php @@ -20,12 +20,15 @@ * @file */ +use MediaWiki\Block\DatabaseBlock; + /** * Query module to get information about a list of users * * @ingroup API */ class ApiQueryUsers extends ApiQueryBase { + use ApiQueryBlockInfoTrait; private $tokenFunctions, $prop; @@ -150,7 +153,7 @@ class ApiQueryUsers extends ApiQueryBase { $this->addWhereFld( 'user_id', $userids ); } - $this->showHiddenUsersAddBlockInfo( isset( $this->prop['blockinfo'] ) ); + $this->addBlockInfoToQuery( isset( $this->prop['blockinfo'] ) ); $data = []; $res = $this->select( __METHOD__ ); @@ -232,13 +235,7 @@ class ApiQueryUsers extends ApiQueryBase { $data[$key]['hidden'] = true; } if ( isset( $this->prop['blockinfo'] ) && !is_null( $row->ipb_by_text ) ) { - $data[$key]['blockid'] = (int)$row->ipb_id; - $data[$key]['blockedby'] = $row->ipb_by_text; - $data[$key]['blockedbyid'] = (int)$row->ipb_by; - $data[$key]['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $row->ipb_timestamp ); - $data[$key]['blockreason'] = $commentStore->getComment( 'ipb_reason', $row ) - ->text; - $data[$key]['blockexpiry'] = $row->ipb_expiry; + $data[$key] += $this->getBlockDetails( DatabaseBlock::newFromRow( $row ) ); } if ( isset( $this->prop['emailable'] ) ) { diff --git a/tests/phpunit/includes/api/ApiQueryBlockInfoTraitTest.php b/tests/phpunit/includes/api/ApiQueryBlockInfoTraitTest.php new file mode 100644 index 0000000000..cded9dd942 --- /dev/null +++ b/tests/phpunit/includes/api/ApiQueryBlockInfoTraitTest.php @@ -0,0 +1,77 @@ +assertTrue( method_exists( ApiQueryBlockInfoTrait::class, 'getBlockDetails' ), + 'ApiQueryBlockInfoTrait::getBlockDetails exists' ); + } + + /** + * @dataProvider provideAddBlockInfoToQuery + */ + public function testAddBlockInfoToQuery( $args, $expect ) { + // Fake timestamp to show up in the queries + $reset = ConvertibleTimestamp::setFakeTime( '20190101000000' ); + + $data = []; + + $mock = $this->getMockForTrait( ApiQueryBlockInfoTrait::class ); + $mock->method( 'getDB' )->willReturn( wfGetDB( DB_REPLICA ) ); + $mock->method( 'getPermissionManager' ) + ->willReturn( MediaWikiServices::getInstance()->getPermissionManager() ); + $mock->method( 'getUser' ) + ->willReturn( $this->getMutableTestUser()->getUser() ); + $mock->method( 'addTables' )->willReturnCallback( function ( $v ) use ( &$data ) { + $data['tables'] = array_merge( $data['tables'] ?? [], (array)$v ); + } ); + $mock->method( 'addFields' )->willReturnCallback( function ( $v ) use ( &$data ) { + $data['fields'] = array_merge( $data['fields'] ?? [], (array)$v ); + } ); + $mock->method( 'addWhere' )->willReturnCallback( function ( $v ) use ( &$data ) { + $data['where'] = array_merge( $data['where'] ?? [], (array)$v ); + } ); + $mock->method( 'addJoinConds' )->willReturnCallback( function ( $v ) use ( &$data ) { + $data['joins'] = array_merge( $data['joins'] ?? [], (array)$v ); + } ); + + TestingAccessWrapper::newFromObject( $mock )->addBlockInfoToQuery( ...$args ); + $this->assertEquals( $expect, $data ); + } + + public function provideAddBlockInfoToQuery() { + $queryInfo = DatabaseBlock::getQueryInfo(); + + $db = wfGetDB( DB_REPLICA ); + $ts = $db->addQuotes( $db->timestamp( '20190101000000' ) ); + + return [ + [ [ false ], [ + 'tables' => [ 'blk' => [ 'ipblocks' ] ], + 'fields' => [ 'ipb_deleted' ], + 'where' => [ 'ipb_deleted = 0 OR ipb_deleted IS NULL' ], + 'joins' => [ + 'blk' => [ 'LEFT JOIN', [ 'ipb_user=user_id', "ipb_expiry > $ts" ] ] + ], + ] ], + + [ [ true ], [ + 'tables' => [ 'blk' => $queryInfo['tables'] ], + 'fields' => $queryInfo['fields'], + 'where' => [ 'ipb_deleted = 0 OR ipb_deleted IS NULL' ], + 'joins' => $queryInfo['joins'] + [ + 'blk' => [ 'LEFT JOIN', [ 'ipb_user=user_id', "ipb_expiry > $ts" ] ] + ], + ] ], + ]; + } + +} -- 2.20.1