Change RevisionStoreCacheRecord callback signature

In order to convert the tests into tru unit tests.

Bug: T272669
Change-Id: I6055b43381fdcc6dcdf69e9311979db0dccdb16b

includes/Revision/RevisionStore.php

@@ -1574,19 +1574,30 @@ class RevisionStore
 					if ( !$row && !( $queryFlags & self::READ_LATEST ) ) {
 						// If we found no slots, try looking on the master database (T259738)
 						$this->logger->info(
-							 'RevisionStoreCacheRecord refresh callback  falling back to READ_LATEST.',
+							 'RevisionStoreCacheRecord refresh callback falling back to READ_LATEST.',
 							[
 								'revid' => $revId,
 								'trace' => wfBacktrace( true )
 							]
 						);
 						$dbw = $this->getDBConnectionRefForQueryFlags( self::READ_LATEST );
-						return $this->fetchRevisionRowFromConds(
+						$row = $this->fetchRevisionRowFromConds(
 							$dbw,
 							[ 'rev_id' => intval( $revId ) ]
 						);
 					}
-					return $row;
+					if ( !$row ) {
+						return [ null, null ];
+					}
+					return [
+						$row->rev_deleted,
+						User::newFromAnyId(
+							$row->rev_user ?? null,
+							$row->rev_user_text ?? null,
+							$row->rev_actor ?? null,
+							$this->dbDomain
+						)
+					];
 				},
 				$title, $user, $comment, $row, $slots, $this->dbDomain
 			);

includes/Revision/RevisionStoreCacheRecord.php

@@ -23,23 +23,21 @@
 namespace MediaWiki\Revision;
 
 use CommentStoreComment;
-use InvalidArgumentException;
 use MediaWiki\Page\PageIdentity;
 use MediaWiki\Permissions\Authority;
 use MediaWiki\User\UserIdentity;
