Remove ParserOptions::legacyOptions() and cleanup related code

ParserOptions::legacyOptions() has been sitting around since 1.17.
Originally it seems to have been intended as a way to avoid a mass cache
invalidation (similar to optionsHashPre30() from I7fb9ffca9). That code
was mostly removed in 1.23, but legacyOptions() was left behind because
it was also being used in a few places as "all cache-varying options"
(despite it not being documented for that purpose) where we'd rather
have any key than no key at all.

This patch creates an actual ParserOptions::allCacheVaryingOptions()
method for those use cases and deprecates the long-obsolete
legacyOptions().

It also makes more explicit the use of the "all cache-varying options"
fallback in ParserCache::getKey(), and doesn't bother trying to use that
fallback in ParserCache::get() where it no longer makes sense.

Change-Id: Ife1e54744155136a570210c03fe907f18f8e8ece

diff --git a/includes/parser/ParserCache.php b/includes/parser/ParserCache.php
index 76a7e1e..3b84c4b 100644 (file)
--- a/includes/parser/ParserCache.php
+++ b/includes/parser/ParserCache.php
@@ -26,6 +26,26 @@
  * @todo document
  */
 class ParserCache {
+       /**
+        * Constants for self::getKey()
+        * @since 1.30
+        */
+
+       /** Use only current data */
+       const USE_CURRENT_ONLY = 0;
+
+       /** Use expired data if current data is unavailable */
+       const USE_EXPIRED = 1;
+
+       /** Use expired data or data from different revisions if current data is unavailable */
+       const USE_OUTDATED = 2;
+
+       /**
+        * Use expired data and data from different revisions, and if all else
+        * fails vary on all variable options
+        */
+       const USE_ANYTHING = 3;
+
        /** @var BagOStuff */
        private $mMemc;
        /**
@@ -103,7 +123,7 @@ class ParserCache {
         */
        public function getETag( $article, $popts ) {
                return 'W/"' . $this->getParserOutputKey( $article,
-                       $popts->optionsHash( ParserOptions::legacyOptions(), $article->getTitle() ) ) .
+                       $popts->optionsHash( ParserOptions::allCacheVaryingOptions(), $article->getTitle() ) ) .
                                "--" . $article->getTouched() . '"';
        }
 
