Merge "MessageCache: replace should actually replace, not reload"

diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php
index 733b1b7..4e0d0a7 100644 (file)
--- a/includes/cache/MessageCache.php
+++ b/includes/cache/MessageCache.php
@@ -619,11 +619,17 @@ class MessageCache {
                        return;
                }
 
-               // Reload messages from the database and pre-populate dc-local caches
-               // as optimisation. Use the master DB to avoid race conditions.
-               $cache = $this->loadFromDB( $code, self::FOR_UPDATE );
+               // Load the existing cache to update it in the local DC cache.
+               // The other DCs will see a hash mismatch.
+               if ( $this->load( $code, self::FOR_UPDATE ) ) {
+                       $cache = $this->cache->get( $code );
+               } else {
+                       // Err? Fall back to loading from the database.
+                       $cache = $this->loadFromDB( $code, self::FOR_UPDATE );
+               }
                // Check if individual cache keys should exist and update cache accordingly
                $newTextByTitle = []; // map of (title => content)
+               $newBigTitles = []; // map of (title => latest revision ID), like EXCESSIVE in loadFromDB()
                foreach ( $replacements as list( $title ) ) {
                        $page = WikiPage::factory( Title::makeTitle( NS_MEDIAWIKI, $title ) );
                        $page->loadPageData( $page::READ_LATEST );
@@ -631,15 +637,29 @@ class MessageCache {
                        // Remember the text for the blob store update later on
                        $newTextByTitle[$title] = $text;
                        // Note that if $text is false, then $cache should have a !NONEXISTANT entry
-                       if ( is_string( $text ) && strlen( $text ) > $wgMaxMsgCacheEntrySize ) {
-                               // Match logic of loadCachedMessagePageEntry()
-                               $this->wanCache->set(
-                                       $this->bigMessageCacheKey( $cache['HASH'], $title ),
-                                       ' ' . $text,
-                                       $this->mExpiry
-                               );
+                       if ( !is_string( $text ) ) {
+                               $cache[$title] = '!NONEXISTENT';
+                       } elseif ( strlen( $text ) > $wgMaxMsgCacheEntrySize ) {
+                               $cache[$title] = '!TOO BIG';
+                               $newBigTitles[$title] = $page->getLatest();
+                       } else {
+                               $cache[$title] = ' ' . $text;
                        }
                }
+               // Update HASH for the new key. Incorporates various administrative keys,
+               // including the old HASH (and thereby the EXCESSIVE value from loadFromDB()
+               // and previous replace() calls), but that doesn't really matter since we
+               // only ever compare it for equality with a copy saved by saveToCaches().
+               $cache['HASH'] = md5( serialize( $cache + [ 'EXCESSIVE' => $newBigTitles ] ) );
+               // Update the too-big WAN cache entries now that we have the new HASH
+               foreach ( $newBigTitles as $title => $id ) {
+                       // Match logic of loadCachedMessagePageEntry()
+                       $this->wanCache->set(
+                               $this->bigMessageCacheKey( $cache['HASH'], $title ),
+                               ' ' . $newTextByTitle[$title],
+                               $this->mExpiry
+                       );
+               }
                // Mark this cache as definitely being "latest" (non-volatile) so
                // load() calls do not try to refresh the cache with replica DB data
                $cache['LATEST'] = time();

diff --git a/tests/phpunit/includes/cache/MessageCacheTest.php b/tests/phpunit/includes/cache/MessageCacheTest.php
index 16448ee..661f325 100644 (file)
--- a/tests/phpunit/includes/cache/MessageCacheTest.php
+++ b/tests/phpunit/includes/cache/MessageCacheTest.php
@@ -129,6 +129,51 @@ class MessageCacheTest extends MediaWikiLangTestCase {
                $this->assertEquals( $oldText, $messageCache->get( $message ), 'Content restored' );
        }
 
+       public function testReplaceCache() {
+               global $wgWANObjectCaches;
+
+               // We need a WAN cache for this.
+               $this->setMwGlobals( [
+                       'wgMainWANCache' => 'hash',
+                       'wgWANObjectCaches' => $wgWANObjectCaches + [
+                               'hash' => [
+                                       'class'    => WANObjectCache::class,
+                                       'cacheId'  => 'hash',
+                                       'channels' => []
+                               ]
+                       ]
+               ] );
+               $this->overrideMwServices();
+
+               MessageCache::destroyInstance();
+               $messageCache = MessageCache::singleton();
+               $messageCache->enable();
+
+               // Populate one key
+               $this->makePage( 'Key1', 'de', 'Value1' );
+               $this->assertEquals( 0,
+                       DeferredUpdates::pendingUpdatesCount(),
+                       'Post-commit deferred update triggers a run of all updates' );
+               $this->assertEquals( 'Value1', $messageCache->get( 'Key1' ), 'Key1 was successfully edited' );
+
+               // Screw up the database so MessageCache::loadFromDB() will
+               // produce the wrong result for reloading Key1
+               $this->db->delete(
+                       'page', [ 'page_namespace' => NS_MEDIAWIKI, 'page_title' => 'Key1' ], __METHOD__
+               );
+
+               // Populate the second key
+               $this->makePage( 'Key2', 'de', 'Value2' );
+               $this->assertEquals( 0,
+                       DeferredUpdates::pendingUpdatesCount(),
+                       'Post-commit deferred update triggers a run of all updates' );
+               $this->assertEquals( 'Value2', $messageCache->get( 'Key2' ), 'Key2 was successfully edited' );
+
+               // Now test that the second edit didn't reload Key1
+               $this->assertEquals( 'Value1', $messageCache->get( 'Key1' ),
+                       'Key1 wasn\'t reloaded by edit of Key2' );
+       }
+
        /**
         * @dataProvider provideNormalizeKey
         */