From 24ae5a631804559d2c78000b794b802b09880133 Mon Sep 17 00:00:00 2001 From: Petr Pchelko Date: Fri, 25 Jun 2021 08:36:19 -0700 Subject: [PATCH] Use CentralIdLookup service instead of static factory Change-Id: Ia0f263b4eff00cc6efee7a88c62d562dafd57950 --- RELEASE-NOTES-1.37 | 2 ++ includes/api/ApiQueryUserInfo.php | 16 +++++++--- includes/preferences/MultiUsernameFilter.php | 3 +- includes/specialpage/SpecialPageFactory.php | 4 ++- includes/specials/SpecialBotPasswords.php | 13 ++++++-- includes/specials/SpecialEmailUser.php | 5 ++-- includes/specials/SpecialMute.php | 30 +++++++++++-------- includes/user/User.php | 4 ++- tests/phpunit/includes/api/ApiLoginTest.php | 4 ++- .../BotPasswordSessionProviderTest.php | 5 +++- .../includes/specials/SpecialMuteTest.php | 11 ++----- 11 files changed, 62 insertions(+), 35 deletions(-) diff --git a/RELEASE-NOTES-1.37 b/RELEASE-NOTES-1.37 index b4ba1e0bd4d..2f3feea94d5 100644 --- a/RELEASE-NOTES-1.37 +++ b/RELEASE-NOTES-1.37 @@ -357,6 +357,8 @@ because of Phabricator reports. ContentHandler::getSecondaryDataUpdates() instead. * wfDiff(), deprecated since 1.25, has been removed. * Language::$mLangObjCache, deprecated since 1.35, was removed. +* SpecialMute::getTarget(), unused outside of the SpecialMute class, was + made private. * … === Deprecations in 1.37 === diff --git a/includes/api/ApiQueryUserInfo.php b/includes/api/ApiQueryUserInfo.php index 6dc1f212d91..074b92e4477 100644 --- a/includes/api/ApiQueryUserInfo.php +++ b/includes/api/ApiQueryUserInfo.php @@ -20,9 +20,11 @@ * @file */ +use MediaWiki\MediaWikiServices; use MediaWiki\User\TalkPageNotificationManager; use MediaWiki\User\UserEditTracker; use MediaWiki\User\UserGroupManager; +use MediaWiki\User\UserIdentity; use MediaWiki\User\UserOptionsLookup; /** @@ -106,8 +108,8 @@ class ApiQueryUserInfo extends ApiQueryBase { /** * Get central user info * @param Config $config - * @param User $user - * @param string|null $attachedWiki + * @param UserIdentity $user + * @param string|false $attachedWiki * @return array Central user info * - centralids: Array mapping non-local Central ID provider names to IDs * - attachedlocal: Array mapping Central ID provider names to booleans @@ -115,7 +117,11 @@ class ApiQueryUserInfo extends ApiQueryBase { * - attachedwiki: Array mapping Central ID provider names to booleans * indicating whether the user is attached to $attachedWiki. */ - public static function getCentralUserInfo( Config $config, User $user, $attachedWiki = null ) { + public static function getCentralUserInfo( + Config $config, + UserIdentity $user, + $attachedWiki = UserIdentity::LOCAL + ) { $providerIds = array_keys( $config->get( 'CentralIdLookupProviders' ) ); $ret = [ @@ -130,8 +136,10 @@ class ApiQueryUserInfo extends ApiQueryBase { } $name = $user->getName(); + $centralIdLookupFactory = MediaWikiServices::getInstance() + ->getCentralIdLookupFactory(); foreach ( $providerIds as $providerId ) { - $provider = CentralIdLookup::factory( $providerId ); + $provider = $centralIdLookupFactory->getLookup( $providerId ); $ret['centralids'][$providerId] = $provider->centralIdFromName( $name ); $ret['attachedlocal'][$providerId] = $provider->isAttached( $user ); if ( $attachedWiki ) { diff --git a/includes/preferences/MultiUsernameFilter.php b/includes/preferences/MultiUsernameFilter.php index f55297fb1e1..e496f7fe901 100644 --- a/includes/preferences/MultiUsernameFilter.php +++ b/includes/preferences/MultiUsernameFilter.php @@ -21,6 +21,7 @@ namespace MediaWiki\Preferences; use CentralIdLookup; +use MediaWiki\MediaWikiServices; use MediaWiki\Permissions\Authority; class MultiUsernameFilter implements Filter { @@ -82,7 +83,7 @@ class MultiUsernameFilter implements Filter { * @return CentralIdLookup */ private function getLookup() { - $this->lookup = $this->lookup ?? CentralIdLookup::factory(); + $this->lookup = $this->lookup ?? MediaWikiServices::getInstance()->getCentralIdLookup(); return $this->lookup; } } diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php index 627a29cdd2b..457cf85b26b 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -400,6 +400,7 @@ class SpecialPageFactory { 'services' => [ 'PasswordFactory', 'AuthManager', + 'CentralIdLookup', ] ], 'PasswordReset' => [ @@ -1066,8 +1067,9 @@ class SpecialPageFactory { $this->list['Mute'] = [ 'class' => \SpecialMute::class, 'services' => [ + 'CentralIdLookup', 'UserOptionsManager', - 'UserFactory', + 'UserIdentityLookup', ] ]; } diff --git a/includes/specials/SpecialBotPasswords.php b/includes/specials/SpecialBotPasswords.php index fb2a9e11c5a..584de8eda3e 100644 --- a/includes/specials/SpecialBotPasswords.php +++ b/includes/specials/SpecialBotPasswords.php @@ -49,14 +49,23 @@ class SpecialBotPasswords extends FormSpecialPage { /** @var PasswordFactory */ private $passwordFactory; + /** @var CentralIdLookup */ + private $centralIdLookup; + /** * @param PasswordFactory $passwordFactory * @param AuthManager $authManager + * @param CentralIdLookup $centralIdLookup */ - public function __construct( PasswordFactory $passwordFactory, AuthManager $authManager ) { + public function __construct( + PasswordFactory $passwordFactory, + AuthManager $authManager, + CentralIdLookup $centralIdLookup + ) { parent::__construct( 'BotPasswords', 'editmyprivateinfo' ); $this->logger = LoggerFactory::getInstance( 'authentication' ); $this->passwordFactory = $passwordFactory; + $this->centralIdLookup = $centralIdLookup; $this->setAuthManager( $authManager ); } @@ -98,7 +107,7 @@ class SpecialBotPasswords extends FormSpecialPage { throw new ErrorPageError( 'botpasswords', 'botpasswords-disabled' ); } - $this->userId = CentralIdLookup::factory()->centralIdFromLocalUser( $this->getUser() ); + $this->userId = $this->centralIdLookup->centralIdFromLocalUser( $this->getUser() ); if ( !$this->userId ) { throw new ErrorPageError( 'botpasswords', 'botpasswords-no-central-id' ); } diff --git a/includes/specials/SpecialEmailUser.php b/includes/specials/SpecialEmailUser.php index e613e54b61e..6144df98453 100644 --- a/includes/specials/SpecialEmailUser.php +++ b/includes/specials/SpecialEmailUser.php @@ -244,8 +244,9 @@ class SpecialEmailUser extends UnlistedSpecialPage { $muteList = $target->getOption( 'email-blacklist', '' ); if ( $muteList ) { $muteList = MultiUsernameFilter::splitIds( $muteList ); - $lookup = CentralIdLookup::factory(); - $senderId = $lookup->centralIdFromLocalUser( $sender ); + $senderId = MediaWikiServices::getInstance() + ->getCentralIdLookup() + ->centralIdFromLocalUser( $sender ); if ( $senderId !== 0 && in_array( $senderId, $muteList ) ) { wfDebug( "User does not allow user emails from this user." ); diff --git a/includes/specials/SpecialMute.php b/includes/specials/SpecialMute.php index 465a182cab8..38b30cf0ba4 100644 --- a/includes/specials/SpecialMute.php +++ b/includes/specials/SpecialMute.php @@ -20,7 +20,8 @@ */ use MediaWiki\Preferences\MultiUsernameFilter; -use MediaWiki\User\UserFactory; +use MediaWiki\User\UserIdentity; +use MediaWiki\User\UserIdentityLookup; use MediaWiki\User\UserOptionsManager; /** @@ -33,7 +34,7 @@ class SpecialMute extends FormSpecialPage { private const PAGE_NAME = 'Mute'; - /** @var User|null */ + /** @var UserIdentity|null */ private $target; /** @var int */ @@ -45,21 +46,23 @@ class SpecialMute extends FormSpecialPage { /** @var UserOptionsManager */ private $userOptionsManager; - /** @var UserFactory */ - private $userFactory; + /** @var UserIdentityLookup */ + private $userIdentityLookup; /** + * @param CentralIdLookup $centralIdLookup * @param UserOptionsManager $userOptionsManager - * @param UserFactory $userFactory + * @param UserIdentityLookup $userIdentityLookup */ public function __construct( + CentralIdLookup $centralIdLookup, UserOptionsManager $userOptionsManager, - UserFactory $userFactory + UserIdentityLookup $userIdentityLookup ) { parent::__construct( self::PAGE_NAME, '', false ); - $this->centralIdLookup = CentralIdLookup::factory(); + $this->centralIdLookup = $centralIdLookup; $this->userOptionsManager = $userOptionsManager; - $this->userFactory = $userFactory; + $this->userIdentityLookup = $userIdentityLookup; } /** @@ -134,9 +137,9 @@ class SpecialMute extends FormSpecialPage { } /** - * @return User|null + * @return UserIdentity|null */ - public function getTarget(): ?User { + private function getTarget(): ?UserIdentity { return $this->target; } @@ -212,7 +215,8 @@ class SpecialMute extends FormSpecialPage { ]; } - $this->getHookRunner()->onSpecialMuteModifyFormFields( $this->getTarget(), $this->getUser(), $fields ); + $legacyUser = $this->getTarget() ? User::newFromIdentity( $this->getTarget() ) : null; + $this->getHookRunner()->onSpecialMuteModifyFormFields( $legacyUser, $this->getUser(), $fields ); if ( count( $fields ) == 0 ) { throw new ErrorPageError( 'specialmute', 'specialmute-error-no-options' ); @@ -227,9 +231,9 @@ class SpecialMute extends FormSpecialPage { private function loadTarget( $username ) { $target = null; if ( $username !== null ) { - $target = $this->userFactory->newFromName( $username ); + $target = $this->userIdentityLookup->getUserIdentityByName( $username ); } - if ( !$target || !$target->getId() ) { + if ( !$target || !$target->isRegistered() ) { throw new ErrorPageError( 'specialmute', 'specialmute-error-invalid-user' ); } else { $this->target = $target; diff --git a/includes/user/User.php b/includes/user/User.php index 7c8eb514b29..4e819fde029 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -1718,7 +1718,9 @@ class User implements Authority, UserIdentity, UserEmailContact { } else { // "global per name" limit, across sites if ( isset( $limits['user-global'] ) ) { - $lookup = CentralIdLookup::factoryNonLocal(); + $lookup = MediaWikiServices::getInstance() + ->getCentralIdLookupFactory() + ->getNonLocalLookup(); $centralId = $lookup ? $lookup->centralIdFromLocalUser( $this, CentralIdLookup::AUDIENCE_RAW ) diff --git a/tests/phpunit/includes/api/ApiLoginTest.php b/tests/phpunit/includes/api/ApiLoginTest.php index d49e96c056f..f89b974f8f7 100644 --- a/tests/phpunit/includes/api/ApiLoginTest.php +++ b/tests/phpunit/includes/api/ApiLoginTest.php @@ -291,7 +291,9 @@ class ApiLoginTest extends ApiTestCase { ); $user = self::$users['sysop']; - $centralId = CentralIdLookup::factory()->centralIdFromLocalUser( $user->getUser() ); + $centralId = $this->getServiceContainer() + ->getCentralIdLookup() + ->centralIdFromLocalUser( $user->getUser() ); $this->assertNotSame( 0, $centralId, 'sanity check' ); $password = 'ngfhmjm64hv0854493hsj5nncjud2clk'; diff --git a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php index cd997067f18..f7f0165c1bc 100644 --- a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php +++ b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php @@ -71,7 +71,10 @@ class BotPasswordSessionProviderTest extends MediaWikiIntegrationTestCase { $passwordHash = $passwordFactory->newFromPlaintext( 'foobaz' ); $sysop = static::getTestSysop()->getUser(); - $userId = \CentralIdLookup::factory( 'local' )->centralIdFromName( $sysop->getName() ); + $userId = $this->getServiceContainer() + ->getCentralIdLookupFactory() + ->getLookup( 'local' ) + ->centralIdFromName( $sysop->getName() ); $dbw = wfGetDB( DB_PRIMARY ); $dbw->delete( diff --git a/tests/phpunit/includes/specials/SpecialMuteTest.php b/tests/phpunit/includes/specials/SpecialMuteTest.php index 92c72bb432b..60edf037287 100644 --- a/tests/phpunit/includes/specials/SpecialMuteTest.php +++ b/tests/phpunit/includes/specials/SpecialMuteTest.php @@ -25,8 +25,9 @@ class SpecialMuteTest extends SpecialPageTestBase { */ protected function newSpecialPage() { return new SpecialMute( + $this->getServiceContainer()->getCentralIdLookupFactory()->getLookup( 'local' ), $this->userOptionsManager, - $this->getServiceContainer()->getUserFactory() + $this->getServiceContainer()->getUserIdentityLookup() ); } @@ -75,10 +76,6 @@ class SpecialMuteTest extends SpecialPageTestBase { * @covers SpecialMute::execute */ public function testMuteAddsUserToEmailBlacklist() { - $this->setMwGlobals( [ - 'wgCentralIdLookupProvider' => 'local', - ] ); - $targetUser = $this->getTestUser()->getUser(); $loggedInUser = $this->getMutableTestUser()->getUser(); @@ -102,10 +99,6 @@ class SpecialMuteTest extends SpecialPageTestBase { * @covers SpecialMute::execute */ public function testUnmuteRemovesUserFromEmailBlacklist() { - $this->setMwGlobals( [ - 'wgCentralIdLookupProvider' => 'local', - ] ); - $targetUser = $this->getTestUser()->getUser(); $loggedInUser = $this->getMutableTestUser()->getUser();