Merge "Hard-deprecate AbstractBlock::getTargetAndType() and getTarget()"
This commit is contained in:
commit
7afc309346
11 changed files with 81 additions and 28 deletions
|
|
@ -280,8 +280,8 @@ because of Phabricator reports.
|
|||
=== Deprecations in 1.37 ===
|
||||
* JobQueue::getWiki, deprecated in 1.33, now emits deprecation warnings.
|
||||
* In AbstractBlock, the getTargetAndType() and getTarget() methods are
|
||||
deprecated. Instead use getTargetName() and getTargetUserIdentity() together
|
||||
with getType().
|
||||
hard deprecated. Instead use getTargetName() and getTargetUserIdentity()
|
||||
together with getType().
|
||||
* Deprecated passing UserIdentity to
|
||||
WatchlistManager::clearAllUserNotifications() and
|
||||
WatchlistManager::clearTitleUserNotifications(). Pass Authority instead.
|
||||
|
|
|
|||
|
|
@ -379,7 +379,8 @@ return [
|
|||
$services->getCommentStore(),
|
||||
$services->getHookContainer(),
|
||||
$services->getDBLoadBalancer(),
|
||||
$services->getReadOnlyMode()
|
||||
$services->getReadOnlyMode(),
|
||||
$services->getUserFactory()
|
||||
);
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -109,11 +109,12 @@ class ApiUnblock extends ApiBase {
|
|||
}
|
||||
|
||||
$block = $status->getValue();
|
||||
$target = $block->getType() == DatabaseBlock::TYPE_AUTO ? '' : $block->getTarget();
|
||||
$targetName = $block->getType() === DatabaseBlock::TYPE_AUTO ? '' : $block->getTargetName();
|
||||
$targetUserId = $block->getTargetUserIdentity() ? $block->getTargetUserIdentity()->getId() : 0;
|
||||
$res = [
|
||||
'id' => $block->getId(),
|
||||
'user' => $target instanceof User ? $target->getName() : $target,
|
||||
'userid' => $target instanceof User ? $target->getId() : 0,
|
||||
'user' => $targetName,
|
||||
'userid' => $targetUserId,
|
||||
'reason' => $params['reason']
|
||||
];
|
||||
$this->getResult()->addValue( null, $this->getModuleName(), $res );
|
||||
|
|
|
|||
|
|
@ -340,6 +340,7 @@ abstract class AbstractBlock implements Block {
|
|||
* @todo FIXME: This should be an integral part of the block member variables
|
||||
*/
|
||||
public function getTargetAndType() {
|
||||
wfDeprecated( __METHOD__, '1.37' );
|
||||
return [ $this->getTarget(), $this->getType() ];
|
||||
}
|
||||
|
||||
|
|
@ -352,6 +353,7 @@ abstract class AbstractBlock implements Block {
|
|||
* @return User|string|null
|
||||
*/
|
||||
public function getTarget() {
|
||||
wfDeprecated( __METHOD__, '1.37' );
|
||||
return $this->target;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -30,11 +30,11 @@ use MediaWiki\Config\ServiceOptions;
|
|||
use MediaWiki\HookContainer\HookContainer;
|
||||
use MediaWiki\HookContainer\HookRunner;
|
||||
use MediaWiki\User\ActorStoreFactory;
|
||||
use MediaWiki\User\UserFactory;
|
||||
use MediaWiki\User\UserIdentity;
|
||||
use MWException;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use ReadOnlyMode;
|
||||
use User;
|
||||
use Wikimedia\Assert\Assert;
|
||||
use Wikimedia\Rdbms\IDatabase;
|
||||
use Wikimedia\Rdbms\ILoadBalancer;
|
||||
|
|
@ -78,6 +78,9 @@ class DatabaseBlockStore {
|
|||
/** @var ReadOnlyMode */
|
||||
private $readOnlyMode;
|
||||
|
||||
/** @var UserFactory */
|
||||
private $userFactory;
|
||||
|
||||
/**
|
||||
* @param ServiceOptions $options
|
||||
* @param LoggerInterface $logger
|
||||
|
|
@ -87,6 +90,7 @@ class DatabaseBlockStore {
|
|||
* @param HookContainer $hookContainer
|
||||
* @param ILoadBalancer $loadBalancer
|
||||
* @param ReadOnlyMode $readOnlyMode
|
||||
* @param UserFactory $userFactory
|
||||
*/
|
||||
public function __construct(
|
||||
ServiceOptions $options,
|
||||
|
|
@ -96,7 +100,8 @@ class DatabaseBlockStore {
|
|||
CommentStore $commentStore,
|
||||
HookContainer $hookContainer,
|
||||
ILoadBalancer $loadBalancer,
|
||||
ReadOnlyMode $readOnlyMode
|
||||
ReadOnlyMode $readOnlyMode,
|
||||
UserFactory $userFactory
|
||||
) {
|
||||
$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
|
||||
|
||||
|
|
@ -108,6 +113,7 @@ class DatabaseBlockStore {
|
|||
$this->hookRunner = new HookRunner( $hookContainer );
|
||||
$this->loadBalancer = $loadBalancer;
|
||||
$this->readOnlyMode = $readOnlyMode;
|
||||
$this->userFactory = $userFactory;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -236,11 +242,12 @@ class DatabaseBlockStore {
|
|||
$autoBlockIds = $this->doRetroactiveAutoblock( $block );
|
||||
|
||||
if ( $this->options->get( 'BlockDisablesLogin' ) ) {
|
||||
$target = $block->getTarget();
|
||||
if ( $target instanceof User ) {
|
||||
$targetUserIdentity = $block->getTargetUserIdentity();
|
||||
if ( $targetUserIdentity ) {
|
||||
$targetUser = $this->userFactory->newFromUserIdentity( $targetUserIdentity );
|
||||
// Change user login token to force them to be logged out.
|
||||
$target->setToken();
|
||||
$target->saveSettings();
|
||||
$targetUser->setToken();
|
||||
$targetUser->saveSettings();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ class ApiBlockTest extends ApiTestCase {
|
|||
|
||||
$this->assertTrue( $block !== null, 'Block is valid' );
|
||||
|
||||
$this->assertSame( $this->mUser->getName(), (string)$block->getTarget() );
|
||||
$this->assertSame( $this->mUser->getName(), $block->getTargetName() );
|
||||
$this->assertSame( 'Some reason', $block->getReasonComment()->text );
|
||||
|
||||
return $ret;
|
||||
|
|
|
|||
|
|
@ -192,7 +192,9 @@ class CompositeBlockTest extends MediaWikiLangTestCase {
|
|||
'originalBlocks' => $blocks,
|
||||
] );
|
||||
|
||||
$title = $blocks[ 'user' ]->getTarget()->getTalkPage();
|
||||
$userFactory = $this->getServiceContainer()->getUserFactory();
|
||||
$targetIdentity = $userFactory->newFromUserIdentity( $blocks[ 'user' ]->getTargetUserIdentity() );
|
||||
$title = $targetIdentity->getTalkPage();
|
||||
$page = $this->getExistingTestPage( 'User talk:' . $title->getText() );
|
||||
|
||||
$this->getBlockRestrictionStore()->insert( [
|
||||
|
|
@ -200,7 +202,7 @@ class CompositeBlockTest extends MediaWikiLangTestCase {
|
|||
new NamespaceRestriction( $blocks[ 'ip' ]->getId(), NS_USER ),
|
||||
] );
|
||||
|
||||
$this->assertTrue( $block->appliesToUsertalk( $blocks[ 'user' ]->getTarget()->getTalkPage() ) );
|
||||
$this->assertTrue( $block->appliesToUsertalk( $title ) );
|
||||
|
||||
$this->deleteBlocks( $blocks );
|
||||
}
|
||||
|
|
|
|||
|
|
@ -153,9 +153,9 @@ class DatabaseBlockTest extends MediaWikiLangTestCase {
|
|||
}
|
||||
|
||||
// Should find the block with the narrowest range
|
||||
$blockTarget = DatabaseBlock::newFromTarget( $this->getTestUser()->getUserIdentity(), $ip )->getTarget();
|
||||
$block = DatabaseBlock::newFromTarget( $this->getTestUser()->getUserIdentity(), $ip );
|
||||
$this->assertSame(
|
||||
$blockTarget instanceof User ? $blockTarget->getName() : $blockTarget,
|
||||
$block->getTargetName(),
|
||||
$expectedTarget
|
||||
);
|
||||
|
||||
|
|
@ -315,7 +315,7 @@ class DatabaseBlockTest extends MediaWikiLangTestCase {
|
|||
$block = DatabaseBlock::newFromID( $res['id'] );
|
||||
$this->assertEquals(
|
||||
'UserOnForeignWiki',
|
||||
$block->getTarget()->getName(),
|
||||
$block->getTargetName(),
|
||||
'Correct blockee name'
|
||||
);
|
||||
$this->assertEquals(
|
||||
|
|
@ -323,7 +323,7 @@ class DatabaseBlockTest extends MediaWikiLangTestCase {
|
|||
$block->getTargetUserIdentity()->getName(),
|
||||
'Correct blockee name'
|
||||
);
|
||||
$this->assertEquals( $userId, $block->getTarget()->getId(), 'Correct blockee id' );
|
||||
$this->assertEquals( $userId, $block->getTargetUserIdentity()->getId(), 'Correct blockee id' );
|
||||
$this->assertEquals( $userId, $block->getTargetUserIdentity()->getId(), 'Correct blockee id' );
|
||||
$this->assertEquals( 'UserOnForeignWiki', $block->getTargetName(), 'Correct blockee name' );
|
||||
$this->assertTrue( $block->isBlocking( 'UserOnForeignWiki' ), 'Is blocking blockee' );
|
||||
|
|
@ -492,7 +492,7 @@ class DatabaseBlockTest extends MediaWikiLangTestCase {
|
|||
$block = DatabaseBlock::newFromRow( $row );
|
||||
$this->assertInstanceOf( DatabaseBlock::class, $block );
|
||||
$this->assertEquals( $block->getBy(), $sysop->getId() );
|
||||
$this->assertEquals( $block->getTarget()->getName(), $badActor->getName() );
|
||||
$this->assertEquals( $block->getTargetName(), $badActor->getName() );
|
||||
$this->assertEquals( $block->getTargetName(), $badActor->getName() );
|
||||
$this->assertTrue( $block->isBlocking( $badActor ), 'Is blocking expected user' );
|
||||
$this->assertEquals( $block->getTargetUserIdentity()->getId(), $badActor->getId() );
|
||||
|
|
|
|||
|
|
@ -86,15 +86,15 @@ class SpecialBlockTest extends SpecialPageTestBase {
|
|||
$block = $this->insertBlock();
|
||||
|
||||
// Refresh the block from the database.
|
||||
$block = DatabaseBlock::newFromTarget( $block->getTarget() );
|
||||
$block = DatabaseBlock::newFromTarget( $block->getTargetUserIdentity() );
|
||||
|
||||
$page = $this->newSpecialPage();
|
||||
|
||||
$wrappedPage = TestingAccessWrapper::newFromObject( $page );
|
||||
$wrappedPage->target = $block->getTarget();
|
||||
$wrappedPage->target = $block->getTargetUserIdentity();
|
||||
$fields = $wrappedPage->getFormFields();
|
||||
|
||||
$this->assertSame( (string)$block->getTarget(), $fields['Target']['default'] );
|
||||
$this->assertSame( $block->getTargetName(), $fields['Target']['default'] );
|
||||
$this->assertSame( $block->isHardblock(), $fields['HardBlock']['default'] );
|
||||
$this->assertSame( $block->isCreateAccountBlocked(), $fields['CreateAccount']['default'] );
|
||||
$this->assertSame( $block->isAutoblocking(), $fields['AutoBlock']['default'] );
|
||||
|
|
@ -137,12 +137,12 @@ class SpecialBlockTest extends SpecialPageTestBase {
|
|||
MediaWikiServices::getInstance()->getDatabaseBlockStore()->insertBlock( $block );
|
||||
|
||||
// Refresh the block from the database.
|
||||
$block = DatabaseBlock::newFromTarget( $block->getTarget() );
|
||||
$block = DatabaseBlock::newFromTarget( $block->getTargetUserIdentity() );
|
||||
|
||||
$page = $this->newSpecialPage();
|
||||
|
||||
$wrappedPage = TestingAccessWrapper::newFromObject( $page );
|
||||
$wrappedPage->target = $block->getTarget();
|
||||
$wrappedPage->target = $block->getTargetUserIdentity();
|
||||
$fields = $wrappedPage->getFormFields();
|
||||
|
||||
$titles = [
|
||||
|
|
@ -150,7 +150,7 @@ class SpecialBlockTest extends SpecialPageTestBase {
|
|||
$pageSaturn->getTitle()->getPrefixedText(),
|
||||
];
|
||||
|
||||
$this->assertSame( (string)$block->getTarget(), $fields['Target']['default'] );
|
||||
$this->assertSame( $block->getTargetName(), $fields['Target']['default'] );
|
||||
$this->assertSame( 'partial', $fields['EditingRestriction']['default'] );
|
||||
$this->assertSame( implode( "\n", $titles ), $fields['PageRestrictions']['default'] );
|
||||
$this->assertSame( [ $actionId ], $fields['ActionRestrictions']['default'] );
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ class DatabaseBlockStoreTest extends MediaWikiIntegrationTestCase {
|
|||
'hookContainer' => $hookContainer,
|
||||
'loadBalancer' => $services->getDBLoadBalancer(),
|
||||
'readOnlyMode' => $readOnlyMode,
|
||||
'userFactory' => $services->getUserFactory(),
|
||||
];
|
||||
$constructorArgs = array_merge( $defaultConstructorArgs, $overrideConstructorArgs );
|
||||
|
||||
|
|
@ -192,14 +193,15 @@ class DatabaseBlockStoreTest extends MediaWikiIntegrationTestCase {
|
|||
*/
|
||||
public function testInsertBlockLogout( $options, $expectTokenEqual ) {
|
||||
$block = $this->getBlock();
|
||||
$targetToken = $block->getTarget()->getToken();
|
||||
$userFactory = $this->getServiceContainer()->getUserFactory();
|
||||
$targetToken = $userFactory->newFromUserIdentity( $block->getTargetUserIdentity() )->getToken();
|
||||
|
||||
$store = $this->getStore( $options );
|
||||
$result = $store->insertBlock( $block );
|
||||
|
||||
$this->assertSame(
|
||||
$expectTokenEqual,
|
||||
$targetToken === $block->getTarget()->getToken()
|
||||
$targetToken === $userFactory->newFromUserIdentity( $block->getTargetUserIdentity() )->getToken()
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
38
tests/phpunit/unit/includes/block/AbstractBlockTest.php
Normal file
38
tests/phpunit/unit/includes/block/AbstractBlockTest.php
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
<?php
|
||||
|
||||
namespace MediaWiki\Tests\Unit\Block;
|
||||
|
||||
use MediaWiki\Block\AbstractBlock;
|
||||
use MediaWikiUnitTestCase;
|
||||
|
||||
/**
|
||||
* @group Blocking
|
||||
* @coversDefaultClass MediaWiki\Block\AbstractBlock
|
||||
*/
|
||||
class AbstractBlockTest extends MediaWikiUnitTestCase {
|
||||
|
||||
/**
|
||||
* @covers ::getTarget
|
||||
*/
|
||||
public function testGetTarget_deprecated() {
|
||||
$this->expectDeprecation();
|
||||
$block = $this->createNoOpAbstractMock(
|
||||
AbstractBlock::class,
|
||||
[ 'getTarget' ]
|
||||
);
|
||||
$block->getTarget();
|
||||
}
|
||||
|
||||
/**
|
||||
* @covers ::getTargetAndType
|
||||
*/
|
||||
public function testGetTargetAndType_deprecated() {
|
||||
$this->expectDeprecation();
|
||||
$this->expectDeprecationMessageMatches( '/AbstractBlock::getTargetAndType was deprecated in MediaWiki 1.37/' );
|
||||
$block = $this->createNoOpAbstractMock(
|
||||
AbstractBlock::class,
|
||||
[ 'getTargetAndType' ]
|
||||
);
|
||||
$block->getTargetAndType();
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue