Provide new, unsaved revision to PST to fix magic words.
authordaniel <daniel.kinzler@wikimedia.de>
Wed, 5 Sep 2018 18:03:15 +0000 (20:03 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Thu, 6 Sep 2018 16:33:44 +0000 (18:33 +0200)
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

includes/EditPage.php
includes/Storage/DerivedPageDataUpdater.php
includes/Storage/PageUpdater.php
includes/parser/Parser.php
tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
tests/phpunit/includes/Storage/PageUpdaterTest.php
tests/phpunit/includes/parser/ParserMethodsTest.php

index ef111c4..d8903c3 100644 (file)
@@ -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 [
index a7dfb4b..99c31b2 100644 (file)
@@ -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 );
                }
        }
 
index ec1f6e2..1621213 100644 (file)
@@ -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__ );
 
index 51c04ea..7d5a362 100644 (file)
@@ -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 );
                }
index 19fa104..7c4be97 100644 (file)
@@ -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() );
        }
index 517e7c6..81f726c 100644 (file)
@@ -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' );
        }
 
 }
index 497c6b5..1427f01 100644 (file)
@@ -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()
 }