diff --git a/RELEASE-NOTES-1.37 b/RELEASE-NOTES-1.37 index dcac1f3f69d..09c2e6d34e8 100644 --- a/RELEASE-NOTES-1.37 +++ b/RELEASE-NOTES-1.37 @@ -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. diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 16380969416..586704dce64 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -379,7 +379,8 @@ return [ $services->getCommentStore(), $services->getHookContainer(), $services->getDBLoadBalancer(), - $services->getReadOnlyMode() + $services->getReadOnlyMode(), + $services->getUserFactory() ); }, diff --git a/includes/api/ApiUnblock.php b/includes/api/ApiUnblock.php index 1b6e5a9bec5..7a1e30a10ca 100644 --- a/includes/api/ApiUnblock.php +++ b/includes/api/ApiUnblock.php @@ -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 ); diff --git a/includes/block/AbstractBlock.php b/includes/block/AbstractBlock.php index e8f291f3d06..cc4d8f3b8d3 100644 --- a/includes/block/AbstractBlock.php +++ b/includes/block/AbstractBlock.php @@ -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; } diff --git a/includes/block/DatabaseBlockStore.php b/includes/block/DatabaseBlockStore.php index c5b56c6d1b0..043e7d4da98 100644 --- a/includes/block/DatabaseBlockStore.php +++ b/includes/block/DatabaseBlockStore.php @@ -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(); } } diff --git a/tests/phpunit/includes/api/ApiBlockTest.php b/tests/phpunit/includes/api/ApiBlockTest.php index 2004e6d80a4..4adf163c1f2 100644 --- a/tests/phpunit/includes/api/ApiBlockTest.php +++ b/tests/phpunit/includes/api/ApiBlockTest.php @@ -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; diff --git a/tests/phpunit/includes/block/CompositeBlockTest.php b/tests/phpunit/includes/block/CompositeBlockTest.php index 964abe2ffb2..33840031ffd 100644 --- a/tests/phpunit/includes/block/CompositeBlockTest.php +++ b/tests/phpunit/includes/block/CompositeBlockTest.php @@ -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 ); } diff --git a/tests/phpunit/includes/block/DatabaseBlockTest.php b/tests/phpunit/includes/block/DatabaseBlockTest.php index 88d2412c637..7d3809eeea9 100644 --- a/tests/phpunit/includes/block/DatabaseBlockTest.php +++ b/tests/phpunit/includes/block/DatabaseBlockTest.php @@ -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() ); diff --git a/tests/phpunit/includes/specials/SpecialBlockTest.php b/tests/phpunit/includes/specials/SpecialBlockTest.php index dedd16274ee..6f88794356b 100644 --- a/tests/phpunit/includes/specials/SpecialBlockTest.php +++ b/tests/phpunit/includes/specials/SpecialBlockTest.php @@ -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'] ); diff --git a/tests/phpunit/integration/includes/block/DatabaseBlockStoreTest.php b/tests/phpunit/integration/includes/block/DatabaseBlockStoreTest.php index 02915132e90..15347dc9dc2 100644 --- a/tests/phpunit/integration/includes/block/DatabaseBlockStoreTest.php +++ b/tests/phpunit/integration/includes/block/DatabaseBlockStoreTest.php @@ -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() ); } diff --git a/tests/phpunit/unit/includes/block/AbstractBlockTest.php b/tests/phpunit/unit/includes/block/AbstractBlockTest.php new file mode 100644 index 00000000000..e9e3f7fc088 --- /dev/null +++ b/tests/phpunit/unit/includes/block/AbstractBlockTest.php @@ -0,0 +1,38 @@ +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(); + } +}