From 85be3c57bcff9a90e89022a43e156d939ee2c359 Mon Sep 17 00:00:00 2001 From: Vadim Kovalenko Date: Tue, 9 Mar 2021 12:56:04 -0500 Subject: [PATCH] Replace RecentChange::getPerformer with RecentChange::getPerformerIdentity Bug: T276412 Change-Id: I8a55bd5cb17bbc259ec36c40261058e0b46ee4a6 --- includes/ServiceWiring.php | 3 ++ .../VariableGenerator/RCVariableGenerator.php | 22 +++++----- .../VariableGenerator/VariableGenerator.php | 13 +++--- includes/Variables/LazyVariableComputer.php | 40 ++++++++++++++----- .../Variables/LazyVariableComputerTest.php | 39 +++++++++++++----- 5 files changed, 81 insertions(+), 36 deletions(-) diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 346e51ac1..54c2fe59c 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -325,6 +325,9 @@ return [ $services->getRevisionStore(), $services->getContentLanguage(), $services->getParser(), + $services->getUserEditTracker(), + $services->getUserGroupManager(), + $services->getPermissionManager(), WikiMap::getCurrentWikiDbDomain()->getId() ); }, diff --git a/includes/VariableGenerator/RCVariableGenerator.php b/includes/VariableGenerator/RCVariableGenerator.php index 326fe090b..e299d8cdf 100644 --- a/includes/VariableGenerator/RCVariableGenerator.php +++ b/includes/VariableGenerator/RCVariableGenerator.php @@ -102,12 +102,12 @@ class RCVariableGenerator extends VariableGenerator { * @return $this */ private function addMoveVars() : self { - $user = $this->rc->getPerformer(); + $userIdentity = $this->rc->getPerformerIdentity(); $oldTitle = $this->rc->getTitle(); $newTitle = Title::newFromText( $this->rc->getParam( '4::target' ) ); - $this->addUserVars( $user, $this->rc ) + $this->addUserVars( $userIdentity, $this->rc ) ->addTitleVars( $oldTitle, 'moved_from', $this->rc ) ->addTitleVars( $newTitle, 'moved_to', $this->rc ); @@ -130,9 +130,9 @@ class RCVariableGenerator extends VariableGenerator { $name = $this->rc->getTitle()->getText(); // Add user data if the account was created by a registered user - $user = $this->rc->getPerformer(); - if ( !$user->isAnon() && $name !== $user->getName() ) { - $this->addUserVars( $user, $this->rc ); + $userIdentity = $this->rc->getPerformerIdentity(); + if ( $userIdentity->isRegistered() && $name !== $userIdentity->getName() ) { + $this->addUserVars( $userIdentity, $this->rc ); } $this->vars->setVar( 'accountname', $name ); @@ -145,9 +145,9 @@ class RCVariableGenerator extends VariableGenerator { */ private function addDeleteVars() : self { $title = $this->rc->getTitle(); - $user = $this->rc->getPerformer(); + $userIdentity = $this->rc->getPerformerIdentity(); - $this->addUserVars( $user, $this->rc ) + $this->addUserVars( $userIdentity, $this->rc ) ->addTitleVars( $title, 'page', $this->rc ); $this->vars->setVar( 'action', 'delete' ); @@ -161,9 +161,9 @@ class RCVariableGenerator extends VariableGenerator { */ private function addUploadVars() : self { $title = $this->rc->getTitle(); - $user = $this->rc->getPerformer(); + $userIdentity = $this->rc->getPerformerIdentity(); - $this->addUserVars( $user, $this->rc ) + $this->addUserVars( $userIdentity, $this->rc ) ->addTitleVars( $title, 'page', $this->rc ); $this->vars->setVar( 'action', 'upload' ); @@ -206,9 +206,9 @@ class RCVariableGenerator extends VariableGenerator { */ private function addEditVarsForRow() : self { $title = $this->rc->getTitle(); - $user = $this->rc->getPerformer(); + $userIdentity = $this->rc->getPerformerIdentity(); - $this->addUserVars( $user, $this->rc ) + $this->addUserVars( $userIdentity, $this->rc ) ->addTitleVars( $title, 'page', $this->rc ); // @todo Set old_content_model and new_content_model diff --git a/includes/VariableGenerator/VariableGenerator.php b/includes/VariableGenerator/VariableGenerator.php index f2525dbf0..a0f86402c 100644 --- a/includes/VariableGenerator/VariableGenerator.php +++ b/includes/VariableGenerator/VariableGenerator.php @@ -4,6 +4,7 @@ namespace MediaWiki\Extension\AbuseFilter\VariableGenerator; use MediaWiki\Extension\AbuseFilter\Hooks\AbuseFilterHookRunner; use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder; +use MediaWiki\User\UserIdentity; use RecentChange; use Title; use User; @@ -59,16 +60,18 @@ class VariableGenerator { } /** - * @param User $user + * @param UserIdentity $userIdentity * @param RecentChange|null $rc If the variables should be generated for an RC entry, * this is the entry. Null if it's for the current action being filtered. * @return $this For chaining */ - public function addUserVars( User $user, RecentChange $rc = null ) : self { + public function addUserVars( UserIdentity $userIdentity, RecentChange $rc = null ) : self { + $user = User::newFromIdentity( $userIdentity ); + $this->vars->setLazyLoadVar( 'user_editcount', 'user-editcount', - [ 'user' => $user ] + [ 'user-identity' => $userIdentity ] ); $this->vars->setVar( 'user_name', $user->getName() ); @@ -88,13 +91,13 @@ class VariableGenerator { $this->vars->setLazyLoadVar( 'user_groups', 'user-groups', - [ 'user' => $user ] + [ 'user-identity' => $userIdentity ] ); $this->vars->setLazyLoadVar( 'user_rights', 'user-rights', - [ 'user' => $user ] + [ 'user-identity' => $userIdentity ] ); $this->vars->setLazyLoadVar( diff --git a/includes/Variables/LazyVariableComputer.php b/includes/Variables/LazyVariableComputer.php index b869ed0a9..77871167f 100644 --- a/includes/Variables/LazyVariableComputer.php +++ b/includes/Variables/LazyVariableComputer.php @@ -8,8 +8,11 @@ use Language; use MediaWiki\Extension\AbuseFilter\Hooks\AbuseFilterHookRunner; use MediaWiki\Extension\AbuseFilter\Parser\AFPData; use MediaWiki\Extension\AbuseFilter\TextExtractor; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Storage\RevisionLookup; use MediaWiki\Storage\RevisionStore; +use MediaWiki\User\UserEditTracker; +use MediaWiki\User\UserGroupManager; use MWException; use Parser; use ParserOptions; @@ -65,6 +68,15 @@ class LazyVariableComputer { /** @var Parser */ private $parser; + /** @var UserEditTracker */ + private $userEditTracker; + + /** @var UserGroupManager */ + private $userGroupManager; + + /** @var PermissionManager */ + private $permissionManager; + /** @var string */ private $wikiID; @@ -78,6 +90,9 @@ class LazyVariableComputer { * @param RevisionStore $revisionStore * @param Language $contentLanguage * @param Parser $parser + * @param UserEditTracker $userEditTracker + * @param UserGroupManager $userGroupManager + * @param PermissionManager $permissionManager * @param string $wikiID */ public function __construct( @@ -90,6 +105,9 @@ class LazyVariableComputer { RevisionStore $revisionStore, Language $contentLanguage, Parser $parser, + UserEditTracker $userEditTracker, + UserGroupManager $userGroupManager, + PermissionManager $permissionManager, string $wikiID ) { $this->textExtractor = $textExtractor; @@ -101,6 +119,9 @@ class LazyVariableComputer { $this->revisionStore = $revisionStore; $this->contentLanguage = $contentLanguage; $this->parser = $parser; + $this->userEditTracker = $userEditTracker; + $this->userGroupManager = $userGroupManager; + $this->permissionManager = $permissionManager; $this->wikiID = $wikiID; } @@ -273,13 +294,12 @@ class LazyVariableComputer { $action = $parameters['action']; /** @var Title $title */ $title = $parameters['title']; - $result = $title->getRestrictions( $action ); break; case 'user-editcount': - /** @var User $user */ - $user = $parameters['user']; - $result = $user->getEditCount(); + /** @var UserIdentity $user */ + $userIdentity = $parameters['user-identity']; + $result = $this->userEditTracker->getUserEditCount( $userIdentity ); break; case 'user-emailconfirm': /** @var User $user */ @@ -287,14 +307,14 @@ class LazyVariableComputer { $result = $user->getEmailAuthenticationTimestamp(); break; case 'user-groups': - /** @var User $user */ - $user = $parameters['user']; - $result = $user->getEffectiveGroups(); + /** @var UserIdentity $user */ + $userIdentity = $parameters['user-identity']; + $result = $this->userGroupManager->getUserEffectiveGroups( $userIdentity ); break; case 'user-rights': - /** @var User $user */ - $user = $parameters['user']; - $result = $user->getRights(); + /** @var UserIdentity $user */ + $userIdentity = $parameters['user-identity']; + $result = $this->permissionManager->getUserPermissions( $userIdentity ); break; case 'user-block': // @todo Support partial blocks? diff --git a/tests/phpunit/unit/Variables/LazyVariableComputerTest.php b/tests/phpunit/unit/Variables/LazyVariableComputerTest.php index 14344d9eb..a97710e93 100644 --- a/tests/phpunit/unit/Variables/LazyVariableComputerTest.php +++ b/tests/phpunit/unit/Variables/LazyVariableComputerTest.php @@ -11,9 +11,12 @@ use MediaWiki\Extension\AbuseFilter\TextExtractor; use MediaWiki\Extension\AbuseFilter\Variables\LazyLoadedVariable; use MediaWiki\Extension\AbuseFilter\Variables\LazyVariableComputer; use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder; +use MediaWiki\Permissions\PermissionManager; use MediaWiki\Revision\RevisionLookup; use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\RevisionStore; +use MediaWiki\User\UserEditTracker; +use MediaWiki\User\UserGroupManager; use MediaWiki\User\UserIdentityValue; use MediaWikiUnitTestCase; use MWException; @@ -43,7 +46,10 @@ class LazyVariableComputerTest extends MediaWikiUnitTestCase { Language $contentLanguage = null, array $hookHandlers = [], RevisionLookup $revisionLookup = null, - string $wikiID = '' + string $wikiID = '', + UserEditTracker $userEditTracker = null, + UserGroupManager $userGroupManager = null, + PermissionManager $permissionManager = null ) : LazyVariableComputer { return new LazyVariableComputer( $this->createMock( TextExtractor::class ), @@ -55,6 +61,9 @@ class LazyVariableComputerTest extends MediaWikiUnitTestCase { $this->createMock( RevisionStore::class ), $contentLanguage ?? $this->createMock( Language::class ), $this->createMock( Parser::class ), + $userEditTracker ?? $this->createMock( UserEditTracker::class ), + $userGroupManager ?? $this->createMock( UserGroupManager::class ), + $permissionManager ?? $this->createMock( PermissionManager::class ), $wikiID ); } @@ -145,11 +154,16 @@ class LazyVariableComputerTest extends MediaWikiUnitTestCase { /** * @param LazyLoadedVariable $var * @param mixed $expected + * @param array $services * @covers ::compute * @dataProvider provideUserRelatedVars */ - public function testUserRelatedVars( LazyLoadedVariable $var, $expected ) { - $computer = $this->getComputer(); + public function testUserRelatedVars( + LazyLoadedVariable $var, + $expected, + $services = [ null, null, null ] + ) { + $computer = $this->getComputer( null, [], null, '', ...$services ); $this->assertSame( $expected, $computer->compute( $var, new VariableHolder(), $this->getForbidComputeCB() )->toNative() @@ -161,14 +175,17 @@ class LazyVariableComputerTest extends MediaWikiUnitTestCase { $getUserVar = function ( $user, $method ) : LazyLoadedVariable { return new LazyLoadedVariable( $method, - [ 'user' => $user ] + [ 'user' => $user, 'user-identity' => $user ] ); }; $editCount = 7; - $user->method( 'getEditCount' )->willReturn( $editCount ); + + $userEditTracker = $this->createMock( UserEditTracker::class ); + + $userEditTracker->method( 'getUserEditCount' )->with( $user )->willReturn( $editCount ); $var = $getUserVar( $user, 'user-editcount' ); - yield 'user_editcount' => [ $var, $editCount ]; + yield 'user_editcount' => [ $var, $editCount, [ $userEditTracker, null, null ] ]; $emailConfirm = '20000101000000'; $user->method( 'getEmailAuthenticationTimestamp' )->willReturn( $emailConfirm ); @@ -176,14 +193,16 @@ class LazyVariableComputerTest extends MediaWikiUnitTestCase { yield 'user_emailconfirm' => [ $var, $emailConfirm ]; $groups = [ '*', 'group1', 'group2' ]; - $user->method( 'getEffectiveGroups' )->willReturn( $groups ); + $userGroupManager = $this->createMock( UserGroupManager::class ); + $userGroupManager->method( 'getUserEffectiveGroups' )->with( $user )->willReturn( $groups ); $var = $getUserVar( $user, 'user-groups' ); - yield 'user_groups' => [ $var, $groups ]; + yield 'user_groups' => [ $var, $groups, [ null, $userGroupManager, null ] ]; $rights = [ 'abusefilter-foo', 'abusefilter-bar' ]; - $user->method( 'getRights' )->willReturn( $rights ); + $permissionManager = $this->createMock( PermissionManager::class ); + $permissionManager->method( 'getUserPermissions' )->with( $user )->willReturn( $rights ); $var = $getUserVar( $user, 'user-rights' ); - yield 'user_rights' => [ $var, $rights ]; + yield 'user_rights' => [ $var, $rights, [ null, null, $permissionManager ] ]; $blocked = true; $user->method( 'getBlock' )->willReturn( $blocked );