From 693c8b2f5ad07a53c2655954d11f172889f466c3 Mon Sep 17 00:00:00 2001 From: Amir Sarabadani Date: Mon, 29 Apr 2019 09:47:31 +0200 Subject: [PATCH] Move ApiQueryUserInfo::getBlockInfo() to ApiBase ApiBase directly uses this method causing a cyclic dependency between ApiBase and ApiQueryUserInfo Change-Id: I84ed21641c44b2f65ebe1980b0893d1846db3b34 --- autoload.php | 1 + includes/api/ApiBase.php | 12 +++-- includes/api/ApiBlock.php | 4 +- includes/api/ApiBlockInfoTrait.php | 53 +++++++++++++++++++ includes/api/ApiQueryUserInfo.php | 31 ++--------- includes/api/ApiUnblock.php | 4 +- tests/phpunit/includes/api/ApiBaseTest.php | 10 +++- ...InfoTest.php => ApiBlockInfoTraitTest.php} | 24 ++++----- 8 files changed, 88 insertions(+), 51 deletions(-) create mode 100644 includes/api/ApiBlockInfoTrait.php rename tests/phpunit/includes/api/{ApiQueryUserInfoTest.php => ApiBlockInfoTraitTest.php} (57%) diff --git a/autoload.php b/autoload.php index 13037ff98e..b113613e07 100644 --- a/autoload.php +++ b/autoload.php @@ -27,6 +27,7 @@ $wgAutoloadLocalClasses = [ 'ApiAuthManagerHelper' => __DIR__ . '/includes/api/ApiAuthManagerHelper.php', 'ApiBase' => __DIR__ . '/includes/api/ApiBase.php', 'ApiBlock' => __DIR__ . '/includes/api/ApiBlock.php', + 'ApiBlockInfoTrait' => __DIR__ . '/includes/api/ApiBlockInfoTrait.php', 'ApiCSPReport' => __DIR__ . '/includes/api/ApiCSPReport.php', 'ApiChangeAuthenticationData' => __DIR__ . '/includes/api/ApiChangeAuthenticationData.php', 'ApiCheckToken' => __DIR__ . '/includes/api/ApiCheckToken.php', diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 8ab92aff4b..19d84f74b8 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -36,6 +36,8 @@ use Wikimedia\Rdbms\IDatabase; */ abstract class ApiBase extends ContextSource { + use ApiBlockInfoTrait; + /** * @name Constants for ::getAllowedParams() arrays * These constants are keys in the arrays returned by ::getAllowedParams() @@ -1811,7 +1813,7 @@ abstract class ApiBase extends ContextSource { if ( is_string( $error[0] ) && isset( self::$blockMsgMap[$error[0]] ) && $user->getBlock() ) { list( $msg, $code ) = self::$blockMsgMap[$error[0]]; $status->fatal( ApiMessage::create( $msg, $code, - [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ] + [ 'blockinfo' => $this->getBlockInfo( $user->getBlock() ) ] ) ); } else { $status->fatal( ...$error ); @@ -1834,7 +1836,7 @@ abstract class ApiBase extends ContextSource { foreach ( self::$blockMsgMap as $msg => list( $apiMsg, $code ) ) { if ( $status->hasMessage( $msg ) && $user->getBlock() ) { $status->replaceMessage( $msg, ApiMessage::create( $apiMsg, $code, - [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ] + [ 'blockinfo' => $this->getBlockInfo( $user->getBlock() ) ] ) ); } } @@ -2033,19 +2035,19 @@ abstract class ApiBase extends ContextSource { $this->dieWithError( 'apierror-autoblocked', 'autoblocked', - [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ] + [ 'blockinfo' => $this->getBlockInfo( $block ) ] ); } elseif ( !$block->isSitewide() ) { $this->dieWithError( 'apierror-blocked-partial', 'blocked', - [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ] + [ 'blockinfo' => $this->getBlockInfo( $block ) ] ); } else { $this->dieWithError( 'apierror-blocked', 'blocked', - [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ] + [ 'blockinfo' => $this->getBlockInfo( $block ) ] ); } } diff --git a/includes/api/ApiBlock.php b/includes/api/ApiBlock.php index b5d51aae8c..336943d054 100644 --- a/includes/api/ApiBlock.php +++ b/includes/api/ApiBlock.php @@ -28,6 +28,8 @@ */ class ApiBlock extends ApiBase { + use ApiBlockInfoTrait; + /** * Blocks the user specified in the parameters for the given expiry, with the * given reason, and with all other settings provided in the params. If the block @@ -50,7 +52,7 @@ class ApiBlock extends ApiBase { $this->dieWithError( $status, null, - [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ] + [ 'blockinfo' => $this->getBlockInfo( $block ) ] ); } } diff --git a/includes/api/ApiBlockInfoTrait.php b/includes/api/ApiBlockInfoTrait.php new file mode 100644 index 0000000000..26634854de --- /dev/null +++ b/includes/api/ApiBlockInfoTrait.php @@ -0,0 +1,53 @@ +getId(); + $vals['blockedby'] = $block->getByName(); + $vals['blockedbyid'] = $block->getBy(); + $vals['blockreason'] = $block->getReason(); + $vals['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $block->getTimestamp() ); + $vals['blockexpiry'] = ApiResult::formatExpiry( $block->getExpiry(), 'infinite' ); + $vals['blockpartial'] = !$block->isSitewide(); + if ( $block->getSystemBlockType() !== null ) { + $vals['systemblocktype'] = $block->getSystemBlockType(); + } + return $vals; + } + +} diff --git a/includes/api/ApiQueryUserInfo.php b/includes/api/ApiQueryUserInfo.php index 00d7d84de8..c495c6db13 100644 --- a/includes/api/ApiQueryUserInfo.php +++ b/includes/api/ApiQueryUserInfo.php @@ -29,6 +29,8 @@ use MediaWiki\MediaWikiServices; */ class ApiQueryUserInfo extends ApiQueryBase { + use ApiBlockInfoTrait; + const WL_UNREAD_LIMIT = 1000; private $params = []; @@ -50,33 +52,6 @@ class ApiQueryUserInfo extends ApiQueryBase { $result->addValue( 'query', $this->getModuleName(), $r ); } - /** - * Get basic info about a given block - * @param Block $block - * @return array Array containing several keys: - * - blockid - ID of the block - * - blockedby - username of the blocker - * - blockedbyid - user ID of the blocker - * - blockreason - reason provided for the block - * - blockedtimestamp - timestamp for when the block was placed/modified - * - blockexpiry - expiry time of the block - * - systemblocktype - system block type, if any - */ - public static function getBlockInfo( Block $block ) { - $vals = []; - $vals['blockid'] = $block->getId(); - $vals['blockedby'] = $block->getByName(); - $vals['blockedbyid'] = $block->getBy(); - $vals['blockreason'] = $block->getReason(); - $vals['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $block->getTimestamp() ); - $vals['blockexpiry'] = ApiResult::formatExpiry( $block->getExpiry(), 'infinite' ); - $vals['blockpartial'] = !$block->isSitewide(); - if ( $block->getSystemBlockType() !== null ) { - $vals['systemblocktype'] = $block->getSystemBlockType(); - } - return $vals; - } - /** * Get central user info * @param Config $config @@ -129,7 +104,7 @@ class ApiQueryUserInfo extends ApiQueryBase { if ( isset( $this->prop['blockinfo'] ) ) { $block = $user->getBlock(); if ( $block ) { - $vals = array_merge( $vals, self::getBlockInfo( $block ) ); + $vals = array_merge( $vals, $this->getBlockInfo( $block ) ); } } diff --git a/includes/api/ApiUnblock.php b/includes/api/ApiUnblock.php index 3aad8f4199..f038b9683b 100644 --- a/includes/api/ApiUnblock.php +++ b/includes/api/ApiUnblock.php @@ -28,6 +28,8 @@ */ class ApiUnblock extends ApiBase { + use ApiBlockInfoTrait; + /** * Unblocks the specified user or provides the reason the unblock failed. */ @@ -48,7 +50,7 @@ class ApiUnblock extends ApiBase { $this->dieWithError( $status, null, - [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ] + [ 'blockinfo' => $this->getBlockInfo( $block ) ] ); } } diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php index 0dc64df87f..e02e8a4984 100644 --- a/tests/phpunit/includes/api/ApiBaseTest.php +++ b/tests/phpunit/includes/api/ApiBaseTest.php @@ -1332,7 +1332,10 @@ class ApiBaseTest extends ApiTestCase { 'expiry' => time() + 100500, ] ); $block->insert(); - $blockinfo = [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ]; + $userInfoTrait = TestingAccessWrapper::newFromObject( + $this->getMockForTrait( ApiBlockInfoTrait::class ) + ); + $blockinfo = [ 'blockinfo' => $userInfoTrait->getBlockInfo( $block ) ]; $expect = Status::newGood(); $expect->fatal( ApiMessage::create( 'apierror-blocked', 'blocked', $blockinfo ) ); @@ -1387,7 +1390,10 @@ class ApiBaseTest extends ApiTestCase { 'expiry' => time() + 100500, ] ); $block->insert(); - $blockinfo = [ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $block ) ]; + $userInfoTrait = TestingAccessWrapper::newFromObject( + $this->getObjectForTrait( ApiBlockInfoTrait::class ) + ); + $blockinfo = [ 'blockinfo' => $userInfoTrait->getBlockInfo( $block ) ]; $expect = Status::newGood(); $expect->fatal( ApiMessage::create( 'apierror-blocked', 'blocked', $blockinfo ) ); diff --git a/tests/phpunit/includes/api/ApiQueryUserInfoTest.php b/tests/phpunit/includes/api/ApiBlockInfoTraitTest.php similarity index 57% rename from tests/phpunit/includes/api/ApiQueryUserInfoTest.php rename to tests/phpunit/includes/api/ApiBlockInfoTraitTest.php index 7dcb75c486..f05cfbcdc1 100644 --- a/tests/phpunit/includes/api/ApiQueryUserInfoTest.php +++ b/tests/phpunit/includes/api/ApiBlockInfoTraitTest.php @@ -1,18 +1,16 @@ apiContext ), 'userinfo' ), - 'userinfo' - ); +class ApiBlockInfoTraitTest extends MediaWikiTestCase { + public function testGetBlockInfo() { $block = new Block(); - $info = $apiQueryUserInfo->getBlockInfo( $block ); + $mock = $this->getMockForTrait( ApiBlockInfoTrait::class ); + $info = TestingAccessWrapper::newFromObject( $mock )->getBlockInfo( $block ); $subset = [ 'blockid' => null, 'blockedby' => '', @@ -25,15 +23,12 @@ class ApiQueryUserInfoTest extends ApiTestCase { } public function testGetBlockInfoPartial() { - $apiQueryUserInfo = new ApiQueryUserInfo( - new ApiQuery( new ApiMain( $this->apiContext ), 'userinfo' ), - 'userinfo' - ); + $mock = $this->getMockForTrait( ApiBlockInfoTrait::class ); $block = new Block( [ 'sitewide' => false, ] ); - $info = $apiQueryUserInfo->getBlockInfo( $block ); + $info = TestingAccessWrapper::newFromObject( $mock )->getBlockInfo( $block ); $subset = [ 'blockid' => null, 'blockedby' => '', @@ -44,4 +39,5 @@ class ApiQueryUserInfoTest extends ApiTestCase { ]; $this->assertArraySubset( $subset, $info ); } + } -- 2.20.1