Provide new, unsaved revision to PST to fix magic words.

This injects the new, unsaved RevisionRecord object into the Parser used for Pre-Save Transform, and sets the user and timestamp on that revision, to allow {{subst:REVISIONUSER}} and {{subst:REVISIONTIMESTAMP}} to function. Bug: T203583 Change-Id: I31a97d0168ac22346b2dad6b88bf7f6f8a0dd9d0
2018-09-05 20:03:15 +02:00 · 2018-09-05 20:03:15 +02:00 · 465954aa23
commit 465954aa23
parent d8e5553237
7 changed files with 263 additions and 43 deletions
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@ -3997,6 +3997,12 @@ ERROR;
 		$parserOptions->setIsPreview( true );
 		$parserOptions->setIsSectionPreview( !is_null( $this->section ) && $this->section !== '' );
 		$parserOptions->enableLimitReport();
+
+		// XXX: we could call $parserOptions->setCurrentRevisionCallback here to force the
+		// current revision to be null during PST, until setupFakeRevision is called on
+		// the ParserOptions. Currently, we rely on Parser::getRevisionObject() to ignore
+		// existing revisions in preview mode.
+
 		return $parserOptions;
 	}

@ -4012,9 +4018,14 @@ ERROR;
 	protected function doPreviewParse( Content $content ) {
 		$user = $this->context->getUser();
 		$parserOptions = $this->getPreviewParserOptions();
+
+		// NOTE: preSaveTransform doesn't have a fake revision to operate on.
+		// Parser::getRevisionObject() will return null in preview mode,
+		// causing the context user to be used for {{subst:REVISIONUSER}}.
+		// XXX: Alternatively, we could also call setupFakeRevision() a second time:
+		// once before PST with $content, and then after PST with $pstContent.
 		$pstContent = $content->preSaveTransform( $this->mTitle, $user, $parserOptions );
-		$scopedCallback = $parserOptions->setupFakeRevision(
-			$this->mTitle, $pstContent, $user );
+		$scopedCallback = $parserOptions->setupFakeRevision( $this->mTitle, $pstContent, $user );
 		$parserOutput = $pstContent->getParserOutput( $this->mTitle, null, $parserOptions );
 		ScopedCallback::consume( $scopedCallback );
 		return [
--- a/includes/Storage/DerivedPageDataUpdater.php
+++ b/includes/Storage/DerivedPageDataUpdater.php
@ -557,14 +557,6 @@ class DerivedPageDataUpdater implements IDBAccessObject {
 		return $this->wikiPage->getId();
 	}

-	/**
-	 * @return string
-	 */
-	private function getTimestampNow() {
-		// TODO: allow an override to be injected for testing
-		return wfTimestampNow();
-	}
-
 	/**
 	 * Whether the content is deleted and thus not visible to the public.
 	 *
@ -777,6 +769,24 @@ class DerivedPageDataUpdater implements IDBAccessObject {
 			$this->revision = new MutableRevisionRecord( $title );
 		}

+		// NOTE: user and timestamp must be set, so they can be used for
+		// {{subst:REVISIONUSER}} and {{subst:REVISIONTIMESTAMP}} in PST!
+		$this->revision->setTimestamp( wfTimestampNow() );
+		$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 ) {
+				if ( $parserTitle->equals( $title ) ) {
+					$legacyRevision = new Revision( $this->revision );
+					return $legacyRevision;
+				} else {
+					return call_user_func( $oldCallback, $parserTitle, $parser );
+				}
+			}
+		);
+
 		$pstContentSlots = $this->revision->getSlots();

 		foreach ( $slotsUpdate->getModifiedRoles() as $role ) {
@ -824,8 +834,6 @@ class DerivedPageDataUpdater implements IDBAccessObject {

 			// prepareUpdate() is redundant for null-edits
 			$this->doTransition( 'has-revision' );
-		} else {
-			$this->revision->setUser( $user );
 		}
 	}

--- a/includes/Storage/PageUpdater.php
+++ b/includes/Storage/PageUpdater.php
@ -840,7 +840,6 @@ class PageUpdater {
 	 *
 	 * @param CommentStoreComment $comment
 	 * @param User $user
-	 * @param string $timestamp
 	 * @param int $flags
 	 * @param Status $status
 	 *
@ -849,7 +848,6 @@ class PageUpdater {
 	private function makeNewRevision(
 		CommentStoreComment $comment,
 		User $user,
-		$timestamp,
 		$flags,
 		Status $status
 	) {
@ -873,7 +871,6 @@ class PageUpdater {

 		$rev->setComment( $comment );
 		$rev->setUser( $user );
-		$rev->setTimestamp( $timestamp );
 		$rev->setMinorEdit( ( $flags & EDIT_MINOR ) > 0 );

 		foreach ( $rev->getSlots()->getSlots() as $slot ) {
@ -904,9 +901,6 @@ class PageUpdater {
 		// Update article, but only if changed.
 		$status = Status::newGood( [ 'new' => false, 'revision' => null, 'revision-record' => null ] );

-		// Convenience variables
-		$now = $this->getTimestampNow();
-
 		$oldRev = $this->grabParentRevision();
 		$oldid = $oldRev ? $oldRev->getId() : 0;

@ -920,7 +914,6 @@ class PageUpdater {
 		$newRevisionRecord = $this->makeNewRevision(
 			$summary,
 			$user,
-			$now,
 			$flags,
 			$status
 		);
@ -929,6 +922,8 @@ class PageUpdater {
 			return $status;
 		}

+		$now = $newRevisionRecord->getTimestamp();
+
 		// XXX: we may want a flag that allows a null revision to be forced!
 		$changed = $this->derivedDataUpdater->isChange();

@ -1060,12 +1055,9 @@ class PageUpdater {

 		$status = Status::newGood( [ 'new' => true, 'revision' => null, 'revision-record' => null ] );

-		$now = $this->getTimestampNow();
-
 		$newRevisionRecord = $this->makeNewRevision(
 			$summary,
 			$user,
-			$now,
 			$flags,
 			$status
 		);
@ -1074,6 +1066,8 @@ class PageUpdater {
 			return $status;
 		}

+		$now = $newRevisionRecord->getTimestamp();
+
 		$dbw = $this->getDBConnectionRef( DB_MASTER );
 		$dbw->startAtomic( __METHOD__ );

--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@ -5770,13 +5770,27 @@ class Parser {

 		// NOTE: try to get the RevisionObject even if mRevisionId is null.
 		// This is useful when parsing revision that has not yet been saved.
+		// However, if we get back a saved revision even though we are in
+		// preview mode, we'll have to ignore it, see below.
+		// NOTE: This callback may be used to inject an OLD revision that was
+		// already loaded, so "current" is a bit of a misnomer. We can't just
+		// skip it if mRevisionId is set.
 		$rev = call_user_func(
 			$this->mOptions->getCurrentRevisionCallback(), $this->getTitle(), $this
 		);

-		# If the parse is for a new revision, then the callback should have
-		# already been set to force the object and should match mRevisionId.
-		# If not, try to fetch by mRevisionId for sanity.
+		if ( $this->mRevisionId === null && $rev && $rev->getId() ) {
+			// We are in preview mode (mRevisionId is null), and the current revision callback
+			// returned an existing revision. Ignore it and return null, it's probably the page's
+			// current revision, which is not what we want here. Note that we do want to call the
+			// callback to allow the unsaved revision to be injected here, e.g. for
+			// self-transclusion previews.
+			return null;
+		}
+
+		// If the parse is for a new revision, then the callback should have
+		// already been set to force the object and should match mRevisionId.
+		// If not, try to fetch by mRevisionId for sanity.
 		if ( $this->mRevisionId && $rev && $rev->getId() != $this->mRevisionId ) {
 			$rev = Revision::newFromId( $this->mRevisionId );
 		}
--- a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
+++ b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
@ -246,11 +246,13 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase {
 		$sysop = $this->getTestUser( [ 'sysop' ] )->getUser();
 		$page = $this->getPage( __METHOD__ );

-		$mainContent1 = new WikitextContent( 'first [[main]] ({{REVISIONUSER}}) ~~~' );
-		$mainContent2 = new WikitextContent( 'second' );
+		$mainContent1 = new WikitextContent( 'first [[main]] ({{REVISIONUSER}}) #~~~#' );
+		$mainContent2 = new WikitextContent( 'second ({{subst:REVISIONUSER}}) #~~~#' );

 		$rev = $this->createRevision( $page, 'first', $mainContent1 );
 		$mainContent1 = $rev->getContent( 'main' ); // get post-pst content
+		$userName = $rev->getUser()->getName();
+		$sysopName = $sysop->getName();

 		$update = new RevisionSlotsUpdate();
 		$update->modifyContent( 'main', $mainContent1 );
@ -268,9 +270,11 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase {

 		// parser-output for null-edit uses the original author's name
 		$html = $updater1->getRenderedRevision()->getRevisionParserOutput()->getText();
-		$this->assertNotContains( $sysop->getName(), $html, '{{REVISIONUSER}}' );
+		$this->assertNotContains( $sysopName, $html, '{{REVISIONUSER}}' );
 		$this->assertNotContains( '{{REVISIONUSER}}', $html, '{{REVISIONUSER}}' );
-		$this->assertContains( '(' . $rev->getUser()->getName() . ')', $html, '{{REVISIONUSER}}' );
+		$this->assertNotContains( '~~~', $html, 'signature ~~~' );
+		$this->assertContains( '(' . $userName . ')', $html, '{{REVISIONUSER}}' );
+		$this->assertContains( '>' . $userName . '<', $html, 'signature ~~~' );

 		// TODO: MCR: test inheritance from parent
 		$update = new RevisionSlotsUpdate();
@ -278,6 +282,13 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase {
 		$updater2 = $this->getDerivedPageDataUpdater( $page );
 		$updater2->prepareContent( $sysop, $update, false );

+		// non-null edit use the new user name in PST
+		$pstText = $updater2->getSlots()->getContent( 'main' )->serialize();
+		$this->assertNotContains( '{{subst:REVISIONUSER}}', $pstText, '{{subst:REVISIONUSER}}' );
+		$this->assertNotContains( '~~~', $pstText, 'signature ~~~' );
+		$this->assertContains( '(' . $sysopName . ')', $pstText, '{{subst:REVISIONUSER}}' );
+		$this->assertContains( ':' . $sysopName . '|', $pstText, 'signature ~~~' );
+
 		$this->assertFalse( $updater2->isCreation() );
 		$this->assertTrue( $updater2->isChange() );
 	}
--- a/tests/phpunit/includes/Storage/PageUpdaterTest.php
+++ b/tests/phpunit/includes/Storage/PageUpdaterTest.php
@ -12,6 +12,7 @@ use RecentChange;
 use Revision;
 use TextContent;
 use Title;
+use User;
 use WikiPage;

 /**
@ -19,6 +20,7 @@ use WikiPage;
 * @group Database
 */
 class PageUpdaterTest extends MediaWikiTestCase {
+
 	private function getDummyTitle( $method ) {
 		return Title::newFromText( $method, $this->getDefaultWikitextNS() );
 	}
@ -520,14 +522,16 @@ class PageUpdaterTest extends MediaWikiTestCase {
 			'Test {{subst:REVISIONUSER}} Test',
 			function ( RevisionRecord $rev ) {
 				return $rev->getUser()->getName();
-			}
+			},
+			'subst'
 		];

 		yield 'subst:PAGENAME' => [
 			'Test {{subst:PAGENAME}} Test',
 			function ( RevisionRecord $rev ) {
 				return 'PageUpdaterTest::testMagicWords';
-			}
+			},
+			'subst'
 		];
 	}

