diff --git a/autoload.php b/autoload.php index 37e87ea7d44..60b748bf079 100644 --- a/autoload.php +++ b/autoload.php @@ -1085,6 +1085,7 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Languages\\LanguageNameUtils' => __DIR__ . '/includes/language/LanguageNameUtils.php', 'MediaWiki\\Parser\\ParserCacheFactory' => __DIR__ . '/includes/parser/ParserCacheFactory.php', 'MediaWiki\\Parser\\ParserCacheMetadata' => __DIR__ . '/includes/parser/ParserCacheMetadata.php', + 'MediaWiki\\Parser\\ParserObserver' => __DIR__ . '/includes/parser/ParserObserver.php', 'MediaWiki\\Parser\\RevisionOutputCache' => __DIR__ . '/includes/parser/RevisionOutputCache.php', 'MediaWiki\\ProcOpenError' => __DIR__ . '/includes/exception/ProcOpenError.php', 'MediaWiki\\ShellDisabledError' => __DIR__ . '/includes/exception/ShellDisabledError.php', diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 70b812a2bb8..140e88f7f54 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -102,6 +102,7 @@ use MediaWiki\Page\ParserOutputAccess; use MediaWiki\Page\RollbackPageFactory; use MediaWiki\Page\WikiPageFactory; use MediaWiki\Parser\ParserCacheFactory; +use MediaWiki\Parser\ParserObserver; use MediaWiki\Permissions\GrantsInfo; use MediaWiki\Permissions\GrantsLocalization; use MediaWiki\Permissions\GroupPermissionsLookup; @@ -1939,6 +1940,10 @@ return [ ); }, + '_ParserObserver' => static function ( MediaWikiServices $services ): ParserObserver { + return new ParserObserver( LoggerFactory::getInstance( 'DuplicateParse' ) ); + }, + '_SqlBlobStore' => static function ( MediaWikiServices $services ): SqlBlobStore { return $services->getBlobStoreFactory()->newSqlBlobStore(); }, diff --git a/includes/content/AbstractContent.php b/includes/content/AbstractContent.php index 59b4b6d3654..23212b1d3ce 100644 --- a/includes/content/AbstractContent.php +++ b/includes/content/AbstractContent.php @@ -535,24 +535,30 @@ abstract class AbstractContent implements Content { $options = ParserOptions::newCanonical( 'canonical' ); } - $po = new ParserOutput(); - $options->registerWatcher( [ $po, 'recordOption' ] ); + $output = new ParserOutput(); + $options->registerWatcher( [ $output, 'recordOption' ] ); if ( Hooks::runner()->onContentGetParserOutput( - $this, $title, $revId, $options, $generateHtml, $po ) + $this, $title, $revId, $options, $generateHtml, $output ) ) { // Save and restore the old value, just in case something is reusing // the ParserOptions object in some weird way. $oldRedir = $options->getRedirectTarget(); $options->setRedirectTarget( $this->getRedirectTarget() ); - $this->fillParserOutput( $title, $revId, $options, $generateHtml, $po ); + $this->fillParserOutput( $title, $revId, $options, $generateHtml, $output ); + MediaWikiServices::getInstance()->get( '_ParserObserver' )->notifyParse( + $title, + $revId, + $options, + $output + ); $options->setRedirectTarget( $oldRedir ); } - Hooks::runner()->onContentAlterParserOutput( $this, $title, $po ); + Hooks::runner()->onContentAlterParserOutput( $this, $title, $output ); $options->registerWatcher( null ); - return $po; + return $output; } /** diff --git a/includes/content/WikitextContent.php b/includes/content/WikitextContent.php index d419c683a1a..e7c2b1ecb04 100644 --- a/includes/content/WikitextContent.php +++ b/includes/content/WikitextContent.php @@ -25,7 +25,6 @@ * @author Daniel Kinzler */ -use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; /** @@ -42,11 +41,6 @@ class WikitextContent extends TextContent { */ private $preSaveTransformFlags = []; - /** - * @var string|null Stack trace of the previous parse - */ - private $previousParseStackTrace = null; - /** * @stable to call * @@ -300,28 +294,6 @@ class WikitextContent extends TextContent { protected function fillParserOutput( Title $title, $revId, ParserOptions $options, $generateHtml, ParserOutput &$output ) { - $stackTrace = ( new RuntimeException() )->getTraceAsString(); - if ( $this->previousParseStackTrace ) { - // NOTE: there may be legitimate changes to re-parse the same WikiText content, - // e.g. if predicted revision ID for the REVISIONID magic word mismatched. - // But that should be rare. - $logger = LoggerFactory::getInstance( 'DuplicateParse' ); - $logger->debug( - __METHOD__ . ': Possibly redundant parse!', - [ - 'title' => $title->getPrefixedDBkey(), - 'rev' => $revId, - 'options-hash' => $options->optionsHash( - ParserOptions::allCacheVaryingOptions(), - $title - ), - 'trace' => $stackTrace, - 'previous-trace' => $this->previousParseStackTrace, - ] - ); - } - $this->previousParseStackTrace = $stackTrace; - list( $redir, $text ) = $this->getRedirectTargetAndText(); $output = MediaWikiServices::getInstance()->getParser() ->parse( $text, $title, $options, true, true, $revId ); diff --git a/includes/parser/ParserObserver.php b/includes/parser/ParserObserver.php new file mode 100644 index 00000000000..dcffc81d21c --- /dev/null +++ b/includes/parser/ParserObserver.php @@ -0,0 +1,114 @@ +logger = $logger; + $this->previousParseStackTraces = []; + } + + /** + * @param PageReference $page + * @param int|null $revId + * @param ParserOptions $options + * @param ParserOutput $output + */ + public function notifyParse( + PageReference $page, ?int $revId, ParserOptions $options, ParserOutput $output + ) { + $pageKey = CacheKeyHelper::getKeyForPage( $page ); + + $optionsHash = $options->optionsHash( + $output->getUsedOptions(), + Title::castFromPageReference( $page ) + ); + + $index = $this->getParseId( $pageKey, $revId, $optionsHash ); + + $stackTrace = ( new RuntimeException() )->getTraceAsString(); + if ( array_key_exists( $index, $this->previousParseStackTraces ) ) { + + // NOTE: there may be legitimate changes to re-parse the same WikiText content, + // e.g. if predicted revision ID for the REVISIONID magic word mismatched. + // But that should be rare. + $this->logger->debug( + __METHOD__ . ': Possibly redundant parse!', + [ + 'page' => $pageKey, + 'rev' => $revId, + 'options-hash' => $optionsHash, + 'trace' => $stackTrace, + 'previous-trace' => $this->previousParseStackTraces[$index], + ] + ); + } + $this->previousParseStackTraces[$index] = $stackTrace; + } + + /** + * @param string $titleStr + * @param int|null $revId + * @param string $optionsHash + * @return string + */ + private function getParseId( string $titleStr, ?int $revId, string $optionsHash ): string { + // $revId may be null when previewing a new page + $revIdStr = $revId ?? ""; + + return "$titleStr.$revIdStr.$optionsHash"; + } + +} diff --git a/tests/phpunit/integration/includes/parser/ParserObserverIntegrationTest.php b/tests/phpunit/integration/includes/parser/ParserObserverIntegrationTest.php new file mode 100644 index 00000000000..ee4dd3ec2e5 --- /dev/null +++ b/tests/phpunit/integration/includes/parser/ParserObserverIntegrationTest.php @@ -0,0 +1,36 @@ +setService( '_ParserObserver', $observer ); + + // Create a test page. Parse it twice if a duplicate is desired, or once otherwise. + $page = $this->getExistingTestPage(); + $page->getContent()->getParserOutput( $page->getTitle() ); + if ( $duplicate ) { + $page->getContent()->getParserOutput( $page->getTitle() ); + } + + $this->assertCount( $count, $logger->getBuffer() ); + } + + public function provideDuplicateParse() { + yield [ true, 1 ]; + yield [ false, 0 ]; + } +} diff --git a/tests/phpunit/unit/includes/parser/ParserObserverTest.php b/tests/phpunit/unit/includes/parser/ParserObserverTest.php new file mode 100644 index 00000000000..bf6f396109d --- /dev/null +++ b/tests/phpunit/unit/includes/parser/ParserObserverTest.php @@ -0,0 +1,53 @@ +createNoOpMock( ParserOptions::class, [ 'optionsHash' ] ); + $options->method( 'optionsHash' )->willReturnOnConsecutiveCalls( $hashOne, $hashTwo ); + + $output = new ParserOutput(); + $observer = new ParserObserver( $logger ); + $observer->notifyParse( $title, null, $options, $output ); + $observer->notifyParse( $title, null, $options, $output ); + + $this->assertArrayEquals( $expects, $logger->getBuffer() ); + } + + public function provideDuplicateParse() { + yield [ + 'foo', + 'bar', + [] + ]; + yield [ + 'foo', + 'foo', + [ + [ + 'debug', + 'MediaWiki\Parser\ParserObserver::notifyParse: Possibly redundant parse!' + ] + ] + ]; + } +}