Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "Remove unnecessary calls to SpecialBlock::getTargetAndType"

Browse source
This commit is contained in:
jenkins-bot 2020-09-21 20:56:53 +00:00 committed by Gerrit Code Review
commit 469c66a6a8
3 changed files with 92 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
includes/api/ApiBlock.php
View file

@ -20,6 +20,7 @@
* @file
*/
use MediaWiki\Block\AbstractBlock;
use MediaWiki\Block\BlockPermissionCheckerFactory;
use MediaWiki\Block\DatabaseBlock;
use MediaWiki\MediaWikiServices;
@ -93,7 +94,7 @@ class ApiBlock extends ApiBase {
$params['user'] = $username;
}
} else {
list( $target, $type ) = SpecialBlock::getTargetAndType( $params['user'] );
list( $target, $type ) = AbstractBlock::parseTarget( $params['user'] );
// T40633 - if the target is a user (not an IP address), but it
// doesn't exist or is unusable, error.
@ -165,7 +166,7 @@ class ApiBlock extends ApiBase {
$res = [];
$res['user'] = $params['user'];
list( $target, /*...*/ ) = SpecialBlock::getTargetAndType( $params['user'] );
list( $target, /*...*/ ) = AbstractBlock::parseTarget( $params['user'] );
$res['userID'] = $target instanceof User ? $target->getId() : 0;
$block = DatabaseBlock::newFromTarget( $target, null, true );

63
includes/specials/SpecialBlock.php
View file

@ -21,6 +21,7 @@
* @ingroup SpecialPage
*/
use MediaWiki\Block\AbstractBlock;
use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Block\Restriction\NamespaceRestriction;
use MediaWiki\Block\Restriction\PageRestriction;
@ -626,8 +627,10 @@ class SpecialBlock extends FormSpecialPage {
}
/**
* Determine the target of the block, and the type of target
* @todo Should be in DatabaseBlock.php?
* Get the target and type, given the request and the subpage parameter.
* Several parameters are handled for backwards compatability. 'wpTarget' is
* prioritized, since it matches the HTML form.
*
* @param string $par Subpage parameter passed to setup, or data value from
* the HTMLForm
* @param WebRequest|null $request Optionally try and get data from a request too
@ -635,46 +638,25 @@ class SpecialBlock extends FormSpecialPage {
* @phan-return array{0:User|string|null,1:int|null}
*/
public static function getTargetAndType( $par, WebRequest $request = null ) {
$i = 0;
$target = null;
while ( true ) {
switch ( $i++ ) {
case 0:
# The HTMLForm will check wpTarget first and only if it doesn't get
# a value use the default, which will be generated from the options
# below; so this has to have a higher precedence here than $par, or
# we could end up with different values in $this->target and the HTMLForm!
if ( $request instanceof WebRequest ) {
$target = $request->getText( 'wpTarget', null );
}
break;
case 1:
$target = $par;
break;
case 2:
if ( $request instanceof WebRequest ) {
$target = $request->getText( 'ip', null );
}
break;
case 3:
# B/C @since 1.18
if ( $request instanceof WebRequest ) {
$target = $request->getText( 'wpBlockAddress', null );
}
break;
case 4:
break 2;
if ( !$request instanceof WebRequest ) {
return AbstractBlock::parseTarget( $par );
}
list( $target, $type ) = DatabaseBlock::parseTarget( $target );
if ( $type !== null ) {
return [ $target, $type ];
$possibleTargets = [
$request->getVal( 'wpTarget', null ),
$par,
$request->getVal( 'ip', null ),
// B/C @since 1.18
$request->getVal( 'wpBlockAddress', null ),
];
foreach ( $possibleTargets as $possibleTarget ) {
$targetAndType = AbstractBlock::parseTarget( $possibleTarget );
// If type is not null then target is valid
if ( $targetAndType[ 1 ] !== null ) {
break;
}
}
return [ null, null ];
return $targetAndType;
}
/**
@ -708,7 +690,7 @@ class SpecialBlock extends FormSpecialPage {
global $wgBlockCIDRLimit;
/** @var User $target */
list( $target, $type ) = self::getTargetAndType( $value );
list( $target, $type ) = AbstractBlock::parseTarget( $value );
$status = Status::newGood( $target );
if ( $type == DatabaseBlock::TYPE_USER ) {
@ -775,7 +757,7 @@ class SpecialBlock extends FormSpecialPage {
$data['Confirm'] = !in_array( $data['Confirm'], [ '', '0', null, false ], true );
/** @var User $target */
list( $target, $type ) = self::getTargetAndType( $data['Target'] );
list( $target, $type ) = AbstractBlock::parseTarget( $data['Target'] );
if ( $type == DatabaseBlock::TYPE_USER ) {
$user = $target;
$target = $user->getName();
@ -863,6 +845,7 @@ class SpecialBlock extends FormSpecialPage {
# Bad expiry.
return [ 'ipb_expiry_temp' ];
} elseif ( $hideUserContribLimit !== false
/** @phan-suppress-next-line PhanNonClassMethodCall */
&& $user->getEditCount() > $hideUserContribLimit
) {
# Typically, the user should have a handful of edits.

63
tests/phpunit/includes/specials/SpecialBlockTest.php
View file

@ -804,6 +804,69 @@ class SpecialBlockTest extends SpecialPageTestBase {
];
}
/**
* @dataProvider provideGetTargetAndType
* @covers ::getTargetAndType
*/
public function testGetTargetAndType( $par, $requestData, $expectedTarget ) {
$request = $requestData ? new FauxRequest( $requestData ) : null;
$page = $this->newSpecialPage();
list( $target, $type ) = $page->getTargetAndType( $par, $request );
$this->assertSame( $target, $expectedTarget );
}
public function provideGetTargetAndType() {
$invalidTarget = '';
return [
'Choose \'wpTarget\' parameter first' => [
'2.2.2.0/24',
[
'wpTarget' => '1.1.1.0/24',
'ip' => '3.3.3.0/24',
'wpBlockAddress' => '4.4.4.0/24',
],
'1.1.1.0/24',
],
'Choose subpage parameter second' => [
'2.2.2.0/24',
[
'wpTarget' => $invalidTarget,
'ip' => '3.3.3.0/24',
'wpBlockAddress' => '4.4.4.0/24',
],
'2.2.2.0/24',
],
'Choose \'ip\' parameter third' => [
$invalidTarget,
[
'wpTarget' => $invalidTarget,
'ip' => '3.3.3.0/24',
'wpBlockAddress' => '4.4.4.0/24',
],
'3.3.3.0/24',
],
'Choose \'wpBlockAddress\' parameter fourth' => [
$invalidTarget,
[
'wpTarget' => $invalidTarget,
'ip' => $invalidTarget,
'wpBlockAddress' => '4.4.4.0/24',
],
'4.4.4.0/24',
],
'No web request' => [
'2.2.2.0/24',
false,
'2.2.2.0/24',
],
'No valid request data or subpage parameter' => [
null,
[],
null,
],
];
}
protected function insertBlock() {
$badActor = $this->getTestUser()->getUser();
$sysop = $this->getTestSysop()->getUser();