Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Move limit report rendering to ParserOutput

Browse source 
This does not move the actual limit report data into
ParserOptions yet, that should be done separately
given that it will require serialization changes.
Let's get this change settled first before messing
with serialization.

This unifies canonical and non-canonical ParserOptions,
so ParserCache can now be used with both. It is hard
to say how this will affect the ParserCache capacity,
so we should monitor it after releasing this.

Change-Id: I154c0a77a5b0287b5572614d56339fb57ac56c33
This commit is contained in:
Petr Pchelko 2021-10-07 07:45:45 -07:00
parent ba7ac9f655
commit 89028e0b8e
13 changed files with 117 additions and 107 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
RELEASE-NOTES-1.38
View file

@ -212,6 +212,10 @@ because of Phabricator reports.
* Http::$httpEngine, deprecated since 1.34, has been removed. The only available
HTTP engine is now Guzzle. CurlHttpRequest and PhpHttpRequest classes were
removed.
* Parser option enableLimitReport was deprecated. The report is now generated
post-parse and can be included by providing 'includeLimitReport' option
to ParserOutput::getText. Thus, ParserOptions::enableLimitReport and
::getEnableLimitReport methods were deprecated.
* …
=== Deprecations in 1.38 ===
@ -305,6 +309,7 @@ because of Phabricator reports.
`mw-delete-editreasons`
The goal of these changes is to make the HTML more similar to that of
normal page deletion.
* ParserOptions created with ::newFrom* or ::newCanonical are now identical.
* …
== Compatibility ==

4
includes/EditPage.php
View file

