Remove last User dependencies from File

Change-Id: I953fcc66b5cde1ef481178b08e16c50b8a118702

includes/Revision/RevisionStore.php

@@ -1614,22 +1614,12 @@ class RevisionStore
 			$row->$field = $value;
 		}
 
-		try {
-			$user = $this->actorStore->newActorFromRowFields(
-				$row->ar_user ?? null,
-				$row->ar_user_text ?? null,
-				$row->ar_actor ?? null
-			);
-		} catch ( InvalidArgumentException $ex ) {
-			$this->logger->warning( 'Could not load user for archive revision {rev_id}', [
-				'ar_rev_id' => $row->ar_rev_id,
-				'ar_actor' => $row->ar_actor ?? 'null',
-				'ar_user_text' => $row->ar_user_text ?? 'null',
-				'ar_user' => $row->ar_user ?? 'null',
-				'exception' => $ex
-			] );
-			$user = $this->actorStore->getUnknownActor();
-		}
+		$user = $this->actorStore->newActorFromRowFields(
+			$row->ar_user ?? null,
+			$row->ar_user_text ?? null,
+			$row->ar_actor ?? null,
+			$this->actorStore->getUnknownActor()
+		);
 
 		$db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
 		// Legacy because $row may have come from self::selectFields()
@@ -1697,22 +1687,12 @@ class RevisionStore
 			);
 		}
 
-		try {
-			$user = $this->actorStore->newActorFromRowFields(
-				$row->rev_user ?? null,
-				$row->rev_user_text ?? null,
-				$row->rev_actor ?? null
-			);
-		} catch ( InvalidArgumentException $ex ) {
-			$this->logger->warning( 'Could not load user for revision {rev_id}', [
-				'rev_id' => $row->rev_id,
-				'rev_actor' => $row->rev_actor ?? 'null',
-				'rev_user_text' => $row->rev_user_text ?? 'null',
-				'rev_user' => $row->rev_user ?? 'null',
-				'exception' => $ex
-			] );
-			$user = $this->actorStore->getUnknownActor();
-		}
+		$user = $this->actorStore->newActorFromRowFields(
+			$row->rev_user ?? null,
+			$row->rev_user_text ?? null,
+			$row->rev_actor ?? null,
+			$this->actorStore->getUnknownActor()
+		);
 
 		$db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
 		// Legacy because $row may have come from self::selectFields()

includes/filerepo/file/ArchivedFile.php

