From 465954aa23cec76ca47e51a58ff342f46fbbdcab Mon Sep 17 00:00:00 2001 From: daniel Date: Wed, 5 Sep 2018 20:03:15 +0200 Subject: [PATCH] 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 --- includes/EditPage.php | 15 +- includes/Storage/DerivedPageDataUpdater.php | 28 ++- includes/Storage/PageUpdater.php | 14 +- includes/parser/Parser.php | 20 +- .../Storage/DerivedPageDataUpdaterTest.php | 19 +- .../includes/Storage/PageUpdaterTest.php | 33 ++-- .../includes/parser/ParserMethodsTest.php | 177 ++++++++++++++++++ 7 files changed, 263 insertions(+), 43 deletions(-) diff --git a/includes/EditPage.php b/includes/EditPage.php index ef111c46cc..d8903c399f 100644 --- 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 [ diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php index a7dfb4b707..99c31b2d69 100644 --- 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 ); } } diff --git a/includes/Storage/PageUpdater.php b/includes/Storage/PageUpdater.php index ec1f6e2e0d..1621213c49 100644 --- 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__ ); diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php index 51c04ea035..7d5a362b38 100644 --- 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 ); } diff --git a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php index 19fa1047a4..7c4be9771e 100644 --- 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() ); } diff --git a/tests/phpunit/includes/Storage/PageUpdaterTest.php b/tests/phpunit/includes/Storage/PageUpdaterTest.php index 517e7c676b..81f726c0bb 100644 --- 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' ); } } diff --git a/tests/phpunit/includes/parser/ParserMethodsTest.php b/tests/phpunit/includes/parser/ParserMethodsTest.php index 497c6b5bca..1427f01f20 100644 --- a/tests/phpunit/includes/parser/ParserMethodsTest.php +++ b/tests/phpunit/includes/parser/ParserMethodsTest.php @@ -1,4 +1,7 @@ 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)(TWO)" + . "#{{:ParserRevisionAccessTest}}#"; + + $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() } -- 2.20.1