@ -4212,7 +4212,6 @@ class EditPage implements IEditObject {
$parserOptions = $this->page->makeParserOptions( $this->context );
$parserOptions->setIsPreview( true );
$parserOptions->setIsSectionPreview( $this->section !== null && $this->section !== '' );
$parserOptions->enableLimitReport();
// XXX: we could call $parserOptions->setCurrentRevisionRecordCallback here to force the
// current revision to be null during PST, until setupFakeRevision is called on
@ -4250,7 +4249,8 @@ class EditPage implements IEditObject {
return [
'parserOutput' => $parserOutput,
'html' => $parserOutput->getText( [
'enableSectionEditLinks' => false
'enableSectionEditLinks' => false,
'includeLimitReport' => true,
] )
];
}

6
includes/actions/McrUndoAction.php
View file

@ -290,12 +290,14 @@ class McrUndoAction extends FormAction {
$parserOptions = $this->getWikiPage()->makeParserOptions( $this->context );
$parserOptions->setIsPreview( true );
$parserOptions->setIsSectionPreview( false );
$parserOptions->enableLimitReport();
$parserOutput = $this->revisionRenderer
->getRenderedRevision( $rev, $parserOptions, $this->context->getUser() )
->getRevisionParserOutput();
$previewHTML = $parserOutput->getText( [ 'enableSectionEditLinks' => false ] );
$previewHTML = $parserOutput->getText( [
'enableSectionEditLinks' => false,
'includeLimitReport' => true,
] );
$out->addParserOutputMetadata( $parserOutput );
if ( count( $parserOutput->getWarnings() ) ) {

1
includes/api/ApiFormatBase.php
View file

@ -273,6 +273,7 @@ abstract class ApiFormatBase extends ApiBase {
}
$header = $msg->parseAsBlock();
// @phan-suppress-next-line SecurityCheck-XSS
$out->addHTML(
Html::rawElement( 'div', [ 'class' => 'api-pretty-header' ],
ApiHelp::fixHelpLinks( $header )

1
includes/api/ApiHelp.php
View file

@ -187,6 +187,7 @@ class ApiHelp extends ApiBase {
$helptitle = $options['helptitle'] ?? null;
$html = self::fixHelpLinks( $out->getHTML(), $helptitle, $haveModules );
$out->clearHTML();
// @phan-suppress-next-line SecurityCheck-XSS
$out->addHTML( $html );
if ( $cacheKey !== null ) {

2
includes/api/ApiParse.php
View file

@ -502,6 +502,7 @@ class ApiParse extends ApiBase {
'wrapperDivClass' => $params['wrapoutputclass'],
'deduplicateStyles' => !$params['disablestylededuplication'],
'skin' => $skin,
'includeLimitReport' => !$params['disablepp'] && !$params['disablelimitreport']
] );
$result_array[ApiResult::META_BC_SUBELEMENTS][] = 'text';
if ( $context ) {
@ -717,7 +718,6 @@ class ApiParse extends ApiBase {
* @return array [ ParserOptions, ScopedCallback, bool $suppressCache ]
*/
private function tweakParserOptions( ParserOptions $popts, Title $title, array $params ) {
$popts->enableLimitReport( !$params['disablepp'] && !$params['disablelimitreport'] );
$popts->setIsPreview( $params['preview'] || $params['sectionpreview'] );
$popts->setIsSectionPreview( $params['sectionpreview'] );

2
includes/page/Article.php
View file

@ -511,6 +511,8 @@ class Article implements Page {
if ( $this->viewIsRenderAction ) {
$poOptions += [ 'absoluteURLs' => true ];
}
$poOptions += [ 'includeLimitReport' => true ];
$continue =
$this->generateContentOutput( $user, $parserOptions, $oldid, $outputPage, $poOptions );

53
includes/parser/Parser.php
View file

@ -675,9 +675,7 @@ class Parser {
$this->currentRevisionCache = null;
$this->mInputSize = strlen( $text );
if ( $this->mOptions->getEnableLimitReport() ) {
$this->mOutput->resetParseStartTime();
}
$this->mOutput->resetParseStartTime();
$oldRevisionId = $this->mRevisionId;
$oldRevisionRecordObject = $this->mRevisionRecordObject;
@ -730,8 +728,8 @@ class Parser {
}
# Information on limits, for the benefit of users who try to skirt them
if ( $this->mOptions->getEnableLimitReport() ) {
$text .= $this->makeLimitReport();
if ( MediaWikiServices::getInstance()->getMainConfig()->get( 'EnableParserLimitReporting' ) ) {
$this->makeLimitReport();
}
# Wrap non-interface parser output in a <div> so it can be targeted
@ -755,10 +753,7 @@ class Parser {
}
/**
* Set the limit report data in the current ParserOutput, and return the
* limit report HTML comment.
*
* @return string
* Set the limit report data in the current ParserOutput.
*/
protected function makeLimitReport() {
$maxIncludeSize = $this->mOptions->getMaxIncludeSize();
@ -797,42 +792,6 @@ class Parser {
$this->hookRunner->onParserLimitReportPrepare( $this, $this->mOutput );
$limitReport = "NewPP limit report\n";
if ( $this->svcOptions->get( 'ShowHostnames' ) ) {
$limitReport .= 'Parsed by ' . wfHostname() . "\n";
}
$limitReport .= 'Cached time: ' . $this->mOutput->getCacheTime() . "\n";
$limitReport .= 'Cache expiry: ' . $this->mOutput->getCacheExpiry() . "\n";
$limitReport .= 'Reduced expiry: ' .
( $this->mOutput->hasReducedExpiry() ? 'true' : 'false' ) .
"\n";
$limitReport .= 'Complications: [' . implode( ', ', $this->mOutput->getAllFlags() ) . "]\n";
foreach ( $this->mOutput->getLimitReportData() as $key => $value ) {
if ( $this->hookRunner->onParserLimitReportFormat(
$key, $value, $limitReport, false, false )
) {
$keyMsg = wfMessage( $key )->inLanguage( 'en' )->useDatabase( false );
$valueMsg = wfMessage( [ "$key-value-text", "$key-value" ] )
->inLanguage( 'en' )->useDatabase( false );
if ( !$valueMsg->exists() ) {
$valueMsg = new RawMessage( '$1' );
}
if ( !$keyMsg->isDisabled() && !$valueMsg->isDisabled() ) {
$valueMsg->params( $value );
$limitReport .= "{$keyMsg->text()}: {$valueMsg->text()}\n";
}
}
}
// Since we're not really outputting HTML, decode the entities and
// then re-encode the things that need hiding inside HTML comments.
$limitReport = htmlspecialchars_decode( $limitReport );
// Sanitize for comment. Note '‐' in the replacement is U+2010,
// which looks much like the problematic '-'.
$limitReport = str_replace( [ '-', '&' ], [ '‐', '&amp;' ], $limitReport );
$text = "\n<!-- \n$limitReport-->\n";
// Add on template profiling data in human/machine readable way
$dataByFunc = $this->mProfiler->getFunctionStats();
uasort( $dataByFunc, static function ( $a, $b ) {
@ -844,8 +803,6 @@ class Parser {
$item['%real'], $item['real'], $item['calls'],
htmlspecialchars( $item['name'] ) );
}
$text .= "<!--\nTransclusion expansion time report (%,ms,calls,template)\n";
$text .= implode( "\n", $profileReport ) . "\n-->\n";
$this->mOutput->setLimitReportData( 'limitreport-timingprofile', $profileReport );
@ -859,8 +816,6 @@ class Parser {
$this->mOutput->getCacheExpiry() );
$this->mOutput->setLimitReportData( 'cachereport-transientcontent',
$this->mOutput->hasDynamicContent() );
return $text;
}
/**

52
includes/parser/ParserOptions.php
View file

@ -476,20 +476,26 @@ class ParserOptions {
}
/**
* @deprecated since 1.38. This does nothing now, to control limit reporting
* please provide 'includeLimitReport' option to ParserOutput::getText.
*
* Enable limit report in an HTML comment on output
* @return bool
*/
public function getEnableLimitReport() {
return $this->getOption( 'enableLimitReport' );
return false;
}
/**
* @deprecated since 1.38. This does nothing now, to control limit reporting
* please provide 'includeLimitReport' option to ParserOutput::getText.
*
* Enable limit report in an HTML comment on output
* @param bool|null $x New value (null is no change)
* @return bool Old value
*/
public function enableLimitReport( $x = true ) {
return $this->setOptionLegacy( 'enableLimitReport', $x );
return false;
}
/**
@ -1006,8 +1012,6 @@ class ParserOptions {
}
/**
* @warning For interaction with the parser cache, use
* WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
* @param UserIdentity $user
* @param Language|null $lang
*/
@ -1022,8 +1026,6 @@ class ParserOptions {
/**
* Get a ParserOptions object for an anonymous user
* @warning For interaction with the parser cache, use
* WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
* @since 1.27
* @return ParserOptions
*/
@ -1036,8 +1038,6 @@ class ParserOptions {
* Get a ParserOptions object from a given user.
* Language will be taken from $wgLang.
*
* @warning For interaction with the parser cache, use
* WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
* @param UserIdentity $user
* @return ParserOptions
*/
@ -1048,8 +1048,6 @@ class ParserOptions {
/**
* Get a ParserOptions object from a given user and language
*
* @warning For interaction with the parser cache, use
* WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
* @param UserIdentity $user
* @param Language $lang
* @return ParserOptions
@ -1061,8 +1059,6 @@ class ParserOptions {
/**
* Get a ParserOptions object from a IContextSource object
*
* @warning For interaction with the parser cache, use
* WikiPage::makeParserOptions() or ParserOptions::newCanonical() instead.
* @param IContextSource $context
* @return ParserOptions
*/
@ -1079,6 +1075,8 @@ class ParserOptions {
* @since 1.30
* @since 1.32 Added string and IContextSource as options for the first parameter
* @since 1.36 UserIdentity is also allowed
* @deprecated since 1.38. Use ::newFromContext, ::newFromAnon or ::newFromUserAndLang instead.
* Canonical ParserOptions are now exactly the same as non-canonical.
* @param IContextSource|string|UserIdentity $context
* - If an IContextSource, the options are initialized based on the source's UserIdentity and Language.
* - If the string 'canonical', the options are initialized with an anonymous user and
@ -1100,10 +1098,6 @@ class ParserOptions {
'$context must be an IContextSource, the string "canonical", or a UserIdentity'
);
}
foreach ( self::getCanonicalOverrides() as $k => $v ) {
$ret->setOption( $k, $v );
}
return $ret;
}
@ -1122,9 +1116,8 @@ class ParserOptions {
/**
* Get default option values
* @warning If you change the default for an existing option (unless it's
* being overridden by self::getCanonicalOverrides()), all existing parser
* cache entries will be invalid. To avoid bugs, you'll need to handle
* @warning If you change the default for an existing option, all existing
* parser cache entries will be invalid. To avoid bugs, you'll need to handle
* that somehow (e.g. with the RejectParserCacheValue hook) because
* MediaWiki won't do it for you.
* @return array
@ -1198,23 +1191,6 @@ class ParserOptions {
];
}
/**
* Get "canonical" non-default option values
* @see self::newCanonical
* @warning If you change the override for an existing option, all existing
* parser cache entries will be invalid. To avoid bugs, you'll need to
* handle that somehow (e.g. with the RejectParserCacheValue hook) because
* MediaWiki won't do it for you.
* @return array
*/
private static function getCanonicalOverrides() {
global $wgEnableParserLimitReporting;
return [
'enableLimitReport' => $wgEnableParserLimitReporting,
];
}
/**
* Get user options
*
@ -1382,7 +1358,7 @@ class ParserOptions {
}
$options = $this->options;
$defaults = self::getCanonicalOverrides() + self::getDefaults();
$defaults = self::getDefaults();
// We only include used options with non-canonical values in the key
// so adding a new option doesn't invalidate the entire parser cache.
@ -1435,7 +1411,7 @@ class ParserOptions {
* @since 1.30
*/
public function isSafeToCache( array $usedOptions = null ) {
$defaults = self::getCanonicalOverrides() + self::getDefaults();
$defaults = self::getDefaults();
$inCacheKey = self::getCacheVaryingOptionsHash();
$usedOptions = $usedOptions ?? array_keys( $this->options );
foreach ( $usedOptions as $option ) {

79
includes/parser/ParserOutput.php
View file

@ -354,6 +354,7 @@ class ParserOutput extends CacheTime {
* the scheme-specific-part of the href is the (percent-encoded) value
* of the `data-mw-deduplicate` attribute.
* - absoluteURLs: (bool) use absolute URLs in all links. Default: false
* - includeLimitReport: (bool) render PP limit report in HTML. Default: false
* @return string HTML
* @return-taint escaped
*/
@ -366,9 +367,14 @@ class ParserOutput extends CacheTime {
'unwrap' => false,
'deduplicateStyles' => true,
'wrapperDivClass' => $this->getWrapperDivClass(),
'includeLimitReport' => false,
];
$text = $this->getRawText();
if ( $options['includeLimitReport'] ) {
$text .= $this->renderLimitReport();
}
Hooks::runner()->onParserOutputPostCacheTransform( $this, $text, $options );
if ( $options['wrapperDivClass'] !== '' && !$options['unwrap'] ) {
@ -1603,6 +1609,79 @@ class ParserOutput extends CacheTime {
}
}
private function renderLimitReport(): string {
$limitReportData = $this->getLimitReportData();
// If nothing set it, we can't get it.
if ( !$limitReportData ) {
return '';
}
$limitReport = "NewPP limit report\n";
if ( array_key_exists( 'cachereport-origin', $limitReportData ) ) {
$limitReport .= "Parsed by {$limitReportData['cachereport-origin']}\n";
}
if ( array_key_exists( 'cachereport-timestamp', $limitReportData ) ) {
$limitReport .= "Cached time: {$limitReportData['cachereport-timestamp']}\n";
}
if ( array_key_exists( 'cachereport-ttl', $limitReportData ) ) {
$limitReport .= "Cache expiry: {$limitReportData['cachereport-ttl']}\n";
}
if ( array_key_exists( 'cachereport-transientcontent', $limitReportData ) ) {
$transient = $limitReportData['cachereport-transientcontent'] ? 'true' : 'false';
$limitReport .= "Reduced expiry: $transient\n";
}
// TODO: flags should go into limit report too.
$limitReport .= 'Complications: [' . implode( ', ', $this->getAllFlags() ) . "]\n";
foreach ( $limitReportData as $key => $value ) {
if ( in_array( $key, [
'cachereport-origin',
'cachereport-timestamp',
'cachereport-ttl',
'cachereport-transientcontent',
'limitreport-timingprofile'
] ) ) {
// These keys are processed separately.
continue;
}
if ( Hooks::runner()->onParserLimitReportFormat(
$key, $value, $limitReport, false, false )
) {
$keyMsg = wfMessage( $key )->inLanguage( 'en' )->useDatabase( false );
$valueMsg = wfMessage( [ "$key-value-text", "$key-value" ] )
->inLanguage( 'en' )->useDatabase( false );
if ( !$valueMsg->exists() ) {
$valueMsg = new RawMessage( '$1' );
}
if ( !$keyMsg->isDisabled() && !$valueMsg->isDisabled() ) {
$valueMsg->params( $value );
$limitReport .= "{$keyMsg->text()}: {$valueMsg->text()}\n";
}
}
}
// Since we're not really outputting HTML, decode the entities and
// then re-encode the things that need hiding inside HTML comments.
$limitReport = htmlspecialchars_decode( $limitReport );
// Sanitize for comment. Note '‐' in the replacement is U+2010,
// which looks much like the problematic '-'.
$limitReport = str_replace( [ '-', '&' ], [ '‐', '&amp;' ], $limitReport );
$text = "\n<!-- \n$limitReport-->\n";
$profileReport = $limitReportData['limitreport-timingprofile'] ?? null;
if ( $profileReport ) {
$text .= "<!--\nTransclusion expansion time report (%,ms,calls,template)\n";
$text .= implode( "\n", $profileReport ) . "\n-->\n";
}
return $text;
}
/**
* Check whether the cache TTL was lowered from the site default.
*

5
tests/phpunit/includes/api/ApiParseTest.php
View file

@ -100,6 +100,9 @@ class ApiParseTest extends ApiTestCase {
$html = substr( $html, strlen( $expectedStart ) );
$possibleParserCache = '/\n<!-- Saved in (?>parser cache|RevisionOutputCache) (?>.*?\n -->)\n/';
$html = preg_replace( $possibleParserCache, '', $html );
if ( $res[1]->getBool( 'disablelimitreport' ) ) {
$expectedEnd = "</div>";
$this->assertSame( $expectedEnd, substr( $html, -strlen( $expectedEnd ) ) );
@ -112,7 +115,7 @@ class ApiParseTest extends ApiTestCase {
} else {
$expectedEnd = '#\n<!-- \nNewPP limit report\n(?>.+?\n-->)\n' .
'<!--\nTransclusion expansion time report \(%,ms,calls,template\)\n(?>.*?\n-->)\n' .
'(\n<!-- Saved in (?>parser cache|RevisionOutputCache) (?>.*?\n -->)\n)?</div>$#s';
'</div>$#s';
$this->assertRegExp( $expectedEnd, $html );
$html = preg_replace( $expectedEnd, '', $html );

2
tests/phpunit/includes/parser/ParserMethodsTest.php
View file

@ -323,8 +323,6 @@ class ParserMethodsTest extends MediaWikiLangTestCase {
'language' => MediaWikiServices::getInstance()->getLanguageFactory()->getLanguage( 'en' )
] );
$po->enableLimitReport( false );
$oldRevision = new MutableRevisionRecord( $title );
$oldRevision->setId( 100 );
$oldRevision->setUser( new UserIdentityValue( 7, 'OldAuthor' ) );

12
tests/phpunit/includes/parser/ParserOptionsTest.php
View file

@ -90,8 +90,6 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
}
public static function provideIsSafeToCache() {
global $wgEnableParserLimitReporting;
$seven = static function () {
return 7;
};
@ -127,12 +125,6 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
'Callback not default' => [ true, [
'speculativeRevIdCallback' => $seven,
] ],
'Canonical override, not default (1)' => [ true, [
'enableLimitReport' => $wgEnableParserLimitReporting,
] ],
'Canonical override, not default (2)' => [ false, [
'enableLimitReport' => !$wgEnableParserLimitReporting,
] ],
];
}
@ -273,10 +265,6 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
$popt2 = ParserOptions::newCanonical( 'canonical' );
$this->assertTrue( $popt1->matches( $popt2 ) );
$popt1->enableLimitReport( true );
$popt2->enableLimitReport( false );
$this->assertTrue( $popt1->matches( $popt2 ) );
$popt2->setInterfaceMessage( !$popt2->getInterfaceMessage() );
$this->assertFalse( $popt1->matches( $popt2 ) );