@@ -302,7 +302,13 @@ class ArchivedFile {
 		$this->description = MediaWikiServices::getInstance()->getCommentStore()
 			// Legacy because $row may have come from self::selectFields()
 			->getCommentLegacy( wfGetDB( DB_REPLICA ), 'fa_description', $row )->text;
-		$this->user = User::newFromAnyId( $row->fa_user, $row->fa_user_text, $row->fa_actor );
+		$actorStore = MediaWikiServices::getInstance()->getActorStore();
+		$this->user = $actorStore->newActorFromRowFields(
+			$row->fa_user ?? null,
+			$row->fa_user_text ?? null,
+			$row->fa_actor ?? null,
+			$actorStore->getUnknownActor()
+		);
 		$this->timestamp = $row->fa_timestamp;
 		$this->deleted = $row->fa_deleted;
 		if ( isset( $row->fa_sha1 ) ) {

includes/filerepo/file/LocalFile.php

@@ -658,10 +658,12 @@ class LocalFile extends File {
 		$this->description = MediaWikiServices::getInstance()->getCommentStore()
 			->getComment( "{$prefix}description", $row )->text;
 
-		$this->user = User::newFromAnyId(
+		$actorStore = MediaWikiServices::getInstance()->getActorStore();
+		$this->user = $actorStore->newActorFromRowFields(
 			$unprefixed['user'] ?? null,
 			$unprefixed['user_text'] ?? null,
-			$unprefixed['actor'] ?? null
+			$unprefixed['actor'] ?? null,
+			$actorStore->getUnknownActor()
 		);
 
 		$this->timestamp = wfTimestamp( TS_MW, $unprefixed['timestamp'] );

includes/filerepo/file/LocalFileDeleteBatch.php

@@ -257,7 +257,6 @@ class LocalFileDeleteBatch {
 				$reason = $commentStore->createComment( $dbw, $this->reason );
 				foreach ( $res as $row ) {
 					$comment = $commentStore->getComment( 'oi_description', $row );
-					$user = User::newFromAnyId( $row->oi_user, $row->oi_user_text, $row->oi_actor );
 					$rowsInsert[] = [
 						// Deletion-specific fields
 						'fa_storage_group' => 'deleted',

includes/user/ActorNormalization.php

@@ -61,9 +61,15 @@ interface ActorNormalization {
 	 * @param int|null $userId
 	 * @param string|null $name
 	 * @param int|null $actorId
+	 * @param UserIdentity|null $fallback
 	 * @return UserIdentity
 	 */
-	public function newActorFromRowFields( $userId, $name, $actorId ): UserIdentity;
+	public function newActorFromRowFields(
+		$userId,
+		$name,
+		$actorId,
+		UserIdentity $fallback = null
+	): UserIdentity;
 
 	/**
 	 * Find the actor_id for the given name.

includes/user/ActorStore.php

@@ -135,13 +135,44 @@ class ActorStore implements UserIdentityLookup, ActorNormalization {
 	 * is completed and all queries use explicit join with actor table, this method will be
 	 * deprecated and removed.
 	 *
+	 * @param int|null $userId
+	 * @param string|null $name
+	 * @param int|null $actorId
+	 * @param UserIdentity|null $fallback in case actor can not be created, use this as fallback.
+	 * @return UserIdentity
 	 * @throws InvalidArgumentException
+	 */
+	public function newActorFromRowFields(
+		$userId,
+		$name,
+		$actorId,
+		UserIdentity $fallback = null
+	): UserIdentity {
+		try {
+			return $this->doCreateActorFromRowFields( $userId, $name, $actorId );
+		} catch ( InvalidArgumentException $e ) {
+			$this->logger->warning( 'Failed to create actor from row fields', [
+				'exception' => $e,
+				'user_id' => $userId,
+				'actor_id' => $actorId,
+				'actor_name' => $name,
+			] );
+			if ( $fallback ) {
+				return $fallback;
+			}
+			throw $e;
+		}
+	}
+
+	/**
+	 * Instantiate a new UserIdentity object based on field values from a DB row.
+	 *
 	 * @param int|null $userId
 	 * @param string|null $name
 	 * @param int|null $actorId
 	 * @return UserIdentity
 	 */
-	public function newActorFromRowFields( $userId, $name, $actorId ): UserIdentity {
+	private function doCreateActorFromRowFields( $userId, $name, $actorId ): UserIdentity {
 		// For backwards compatibility we are quite relaxed about what to accept,
 		// but try not to create entirely incorrect objects. As we move more code
 		// from ActorMigration aliases to proper join with the actor table,

tests/phpunit/includes/filerepo/LocalRepoTest.php

@@ -48,6 +48,8 @@ class LocalRepoTest extends MediaWikiIntegrationTestCase {
 		$row = (object)[
 			"{$prefix}_name" => 'Test_file',
 			"{$prefix}_user" => '1',
+			"{$prefix}_user_text" => 'TestUser',
+			"{$prefix}_actor" => '42',
 			"{$prefix}_timestamp" => '12345678910111',
 			"{$prefix}_metadata" => '',
 			"{$prefix}_sha1" => sha1( '' ),
@@ -67,6 +69,7 @@ class LocalRepoTest extends MediaWikiIntegrationTestCase {
 		$this->assertInstanceOf( $expectedClass, $file );
 		$this->assertSame( 'Test_file', $file->getName() );
 		$this->assertSame( 1, $file->getUploader()->getId() );
+		$this->assertSame( 'TestUser', $file->getUploader()->getName() );
 	}
 
 	public static function provideNewFileFromRow() {

tests/phpunit/integration/includes/user/ActorStoreTest.php

@@ -309,6 +309,7 @@ class ActorStoreTest extends ActorStoreTestBase {
 	/**
 	 * @dataProvider provideNewActorFromRowFields
 	 * @covers ::newActorFromRowFields
+	 * @covers ::doCreateActorFromRowFields
 	 */
 	public function testNewActorFromRowFields( $wikiId, $actorId, $name, $userId, UserIdentity $expected ) {
 		$actor = $this->getStore( $wikiId )->newActorFromRowFields( $userId, $name, $actorId );
@@ -341,12 +342,23 @@ class ActorStoreTest extends ActorStoreTestBase {
 	/**
 	 * @dataProvider provideNewActorFromRowFields_exception
 	 * @covers ::newActorFromRowFields
+	 * @covers ::doCreateActorFromRowFields
 	 */
 	public function testNewActorFromRowFields_exception( $actorId, $name, $userId ) {
 		$this->expectException( InvalidArgumentException::class );
 		$this->getStore()->newActorFromRowFields( $userId, $name, $actorId );
 	}
 
+	/**
+	 * @dataProvider provideNewActorFromRowFields_exception
+	 * @covers ::newActorFromRowFields
+	 */
+	public function testNewActorFromRowFields_fallback( $actorId, $name, $userId ) {
+		$store = $this->getStore();
+		$fallback = $store->getUnknownActor();
+		$this->assertSame( $fallback, $store->newActorFromRowFields( $userId, $name, $actorId, $fallback ) );
+	}
+
 	public function provideFindActorId() {
 		yield 'anon, local' => [
 			static function () {