Only load latest revision in MessageCache::loadFromDB
authordaniel <dkinzler@wikimedia.org>
Fri, 22 Mar 2019 12:26:21 +0000 (13:26 +0100)
committerdaniel <dkinzler@wikimedia.org>
Fri, 22 Mar 2019 14:44:14 +0000 (15:44 +0100)
In Id044b8dcd7c, we lost a condition that ensured that the cache would
be populated with the latest revision. Now it was being populated with
all revisions, with a random one winning.

Bug: T218918
Change-Id: I1a47356ea35f0abf35bb1a3489d0d3442a3400a5

includes/cache/MessageCache.php
tests/phpunit/includes/cache/MessageCacheTest.php

index e78dfa1..157d88e 100644 (file)
@@ -540,7 +540,10 @@ class MessageCache {
                $res = $dbr->select(
                        $revQuery['tables'],
                        $revQuery['fields'],
-                       array_merge( $conds, [ 'page_len <= ' . intval( $wgMaxMsgCacheEntrySize ) ] ),
+                       array_merge( $conds, [
+                               'page_len <= ' . intval( $wgMaxMsgCacheEntrySize ),
+                               'page_latest = rev_id' // get the latest revision only
+                       ] ),
                        __METHOD__ . "($code)-small",
                        [],
                        $revQuery['joins']
index c340c08..8fdc1f1 100644 (file)
@@ -1,6 +1,7 @@
 <?php
 
 use MediaWiki\MediaWikiServices;
+use Wikimedia\TestingAccessWrapper;
 
 /**
  * @group Database
@@ -58,6 +59,8 @@ class MessageCacheTest extends MediaWikiLangTestCase {
         * @param string $title Title of page to be created
         * @param string $lang Language and content of the created page
         * @param string|null $content Content of the created page, or null for a generic string
+        *
+        * @return Revision
         */
        protected function makePage( $title, $lang, $content = null ) {
                if ( $content === null ) {
@@ -70,7 +73,11 @@ class MessageCacheTest extends MediaWikiLangTestCase {
                $title = Title::newFromText( $title, NS_MEDIAWIKI );
                $wikiPage = new WikiPage( $title );
                $contentHandler = ContentHandler::makeContent( $content, $title );
-               $wikiPage->doEditContent( $contentHandler, "$lang translation test case" );
+               $status = $wikiPage->doEditContent( $contentHandler, "$lang translation test case" );
+
+               // sanity
+               $this->assertTrue( $status->isOK(), 'Create page ' . $title->getPrefixedDBkey() );
+               return $status->value['revision'];
        }
 
        /**
@@ -213,4 +220,42 @@ class MessageCacheTest extends MediaWikiLangTestCase {
 
                $this->assertEquals( 0, $dbr->trxLevel(), "No DB read queries" );
        }
+
+       /**
+        * Regression test for T218918
+        */
+       public function testLoadFromDB_fetchLatestRevision() {
+               // Create three revisions of the same message page.
+               // Must be an existing message key.
+               $key = 'Log';
+               $this->makePage( $key, 'de', 'Test eins' );
+               $this->makePage( $key, 'de', 'Test zwei' );
+               $r3 = $this->makePage( $key, 'de', 'Test drei' );
+
+               // Create an out-of-sequence revision by importing a
+               // revision with an old timestamp. Hacky.
+               $importRevision = new WikiRevision( new HashConfig() );
+               $importRevision->setTitle( $r3->getTitle() );
+               $importRevision->setComment( 'Imported edit' );
+               $importRevision->setTimestamp( '19991122334455' );
+               $importRevision->setText( 'IMPORTED OLD TEST' );
+               $importRevision->setUsername( 'Alan Smithee' );
+
+               $importer = MediaWikiServices::getInstance()->getWikiRevisionOldRevisionImporterNoUpdates();
+               $importer->import( $importRevision );
+
+               // Now, load the message from the wiki page
+               MessageCache::destroyInstance();
+               $messageCache = MessageCache::singleton();
+               $messageCache->enable();
+               $messageCache = TestingAccessWrapper::newFromObject( $messageCache );
+
+               $cache = $messageCache->loadFromDB( 'de' );
+
+               $this->assertArrayHasKey( $key, $cache );
+
+               // Text in the cache has an extra space in front!
+               $this->assertSame( ' ' . 'Test drei', $cache[$key] );
+       }
+
 }