Remove $actor field from UsererIdentityValue

Code that needs to store an actor ID in the database to
represent a UserIdentity, or needs to construct a UserIdentity based on
an actor ID loaded from the database, should use the ActorNormalization
service.

Note: The getActorId() method is removed from the UserIdentity interface,
but all concrete classes continue to support it for now.
UsererIdentityValue::getActorId() is hard deprecated and should
be removed in 1.37. It always returns 0.
User::getActorId() is not deprecated at this point.

Bug: T274179
Depends-On: Id2b3ddf6a2a7cdf90f8936a69148d2cce6fde237
Change-Id: I9925906d11e47efaec3c1f48d5cb3f9896a982c1

5 files changed, 48 insertions, 89 deletions

diff --git a/includes/user/ActorStore.php b/includes/user/ActorStore.php
index a0c76bca3f5b..4dbd892429ef 100644
--- a/includes/user/ActorStore.php
+++ b/includes/user/ActorStore.php
@@ -128,7 +128,7 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 			}
 		}
 
-		$actor = new UserIdentityValue( $userId, $row->actor_name, $actorId, $this->wikiId );
+		$actor = new UserIdentityValue( $userId, $row->actor_name, $this->wikiId );
 		$this->addUserIdentityToCache( $actorId, $actor );
 		return $actor;
 	}
@@ -182,7 +182,6 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 		$actor = new UserIdentityValue(
 			$userId,
 			$name,
-			$actorId,
 			$this->wikiId
 		);
 
