Merge "Replace core uses and hard deprecate Parser(Options) Revision methods"

8 files changed, 76 insertions, 52 deletions

diff --git a/RELEASE-NOTES-1.35 b/RELEASE-NOTES-1.35
index 6af81316c69f..0eaa7c5a011f 100644
--- a/RELEASE-NOTES-1.35
+++ b/RELEASE-NOTES-1.35
@@ -1105,13 +1105,12 @@ because of Phabricator reports.
   Previously, the third parameter was unused. Using the old signature is
   hard deprecated.
 * Passing Article to ParserCache::get() was deprecated
-* Parser::fetchCurrentRevisionOfTitle and ::statelessFetchRevision were soft
-  deprecated in favor of the new ::fetchCurrentRevisionRecordOfTitle and
-  ::statelessFetchRevisionRecord methods respectively.
-* Parser::getRevisionObject was hard deprecated in favor of the new
-  ::getRevisionRecordObject method.
+* Parser::fetchCurrentRevisionOfTitle, ::statelessFetchRevision, and
+  ::getRevisionObject were hard deprecated in favor of the new
+  ::fetchCurrentRevisionRecordOfTitle, ::statelessFetchRevisionRecord,
+  and ::getRevisionRecordObject methods respectively.
 * ParserOptions::getCurrentRevisionCallback and ::setCurrentRevisionCallback
-  were soft deprecated in favor of the new ::getCurrentRevisionRecordCallback
+  were hard deprecated in favor of the new ::getCurrentRevisionRecordCallback
   and ::setCurrentRevisionRecordCallback methods respectively.
 * Page interface was deprecated. Use Article or WikiPage instead.
 * The following DatabaseBlock methods are deprecated because they are no longer
diff --git a/includes/Revision/RenderedRevision.php b/includes/Revision/RenderedRevision.php
index 46685914c755..e5573c86d453 100644
--- a/includes/Revision/RenderedRevision.php
+++ b/includes/Revision/RenderedRevision.php
@@ -29,7 +29,6 @@ use ParserOptions;
 use ParserOutput;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
-use Revision;
 use Title;
 use User;
 use Wikimedia\Assert\Assert;
