Merge "parser: add vary-revision-sha1 and related ParserOutput methods"

diff --git a/includes/Revision/RenderedRevision.php b/includes/Revision/RenderedRevision.php
index 4acb9c0..cf1cc94 100644 (file)
--- a/includes/Revision/RenderedRevision.php
+++ b/includes/Revision/RenderedRevision.php
@@ -430,6 +430,16 @@ class RenderedRevision implements SlotRenderingProvider {
                                "$method: Prepared output has vary-revision-exists..."
                        );
                        return true;
+               } elseif (
+                       $out->getFlag( 'vary-revision-sha1' ) &&
+                       $out->getRevisionUsedSha1Base36() !== $this->revision->getSha1()
+               ) {
+                       // If a self-transclusion used the proposed page text, it must match the final
+                       // page content after PST transformations and automatically merged edit conflicts
+                       $this->saveParseLogger->info(
+                               "$method: Prepared output has vary-revision-sha1 with wrong SHA-1..."
+                       );
+                       return true;
                } else {
                        // NOTE: In the original fix for T135261, the output was discarded if 'vary-user' was
                        // set for a null-edit. The reason was that the original rendering in that case was

diff --git a/includes/Storage/PageEditStash.php b/includes/Storage/PageEditStash.php
index 6caca29..4671d99 100644 (file)
--- a/includes/Storage/PageEditStash.php
+++ b/includes/Storage/PageEditStash.php
@@ -269,23 +269,28 @@ class PageEditStash {
 
                if ( $editInfo->output->getFlag( 'vary-revision' ) ) {
                        // This can be used for the initial parse, e.g. for filters or doEditContent(),
-                       // but a second parse will be triggered in doEditUpdates(). This is not optimal.
+                       // but a second parse will be triggered in doEditUpdates() no matter what
                        $logger->info(
-                               "Cache for key '{key}' has vary_revision; post-insertion parse inevitable.",
-                               $context
-                       );
-               } elseif ( $editInfo->output->getFlag( 'vary-revision-id' ) ) {
-                       // Similar to the above if we didn't guess the ID correctly.
-                       $logger->debug(
-                               "Cache for key '{key}' has vary_revision_id; post-insertion parse possible.",
-                               $context
-                       );
-               } elseif ( $editInfo->output->getFlag( 'vary-revision-timestamp' ) ) {
-                       // Similar to the above if we didn't guess the timestamp correctly.
-                       $logger->debug(
-                               "Cache for key '{key}' has vary_revision_timestamp; post-insertion parse possible.",
+                               "Cache for key '{key}' has 'vary-revision'; post-insertion parse inevitable.",
                                $context
                        );
+               } else {
+                       static $flagsMaybeReparse = [
+                               // Similar to the above if we didn't guess the ID correctly
+                               'vary-revision-id',
+                               // Similar to the above if we didn't guess the timestamp correctly
+                               'vary-revision-timestamp',
+                               // Similar to the above if we didn't guess the content correctly
+                               'vary-revision-sha1'
+                       ];
+                       foreach ( $flagsMaybeReparse as $flag ) {
+                               if ( $editInfo->output->getFlag( $flag ) ) {
+                                       $logger->debug(
+                                               "Cache for key '{key}' has $flag; post-insertion parse possible.",
+                                               $context
+                                       );
+                               }
+                       }
                }
 
                return $editInfo;

diff --git a/includes/parser/CoreParserFunctions.php b/includes/parser/CoreParserFunctions.php
index 7fece00..5aa1a69 100644 (file)
--- a/includes/parser/CoreParserFunctions.php
+++ b/includes/parser/CoreParserFunctions.php
@@ -823,7 +823,7 @@ class CoreParserFunctions {
                }
 
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $title );
+               $rev = self::getCachedRevisionObject( $parser, $title, 'vary-revision-sha1' );
                $length = $rev ? $rev->getSize() : 0;
                if ( $length === null ) {
                        // We've had bugs where rev_len was not being recorded for empty pages, see T135414
@@ -1126,41 +1126,56 @@ class CoreParserFunctions {
         *
         * @param Parser $parser
         * @param Title $title
+        * @param string $vary ParserOuput vary-* flag
         * @return Revision
         * @since 1.23
         */
-       private static function getCachedRevisionObject( $parser, $title = null ) {
-               if ( is_null( $title ) ) {
+       private static function getCachedRevisionObject( $parser, $title, $vary ) {
+               if ( !$title ) {
                        return null;
                }
 
-               // Use the revision from the parser itself, when param is the current page
-               // and the revision is the current one
-               if ( $title->equals( $parser->getTitle() ) ) {
-                       $parserRev = $parser->getRevisionObject();
-                       if ( $parserRev && $parserRev->isCurrent() ) {
-                               // force reparse after edit with vary-revision flag
-                               $parser->getOutput()->setFlag( 'vary-revision' );
-                               wfDebug( __METHOD__ . ": use current revision from parser, setting vary-revision...\n" );
-                               return $parserRev;
+               $revision = null;
+
+               $isSelfReferential = $title->equals( $parser->getTitle() );
+               if ( $isSelfReferential ) {
+                       // Revision is for the same title that is currently being parsed. Only use the last
+                       // saved revision, regardless of Parser::getRevisionId() or fake revision injection
+                       // callbacks against the current title.
+                       $parserRevision = $parser->getRevisionObject();
+                       if ( $parserRevision && $parserRevision->isCurrent() ) {
+                               $revision = $parserRevision;
+                               wfDebug( __METHOD__ . ": used current revision, setting $vary" );
                        }
                }
 
-               // Normalize name for cache
-               $page = $title->getPrefixedDBkey();
-
-               if ( !( $parser->currentRevisionCache && $parser->currentRevisionCache->has( $page ) )
-                       && !$parser->incrementExpensiveFunctionCount() ) {
-                       return null;
+               $parserOutput = $parser->getOutput();
+               if ( !$revision ) {
+                       if (
+                               !$parser->isCurrentRevisionOfTitleCached( $title ) &&
+                               !$parser->incrementExpensiveFunctionCount()
+                       ) {
+                               return null; // not allowed
+                       }
+                       // Get the current revision, ignoring Parser::getRevisionId() being null/old
+                       $revision = $parser->fetchCurrentRevisionOfTitle( $title );
+                       // Register dependency in templatelinks
+                       $parserOutput->addTemplate(
+                               $title,
+                               $revision ? $revision->getPage() : 0,
+                               $revision ? $revision->getId() : 0
+                       );
                }
-               $rev = $parser->fetchCurrentRevisionOfTitle( $title );
-               $pageID = $rev ? $rev->getPage() : 0;
-               $revID = $rev ? $rev->getId() : 0;
 
-               // Register dependency in templatelinks
-               $parser->getOutput()->addTemplate( $title, $pageID, $revID );
+               if ( $isSelfReferential ) {
+                       // Upon page save, the result of the parser function using this might change
+                       $parserOutput->setFlag( $vary );
+                       if ( $vary === 'vary-revision-sha1' && $revision ) {
+                               $parserOutput->setRevisionUsedSha1Base36( $revision->getSha1() );
+                       }
+               }
 
-               return $rev;
+               return $revision;
        }
 
        /**
@@ -1221,7 +1236,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-revision-id' );
                return $rev ? $rev->getId() : '';
        }
 
@@ -1238,7 +1253,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-revision-timestamp' );
                return $rev ? MWTimestamp::getLocalInstance( $rev->getTimestamp() )->format( 'j' ) : '';
        }
 
@@ -1255,7 +1270,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-revision-timestamp' );
                return $rev ? MWTimestamp::getLocalInstance( $rev->getTimestamp() )->format( 'd' ) : '';
        }
 
@@ -1272,7 +1287,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-revision-timestamp' );
                return $rev ? MWTimestamp::getLocalInstance( $rev->getTimestamp() )->format( 'm' ) : '';
        }
 
@@ -1289,7 +1304,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-revision-timestamp' );
                return $rev ? MWTimestamp::getLocalInstance( $rev->getTimestamp() )->format( 'n' ) : '';
        }
 
@@ -1306,7 +1321,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-revision-timestamp' );
                return $rev ? MWTimestamp::getLocalInstance( $rev->getTimestamp() )->format( 'Y' ) : '';
        }
 
@@ -1323,7 +1338,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-revision-timestamp' );
                return $rev ? MWTimestamp::getLocalInstance( $rev->getTimestamp() )->format( 'YmdHis' ) : '';
        }
 
@@ -1340,7 +1355,7 @@ class CoreParserFunctions {
                        return '';
                }
                // fetch revision from cache/database and return the value
-               $rev = self::getCachedRevisionObject( $parser, $t );
+               $rev = self::getCachedRevisionObject( $parser, $t, 'vary-user' );
                return $rev ? $rev->getUserText() : '';
        }
 

diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index a2c5eec..0721446 100644 (file)
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -3693,6 +3693,18 @@ class Parser {
                return $this->currentRevisionCache->get( $cacheKey );
        }
 
+       /**
+        * @param Title $title
+        * @return bool
+        * @since 1.34
+        */
+       public function isCurrentRevisionOfTitleCached( $title ) {
+               return (
+                       $this->currentRevisionCache &&
+                       $this->currentRevisionCache->has( $title->getPrefixedText() )
+               );
+       }
+
        /**
         * Wrapper around Revision::newFromTitle to allow passing additional parameters
         * without passing them on to it.
@@ -3727,8 +3739,7 @@ class Parser {
                        foreach ( $stuff['deps'] as $dep ) {
                                $this->mOutput->addTemplate( $dep['title'], $dep['page_id'], $dep['rev_id'] );
                                if ( $dep['title']->equals( $this->getTitle() ) ) {
-                                       // If we transclude ourselves, the final result
-                                       // will change based on the new version of the page
+                                       // Self-transclusion; final result may change based on the new page version
                                        $this->mOutput->setFlag( 'vary-revision' );
                                        wfDebug( __METHOD__ . ": self transclusion, setting vary-revision" );
                                }

diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php
index c8113f3..23e5911 100644 (file)
--- a/includes/parser/ParserOutput.php
+++ b/includes/parser/ParserOutput.php
@@ -216,6 +216,9 @@ class ParserOutput extends CacheTime {
        /** @var int|null Assumed rev timestamp for {{REVISIONTIMESTAMP}} if no revision is set */
        private $revisionTimestampUsed;
 
+       /** @var string|null SHA-1 base 36 hash of any self-transclusion */
+       private $revisionUsedSha1Base36;
+
        /** string CSS classes to use for the wrapping div, stored in the array keys.
         * If no class is given, no wrapper is added.
         */
@@ -464,6 +467,33 @@ class ParserOutput extends CacheTime {
                return $this->revisionTimestampUsed;
        }
 
+       /**
+        * @param string $hash Lowercase SHA-1 base 36 hash
+        * @since 1.34
+        */
+       public function setRevisionUsedSha1Base36( $hash ) {
+               if ( $hash === null ) {
+                       return; // e.g. RevisionRecord::getSha1() returned null
+               }
+
+               if (
+                       $this->revisionUsedSha1Base36 !== null &&
+                       $this->revisionUsedSha1Base36 !== $hash
+               ) {
+                       $this->revisionUsedSha1Base36 = ''; // mismatched
+               } else {
+                       $this->revisionUsedSha1Base36 = $hash;
+               }
+       }
+
+       /**
+        * @return string|null Lowercase SHA-1 base 36 hash, null if unused, or "" on inconsistency
+        * @since 1.34
+        */
+       public function getRevisionUsedSha1Base36() {
+               return $this->revisionUsedSha1Base36;
+       }
+
        public function &getLanguageLinks() {
                return $this->mLanguageLinks;
        }

diff --git a/tests/parser/ParserTestRunner.php b/tests/parser/ParserTestRunner.php
index f29b0d7..ba85027 100644 (file)
--- a/tests/parser/ParserTestRunner.php
+++ b/tests/parser/ParserTestRunner.php
@@ -25,6 +25,7 @@
  * @file
  * @ingroup Testing
  */
+
 use Wikimedia\Rdbms\IDatabase;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Tidy\TidyDriverBase;
@@ -129,6 +130,9 @@ class ParserTestRunner {
         */
        private $keepUploads;
 
+       /** @var Title */
+       private $defaultTitle;
+
        /**
         * @param TestRecorder $recorder
         * @param array $options
@@ -165,6 +169,8 @@ class ParserTestRunner {
                if ( isset( $options['upload-dir'] ) ) {
                        $this->uploadDir = $options['upload-dir'];
                }
+
+               $this->defaultTitle = Title::newFromText( 'Parser test' );
        }
 
        /**
@@ -839,10 +845,43 @@ class ParserTestRunner {
                        $options->setTidy( true );
                }
 
-               if ( isset( $opts['title'] ) ) {
-                       $titleText = $opts['title'];
-               } else {
-                       $titleText = 'Parser test';
+               $revId = 1337; // see Parser::getRevisionId()
+               $title = isset( $opts['title'] )
+                       ? Title::newFromText( $opts['title'] )
+                       : $this->defaultTitle;
+
+               if ( isset( $opts['lastsavedrevision'] ) ) {
+                       $content = new WikitextContent( $test['input'] );
+                       $title = Title::newFromRow( (object)[
+                               'page_id' => 187,
+                               'page_len' => $content->getSize(),
+                               'page_latest' => 1337,
+                               'page_namespace' => $title->getNamespace(),
+                               'page_title' => $title->getDBkey(),
+                               'page_is_redirect' => 0
+                       ] );
+                       $rev = new Revision(
+                               [
+                                       'id' => $title->getLatestRevID(),
+                                       'page' => $title->getArticleID(),
+                                       'user' => $user,
+                                       'content' => $content,
+                                       'timestamp' => $this->getFakeTimestamp(),
+                                       'title' => $title
+                               ],
+                               Revision::READ_LATEST,
+                               $title
+                       );
+                       $oldCallback = $options->getCurrentRevisionCallback();
+                       $options->setCurrentRevisionCallback(
+                               function ( Title $t, $parser ) use ( $title, $rev, $oldCallback ) {
+                                       if ( $t->equals( $title ) ) {
+                                               return $rev;
+                                       } else {
+                                               return call_user_func( $oldCallback, $t, $parser );
+                                       }
+                               }
+                       );
                }
 
                if ( isset( $opts['maxincludesize'] ) ) {
@@ -855,7 +894,6 @@ class ParserTestRunner {
                $local = isset( $opts['local'] );
                $preprocessor = $opts['preprocessor'] ?? null;
                $parser = $this->getParser( $preprocessor );
-               $title = Title::newFromText( $titleText );
 
                if ( isset( $opts['styletag'] ) ) {
                        // For testing the behavior of <style> (including those deduplicated
@@ -887,7 +925,7 @@ class ParserTestRunner {
                } elseif ( isset( $opts['preload'] ) ) {
                        $out = $parser->getPreloadText( $test['input'], $title, $options );
                } else {
-                       $output = $parser->parse( $test['input'], $title, $options, true, true, 1337 );
+                       $output = $parser->parse( $test['input'], $title, $options, true, true, $revId );
                        $out = $output->getText( [
                                'allowTOC' => !isset( $opts['notoc'] ),
                                'unwrap' => !isset( $opts['wrap'] ),

diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
index 0facec2..7046a7f 100644 (file)
--- a/tests/parser/parserTests.txt
+++ b/tests/parser/parserTests.txt
@@ -10813,8 +10813,9 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
 !! end
 
 !! test
-Magic Word: {{REVISIONID}}
+Magic Word: {{REVISIONID}} on latest revision
 !! options
+lastsavedrevision
 parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
 showflags
 !! wikitext
@@ -10825,6 +10826,156 @@ showflags
 flags=vary-revision-id
 !! end
 
+!! test
+Magic Word: {{REVISIONID}} on non-latest revision
+!! options
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONID}}
+!! html/*
+<p>1337
+</p>
+flags=vary-revision-id
+!! end
+
+!! test
+Magic Word: {{REVISIONTIMESTAMP}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONTIMESTAMP}}
+!! html/*
+<p>19700101000203
+</p>
+flags=
+!! end
+
+!! test
+Magic Word: {{REVISIONTIMESTAMP}} on non-existing page
+!! options
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONTIMESTAMP}}
+!! html/*
+<p>123
+</p>
+flags=vary-revision-timestamp
+!! end
+
+!! test
+Magic Word: {{REVISIONUSER}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONUSER}}
+!! html/*
+<p>127.0.0.1
+</p>
+flags=vary-user
+!! end
+
+!! test
+Parser Function: {{REVISIONID:{{PAGENAME}}}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONID:{{PAGENAME}}}}
+!! html/*
+<p>1337
+</p>
+flags=vary-revision-id
+!! end
+
+!! test
+Parser Function: {{REVISIONID:{{PAGENAME}}}} on non-saved revision
+!! options
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONID:{{PAGENAME}}}}
+!! html/*
+
+flags=vary-revision-id
+!! end
+
+!! test
+Parser Function: {{REVISIONTIMESTAMP:{{PAGENAME}}}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONTIMESTAMP:{{PAGENAME}}}}
+!! html/*
+<p>19700101000203
+</p>
+flags=vary-revision-timestamp
+!! end
+
+!! test
+Parser Function: {{REVISIONDAY:{{PAGENAME}}}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONDAY:{{PAGENAME}}}}
+!! html/*
+<p>1
+</p>
+flags=vary-revision-timestamp
+!! end
+
+!! test
+Parser Function: {{REVISIONMONTH:{{PAGENAME}}}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONMONTH:{{PAGENAME}}}}
+!! html/*
+<p>01
+</p>
+flags=vary-revision-timestamp
+!! end
+
+!! test
+Parser Function: {{REVISIONYEAR:{{PAGENAME}}}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{REVISIONYEAR:{{PAGENAME}}}}
+!! html/*
+<p>1970
+</p>
+flags=vary-revision-timestamp
+!! end
+
+!! test
+Parser Function: {{PAGESIZE:{{PAGENAME}}}} on latest revision
+!! options
+lastsavedrevision
+parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true }
+showflags
+!! wikitext
+{{PAGESIZE:{{PAGENAME}}}}
+!! html/*
+<p>25
+</p>
+flags=vary-revision-sha1
+!! end
+
 !! test
 Magic Word: {{SCRIPTPATH}}
 !! options