@@ -314,9 +313,7 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 	 * @param int $id
 	 */
 	private function attachActorId( UserIdentity $user, int $id ) {
-		if ( $user instanceof UserIdentityValue ) {
-			$user->setActorId( $id );
-		} elseif ( $user instanceof User ) {
+		if ( $user instanceof User ) {
 			$user->setActorId( $id );
 		}
 	}
@@ -329,9 +326,7 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 	 * @param UserIdentity $user
 	 */
 	private function detachActorId( UserIdentity $user ) {
-		if ( $user instanceof UserIdentityValue ) {
-			$user->setActorId( 0 );
-		} elseif ( $user instanceof User ) {
+		if ( $user instanceof User ) {
 			$user->setActorId( 0 );
 		}
 	}
@@ -363,8 +358,9 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 
 		$id = $this->findActorIdInternal( $name, $db );
 
-		if ( $id ) {
-			$this->attachActorId( $user, $id );
+		// Set the actor ID in the User object. To be removed, see T274148.
+		if ( $id && $user instanceof User ) {
+			$user->setActorId( $id );
 		}
 
 		return $id;
@@ -520,7 +516,10 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 			}
 		}
 
-		$this->attachActorId( $user, $actorId );
+		// Set the actor ID in the User object. To be removed, see T274148.
+		if ( $user instanceof User ) {
+			$user->setActorId( $actorId );
+		}
 
 		// Cache row we've just created
 		$cachedUserIdentity = $this->newActorFromRowFields( $userId, $userName, $actorId );
@@ -599,7 +598,7 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 		if ( $actor ) {
 			return $actor;
 		}
-		$actor = new UserIdentityValue( 0, self::UNKNOWN_USER_NAME, 0, $this->wikiId );
+		$actor = new UserIdentityValue( 0, self::UNKNOWN_USER_NAME, $this->wikiId );
 
 		[ $db, ] = $this->getDBConnectionRefForQueryFlags( self::READ_LATEST );
 		$this->acquireActorId( $actor, $db );
diff --git a/includes/user/User.php b/includes/user/User.php
index c498f7455998..5bd0a86bc279 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -2153,20 +2153,22 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
 	/**
 	 * Get the user's actor ID.
 	 * @since 1.31
-	 * @note This method is deprecated in the UserIdentity interface since 1.36,
+	 * @note This method was removed from the UserIdentity interface in 1.36,
 	 *       but remains supported in the User class for now.
 	 *       New code should use ActorNormalization::findActorId() or
 	 *       ActorNormalization::acquireActorId() instead.
-	 * @param IDatabase|string|false $dbwOrWikiId Assign a new actor ID, using this DB handle,
-	 *        if none exists; wiki ID, if provided, must be self::LOCAL; Usage with IDatabase is
-	 *        deprecated since 1.36
+	 * @param IDatabase|string|false $dbwOrWikiId Deprecated since 1.36.
+	 *        If a database connection is passed, a new actor ID is assigned if needed.
+	 *        ActorNormalization::acquireActorId() should be used for that purpose instead.
 	 * @return int The actor's ID, or 0 if no actor ID exists and $dbw was null
 	 * @throws PreconditionException if $dbwOrWikiId is a string and does not match the local wiki
 	 */
 	public function getActorId( $dbwOrWikiId = self::LOCAL ) : int {
-		if ( $dbwOrWikiId instanceof IDatabase ) {
-			wfDeprecatedMsg( 'Passing parameter of type IDatabase', '1.36' );
-		} else {
+		if ( $dbwOrWikiId ) {
+			wfDeprecatedMsg( 'Passing a parameter to getActorId() is deprecated', '1.36' );
+		}
+
+		if ( is_string( $dbwOrWikiId ) ) {
 			$this->assertWiki( $dbwOrWikiId );
 		}
 
@@ -2191,9 +2193,8 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
 
 	/**
 	 * Sets the actor id.
-	 *
-	 * This method is deprecated upon introduction. It only exists for transition to ActorStore,
-	 * and will be removed shortly - T274148
+	 * For use by ActorStore only.
+	 * Should be removed once callers of getActorId() have been migrated to using ActorNormalization.
 	 *
 	 * @internal
 	 * @deprecated since 1.36
diff --git a/includes/user/UserIdentity.php b/includes/user/UserIdentity.php
index f39edf33e46c..24623a0c7e81 100644
--- a/includes/user/UserIdentity.php
+++ b/includes/user/UserIdentity.php
@@ -55,23 +55,6 @@ interface UserIdentity extends WikiAwareEntity {
 	public function getName() : string;
 
 	/**
-	 * @since 1.31
-	 *
-	 * @param string|false $wikiId The wiki ID expected by the caller.
-	 *        Use self::LOCAL for the local wiki.
-	 *
-	 * @deprecated since 1.36, use ActorNormalization::findActorId() instead.
-	 *
-	 * @return int The user's actor ID. May be 0 if no actor ID is set.
-	 *
-	 * @note This will trigger a deprecation warning when $wikiId mismatches $this->getWikiId().
-	 *       In the future, it will throw PreconditionException.
-	 */
-	public function getActorId( $wikiId = self::LOCAL ) : int;
-
-	// TODO: we may want to (optionally?) provide a global ID, see CentralIdLookup.
-
-	/**
 	 * @since 1.32
 	 *
 	 * @param UserIdentity $user
diff --git a/includes/user/UserIdentityValue.php b/includes/user/UserIdentityValue.php
index 9a005f602ae8..54e7433e27c2 100644
--- a/includes/user/UserIdentityValue.php
+++ b/includes/user/UserIdentityValue.php
@@ -24,8 +24,6 @@ namespace MediaWiki\User;
 
 use MediaWiki\DAO\WikiAwareEntityTrait;
 use Wikimedia\Assert\Assert;
-use Wikimedia\Assert\PostconditionException;
-use Wikimedia\Rdbms\IDatabase;
 
 /**
  * Value object representing a user's identity.
@@ -47,31 +45,41 @@ class UserIdentityValue implements UserIdentity {
 	 */
 	private $name;
 
-	/**
-	 * @var int
-	 */
-	private $actor;
-
 	/** @var bool|string */
 	private $wikiId;
 
 	/**
 	 * @stable to call
 	 *
+	 * @note Signature in 1.35 was: ( $id, $name, $actor ). This is still supported for
+	 *        backwards compatibility until 1.37. $actor is ignored.
+	 *
 	 * @param int $id user ID
 	 * @param string $name user name
-	 * @param int $actor actor ID
 	 * @param string|false $wikiId wiki ID or self::LOCAL for the local wiki
 	 */
-	public function __construct( $id, $name, $actor = 0, $wikiId = self::LOCAL ) {
+	public function __construct( $id, $name, $wikiId = self::LOCAL ) {
+		if ( is_int( $wikiId ) ) {
+			// Handle old signature: ( $id, $name, $actor, $wikiId )
+			$args = func_get_args();
+			$actor = $args[2];
+			$wikiId = $args[3] ?? self::LOCAL;
+
+			if ( $actor > 0 || $wikiId !== self::LOCAL ) {
+				// NOTE: For now, we only trigger a deprecation warning if $actor is not 0,
+				//       or a $wikiId is given.
+				//       A lot of tests construct UserIdentityValue with $actor = 0.
+				// TODO: Trigger a deprecation warning even if $actor is 0
+				wfDeprecatedMsg( 'Old constructor signature: $actor is no longer supported', '1.36' );
+			}
+		}
+
 		Assert::parameterType( 'integer', $id, '$id' );
 		Assert::parameterType( 'string', $name, '$name' );
-		Assert::parameterType( 'integer', $actor, '$actor' );
 		$this->assertWikiIdParam( $wikiId );
 
 		$this->id = $id;
 		$this->name = $name;
-		$this->actor = $actor;
 		$this->wikiId = $wikiId;
 	}
 
@@ -107,34 +115,15 @@ class UserIdentityValue implements UserIdentity {
 	}
 
 	/**
-	 * The numerical actor ID provided to the constructor or 0 if no actor ID has been assigned.
-	 *
 	 * @deprecated since 1.36, use ActorNormalization::acquireActorId instead.
 	 *
-	 * @param string|false $wikiId The wiki ID expected by the caller.
-	 *        Omit if expecting the local wiki. Since 1.36.
+	 * @param string|false $wikiId
 	 *
-	 * @return int The user's actor ID. May be 0 if no actor ID has been assigned.
+	 * @return int always 0.
 	 */
 	public function getActorId( $wikiId = self::LOCAL ) : int {
-		// need to check if $wikiId is an instance of IDatabase, since for legacy reasons,
-		// some callers call this function on a UserIdentity passing an IDatabase
-		if ( $wikiId instanceof IDatabase ) {
-			wfDeprecatedMsg( 'Passing parameter of type IDatabase', '1.36' );
-			if ( $this->actor === 0 ) {
-				// Passing an IDatabase was never actually allowed on this class.
-				// We just support it to prevent hard breakage in places where callers where
-				// assuming they got a User, when they actually just have a UserIdentity.
-				// However, the caller is expecting to get back a valid actorId. Not getting one
-				// may cause data corruption.
-				throw new PostconditionException(
-					'Caller expects non-zero actor ID.'
-				);
-			}
-		} else {
-			$this->deprecateInvalidCrossWiki( $wikiId, '1.36' );
-		}
-		return $this->actor;
+		wfDeprecated( __METHOD__, '1.36' );
+		return 0;
 	}
 
 	/**
@@ -163,17 +152,4 @@ class UserIdentityValue implements UserIdentity {
 		return $this->getName();
 	}
 
-	/**
-	 * Sets the actor id.
-	 *
-	 * This method is deprecated upon introduction. It only exists for transition to ActorStore,
-	 * and will be removed shortly - T274148
-	 *
-	 * @internal
-	 * @deprecated since 1.36
-	 * @param int $actorId
-	 */
-	public function setActorId( int $actorId ) {
-		$this->actor = $actorId;
-	}
 }
diff --git a/includes/user/UserRightsProxy.php b/includes/user/UserRightsProxy.php
index 9ce61235a80a..ff2b7fbc83e4 100644
--- a/includes/user/UserRightsProxy.php
+++ b/includes/user/UserRightsProxy.php
@@ -219,7 +219,7 @@ class UserRightsProxy {
 		// After all the relevant UserGroupMemberships methods are ported into UserGroupManager,
 		// the usages of this class will be changed into usages of the UserGroupManager,
 		// thus the need of this class and the need of this artificial UserIdentityValue will parish.
-		$user = new UserIdentityValue( $this->getId(), $this->getName(), 0 );
+		$user = new UserIdentityValue( $this->getId(), $this->getName() );
 		return $this->userGroupManager->getUserGroupMemberships( $user, IDBAccessObject::READ_LATEST );
 	}
 
@@ -234,7 +234,7 @@ class UserRightsProxy {
 		return $this->userGroupManager->addUserToGroup(
 			// TODO: Artificial UserIdentity just for passing the id and name.
 			// see comment in getGroupMemberships.
-			new UserIdentityValue( $this->getId(), $this->getName(), 0 ),
+			new UserIdentityValue( $this->getId(), $this->getName() ),
 			$group,
 			$expiry
 		);
@@ -250,7 +250,7 @@ class UserRightsProxy {
 		return $this->userGroupManager->removeUserFromGroup(
 			// TODO: Artificial UserIdentity just for passing the id and name.
 			// see comment in getGroupMemberships.
-			new UserIdentityValue( $this->getId(), $this->getName(), 0 ),
+			new UserIdentityValue( $this->getId(), $this->getName() ),
 			$group
 		);
 	}