@@ -130,16 +150,21 @@ class ParserCache {
         * It would be preferable to have this code in get()
         * instead of having Article looking in our internals.
         *
-        * @todo Document parameter $useOutdated
-        *
         * @param WikiPage $article
         * @param ParserOptions $popts
-        * @param bool $useOutdated (default true)
+        * @param int|bool $useOutdated One of the USE constants. For backwards
+        *  compatibility, boolean false is treated as USE_CURRENT_ONLY and
+        *  boolean true is treated as USE_ANYTHING.
         * @return bool|mixed|string
+        * @since 1.30 Changed $useOutdated to an int and added the non-boolean values
         */
-       public function getKey( $article, $popts, $useOutdated = true ) {
+       public function getKey( $article, $popts, $useOutdated = self::USE_ANYTHING ) {
                global $wgCacheEpoch;
 
+               if ( is_bool( $useOutdated ) ) {
+                       $useOutdated = $useOutdated ? self::USE_ANYTHING : self::USE_CURRENT_ONLY;
+               }
+
                if ( $popts instanceof User ) {
                        wfWarn( "Use of outdated prototype ParserCache::getKey( &\$article, &\$user )\n" );
                        $popts = ParserOptions::newFromUser( $popts );
@@ -150,14 +175,16 @@ class ParserCache {
                $optionsKey = $this->mMemc->get(
                        $this->getOptionsKey( $article ), $casToken, BagOStuff::READ_VERIFIED );
                if ( $optionsKey instanceof CacheTime ) {
-                       if ( !$useOutdated && $optionsKey->expired( $article->getTouched() ) ) {
+                       if ( $useOutdated < self::USE_EXPIRED && $optionsKey->expired( $article->getTouched() ) ) {
                                wfIncrStats( "pcache.miss.expired" );
                                $cacheTime = $optionsKey->getCacheTime();
                                wfDebugLog( "ParserCache",
                                        "Parser options key expired, touched " . $article->getTouched()
                                        . ", epoch $wgCacheEpoch, cached $cacheTime\n" );
                                return false;
-                       } elseif ( !$useOutdated && $optionsKey->isDifferentRevision( $article->getLatest() ) ) {
+                       } elseif ( $useOutdated < self::USE_OUTDATED &&
+                               $optionsKey->isDifferentRevision( $article->getLatest() )
+                       ) {
                                wfIncrStats( "pcache.miss.revid" );
                                $revId = $article->getLatest();
                                $cachedRevId = $optionsKey->getCacheRevisionId();
@@ -171,10 +198,10 @@ class ParserCache {
                        $usedOptions = $optionsKey->mUsedOptions;
                        wfDebug( "Parser cache options found.\n" );
                } else {
-                       if ( !$useOutdated ) {
+                       if ( $useOutdated < self::USE_ANYTHING ) {
                                return false;
                        }
-                       $usedOptions = ParserOptions::legacyOptions();
+                       $usedOptions = ParserOptions::allCacheVaryingOptions();
                }
 
                return $this->getParserOutputKey(
@@ -204,7 +231,9 @@ class ParserCache {
 
                $touched = $article->getTouched();
 
-               $parserOutputKey = $this->getKey( $article, $popts, $useOutdated );
+               $parserOutputKey = $this->getKey( $article, $popts,
+                       $useOutdated ? self::USE_OUTDATED : self::USE_CURRENT_ONLY
+               );
                if ( $parserOutputKey === false ) {
                        wfIncrStats( 'pcache.miss.absent' );
                        return false;

diff --git a/includes/parser/ParserOptions.php b/includes/parser/ParserOptions.php
index 4809917..96a4368 100644 (file)
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@ -1211,10 +1211,11 @@ class ParserOptions {
         * Returns the full array of options that would have been used by
         * in 1.16.
         * Used to get the old parser cache entries when available.
-        * @todo 1.16 was years ago, can we remove this?
+        * @deprecated since 1.30. You probably want self::allCacheVaryingOptions() instead.
         * @return array
         */
        public static function legacyOptions() {
+               wfDeprecated( __METHOD__, '1.30' );
                return [
                        'stubthreshold',
                        'numberheadings',
@@ -1225,6 +1226,20 @@ class ParserOptions {
                ];
        }
 
+       /**
+        * Return all option keys that vary the options hash
+        * @since 1.30
+        * @return string[]
+        */
+       public static function allCacheVaryingOptions() {
+               // Trigger a call to the 'ParserOptionsRegister' hook if it hasn't
+               // already been called.
+               if ( self::$defaults === null ) {
+                       self::getDefaults();
+               }
+               return array_keys( array_filter( self::$inCacheKey ) );
+       }
+
        /**
         * Convert an option to a string value
         * @param mixed $value

diff --git a/tests/phpunit/includes/parser/ParserOptionsTest.php b/tests/phpunit/includes/parser/ParserOptionsTest.php
index 264e35d..ad899bd 100644 (file)
--- a/tests/phpunit/includes/parser/ParserOptionsTest.php
+++ b/tests/phpunit/includes/parser/ParserOptionsTest.php
@@ -5,6 +5,42 @@ use Wikimedia\ScopedCallback;
 
 class ParserOptionsTest extends MediaWikiTestCase {
 
+       private static function clearCache() {
+               $wrap = TestingAccessWrapper::newFromClass( ParserOptions::class );
+               $wrap->defaults = null;
+               $wrap->lazyOptions = [
+                       'dateformat' => [ ParserOptions::class, 'initDateFormat' ],
+               ];
+               $wrap->inCacheKey = [
+                       'dateformat' => true,
+                       'numberheadings' => true,
+                       'thumbsize' => true,
+                       'stubthreshold' => true,
+                       'printable' => true,
+                       'userlang' => true,
+                       'wrapclass' => true,
+               ];
+       }
+
+       protected function setUp() {
+               global $wgHooks;
+
+               parent::setUp();
+               self::clearCache();
+
+               $this->setMwGlobals( [
+                       'wgRenderHashAppend' => '',
+                       'wgHooks' => [
+                               'PageRenderingHash' => [],
+                       ] + $wgHooks,
+               ] );
+       }
+
+       protected function tearDown() {
+               self::clearCache();
+               parent::tearDown();
+       }
+
        /**
         * @dataProvider provideIsSafeToCache
         * @param bool $expect Expected value
@@ -48,7 +84,6 @@ class ParserOptionsTest extends MediaWikiTestCase {
                global $wgHooks;
 
                $globals += [
-                       'wgRenderHashAppend' => '',
                        'wgHooks' => [],
                ];
                $globals['wgHooks'] += [
@@ -103,13 +138,6 @@ class ParserOptionsTest extends MediaWikiTestCase {
 
        // Test weird historical behavior is still weird
        public function testOptionsHashEditSection() {
-               global $wgHooks;
-
-               $this->setMwGlobals( [
-                       'wgRenderHashAppend' => '',
-                       'wgHooks' => [ 'PageRenderingHash' => [] ] + $wgHooks,
-               ] );
-
                $popt = ParserOptions::newCanonical();
                $popt->registerWatcher( function ( $name ) {
                        $this->assertNotEquals( 'editsection', $name );
@@ -175,4 +203,33 @@ class ParserOptionsTest extends MediaWikiTestCase {
                ScopedCallback::consume( $reset );
        }
 
+       public function testAllCacheVaryingOptions() {
+               global $wgHooks;
+
+               // $wgHooks is already saved in self::setUp(), so we can modify it freely here
+               $wgHooks['ParserOptionsRegister'] = [];
+               $this->assertSame( [
+                       'dateformat', 'numberheadings', 'printable', 'stubthreshold',
+                       'thumbsize', 'userlang', 'wrapclass',
+               ], ParserOptions::allCacheVaryingOptions() );
+
+               self::clearCache();
+
+               $wgHooks['ParserOptionsRegister'][] = function ( &$defaults, &$inCacheKey ) {
+                       $defaults += [
+                               'foo' => 'foo',
+                               'bar' => 'bar',
+                               'baz' => 'baz',
+                       ];
+                       $inCacheKey += [
+                               'foo' => true,
+                               'bar' => false,
+                       ];
+               };
+               $this->assertSame( [
+                       'dateformat', 'foo', 'numberheadings', 'printable', 'stubthreshold',
+                       'thumbsize', 'userlang', 'wrapclass',
+               ], ParserOptions::allCacheVaryingOptions() );
+       }
+
 }