Stop using the Revision class

Change-Id: Ie257c9b1ea94dcadce59f4541d5947465262bd75

includes/AFComputedVariable.php

@@ -177,6 +177,7 @@ class AFComputedVariable {
 				? $result : AFPData::newFromPHPVar( $result );
 		}
 
+		$services = MediaWikiServices::getInstance();
 		switch ( $this->mMethod ) {
 			case 'diff':
 				// Currently unused. Kept for backwards compatibility since it remains
@@ -464,21 +465,24 @@ class AFComputedVariable {
 				$result = $v1 - $v2;
 				break;
 			case 'revision-text-by-id':
-				$rev = Revision::newFromId( $parameters['revid'] );
-				$result = AbuseFilter::revisionToString( $rev, $wgUser );
+				$revRec = $services
+					->getRevisionLookup()
+					->getRevisionById( $parameters['revid'] );
+				$result = AbuseFilter::revisionToString( $revRec, $wgUser );
 				break;
 			case 'revision-text-by-timestamp':
 				$timestamp = $parameters['timestamp'];
 				$title = $this->buildTitle( $parameters['namespace'], $parameters['title'] );
-				$dbr = wfGetDB( DB_REPLICA );
-				$rev = Revision::loadFromTimestamp( $dbr, $title, $timestamp );
-				$result = AbuseFilter::revisionToString( $rev, $wgUser );
+				$revRec = $services
+					->getRevisionStore()
+					->getRevisionByTimestamp( $title, $timestamp );
+				$result = AbuseFilter::revisionToString( $revRec, $wgUser );
 				break;
 			case 'get-wiki-name':
 				$result = WikiMap::getCurrentWikiDbDomain()->getId();
 				break;
 			case 'get-wiki-language':
-				$result = MediaWikiServices::getInstance()->getContentLanguage()->getCode();
+				$result = $services->getContentLanguage()->getCode();
 				break;
 			default:
 				if ( Hooks::run( 'AbuseFilter-computeVariable',
@@ -510,7 +514,7 @@ class AFComputedVariable {
 				$dbr = wfGetDB( DB_REPLICA );
 				$setOpts += Database::getCacheSetOptions( $dbr );
 				// Get the last 100 edit authors with a trivial query (avoid T116557)
-				$revQuery = Revision::getQueryInfo();
+				$revQuery = MediaWikiServices::getInstance()->getRevisionStore()->getQueryInfo();
 				$revAuthors = $dbr->selectFieldValues(
 					$revQuery['tables'],
 					$revQuery['fields']['rev_user_text'],

includes/AbuseFilter.php

@@ -1793,16 +1793,13 @@ class AbuseFilter {
 	 * @internal
 	 * @todo Move elsewhere. VariableGenerator is a good candidate
 	 *
-	 * @param Revision|RevisionRecord|null $revision a valid revision
+	 * @param RevisionRecord|null $revision a valid revision
 	 * @param User $user the user instance to check for privileged access
 	 * @return string the content of the revision as some kind of string,
 	 *        or an empty string if it can not be found
 	 */
-	public static function revisionToString( $revision, User $user ) {
-		if ( $revision instanceof Revision ) {
-			$revision = $revision->getRevisionRecord();
-		}
-		if ( !$revision instanceof RevisionRecord ) {
+	public static function revisionToString( ?RevisionRecord $revision, User $user ) {
+		if ( !$revision ) {
 			return '';
 		}
 
@@ -1967,4 +1964,22 @@ class AbuseFilter {
 		$logger = LoggerFactory::getInstance( 'AbuseFilter' );
 		return new $wgAbuseFilterParserClass( $contLang, $cache, $logger, $vars );
 	}
+
+	/**
+	 * Shortcut for checking whether $user can view the given revision, with mask
+	 *  SUPPRESSED_ALL.
+	 *
+	 * @note This assumes that a revision with the given ID exists
+	 *
+	 * @param RevisionRecord $revRec
+	 * @param User $user
+	 * @return bool
+	 */
+	public static function userCanViewRev( RevisionRecord $revRec, User $user ) : bool {
+		return $revRec->audienceCan(
+			RevisionRecord::SUPPRESSED_ALL,
+			RevisionRecord::FOR_THIS_USER,
+			$user
+		);
+	}
 }

includes/AbuseFilterChangesList.php

@@ -1,6 +1,6 @@
 <?php
 
-use MediaWiki\Storage\RevisionRecord;
+use MediaWiki\Revision\RevisionRecord;
 
 class AbuseFilterChangesList extends OldChangesList {
 

includes/VariableGenerator/RunVariableGenerator.php

@@ -9,6 +9,7 @@ use Content;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\SlotRecord;
 use MWException;
 use MWFileProps;
 use Title;
@@ -69,6 +70,7 @@ class RunVariableGenerator extends VariableGenerator {
 
 	/**
 	 * Get the text of an edit to be used for filtering
+	 * @todo Full support for multi-slots
 	 *
 	 * @param WikiPage $page
 	 * @param Content $content
@@ -76,17 +78,16 @@ class RunVariableGenerator extends VariableGenerator {
 	 * @return array|null
 	 */
 	protected function getEditTextForFiltering( WikiPage $page, Content $content, $slot ) : ?array {
-		$oldRevision = $page->getRevision();
-		if ( !$oldRevision ) {
+		$oldRevRecord = $page->getRevisionRecord();
+		if ( !$oldRevRecord ) {
 			return null;
 		}
 
-		$oldContent = $oldRevision->getContent( RevisionRecord::RAW );
-		$oldAfText = AbuseFilter::revisionToString( $oldRevision, $this->user );
+		$oldContent = $oldRevRecord->getContent( SlotRecord::MAIN, RevisionRecord::RAW );
+		$oldAfText = AbuseFilter::revisionToString( $oldRevRecord, $this->user );
 
 		// XXX: Recreate what the new revision will probably be so we can get the full AF
 		// text for all slots
-		$oldRevRecord = $oldRevision->getRevisionRecord();
 		$newRevision = MutableRevisionRecord::newFromParentRevision( $oldRevRecord );
 		$newRevision->setContent( $slot, $content );
 		$text = AbuseFilter::revisionToString( $newRevision, $this->user );
@@ -263,12 +264,12 @@ class RunVariableGenerator extends VariableGenerator {
 			// This block is adapted from self::getTextForFiltering()
 			if ( $this->title->exists() ) {
 				$page = WikiPage::factory( $this->title );
-				$revision = $page->getRevision();
-				if ( !$revision ) {
+				$revRec = $page->getRevisionRecord();
+				if ( !$revRec ) {
 					return null;
 				}
 
-				$oldcontent = $revision->getContent( RevisionRecord::RAW );
+				$oldcontent = $revRec->getContent( SlotRecord::MAIN, RevisionRecord::RAW );
 				$oldtext = AbuseFilter::contentToString( $oldcontent );
 
 				// Cache article object so we can share a parse operation

includes/Views/AbuseFilterViewExamine.php

@@ -1,7 +1,8 @@
 <?php
 
 use MediaWiki\Extension\AbuseFilter\VariableGenerator\RCVariableGenerator;
-use MediaWiki\Storage\RevisionRecord;
+use MediaWiki\MediaWikiServices;
+use MediaWiki\Revision\RevisionRecord;
 
 class AbuseFilterViewExamine extends AbuseFilterView {
 	/**
@@ -186,8 +187,10 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 		}
 
 		if ( SpecialAbuseLog::isHidden( $row ) === 'implicit' ) {
-			$rev = Revision::newFromId( $row->afl_rev_id );
-			if ( !$rev->userCan( RevisionRecord::SUPPRESSED_ALL, $user ) ) {
+			$revRec = MediaWikiServices::getInstance()
+				->getRevisionLookup()
+				->getRevisionById( (int)$row->afl_rev_id );
+			if ( !AbuseFilter::userCanViewRev( $revRec, $user ) ) {
 				$out->addWikiMsg( 'abusefilter-log-details-hidden-implicit' );
 				return;
 			}

includes/api/ApiQueryAbuseLog.php

@@ -23,7 +23,7 @@
  * http://www.gnu.org/copyleft/gpl.html
  */
 
-use MediaWiki\Storage\RevisionRecord;
+use MediaWiki\MediaWikiServices;
 use Wikimedia\IPUtils;
 
 /**
@@ -170,9 +170,12 @@ class ApiQueryAbuseLog extends ApiQueryBase {
 			$hidden = SpecialAbuseLog::isHidden( $row );
 			if ( $hidden === true && !SpecialAbuseLog::canSeeHidden( $user ) ) {
 				continue;
-			} elseif ( $hidden === 'implicit' ) {
-				$rev = Revision::newFromId( $row->afl_rev_id );
-				if ( !$rev->userCan( RevisionRecord::SUPPRESSED_ALL, $user ) ) {
+			}
+			if ( $hidden === 'implicit' ) {
+				$revRec = MediaWikiServices::getInstance()
+					->getRevisionLookup()
+					->getRevisionById( (int)$row->afl_rev_id );
+				if ( !AbuseFilter::userCanViewRev( $revRec, $user ) ) {
 					continue;
 				}
 			}

includes/special/SpecialAbuseLog.php

@@ -2,7 +2,6 @@
 
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
-use MediaWiki\Storage\RevisionRecord;
 use MediaWiki\User\UserIdentity;
 
 class SpecialAbuseLog extends AbuseFilterSpecialPage {
@@ -631,9 +630,11 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
 			} elseif ( self::isHidden( $row ) === true && !self::canSeeHidden( $user ) ) {
 				$error = 'abusefilter-log-details-hidden';
 			} elseif ( self::isHidden( $row ) === 'implicit' ) {
-				$rev = Revision::newFromId( $row->afl_rev_id );
-				// The log is visible, but refers to a deleted revision
-				if ( !$rev->userCan( RevisionRecord::SUPPRESSED_ALL, $user ) ) {
+				$revRec = MediaWikiServices::getInstance()
+					->getRevisionLookup()
+					->getRevisionById( (int)$row->afl_rev_id );
+				if ( !AbuseFilter::userCanViewRev( $revRec, $user ) ) {
+					// The log is visible, but refers to a deleted revision
 					$error = 'abusefilter-log-details-hidden-implicit';
 				}
 			}
@@ -1226,7 +1227,9 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
 			return true;
 		}
 		if ( $row->afl_rev_id ) {
-			$revision = Revision::newFromId( $row->afl_rev_id );
+			$revision = MediaWikiServices::getInstance()
+				->getRevisionLookup()
+				->getRevisionById( $row->afl_rev_id );
 			if ( $revision && $revision->getVisibility() !== 0 ) {
 				return 'implicit';
 			}

tests/phpunit/AbuseFilterDBTest.php

@@ -358,14 +358,14 @@ class AbuseFilterDBTest extends MediaWikiTestCase {
 	}
 
 	/**
-	 * @param RevisionRecord $rev The revision being converted
+	 * @param RevisionRecord|null $rev The revision being converted
 	 * @param bool $sysop Whether the user should be a sysop (i.e. able to see deleted stuff)
 	 * @param string $expected The expected textual representation of the Revision
 	 * @covers AbuseFilter::revisionToString
 	 * @dataProvider provideRevisionToString
 	 * @todo This should be a unit test...
 	 */
-	public function testRevisionToString( $rev, $sysop, $expected ) {
+	public function testRevisionToString( ?RevisionRecord $rev, bool $sysop, string $expected ) {
 		/** @var MockObject|User $user */
 		$user = $this->getMockBuilder( User::class )
 			->setMethods( [ 'getEffectiveGroups' ] )
@@ -397,11 +397,6 @@ class AbuseFilterDBTest extends MediaWikiTestCase {
 		$title = Title::newFromText( __METHOD__ );
 		$revRec = new MutableRevisionRecord( $title );
 		$revRec->setContent( SlotRecord::MAIN, new TextContent( 'Main slot text.' ) );
-		yield 'Revision instance' => [
-			new Revision( $revRec ),
-			false,
-			'Main slot text.'
-		];
 
 		yield 'RevisionRecord instance' => [
 			$revRec,
@@ -522,4 +517,39 @@ class AbuseFilterDBTest extends MediaWikiTestCase {
 			$vars->getVar( 'wiki_language' )->toNative()
 		);
 	}
+
+	/**
+	 * @param RevisionRecord $revRec
+	 * @param bool $privileged
+	 * @param bool $expected
+	 * @dataProvider provideUserCanViewRev
+	 * @covers AbuseFilter::userCanViewRev
+	 */
+	public function testUserCanViewRev( RevisionRecord $revRec, bool $privileged, bool $expected ) {
+		$user = $privileged
+			? $this->getTestUser( 'suppress' )->getUser()
+			: $this->getTestUser()->getUser();
+		$this->assertSame( $expected, AbuseFilter::userCanViewRev( $revRec, $user ) );
+	}
+
+	/**
+	 * @return Generator|array
+	 */
+	public function provideUserCanViewRev() {
+		$title = Title::newFromText( __METHOD__ );
+
+		$visible = new MutableRevisionRecord( $title );
+		yield 'Visible, not privileged' => [ $visible, false, true ];
+		yield 'Visible, privileged' => [ $visible, true, true ];
+
+		$userSup = new MutableRevisionRecord( $title );
+		$userSup->setVisibility( RevisionRecord::SUPPRESSED_USER );
+		yield 'User suppressed, not privileged' => [ $userSup, false, false ];
+		yield 'User suppressed, privileged' => [ $userSup, true, true ];
+
+		$allSupp = new MutableRevisionRecord( $title );
+		$allSupp->setVisibility( RevisionRecord::SUPPRESSED_ALL );
+		yield 'All suppressed, not privileged' => [ $allSupp, false, false ];
+		yield 'All suppressed, privileged' => [ $allSupp, true, true ];
+	}
 }