Merge "Accept UserIdentity in code to manage edit counts"

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(),

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

includes/ServiceWiring.php

@@ -1767,7 +1767,8 @@ return [
 			$services->getUserFactory(),
 			$services->getActorMigration(),
 			$services->getActorNormalization(),
-			$services->getTitleFactory()
+			$services->getTitleFactory(),
+			$services->getUserEditTracker()
 		);
 	},
 

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

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();
 	}

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
 		);
 	}
 

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

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 );
 	}
 
 	/**

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
 	 *

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'
+		);
+	}
+
 }