Fix the cache timestamp for forced updates.

Without this patch, the forcelinksupdate parameter of ApiPurge
was inoperational, caused by the fact that RefreshLinksJob got
the original revision's timestamp in the rootJobTimestamp parameter,
instead of the time at which the new ParserOutput was created.

See <https://phabricator.wikimedia.org/T210307#4771586> for details.

Bug: T210307
Change-Id: I281d6d0ed112b35e160775e528d363ce4770990a

diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php
index c401d44..9ce12b4 100644 (file)
--- 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 5f3cba3..3339749 100644 (file)
--- 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 4e09077..89e1d4e 100644 (file)
--- 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 ) {