Use stashed ParserOutput during saving.

The code in DerivedPageDataUpdater that used the stashed ParserOutput from
ApiStashEdit::checkCache was accidentally broken when RevisionRenderer
was introduced in I871978bf79f67c9e7954fb3fc8528d6e365f2cc1.

This is likely the cause for the degraded save timing noted in T205369.

Bug: T205369
Change-Id: I6d8fdda73dccae08d18bfb528b948706f56ad2e0

diff --git a/includes/Revision/RenderedRevision.php b/includes/Revision/RenderedRevision.php
index 6eee3c4..c8f56e9 100644 (file)
--- a/includes/Revision/RenderedRevision.php
+++ b/includes/Revision/RenderedRevision.php
@@ -159,6 +159,28 @@ class RenderedRevision implements SlotRenderingProvider {
                return $this->options;
        }
 
+       /**
+        * Sets a ParserOutput to be returned by getRevisionParserOutput().
+        *
+        * @note For internal use by RevisionRenderer only! This method may be modified
+        * or removed without notice per the deprecation policy.
+        *
+        * @internal
+        *
+        * @param ParserOutput $output
+        */
+       public function setRevisionParserOutput( ParserOutput $output ) {
+               $this->revisionOutput = $output;
+
+               // If there is only one slot, we assume that the combined output is identical
+               // with the main slot's output. This is intended to prevent a redundant re-parse of
+               // the content in case getSlotParserOutput( SlotRecord::MAIN ) is called, for instance
+               // from ContentHandler::getSecondaryDataUpdates.
+               if ( $this->revision->getSlotRoles() === [ SlotRecord::MAIN ] ) {
+                       $this->slotsOutput[ SlotRecord::MAIN ] = $output;
+               }
+       }
+
        /**
         * @param array $hints Hints given as an associative array. Known keys:
         *      - 'generate-html' => bool: Whether the caller is interested in output HTML (as opposed

diff --git a/includes/Revision/RevisionRenderer.php b/includes/Revision/RevisionRenderer.php
index e2e84b6..265ad13 100644 (file)
--- a/includes/Revision/RevisionRenderer.php
+++ b/includes/Revision/RevisionRenderer.php
@@ -82,6 +82,11 @@ class RevisionRenderer {
         *      - 'audience' the audience to use for content access. Default is
         *        RevisionRecord::FOR_PUBLIC if $forUser is not set, RevisionRecord::FOR_THIS_USER
         *        if $forUser is set. Can be set to RevisionRecord::RAW to disable audience checks.
+        *      - 'known-revision-output' a combined ParserOutput for the revision, perhaps from
+        *        some cache. the caller is responsible for ensuring that the ParserOutput indeed
+        *        matched the $rev and $options. This mechanism is intended as a temporary stop-gap,
+        *        for the time until caches have been changed to store RenderedRevision states instead
+        *        of ParserOutput objects.
         *
         * @return RenderedRevision|null The rendered revision, or null if the audience checks fails.
         */
@@ -133,6 +138,10 @@ class RevisionRenderer {
 
                $renderedRevision->setSaveParseLogger( $this->saveParseLogger );
 
+               if ( isset( $hints['known-revision-output'] ) ) {
+                       $renderedRevision->setRevisionParserOutput( $hints['known-revision-output'] );
+               }
+
                return $renderedRevision;
        }
 

diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php
index ad29f91..4556af0 100644 (file)
--- a/includes/Storage/DerivedPageDataUpdater.php
+++ b/includes/Storage/DerivedPageDataUpdater.php
@@ -762,17 +762,6 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                        $stashedEdit = ApiStashEdit::checkCache( $title, $mainContent, $legacyUser );
                }
 
-               if ( $stashedEdit ) {
-                       /** @var ParserOutput $output */
-                       $output = $stashedEdit->output;
-
-                       // TODO: this should happen when stashing the ParserOutput, not now!
-                       $output->setCacheTime( $stashedEdit->timestamp );
-
-                       // TODO: MCR: allow output for all slots to be stashed.
-                       $this->canonicalParserOutput = $output;
-               }
-
                $userPopts = ParserOptions::newFromUserAndLang( $user, $this->contLang );
                Hooks::run( 'ArticlePrepareTextForEdit', [ $wikiPage, $userPopts ] );
 
@@ -853,6 +842,27 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                } else {
                        $this->parentRevision = $parentRevision;
                }
+
+               $renderHints = [ 'use-master' => $this->useMaster(), 'audience' => RevisionRecord::RAW ];
+
+               if ( $stashedEdit ) {
+                       /** @var ParserOutput $output */
+                       $output = $stashedEdit->output;
+
+                       // TODO: this should happen when stashing the ParserOutput, not now!
+                       $output->setCacheTime( $stashedEdit->timestamp );
+
+                       $renderHints['known-revision-output'] = $output;
+               }
+
+               // NOTE: we want a canonical rendering, so don't pass $this->user or ParserOptions
+               // NOTE: the revision is either new or current, so we can bypass audience checks.
+               $this->renderedRevision = $this->revisionRenderer->getRenderedRevision(
+                       $this->revision,
+                       null,
+                       null,
+                       $renderHints
+               );
        }
 
        /**
@@ -879,18 +889,7 @@ class DerivedPageDataUpdater implements IDBAccessObject {
         * @return RenderedRevision
         */
        public function getRenderedRevision() {
-               if ( !$this->renderedRevision ) {
-                       $this->assertPrepared( __METHOD__ );
-
-                       // NOTE: we want a canonical rendering, so don't pass $this->user or ParserOptions
-                       // NOTE: the revision is either new or current, so we can bypass audience checks.
-                       $this->renderedRevision = $this->revisionRenderer->getRenderedRevision(
-                               $this->revision,
-                               null,
-                               null,
-                               [ 'use-master' => $this->useMaster(), 'audience' => RevisionRecord::RAW ]
-                       );
-               }
+               $this->assertPrepared( __METHOD__ );
 
                return $this->renderedRevision;
        }