-use MediaWiki\User\UserIdentityValue;
-use User;
 
 /**
  * A cached RevisionStoreRecord.  Ensures that changes performed "behind the back"
  * of the cache do not cause the revision record to deliver stale data.
  *
+ * @internal
  * @since 1.33
  */
 class RevisionStoreCacheRecord extends RevisionStoreRecord {
 
 	/**
-	 * @var callable
+	 * @var callable ( int $revId ): [ int $rev_deleted, UserIdentity $user ]
 	 */
 	private $mCallback;
 
@@ -47,7 +45,8 @@ class RevisionStoreCacheRecord extends RevisionStoreRecord {
 	 * @note Avoid calling this constructor directly. Use the appropriate methods
 	 * in RevisionStore instead.
 	 *
-	 * @param callable $callback Callback for loading data.  Signature: function ( $id ): \stdClass
+	 * @param callable $callback Callback for loading data.
+	 *        Signature: function ( int $revId ): [ int $rev_deleted, UserIdentity $user ]
 	 * @param PageIdentity $page The page this Revision is associated with.
 	 * @param UserIdentity $user
 	 * @param CommentStoreComment $comment
@@ -57,7 +56,7 @@ class RevisionStoreCacheRecord extends RevisionStoreRecord {
 	 * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one.
 	 */
 	public function __construct(
-		$callback,
+		callable $callback,
 		PageIdentity $page,
 		UserIdentity $user,
 		CommentStoreComment $comment,
@@ -102,31 +101,15 @@ class RevisionStoreCacheRecord extends RevisionStoreRecord {
 	 * @throws RevisionAccessException if the row could not be loaded
 	 */
 	private function loadFreshRow() {
-		$freshRow = call_user_func( $this->mCallback, $this->mId );
+		list( $freshRevDeleted, $freshUser ) = call_user_func( $this->mCallback, $this->mId );
 
 		// Set to null to ensure we do not make unnecessary queries for subsequent getter calls,
 		// and to allow the closure to be freed.
 		$this->mCallback = null;
 
-		if ( $freshRow ) {
+		if ( $freshRevDeleted !== null && $freshUser !== null ) {
-			$this->mDeleted = intval( $freshRow->rev_deleted );
+			$this->mDeleted = intval( $freshRevDeleted );
-
+			$this->mUser = $freshUser;
-			try {
-				$this->mUser = User::newFromAnyId(
-					$freshRow->rev_user ?? null,
-					$freshRow->rev_user_text ?? null,
-					$freshRow->rev_actor ?? null
-				);
-			} catch ( InvalidArgumentException $ex ) {
-				wfWarn(
-					__METHOD__
-					. ': '
-					. $this->mPage
-					. ': '
-					. $ex->getMessage()
-				);
-				$this->mUser = new UserIdentityValue( 0, 'Unknown user', 0 );
-			}
 		} else {
 			throw new RevisionAccessException(
 				'Unable to load fresh row for rev_id: ' . $this->mId

tests/common/TestsAutoLoader.php

@@ -177,7 +177,6 @@ $wgAutoloadClasses += [
 	# tests/phpunit/includes/Revision
 	'MediaWiki\Tests\Revision\RevisionSlotsTest' => "$testDir/phpunit/includes/Revision/RevisionSlotsTest.php",
 	'MediaWiki\Tests\Revision\RevisionStoreDbTestBase' => "$testDir/phpunit/includes/Revision/RevisionStoreDbTestBase.php",
-	'MediaWiki\Tests\Revision\RevisionStoreRecordTest' => "$testDir/phpunit/includes/Revision/RevisionStoreRecordTest.php",
 
 	# test/phpunit/includes/user
 	'UserOptionsLookupTest' => "$testDir/phpunit/includes/user/UserOptionsLookupTest.php",

tests/phpunit/unit/includes/Revision/RevisionStoreCacheRecordTest.php

@@ -1,17 +1,17 @@
 <?php
 
-namespace MediaWiki\Tests\Revision;
+namespace MediaWiki\Tests\Unit\Revision;
 
 use CommentStoreComment;
+use MediaWiki\Page\PageIdentity;
+use MediaWiki\Page\PageIdentityValue;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Revision\RevisionSlots;
 use MediaWiki\Revision\RevisionStoreCacheRecord;
 use MediaWiki\Revision\RevisionStoreRecord;
 use MediaWiki\Revision\SlotRecord;
-use MediaWiki\Tests\Unit\Revision\RevisionRecordTests;
 use MediaWiki\User\UserIdentityValue;
 use TextContent;
-use Title;
 
 /**
  * @covers \MediaWiki\Revision\RevisionStoreCacheRecord
@@ -29,8 +29,7 @@ class RevisionStoreCacheRecordTest extends RevisionStoreRecordTest {
 	 * @return RevisionStoreRecord
 	 */
 	protected function newRevision( array $rowOverrides = [], $callback = false ) {
-		$title = Title::newFromText( 'Dummy' );
+		$title = new PageIdentityValue( 17, NS_MAIN, 'Dummy', PageIdentity::LOCAL );
-		$title->resetArticleID( 17 );
 
 		$user = new UserIdentityValue( 11, 'Tester', 0 );
 		$comment = CommentStoreComment::newUnsavedComment( 'Hello World' );
@@ -41,7 +40,7 @@ class RevisionStoreCacheRecordTest extends RevisionStoreRecordTest {
 
 		$row = [
 			'rev_id' => '7',
-			'rev_page' => strval( $title->getArticleID() ),
+			'rev_page' => '17',
 			'rev_timestamp' => '20200101000000',
 			'rev_deleted' => 0,
 			'rev_minor_edit' => 0,
@@ -56,7 +55,11 @@ class RevisionStoreCacheRecordTest extends RevisionStoreRecordTest {
 
 		if ( !$callback ) {
 			$callback = function ( $revId ) use ( $row ) {
-				return (object)$row;
+				$this->assertSame( 7, $revId );
+				return [
+					$row['rev_deleted'],
+					new UserIdentityValue( intval( $row['rev_user'] ), 'Bla', 10 )
+				];
 			};
 		}
 
@@ -74,14 +77,14 @@ class RevisionStoreCacheRecordTest extends RevisionStoreRecordTest {
 		// Provide a callback that returns non-default values. Asserting the revision returns
 		// these values confirms callback execution and behavior. Also confirm the callback
 		// is only invoked once, even for multiple getter calls.
-		$rowOverrides = [
-			'rev_deleted' => RevisionRecord::DELETED_TEXT,
-			'rev_user' => 12,
-		];
 		$callbackInvoked = 0;
-		$callback = function ( $revId ) use ( &$callbackInvoked, $rowOverrides ) {
+		$callback = function ( $revId ) use ( &$callbackInvoked ) {
+			$this->assertSame( 7, $revId );
 			$callbackInvoked++;
-			return (object)$rowOverrides;
+			return [
+				RevisionRecord::DELETED_TEXT,
+				new UserIdentityValue( 12, 'Lalala', 24 )
+			];
 		};
 		$rev = $this->newRevision( [], $callback );