diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php index c401d44eeb1..9ce12b4b134 100644 --- a/includes/Storage/DerivedPageDataUpdater.php +++ b/includes/Storage/DerivedPageDataUpdater.php @@ -151,6 +151,9 @@ class DerivedPageDataUpdater implements IDBAccessObject { */ private $options = [ 'changed' => true, + // newrev is true if prepareUpdate is handling the creation of a new revision, + // as opposed to a null edit or a forced update. + 'newrev' => false, 'created' => false, 'moved' => false, 'restored' => false, @@ -1110,12 +1113,14 @@ class DerivedPageDataUpdater implements IDBAccessObject { // Override fields defined in $this->options with values from $options. $this->options = array_intersect_key( $options, $this->options ) + $this->options; - if ( isset( $this->pageState['oldId'] ) ) { - $oldId = $this->pageState['oldId']; + if ( $this->revision ) { + $oldId = $this->pageState['oldId'] ?? 0; + $this->options['newrev'] = ( $revision->getId() !== $oldId ); } elseif ( isset( $this->options['oldrevision'] ) ) { /** @var Revision|RevisionRecord $oldRev */ $oldRev = $this->options['oldrevision']; $oldId = $oldRev->getId(); + $this->options['newrev'] = ( $revision->getId() !== $oldId ); } else { $oldId = $revision->getParentId(); } @@ -1611,8 +1616,8 @@ class DerivedPageDataUpdater implements IDBAccessObject { // Save it to the parser cache. Use the revision timestamp in the case of a // freshly saved edit, as that matches page_touched and a mismatch would trigger an // unnecessary reparse. - $timestamp = $this->options['changed'] ? $this->revision->getTimestamp() - : $output->getTimestamp(); + $timestamp = $this->options['newrev'] ? $this->revision->getTimestamp() + : $output->getCacheTime(); $this->parserCache->save( $output, $wikiPage, $this->getCanonicalParserOptions(), $timestamp, $this->revision->getId() diff --git a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php index 5f3cba333b6..3339749377b 100644 --- a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php +++ b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php @@ -656,7 +656,7 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase { RevisionSlotsUpdate $update, User $user, $comment, - $id, + $id = 0, $parentId = 0 ) { $rev = new MutableRevisionRecord( $title ); @@ -664,10 +664,13 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase { $rev->applyUpdate( $update ); $rev->setUser( $user ); $rev->setComment( CommentStoreComment::newUnsavedComment( $comment ) ); - $rev->setId( $id ); $rev->setPageId( $title->getArticleID() ); $rev->setParentId( $parentId ); + if ( $id ) { + $rev->setId( $id ); + } + return $rev; } @@ -942,6 +945,79 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase { // TODO: test category membership update (with setRcWatchCategoryMembership()) } + /** + * @covers \MediaWiki\Storage\DerivedPageDataUpdater::doParserCacheUpdate() + */ + public function testDoParserCacheUpdate() { + if ( $this->hasMultiSlotSupport() ) { + MediaWikiServices::getInstance()->getSlotRoleRegistry()->defineRoleWithModel( + 'aux', + CONTENT_MODEL_WIKITEXT + ); + } + + $page = $this->getPage( __METHOD__ ); + $this->createRevision( $page, 'Dummy' ); + + $user = $this->getTestUser()->getUser(); + + $update = new RevisionSlotsUpdate(); + $update->modifyContent( 'main', new WikitextContent( 'first [[Main]]' ) ); + + if ( $this->hasMultiSlotSupport() ) { + $update->modifyContent( 'aux', new WikitextContent( 'Aux [[Nix]]' ) ); + } + + // Emulate update after edit ---------- + $pcache = MediaWikiServices::getInstance()->getParserCache(); + $pcache->deleteOptionsKey( $page ); + + $rev = $this->makeRevision( $page->getTitle(), $update, $user, 'rev', null ); + $rev->setTimestamp( '20100101000000' ); + $rev->setParentId( $page->getLatest() ); + + $updater = $this->getDerivedPageDataUpdater( $page ); + $updater->prepareContent( $user, $update, false ); + + $rev->setId( 11 ); + $updater->prepareUpdate( $rev ); + + // Force the page timestamp, so we notice whether ParserOutput::getTimestamp + // or ParserOutput::getCacheTime are used. + $page->setTimestamp( $rev->getTimestamp() ); + $updater->doParserCacheUpdate(); + + // The cached ParserOutput should not use the revision timestamp + $cached = $pcache->get( $page, $updater->getCanonicalParserOptions(), true ); + $this->assertInternalType( 'object', $cached ); + $this->assertSame( $updater->getCanonicalParserOutput(), $cached ); + + $this->assertSame( $rev->getTimestamp(), $cached->getCacheTime() ); + $this->assertSame( $rev->getId(), $cached->getCacheRevisionId() ); + + // Emulate forced update of an old revision ---------- + $pcache->deleteOptionsKey( $page ); + + $updater = $this->getDerivedPageDataUpdater( $page ); + $updater->prepareUpdate( $rev ); + + // Force the page timestamp, so we notice whether ParserOutput::getTimestamp + // or ParserOutput::getCacheTime are used. + $page->setTimestamp( $rev->getTimestamp() ); + $updater->doParserCacheUpdate(); + + // The cached ParserOutput should not use the revision timestamp + $cached = $pcache->get( $page, $updater->getCanonicalParserOptions(), true ); + $this->assertInternalType( 'object', $cached ); + $this->assertSame( $updater->getCanonicalParserOutput(), $cached ); + + $this->assertGreaterThan( $rev->getTimestamp(), $cached->getCacheTime() ); + $this->assertSame( $rev->getId(), $cached->getCacheRevisionId() ); + } + + /** + * @return bool + */ private function hasMultiSlotSupport() { global $wgMultiContentRevisionSchemaMigrationStage; diff --git a/tests/phpunit/includes/Storage/PageUpdaterTest.php b/tests/phpunit/includes/Storage/PageUpdaterTest.php index 4e090775b5b..89e1d4ee1fd 100644 --- a/tests/phpunit/includes/Storage/PageUpdaterTest.php +++ b/tests/phpunit/includes/Storage/PageUpdaterTest.php @@ -29,6 +29,9 @@ class PageUpdaterTest extends MediaWikiTestCase { 'aux', CONTENT_MODEL_WIKITEXT ); + + $this->tablesUsed[] = 'logging'; + $this->tablesUsed[] = 'recentchanges'; } private function getDummyTitle( $method ) {