jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/360676 )
Change subject: Remove ParserOptions::legacyOptions() and cleanup related code
......................................................................
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
---
M includes/parser/ParserCache.php
M includes/parser/ParserOptions.php
M tests/phpunit/includes/parser/ParserOptionsTest.php
3 files changed, 120 insertions(+), 19 deletions(-)
Approvals:
Tim Starling: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/parser/ParserCache.php b/includes/parser/ParserCache.php
index 76a7e1e..3b84c4b 100644
--- 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 @@
*/
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,15 +150,20 @@
* 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" );
@@ -150,14 +175,16 @@
$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 @@
$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 @@
$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
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@ -1211,10 +1211,11 @@
* 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',
@@ -1226,6 +1227,20 @@
}
/**
+ * 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
* @return string
diff --git a/tests/phpunit/includes/parser/ParserOptionsTest.php
b/tests/phpunit/includes/parser/ParserOptionsTest.php
index 264e35d..ad899bd 100644
--- a/tests/phpunit/includes/parser/ParserOptionsTest.php
+++ b/tests/phpunit/includes/parser/ParserOptionsTest.php
@@ -5,6 +5,42 @@
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 @@
global $wgHooks;
$globals += [
- 'wgRenderHashAppend' => '',
'wgHooks' => [],
];
$globals['wgHooks'] += [
@@ -103,13 +138,6 @@
// 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 @@
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() );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/360676
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ife1e54744155136a570210c03fe907f18f8e8ece
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: C. Scott Ananian <canan...@wikimedia.org>
Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com>
Gerrit-Reviewer: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