@ -541,17 +545,20 @@ class PageUpdaterTest extends MediaWikiTestCase {
 	 *
 	 * @dataProvider provideMagicWords
 	 */
-	public function testMagicWords( $wikitext, $callback ) {
-		$user = $this->getTestUser()->getUser();
+	public function testMagicWords( $wikitext, $callback, $subst = false ) {
+		$user = User::newFromName( 'A user for ' . __METHOD__ );
+		$user->addToDatabase();

 		$title = $this->getDummyTitle( __METHOD__ . '-' . $this->getName() );
+		$this->insertPage( $title );
+
 		$page = WikiPage::factory( $title );
 		$updater = $page->newPageUpdater( $user );

 		$updater->setContent( 'main', new \WikitextContent( $wikitext ) );

 		$summary = CommentStoreComment::newUnsavedComment( 'Just a test' );
-		$rev = $updater->saveRevision( $summary, EDIT_NEW );
+		$rev = $updater->saveRevision( $summary, EDIT_UPDATE );

 		if ( !$rev ) {
 			$this->fail( $updater->getStatus()->getWikiText() );
@ -559,17 +566,15 @@ class PageUpdaterTest extends MediaWikiTestCase {

 		$expected = strval( $callback( $rev ) );

-		$cache = MediaWikiServices::getInstance()->getParserCache();
-		$output = $cache->get(
-			$page,
-			ParserOptions::newCanonical(
-				'canonical'
-			)
-		);
+		$output = $page->getParserOutput( ParserOptions::newCanonical( 'canonical' ) );
+		$html = $output->getText();
+		$text = $rev->getContent( 'main' )->serialize();

-		$this->assertNotNull( $output, 'ParserCache::get' );
+		if ( $subst ) {
+			$this->assertContains( $expected, $text, 'In Wikitext' );
+		}

-		$this->assertContains( $expected, $output->getText() );
+		$this->assertContains( $expected, $html, 'In HTML' );
 	}

 }
--- a/tests/phpunit/includes/parser/ParserMethodsTest.php
+++ b/tests/phpunit/includes/parser/ParserMethodsTest.php
@ -1,4 +1,7 @@
 <?php
+use MediaWiki\Storage\MutableRevisionRecord;
+use MediaWiki\Storage\RevisionStore;
+use MediaWiki\User\UserIdentityValue;

 /**
 * @group Database
@ -192,6 +195,180 @@ class ParserMethodsTest extends MediaWikiLangTestCase {
 		$this->assertContains( 'class="mw-parser-output"', $text );
 	}

+	/**
+	 * @param string $name
+	 * @return Title
+	 */
+	private function getMockTitle( $name ) {
+		$title = $this->getMock( Title::class );
+		$title->method( 'getPrefixedDBkey' )->willReturn( $name );
+		$title->method( 'getPrefixedText' )->willReturn( $name );
+		$title->method( 'getDBkey' )->willReturn( $name );
+		$title->method( 'getText' )->willReturn( $name );
+		$title->method( 'getNamespace' )->willReturn( 0 );
+		$title->method( 'getPageLanguage' )->willReturn( Language::factory( 'en' ) );
+
+		return $title;
+	}
+
+	public function provideRevisionAccess() {
+		$title = $this->getMockTitle( 'ParserRevisionAccessTest' );
+
+		$frank = $this->getMockBuilder( User::class )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$frank->method( 'getName' )->willReturn( 'Frank' );
+
+		$text = '* user:{{REVISIONUSER}};id:{{REVISIONID}};time:{{REVISIONTIMESTAMP}};';
+		$po = new ParserOptions( $frank );
+
+		yield 'current' => [ $text, $po, 0, 'user:CurrentAuthor;id:200;time:20160606000000;' ];
+		yield 'current with ID' => [ $text, $po, 200, 'user:CurrentAuthor;id:200;time:20160606000000;' ];
+
+		$text = '* user:{{REVISIONUSER}};id:{{REVISIONID}};time:{{REVISIONTIMESTAMP}};';
+		$po = new ParserOptions( $frank );
+
+		yield 'old' => [ $text, $po, 100, 'user:OldAuthor;id:100;time:20140404000000;' ];
+
+		$oldRevision = new MutableRevisionRecord( $title );
+		$oldRevision->setId( 100 );
+		$oldRevision->setUser( new UserIdentityValue( 7, 'FauxAuthor', 0 ) );
+		$oldRevision->setTimestamp( '20141111111111' );
+		$oldRevision->setContent( 'main', new WikitextContent( 'FAUX' ) );
+
+		$po = new ParserOptions( $frank );
+		$po->setCurrentRevisionCallback( function () use ( $oldRevision ) {
+			return new Revision( $oldRevision );
+		} );
+
+		yield 'old with override' => [ $text, $po, 100, 'user:FauxAuthor;id:100;time:20141111111111;' ];
+
+		$text = '* user:{{REVISIONUSER}};user-subst:{{subst:REVISIONUSER}};';
+
+		$po = new ParserOptions( $frank );
+		$po->setIsPreview( true );
+
+		yield 'preview without override, using context' => [
+			$text,
+			$po,
+			null,
+			'user:Frank;',
+			'user-subst:Frank;',
+		];
+
+		$text = '* user:{{REVISIONUSER}};time:{{REVISIONTIMESTAMP}};'
+			. 'user-subst:{{subst:REVISIONUSER}};time-subst:{{subst:REVISIONTIMESTAMP}};';
+
+		$newRevision = new MutableRevisionRecord( $title );
+		$newRevision->setUser( new UserIdentityValue( 9, 'NewAuthor', 0 ) );
+		$newRevision->setTimestamp( '20180808000000' );
+		$newRevision->setContent( 'main', new WikitextContent( 'NEW' ) );
+
+		$po = new ParserOptions( $frank );
+		$po->setIsPreview( true );
+		$po->setCurrentRevisionCallback( function () use ( $newRevision ) {
+			return new Revision( $newRevision );
+		} );
+
+		yield 'preview' => [
+			$text,
+			$po,
+			null,
+			'user:NewAuthor;time:20180808000000;',
+			'user-subst:NewAuthor;time-subst:20180808000000;',
+		];
+
+		$po = new ParserOptions( $frank );
+		$po->setCurrentRevisionCallback( function () use ( $newRevision ) {
+			return new Revision( $newRevision );
+		} );
+
+		yield 'pre-save' => [
+			$text,
+			$po,
+			null,
+			'user:NewAuthor;time:20180808000000;',
+			'user-subst:NewAuthor;time-subst:20180808000000;',
+		];
+
+		$text = "(ONE)<includeonly>(TWO)</includeonly>"
+			. "<noinclude>#{{:ParserRevisionAccessTest}}#</noinclude>";
+
+		$newRevision = new MutableRevisionRecord( $title );
+		$newRevision->setUser( new UserIdentityValue( 9, 'NewAuthor', 0 ) );
+		$newRevision->setTimestamp( '20180808000000' );
+		$newRevision->setContent( 'main', new WikitextContent( $text ) );
+
+		$po = new ParserOptions( $frank );
+		$po->setIsPreview( true );
+		$po->setCurrentRevisionCallback( function () use ( $newRevision ) {
+			return new Revision( $newRevision );
+		} );
+
+		yield 'preview with self-transclude' => [ $text, $po, null, '(ONE)#(ONE)(TWO)#' ];
+	}
+
+	/**
+	 * @dataProvider provideRevisionAccess
+	 */
+	public function testRevisionAccess(
+		$text,
+		ParserOptions $po,
+		$revId,
+		$expectedInHtml,
+		$expectedInPst = null
+	) {
+		global $wgParser;
+
+		$title = $this->getMockTitle( 'ParserRevisionAccessTest' );
+
+		$po->enableLimitReport( false );
+
+		$oldRevision = new MutableRevisionRecord( $title );
+		$oldRevision->setId( 100 );
+		$oldRevision->setUser( new UserIdentityValue( 7, 'OldAuthor', 0 ) );
+		$oldRevision->setTimestamp( '20140404000000' );
+		$oldRevision->setContent( 'main', new WikitextContent( 'OLD' ) );
+
+		$currentRevision = new MutableRevisionRecord( $title );
+		$currentRevision->setId( 200 );
+		$currentRevision->setUser( new UserIdentityValue( 9, 'CurrentAuthor', 0 ) );
+		$currentRevision->setTimestamp( '20160606000000' );
+		$currentRevision->setContent( 'main', new WikitextContent( 'CURRENT' ) );
+
+		$revisionStore = $this->getMockBuilder( RevisionStore::class )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$revisionStore
+			->method( 'getKnownCurrentRevision' )
+			->willReturnMap( [
+				[ $title, 100, $oldRevision ],
+				[ $title, 200, $currentRevision ],
+				[ $title, 0, $currentRevision ],
+			] );
+
+		$revisionStore
+			->method( 'getRevisionById' )
+			->willReturnMap( [
+				[ 100, 0, $oldRevision ],
+				[ 200, 0, $currentRevision ],
+			] );
+
+		$this->setService( 'RevisionStore', $revisionStore );
+
+		$wgParser->parse( $text, $title, $po, true, true, $revId );
+		$html = $wgParser->getOutput()->getText();
+
+		$this->assertContains( $expectedInHtml, $html, 'In HTML' );
+
+		if ( $expectedInPst !== null ) {
+			$pst = $wgParser->preSaveTransform( $text, $title, $po->getUser(), $po );
+			$this->assertContains( $expectedInPst, $pst, 'After Pre-Safe Transform' );
+		}
+	}
+
 	// @todo Add tests for cleanSig() / cleanSigInSig(), getSection(),
 	// replaceSection(), getPreloadText()
 }