From 5a229f691ff1bd5f3d97ac01f8bf61d36fa4aed3 Mon Sep 17 00:00:00 2001 From: DannyS712 Date: Sat, 24 Apr 2021 04:54:48 +0000 Subject: [PATCH] Accept UserIdentity in code to manage edit counts - UserEditCountUpdate: accept UserIdentity instead of User - Move User::incEditCount() to UserEditTracker Change-Id: Ibf1482bcbcbf4d56fc06531bb3e17e2a6e3e2e6f --- RELEASE-NOTES-1.37 | 2 + includes/MovePage.php | 12 ++++-- includes/ServiceWiring.php | 3 +- includes/Storage/PageUpdater.php | 11 +++++- includes/deferred/UserEditCountUpdate.php | 30 ++++++++++----- includes/page/PageCommandFactory.php | 11 +++++- includes/page/WikiPage.php | 1 + includes/user/User.php | 10 +---- includes/user/UserEditTracker.php | 20 ++++++++++ .../includes/user/UserEditTrackerTest.php | 37 +++++++++++++++++++ 10 files changed, 111 insertions(+), 26 deletions(-) diff --git a/RELEASE-NOTES-1.37 b/RELEASE-NOTES-1.37 index f63bfeb1b42..a79266b3048 100644 --- a/RELEASE-NOTES-1.37 +++ b/RELEASE-NOTES-1.37 @@ -355,6 +355,8 @@ because of Phabricator reports. * User::getRights(), deprecated since 1.34, now emits deprecation warnings. * User::changeableGroups and ::changeableByGroup were hard deprecated, use corresponding methods in UserGroupManager instead. +* User::incEditCount() was deprecated in favor of the new method + UserEditTracker::incrementUserEditCount(). * RepoGroup::singleton(), ::destroySingleton() and ::setSingleton(), deprecated since 1.34, now emit deprecation warnings. * AbstractAuthenticationProvider ::setLogger(), ::setManager(), ::setConfig(), diff --git a/includes/MovePage.php b/includes/MovePage.php index cee3ee1b6c2..dfa3434f6a1 100644 --- a/includes/MovePage.php +++ b/includes/MovePage.php @@ -33,6 +33,7 @@ use MediaWiki\Revision\MutableRevisionRecord; use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\RevisionStore; use MediaWiki\Revision\SlotRecord; +use MediaWiki\User\UserEditTracker; use MediaWiki\User\UserFactory; use MediaWiki\User\UserIdentity; use Wikimedia\Rdbms\IDatabase; @@ -111,6 +112,9 @@ class MovePage { */ private $userFactory; + /** @var UserEditTracker */ + private $userEditTracker; + /** * @internal For use by PageCommandFactory */ @@ -135,6 +139,7 @@ class MovePage { * @param HookContainer|null $hookContainer * @param WikiPageFactory|null $wikiPageFactory * @param UserFactory|null $userFactory + * @param UserEditTracker|null $userEditTracker */ public function __construct( Title $oldTitle, @@ -149,7 +154,8 @@ class MovePage { SpamChecker $spamChecker = null, HookContainer $hookContainer = null, WikiPageFactory $wikiPageFactory = null, - UserFactory $userFactory = null + UserFactory $userFactory = null, + UserEditTracker $userEditTracker = null ) { $this->oldTitle = $oldTitle; $this->newTitle = $newTitle; @@ -175,6 +181,7 @@ class MovePage { $this->hookRunner = new HookRunner( $hookContainer ?? $services()->getHookContainer() ); $this->wikiPageFactory = $wikiPageFactory ?? $services()->getWikiPageFactory(); $this->userFactory = $userFactory ?? $services()->getUserFactory(); + $this->userEditTracker = $userEditTracker ?? $services()->getUserEditTracker(); } /** @@ -975,8 +982,7 @@ class MovePage { * Increment user_editcount during page moves * Moved from SpecialMovepage.php per T195550 */ - $userObj = $this->userFactory->newFromUserIdentity( $user ); - $userObj->incEditCount(); + $this->userEditTracker->incrementUserEditCount( $user ); if ( !$redirectContent ) { // Clean up the old title *before* reset article id - T47348 diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index a9e32adc55c..222240792bf 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -1767,7 +1767,8 @@ return [ $services->getUserFactory(), $services->getActorMigration(), $services->getActorNormalization(), - $services->getTitleFactory() + $services->getTitleFactory(), + $services->getUserEditTracker() ); }, diff --git a/includes/Storage/PageUpdater.php b/includes/Storage/PageUpdater.php index faab63550cc..eb8a0387b12 100644 --- a/includes/Storage/PageUpdater.php +++ b/includes/Storage/PageUpdater.php @@ -45,6 +45,7 @@ use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\RevisionStore; use MediaWiki\Revision\SlotRecord; use MediaWiki\Revision\SlotRoleRegistry; +use MediaWiki\User\UserEditTracker; use MediaWiki\User\UserIdentity; use MWException; use RecentChange; @@ -132,6 +133,9 @@ class PageUpdater { */ private $hookContainer; + /** @var UserEditTracker */ + private $userEditTracker; + /** * @var bool see $wgUseAutomaticEditSummaries * @see $wgUseAutomaticEditSummaries @@ -197,6 +201,7 @@ class PageUpdater { * @param SlotRoleRegistry $slotRoleRegistry * @param IContentHandlerFactory $contentHandlerFactory * @param HookContainer $hookContainer + * @param UserEditTracker $userEditTracker * @param ServiceOptions $serviceOptions * @param string[] $softwareTags Array of currently enabled software change tags. Can be * obtained from ChangeTags::getSoftwareTags() @@ -210,6 +215,7 @@ class PageUpdater { SlotRoleRegistry $slotRoleRegistry, IContentHandlerFactory $contentHandlerFactory, HookContainer $hookContainer, + UserEditTracker $userEditTracker, ServiceOptions $serviceOptions, array $softwareTags ) { @@ -226,6 +232,7 @@ class PageUpdater { $this->contentHandlerFactory = $contentHandlerFactory; $this->hookContainer = $hookContainer; $this->hookRunner = new HookRunner( $hookContainer ); + $this->userEditTracker = $userEditTracker; $this->softwareTags = $softwareTags; $this->slotsUpdate = new RevisionSlotsUpdate(); @@ -1269,7 +1276,7 @@ class PageUpdater { ); } - $legacyUser->incEditCount(); + $this->userEditTracker->incrementUserEditCount( $user ); $dbw->endAtomic( __METHOD__ ); @@ -1404,7 +1411,7 @@ class PageUpdater { ); } - $legacyUser->incEditCount(); + $this->userEditTracker->incrementUserEditCount( $user ); if ( $this->usePageCreationLog ) { // Log the page creation diff --git a/includes/deferred/UserEditCountUpdate.php b/includes/deferred/UserEditCountUpdate.php index 400cb24521e..f0349cb0034 100644 --- a/includes/deferred/UserEditCountUpdate.php +++ b/includes/deferred/UserEditCountUpdate.php @@ -21,20 +21,21 @@ */ use MediaWiki\MediaWikiServices; +use MediaWiki\User\UserIdentity; use Wikimedia\Assert\Assert; /** * Handles increment the edit count for a given set of users */ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate { - /** @var array[] Map of (user ID => ('increment': int, 'instances': User[])) */ + /** @var array[] Map of (user ID => ('increment': int, 'instances': UserIdentity[])) */ private $infoByUser; /** - * @param User $user + * @param UserIdentity $user * @param int $increment */ - public function __construct( User $user, $increment ) { + public function __construct( UserIdentity $user, $increment ) { if ( !$user->getId() ) { throw new RuntimeException( "Got user ID of zero" ); } @@ -67,11 +68,14 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate { * Purges the list of URLs passed to the constructor. */ public function doUpdate() { - $lb = MediaWikiServices::getInstance()->getDBLoadBalancer(); + $mwServices = MediaWikiServices::getInstance(); + $lb = $mwServices->getDBLoadBalancer(); $dbw = $lb->getConnectionRef( DB_PRIMARY ); + $userFactory = $mwServices->getUserFactory(); + $editTracker = $mwServices->getUserEditTracker(); $fname = __METHOD__; - ( new AutoCommitUpdate( $dbw, __METHOD__, function () use ( $lb, $dbw, $fname ) { + ( new AutoCommitUpdate( $dbw, __METHOD__, function () use ( $lb, $dbw, $fname, $userFactory, $editTracker ) { foreach ( $this->infoByUser as $userId => $info ) { $dbw->update( 'user', @@ -79,7 +83,7 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate { [ 'user_id' => $userId, 'user_editcount IS NOT NULL' ], $fname ); - /** @var User[] $affectedInstances */ + /** @var UserIdentity[] $affectedInstances */ $affectedInstances = $info['instances']; // Lazy initialization check... if ( $dbw->affectedRows() == 0 ) { @@ -91,7 +95,7 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate { // is harmless and waitForMasterPos() will just no-op. $dbr->flushSnapshot( $fname ); $lb->waitForMasterPos( $dbr ); - $affectedInstances[0]->initEditCountInternal( $dbr ); + $editTracker->initializeUserEditCount( $affectedInstances[0] ); } $newCount = (int)$dbw->selectField( 'user', @@ -102,12 +106,18 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate { // Update the edit count in the instance caches. This is mostly useful // for maintenance scripts, where deferred updates might run immediately - // and user instances might be reused for a long time. + // and user instances might be reused for a long time. Only applies to + // instances where we have User objects, if we have UserIdentity only + // then invalidating the cache should be enough foreach ( $affectedInstances as $affectedInstance ) { - $affectedInstance->setEditCountInternal( $newCount ); + if ( $affectedInstance instanceof User ) { + $affectedInstance->setEditCountInternal( $newCount ); + } } // Clear the edit count in user cache too - $affectedInstances[0]->invalidateCache(); + $userFactory->newFromUserIdentity( $affectedInstances[0] )->invalidateCache(); + // And the cache in UserEditTracker + $editTracker->clearUserEditCache( $affectedInstances[0] ); } } ) )->doUpdate(); } diff --git a/includes/page/PageCommandFactory.php b/includes/page/PageCommandFactory.php index 470c79817c5..c0c3b7597aa 100644 --- a/includes/page/PageCommandFactory.php +++ b/includes/page/PageCommandFactory.php @@ -32,6 +32,7 @@ use MediaWiki\HookContainer\HookContainer; use MediaWiki\Permissions\Authority; use MediaWiki\Revision\RevisionStore; use MediaWiki\User\ActorNormalization; +use MediaWiki\User\UserEditTracker; use MediaWiki\User\UserFactory; use MediaWiki\User\UserIdentity; use MergeHistory; @@ -106,6 +107,9 @@ class PageCommandFactory implements /** @var TitleFactory */ private $titleFactory; + /** @var UserEditTracker */ + private $userEditTracker; + public function __construct( Config $config, ILoadBalancer $loadBalancer, @@ -122,7 +126,8 @@ class PageCommandFactory implements UserFactory $userFactory, ActorMigration $actorMigration, ActorNormalization $actorNormalization, - TitleFactory $titleFactory + TitleFactory $titleFactory, + UserEditTracker $userEditTracker ) { $this->config = $config; $this->loadBalancer = $loadBalancer; @@ -140,6 +145,7 @@ class PageCommandFactory implements $this->actorMigration = $actorMigration; $this->actorNormalization = $actorNormalization; $this->titleFactory = $titleFactory; + $this->userEditTracker = $userEditTracker; } /** @@ -214,7 +220,8 @@ class PageCommandFactory implements $this->spamChecker, $this->hookContainer, $this->wikiPageFactory, - $this->userFactory + $this->userFactory, + $this->userEditTracker ); } diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index f6307348e66..ca72762d4bf 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -1848,6 +1848,7 @@ class WikiPage implements Page, IDBAccessObject, PageRecord { $this->getSlotRoleRegistry(), $this->getContentHandlerFactory(), $this->getHookContainer(), + MediaWikiServices::getInstance()->getUserEditTracker(), new ServiceOptions( PageUpdater::CONSTRUCTOR_OPTIONS, $config diff --git a/includes/user/User.php b/includes/user/User.php index 0927567890f..50359fc3059 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -4102,16 +4102,10 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact /** * Schedule a deferred update to update the user's edit count + * @deprecated since 1.37 */ public function incEditCount() { - if ( $this->isAnon() ) { - return; // sanity - } - - DeferredUpdates::addUpdate( - new UserEditCountUpdate( $this, 1 ), - DeferredUpdates::POSTSEND - ); + MediaWikiServices::getInstance()->getUserEditTracker()->incrementUserEditCount( $this ); } /** diff --git a/includes/user/UserEditTracker.php b/includes/user/UserEditTracker.php index 62cc8792dee..93179a14a2e 100644 --- a/includes/user/UserEditTracker.php +++ b/includes/user/UserEditTracker.php @@ -3,9 +3,11 @@ namespace MediaWiki\User; use ActorMigration; +use DeferredUpdates; use InvalidArgumentException; use JobQueueGroup; use UserEditCountInitJob; +use UserEditCountUpdate; use Wikimedia\Rdbms\ILoadBalancer; use Wikimedia\Timestamp\ConvertibleTimestamp; @@ -116,6 +118,24 @@ class UserEditTracker { return $count; } + /** + * Schedule a job to increase a user's edit count + * + * @since 1.37 + * @param UserIdentity $user + */ + public function incrementUserEditCount( UserIdentity $user ) { + if ( !$user->isRegistered() ) { + // Anonymous users don't have edit counts + return; + } + + DeferredUpdates::addUpdate( + new UserEditCountUpdate( $user, 1 ), + DeferredUpdates::POSTSEND + ); + } + /** * Get the user's first edit timestamp * diff --git a/tests/phpunit/includes/user/UserEditTrackerTest.php b/tests/phpunit/includes/user/UserEditTrackerTest.php index ed1fb7a17e8..e0d90382616 100644 --- a/tests/phpunit/includes/user/UserEditTrackerTest.php +++ b/tests/phpunit/includes/user/UserEditTrackerTest.php @@ -125,4 +125,41 @@ class UserEditTrackerTest extends MediaWikiIntegrationTestCase { $this->assertSame( 1, $tracker->getUserEditCount( $user ) ); } + public function testIncrementUserEditCount() { + $tracker = $this->getServiceContainer()->getUserEditTracker(); + $user = $this->getMutableTestUser()->getUser(); + + $editCountStart = $tracker->getUserEditCount( $user ); + + $this->db->startAtomic( __METHOD__ ); // let deferred updates queue up + + $tracker->incrementUserEditCount( $user ); + $this->assertSame( + 1, + DeferredUpdates::pendingUpdatesCount(), + 'Update queued for registered user' + ); + + $tracker->incrementUserEditCount( UserIdentityValue::newAnonymous( '1.1.1.1' ) ); + $this->assertSame( + 1, + DeferredUpdates::pendingUpdatesCount(), + 'No update queued for anonymous user' + ); + + $this->db->endAtomic( __METHOD__ ); // run deferred updates + $this->assertSame( + 0, + DeferredUpdates::pendingUpdatesCount(), + 'Sanity check: deferred updates ran' + ); + + $editCountEnd = $tracker->getUserEditCount( $user ); + $this->assertSame( + 1, + $editCountEnd - $editCountStart, + 'Edit count was incremented' + ); + } + }