@@ -381,12 +380,11 @@ class RenderedRevision implements SlotRenderingProvider {
 
 		if ( $this->revision->isReadyForInsertion() || !$this->revision->getId() ) {
 			$title = $this->title;
-			$oldCallback = $this->options->getCurrentRevisionCallback();
-			$this->options->setCurrentRevisionCallback(
-				function ( Title $parserTitle, $parser = false ) use ( $title, $oldCallback ) {
+			$oldCallback = $this->options->getCurrentRevisionRecordCallback();
+			$this->options->setCurrentRevisionRecordCallback(
+				function ( Title $parserTitle, $parser = null ) use ( $title, $oldCallback ) {
 					if ( $title->equals( $parserTitle ) ) {
-						$legacyRevision = new Revision( $this->revision );
-						return $legacyRevision;
+						return $this->revision;
 					} else {
 						return call_user_func( $oldCallback, $parserTitle, $parser );
 					}
@@ -465,8 +463,8 @@ class RenderedRevision implements SlotRenderingProvider {
 		// targeting the user making the null-edit, not the user who made the original edit,
 		// causing {{REVISIONUSER}} to return the wrong name.
 		// This case is now expected to be handled by the code in RevisionRenderer that
-		// constructs the ParserOptions: For a null-edit, setCurrentRevisionCallback is called
-		// with the old, existing revision.
+		// constructs the ParserOptions: For a null-edit, setCurrentRevisionRecordCallback is
+		// called with the old, existing revision.
 		$logger->debug( __METHOD__ . ": reusing prepared output for '{title}'", $context );
 		return false;
 	}
diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php
index ad823234db97..3156278762a4 100644
--- a/includes/Storage/DerivedPageDataUpdater.php
+++ b/includes/Storage/DerivedPageDataUpdater.php
@@ -806,12 +806,11 @@ class DerivedPageDataUpdater implements IDBAccessObject, LoggerAwareInterface {
 		$this->revision->setUser( $user );
 
 		// Set up ParserOptions to operate on the new revision
-		$oldCallback = $userPopts->getCurrentRevisionCallback();
-		$userPopts->setCurrentRevisionCallback(
-			function ( Title $parserTitle, $parser = false ) use ( $title, $oldCallback ) {
+		$oldCallback = $userPopts->getCurrentRevisionRecordCallback();
+		$userPopts->setCurrentRevisionRecordCallback(
+			function ( Title $parserTitle, $parser = null ) use ( $title, $oldCallback ) {
 				if ( $parserTitle->equals( $title ) ) {
-					$legacyRevision = new Revision( $this->revision );
-					return $legacyRevision;
+					return $this->revision;
 				} else {
 					return call_user_func( $oldCallback, $parserTitle, $parser );
 				}
diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index db351651a94a..29edc7a83a1e 100644
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -32,6 +32,7 @@ use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\RevisionAccessException;
 use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\SlotRecord;
 use MediaWiki\SpecialPage\SpecialPageFactory;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
@@ -3422,6 +3423,7 @@ class Parser {
 	 * @return Revision|false
 	 */
 	public function fetchCurrentRevisionOfTitle( Title $title ) {
+		wfDeprecated( __METHOD__, '1.35' );
 		$revisionRecord = $this->fetchCurrentRevisionRecordOfTitle( $title );
 		if ( $revisionRecord ) {
 			return new Revision( $revisionRecord );
@@ -3484,6 +3486,7 @@ class Parser {
 	 * @return Revision|bool False if missing
 	 */
 	public static function statelessFetchRevision( Title $title, $parser = false ) {
+		wfDeprecated( __METHOD__, '1.35' );
 		$revRecord = MediaWikiServices::getInstance()
 			->getRevisionLookup()
 			->getKnownCurrentRevision( $title );
@@ -3564,7 +3567,7 @@ class Parser {
 		$text = $skip = false;
 		$finalTitle = $title;
 		$deps = [];
-		$rev = null;
+		$revRecord = null;
 
 		# Loop to fetch the article, with up to 1 redirect
 		$revLookup = MediaWikiServices::getInstance()->getRevisionLookup();
@@ -3587,16 +3590,14 @@ class Parser {
 			# TODO rewrite using only RevisionRecord objects
 			if ( $id ) {
 				$revRecord = $revLookup->getRevisionById( $id );
-				$rev = $revRecord ? new Revision( $revRecord ) : null;
 			} elseif ( $parser ) {
-				$rev = $parser->fetchCurrentRevisionOfTitle( $title );
+				$revRecord = $parser->fetchCurrentRevisionRecordOfTitle( $title );
 			} else {
 				$revRecord = $revLookup->getRevisionByTitle( $title );
-				$rev = $revRecord ? new Revision( $revRecord ) : null;
 			}
-			$rev_id = $rev ? $rev->getId() : 0;
+			$rev_id = $revRecord ? $revRecord->getId() : 0;
 			# If there is no current revision, there is no page
-			if ( $id === false && !$rev ) {
+			if ( $id === false && !$revRecord ) {
 				$linkCache = MediaWikiServices::getInstance()->getLinkCache();
 				$linkCache->addBadLinkObj( $title );
 			}
@@ -3606,20 +3607,33 @@ class Parser {
 				'page_id' => $title->getArticleID(),
 				'rev_id' => $rev_id
 			];
-			if ( $rev && !$title->equals( $rev->getTitle() ) ) {
-				# We fetched a rev from a different title; register it too...
-				$deps[] = [
-					'title' => $rev->getTitle(),
-					'page_id' => $rev->getPage(),
-					'rev_id' => $rev_id
-				];
+			if ( $revRecord ) {
+				$revTitle = Title::newFromLinkTarget(
+					$revRecord->getPageAsLinkTarget()
+				);
+				if ( !$title->equals( $revTitle ) ) {
+					# We fetched a rev from a different title; register it too...
+					$deps[] = [
+						'title' => $revTitle,
+						'page_id' => $revRecord->getPageId(),
+						'rev_id' => $rev_id
+					];
+				}
 			}
 
-			if ( $rev ) {
-				$content = $rev->getContent();
+			if ( $revRecord ) {
+				$content = $revRecord->getContent( SlotRecord::MAIN );
 				$text = $content ? $content->getWikitextForTransclusion() : null;
 
-				Hooks::runner()->onParserFetchTemplate( $parser, $title, $rev, $text, $deps );
+				// Hook is deprecated already
+				$legacyRevision = new Revision( $revRecord );
+				Hooks::runner()->onParserFetchTemplate(
+					$parser,
+					$title,
+					$legacyRevision,
+					$text,
+					$deps
+				);
 
 				if ( $text === false || $text === null ) {
 					$text = false;
@@ -3644,8 +3658,11 @@ class Parser {
 			$finalTitle = $title;
 			$title = $content->getRedirectTarget();
 		}
+
+		// TODO return RevisionRecord instead
+		$legacyRevision = $revRecord ? new Revision( $revRecord ) : null;
 		return [
-			'revision' => $rev,
+			'revision' => $legacyRevision,
 			'text' => $text,
 			'finalTitle' => $finalTitle,
 			'deps' => $deps
diff --git a/includes/parser/ParserOptions.php b/includes/parser/ParserOptions.php
index ccd1251c0a75..aca590e571ac 100644
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@ -791,6 +791,7 @@ class ParserOptions {
 	 * @return callable
 	 */
 	public function getCurrentRevisionCallback() {
+		wfDeprecated( __METHOD__, '1.35' );
 		$revCb = $this->getOption( 'currentRevisionCallback' );
 
 		// As a temporary measure, while both currentRevisionCallback and
@@ -857,6 +858,7 @@ class ParserOptions {
 	 * @return callable Old value
 	 */
 	public function setCurrentRevisionCallback( $x ) {
+		wfDeprecated( __METHOD__, '1.35' );
 		return $this->setOptionLegacy( 'currentRevisionCallback', $x );
 	}
 
@@ -1469,12 +1471,13 @@ class ParserOptions {
 	 * @return ScopedCallback to unset the hook
 	 */
 	public function setupFakeRevision( $title, $content, $user ) {
-		$oldCallback = $this->setCurrentRevisionCallback(
+		$oldCallback = $this->setCurrentRevisionRecordCallback(
 			function (
-				$titleToCheck, $parser = false ) use ( $title, $content, $user, &$oldCallback
+				$titleToCheck, $parser = null ) use ( $title, $content, $user, &$oldCallback
 			) {
 				if ( $titleToCheck->equals( $title ) ) {
-					return new Revision( [
+					// TODO construct a RevisionRecord
+					$rev = new Revision( [
 						'page' => $title->getArticleID(),
 						'user_text' => $user->getName(),
 						'user' => $user->getId(),
@@ -1482,6 +1485,7 @@ class ParserOptions {
 						'title' => $title,
 						'content' => $content
 					] );
+					return $rev->getRevisionRecord();
 				} else {
 					return call_user_func( $oldCallback, $titleToCheck, $parser );
 				}
diff --git a/tests/parser/ParserTestRunner.php b/tests/parser/ParserTestRunner.php
index 67faacc3359a..2d6536cb5eb6 100644
--- a/tests/parser/ParserTestRunner.php
+++ b/tests/parser/ParserTestRunner.php
@@ -842,6 +842,8 @@ class ParserTestRunner {
 				'page_title' => $title->getDBkey(),
 				'page_is_redirect' => 0
 			] );
+
+			// TODO construct a RevisionRecord here instead
 			$rev = new Revision(
 				[
 					'id' => $title->getLatestRevID(),
@@ -854,11 +856,13 @@ class ParserTestRunner {
 				Revision::READ_LATEST,
 				$title
 			);
-			$oldCallback = $options->getCurrentRevisionCallback();
-			$options->setCurrentRevisionCallback(
-				function ( Title $t, $parser ) use ( $title, $rev, $oldCallback ) {
+			$revRecord = $rev->getRevisionRecord();
+
+			$oldCallback = $options->getCurrentRevisionRecordCallback();
+			$options->setCurrentRevisionRecordCallback(
+				function ( Title $t, $parser = null ) use ( $title, $revRecord, $oldCallback ) {
 					if ( $t->equals( $title ) ) {
-						return $rev;
+						return $revRecord;
 					} else {
 						return call_user_func( $oldCallback, $t, $parser );
 					}
diff --git a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
index c4c02e99bf6c..e2cad2c02ba0 100644
--- a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
+++ b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
@@ -153,7 +153,10 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase {
 
 		$options2 = $updater->getCanonicalParserOptions();
 
-		$currentRev = call_user_func( $options2->getCurrentRevisionCallback(), $page->getTitle() );
+		$currentRev = call_user_func(
+			$options2->getCurrentRevisionRecordCallback(),
+			$page->getTitle()
+		);
 		$this->assertSame( $rev->getId(), $currentRev->getId() );
 	}
 
diff --git a/tests/phpunit/includes/parser/ParserMethodsTest.php b/tests/phpunit/includes/parser/ParserMethodsTest.php
index 7f8e4a66fcba..7e8e792205f1 100644
--- a/tests/phpunit/includes/parser/ParserMethodsTest.php
+++ b/tests/phpunit/includes/parser/ParserMethodsTest.php
@@ -249,8 +249,8 @@ class ParserMethodsTest extends MediaWikiLangTestCase {
 		$oldRevision->setContent( SlotRecord::MAIN, new WikitextContent( 'FAUX' ) );
 
 		$po = new ParserOptions( $frank );
-		$po->setCurrentRevisionCallback( function () use ( $oldRevision ) {
-			return new Revision( $oldRevision );
+		$po->setCurrentRevisionRecordCallback( function () use ( $oldRevision ) {
+			return $oldRevision;
 		} );
 
 		yield 'old with override' => [ $text, $po, 100, 'user:FauxAuthor;id:100;time:20141111111111;' ];
@@ -278,8 +278,8 @@ class ParserMethodsTest extends MediaWikiLangTestCase {
 
 		$po = new ParserOptions( $frank );
 		$po->setIsPreview( true );
-		$po->setCurrentRevisionCallback( function () use ( $newRevision ) {
-			return new Revision( $newRevision );
+		$po->setCurrentRevisionRecordCallback( function () use ( $newRevision ) {
+			return $newRevision;
 		} );
 
 		yield 'preview' => [
@@ -291,8 +291,8 @@ class ParserMethodsTest extends MediaWikiLangTestCase {
 		];
 
 		$po = new ParserOptions( $frank );
-		$po->setCurrentRevisionCallback( function () use ( $newRevision ) {
-			return new Revision( $newRevision );
+		$po->setCurrentRevisionRecordCallback( function () use ( $newRevision ) {
+			return $newRevision;
 		} );
 
 		yield 'pre-save' => [
@@ -313,8 +313,8 @@ class ParserMethodsTest extends MediaWikiLangTestCase {
 
 		$po = new ParserOptions( $frank );
 		$po->setIsPreview( true );
-		$po->setCurrentRevisionCallback( function () use ( $newRevision ) {
-			return new Revision( $newRevision );
+		$po->setCurrentRevisionRecordCallback( function () use ( $newRevision ) {
+			return $newRevision;
 		} );
 
 		yield 'preview with self-transclude' => [ $text, $po, null, '(ONE)#(ONE)(TWO)#' ];