@@ -1210,6 +1209,19 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                // Prune any output that depends on the revision ID.
                if ( $this->renderedRevision ) {
                        $this->renderedRevision->updateRevision( $revision );
+               } else {
+
+                       // NOTE: we want a canonical rendering, so don't pass $this->user or ParserOptions
+                       // NOTE: the revision is either new or current, so we can bypass audience checks.
+                       $this->renderedRevision = $this->revisionRenderer->getRenderedRevision(
+                               $this->revision,
+                               null,
+                               null,
+                               [ 'use-master' => $this->useMaster(), 'audience' => RevisionRecord::RAW ]
+                       );
+
+                       // XXX: Since we presumably are dealing with the current revision,
+                       // we could try to get the ParserOutput from the parser cache.
                }
 
                // TODO: optionally get ParserOutput from the ParserCache here.

diff --git a/tests/phpunit/includes/Revision/RenderedRevisionTest.php b/tests/phpunit/includes/Revision/RenderedRevisionTest.php
index 08a8fa6..43fccee 100644 (file)
--- a/tests/phpunit/includes/Revision/RenderedRevisionTest.php
+++ b/tests/phpunit/includes/Revision/RenderedRevisionTest.php
@@ -483,6 +483,23 @@ class RenderedRevisionTest extends MediaWikiTestCase {
                $this->assertContains( 'time:20180101000003!', $html );
        }
 
+       public function testSetRevisionParserOutput() {
+               $title = $this->getMockTitle( 3, 21 );
+               $rev = $this->getMockRevision( RevisionStoreRecord::class, $title );
+
+               $options = ParserOptions::newCanonical( 'canonical' );
+               $rr = new RenderedRevision( $title, $rev, $options, $this->combinerCallback );
+
+               $output = new ParserOutput( 'Kittens' );
+               $rr->setRevisionParserOutput( $output );
+
+               $this->assertSame( $output, $rr->getRevisionParserOutput() );
+               $this->assertSame( 'Kittens', $rr->getRevisionParserOutput()->getText() );
+
+               $this->assertSame( $output, $rr->getSlotParserOutput( SlotRecord::MAIN ) );
+               $this->assertSame( 'Kittens', $rr->getSlotParserOutput( SlotRecord::MAIN )->getText() );
+       }
+
        public function testNoHtml() {
                /** @var MockObject|Content $mockContent */
                $mockContent = $this->getMockBuilder( WikitextContent::class )

diff --git a/tests/phpunit/includes/Revision/RevisionRendererTest.php b/tests/phpunit/includes/Revision/RevisionRendererTest.php
index 469f281..5c75ede 100644 (file)
--- a/tests/phpunit/includes/Revision/RevisionRendererTest.php
+++ b/tests/phpunit/includes/Revision/RevisionRendererTest.php
@@ -240,6 +240,34 @@ class RevisionRendererTest extends MediaWikiTestCase {
                $this->assertSame( $html, $rr->getSlotParserOutput( SlotRecord::MAIN )->getText() );
        }
 
+       public function testGetRenderedRevision_known() {
+               $renderer = $this->newRevisionRenderer( 100, true ); // use master
+               $title = $this->getMockTitle( 7, 21 );
+
+               $rev = new MutableRevisionRecord( $title );
+               $rev->setId( 21 ); // current!
+               $rev->setUser( new UserIdentityValue( 9, 'Frank', 0 ) );
+               $rev->setTimestamp( '20180101000003' );
+               $rev->setComment( CommentStoreComment::newUnsavedComment( '' ) );
+
+               $text = "uncached text";
+               $rev->setContent( SlotRecord::MAIN, new WikitextContent( $text ) );
+
+               $output = new ParserOutput( 'cached text' );
+
+               $options = ParserOptions::newCanonical( 'canonical' );
+               $rr = $renderer->getRenderedRevision(
+                       $rev,
+                       $options,
+                       null,
+                       [ 'known-revision-output' => $output ]
+               );
+
+               $this->assertSame( $output, $rr->getRevisionParserOutput() );
+               $this->assertSame( 'cached text', $rr->getRevisionParserOutput()->getText() );
+               $this->assertSame( 'cached text', $rr->getSlotParserOutput( SlotRecord::MAIN )->getText() );
+       }
+
        public function testGetRenderedRevision_old() {
                $renderer = $this->newRevisionRenderer( 100 );
                $title = $this->getMockTitle( 7, 21 );