jenkins-bot has submitted this change and it was merged.

Change subject: resourceloader: Migrate from msg_resource table to object cache
......................................................................


resourceloader: Migrate from msg_resource table to object cache

MessageBlobStore class:
* Make logger aware.
* Log an error if json encoding fails.
* Stop using the DB table. WANObjectCache supports everything we need:
  - Batch retrieval.
  - Invalidate keys with wildcard selects or cascading check keys.
* Update tests slightly since the actual update now happens on-demand as
  part of get() instead of within updateMessage().

ResourceLoader class:
* Remove all interaction with the msg_resource table. Remove db table later.
* Refactor code to use a hash of the blob instead of a timestamp.
  Timestamps are unreliable and roll over too frequently for message blob store
  because there is no authoritative source. The timestamps were inferred based 
on
  when a change is observed. Message overrides from the local wiki have an
  explicit update event when the page is edited. All other messages, such as
  from MediaWiki core and extensions using LocalisationCache, have a single
  timestamp for all messages which rolls over every time the cache is rebuilt.
  A hash is deterministic, and won't cause needless invalidation (T102578).
* Remove redundant pre-fetching in makeModuleResponse().
  This is already done by preloadModuleInfo() in respond().
* Don't bother storing and retreiving empty "{}" objects.
  Instead, detect whether a module's message list is empty at runtime.

ResourceLoaderModule class:
* Make logger aware.
* Log if a module's message blob was not preloaded.

cleanupRemovedModules:
* Now that blobs have a TTL, there's no need to prune old entries.

Bug: T113092
Bug: T92357
Change-Id: Id8c26f41a82597e34013f95294cdc3971a4f52ae
(cherry picked from commit 5d5b269e0e63641f3dacd663d8a39f376a905434)
---
M includes/cache/MessageBlobStore.php
M includes/cache/MessageCache.php
M includes/libs/objectcache/WANObjectCache.php
M includes/resourceloader/ResourceLoader.php
M includes/resourceloader/ResourceLoaderFileModule.php
M includes/resourceloader/ResourceLoaderModule.php
M maintenance/cleanupRemovedModules.php
M tests/phpunit/includes/OutputPageTest.php
M tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
M tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
10 files changed, 266 insertions(+), 419 deletions(-)

Approvals:
  Catrope: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/cache/MessageBlobStore.php 
b/includes/cache/MessageBlobStore.php
index 6054ecc..e8c05a4 100644
--- a/includes/cache/MessageBlobStore.php
+++ b/includes/cache/MessageBlobStore.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * Resource message blobs storage used by ResourceLoader.
+ * Message blobs storage used by ResourceLoader.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -20,34 +20,50 @@
  * @file
  * @author Roan Kattouw
  * @author Trevor Parscal
+ * @author Timo Tijhof
  */
+
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 
 /**
- * This class provides access to the message blobs used by ResourceLoader 
modules.
+ * This class generates message blobs for use by ResourceLoader modules.
  *
- * A message blob is a JSON object containing the interface messages for a
- * certain module in a certain language. These message blobs are cached
- * in the automatically invalidated when one of their constituent messages,
- * or the module definition, is changed.
+ * A message blob is a JSON object containing the interface messages for a 
certain module in
+ * a certain language.
  */
-class MessageBlobStore {
-       /**
-        * In-process cache for message blobs.
-        *
-        * Keyed by language code, then module name.
-        *
-        * @var array
-        */
-       protected $blobCache = array();
+class MessageBlobStore implements LoggerAwareInterface {
 
-       /* @var ResourceLoader */
-       protected $resourceloader;
+       /* @var ResourceLoader|null */
+       private $resourceloader;
 
        /**
-        * @param ResourceLoader $resourceloader
+        * @var LoggerInterface
         */
-       public function __construct( ResourceLoader $resourceloader = null ) {
-               $this->resourceloader = $resourceloader;
+       protected $logger;
+
+       /**
+        * @var WANObjectCache
+        */
+       protected $wanCache;
+
+       /**
+        * @param ResourceLoader $rl
+        * @param LoggerInterface $logger
+        */
+       public function __construct( ResourceLoader $rl = null, LoggerInterface 
$logger = null ) {
+               $this->resourceloader = $rl;
+               $this->logger = $logger ?: new NullLogger();
+               $this->wanCache = ObjectCache::getMainWANInstance();
+       }
+
+       /**
+        * @since 1.27
+        * @param LoggerInterface $logger
+        */
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
        }
 
        /**
@@ -67,55 +83,46 @@
         * Get the message blobs for a set of modules
         *
         * @since 1.27
-        * @param ResourceLoader $resourceLoader
-        * @param array $modules Array of module objects keyed by module name
+        * @param ResourceLoaderModule[] $modules Array of module objects keyed 
by name
         * @param string $lang Language code
         * @return array An array mapping module names to message blobs
         */
-       public function getBlobs( $modules, $lang ) {
-               if ( !count( $modules ) ) {
-                       return array();
+       public function getBlobs( array $modules, $lang ) {
+               // Each cache key for a message blob by module name and 
language code also has a generic
+               // check key without language code. This is used to invalidate 
any and all language subkeys
+               // that exist for a module from the updateMessage() method.
+               $cache = $this->wanCache;
+               $checkKeys = array(
+                       // Global check key, see clear()
+                       $cache->makeKey( __CLASS__ )
+               );
+               $cacheKeys = array();
+               foreach ( $modules as $name => $module ) {
+                       $cacheKey = $this->makeCacheKey( $module, $lang );
+                       $cacheKeys[$name] = $cacheKey;
+                       // Per-module check key, see updateMessage()
+                       $checkKeys[$cacheKey][] = $cache->makeKey( __CLASS__, 
$name );
                }
+               $curTTLs = array();
+               $result = $cache->getMulti( array_values( $cacheKeys ), 
$curTTLs, $checkKeys );
 
                $blobs = array();
-
-               // Try in-process cache
-               $missingFromCache = array();
                foreach ( $modules as $name => $module ) {
-                       if ( isset( $this->blobCache[$lang][$name] ) ) {
-                               $blobs[$name] = $this->blobCache[$lang][$name];
+                       $key = $cacheKeys[$name];
+                       if ( !isset( $result[$key] ) || $curTTLs[$key] === null 
|| $curTTLs[$key] < 0 ) {
+                               $this->logger->info( 'Message blob cache-miss 
for {module}',
+                                       array( 'module' => $name, 'cacheKey' => 
$key )
+                               );
+                               $blobs[$name] = $this->recacheMessageBlob( 
$key, $module, $lang );
                        } else {
-                               $missingFromCache[$name] = $module;
+                               // Use unexpired cache
+                               $blobs[$name] = $result[$key];
                        }
                }
-
-               // Try DB cache
-               if ( $missingFromCache ) {
-                       $blobs += $this->getFromDB( $missingFromCache, $lang );
-               }
-
-               // Generate new blobs for any remaining modules and store in DB
-               $missingFromDb = array_diff( array_keys( $modules ), 
array_keys( $blobs ) );
-               foreach ( $missingFromDb as $name ) {
-                       $blob = $this->insertMessageBlob( $name, 
$modules[$name], $lang );
-                       if ( $blob ) {
-                               $blobs[$name] = $blob;
-                       }
-               }
-
-               // Update in-process cache
-               if ( isset( $this->blobCache[$lang] ) ) {
-                       $this->blobCache[$lang] += $blobs;
-               } else {
-                       $this->blobCache[$lang] = $blobs;
-               }
-
                return $blobs;
        }
 
        /**
-        * Get the message blobs for a set of modules
-        *
         * @deprecated since 1.27 Use getBlobs() instead
         * @return array
         */
@@ -124,281 +131,100 @@
        }
 
        /**
-        * Generate and insert a new message blob. If the blob was already
-        * present, it is not regenerated; instead, the preexisting blob
-        * is fetched and returned.
-        *
-        * @param string $name Module name
-        * @param ResourceLoaderModule $module
-        * @param string $lang Language code
-        * @return string JSON blob
+        * @deprecated since 1.27 Obsolete. Used to populate a cache table in 
the database.
+        * @return bool
         */
        public function insertMessageBlob( $name, ResourceLoaderModule $module, 
$lang ) {
+               return false;
+       }
+
+       /**
+        * @since 1.27
+        * @param ResourceLoaderModule $module
+        * @param string $lang
+        * @return string Cache key
+        */
+       private function makeCacheKey( ResourceLoaderModule $module, $lang ) {
+               $messages = array_values( array_unique( $module->getMessages() 
) );
+               sort( $messages );
+               return $this->wanCache->makeKey( __CLASS__, $module->getName(), 
$lang,
+                       md5( json_encode( $messages ) )
+               );
+       }
+
+       /**
+        * @since 1.27
+        * @param string $cacheKey
+        * @param ResourceLoaderModule $module
+        * @param string $lang
+        * @return string JSON blob
+        */
+       protected function recacheMessageBlob( $cacheKey, ResourceLoaderModule 
$module, $lang ) {
                $blob = $this->generateMessageBlob( $module, $lang );
-
-               if ( !$blob ) {
-                       return false;
-               }
-
-               try {
-                       $dbw = wfGetDB( DB_MASTER );
-                       $success = $dbw->insert( 'msg_resource', array(
-                                       'mr_lang' => $lang,
-                                       'mr_resource' => $name,
-                                       'mr_blob' => $blob,
-                                       'mr_timestamp' => $dbw->timestamp()
-                               ),
-                               __METHOD__,
-                               array( 'IGNORE' )
-                       );
-
-                       if ( $success && $dbw->affectedRows() == 0 ) {
-                               // Blob was already present, fetch it
-                               $blob = $dbw->selectField( 'msg_resource', 
'mr_blob', array(
-                                               'mr_resource' => $name,
-                                               'mr_lang' => $lang,
-                                       ),
-                                       __METHOD__
-                               );
-                       }
-               } catch ( DBError $e ) {
-                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
-               }
+               $cache = $this->wanCache;
+               $cache->set( $cacheKey, $blob,
+                       // Add part of a day to TTL to avoid all modules 
expiring at once
+                       $cache::TTL_WEEK + mt_rand( 0, $cache::TTL_DAY ),
+                       Database::getCacheSetOptions( wfGetDB( DB_SLAVE ) )
+               );
                return $blob;
        }
 
        /**
-        * Update the message blob for a given module in a given language
-        *
-        * @param string $name Module name
-        * @param ResourceLoaderModule $module
-        * @param string $lang Language code
-        * @return string|null Regenerated message blob, or null if there was 
no blob for
-        *   the given module/language pair.
-        */
-       public function updateModule( $name, ResourceLoaderModule $module, 
$lang ) {
-               $dbw = wfGetDB( DB_MASTER );
-               $row = $dbw->selectRow( 'msg_resource', 'mr_blob',
-                       array( 'mr_resource' => $name, 'mr_lang' => $lang ),
-                       __METHOD__
-               );
-               if ( !$row ) {
-                       return null;
-               }
-
-               $newBlob = $this->generateMessageBlob( $module, $lang );
-
-               try {
-                       $newRow = array(
-                               'mr_resource' => $name,
-                               'mr_lang' => $lang,
-                               'mr_blob' => $newBlob,
-                               'mr_timestamp' => $dbw->timestamp()
-                       );
-
-                       $dbw->replace( 'msg_resource',
-                               array( array( 'mr_resource', 'mr_lang' ) ),
-                               $newRow, __METHOD__
-                       );
-               } catch ( Exception $e ) {
-                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
-               }
-               return $newBlob;
-       }
-
-       /**
-        * Update a single message in all message blobs it occurs in.
+        * Invalidate cache keys for modules using this message key.
+        * Called by MessageCache when a message has changed.
         *
         * @param string $key Message key
         */
        public function updateMessage( $key ) {
-               try {
-                       $dbw = wfGetDB( DB_MASTER );
-
-                       // Keep running until the updates queue is empty.
-                       // Due to update conflicts, the queue might not be 
emptied
-                       // in one iteration.
-                       $updates = null;
-                       do {
-                               $updates = $this->getUpdatesForMessage( $key, 
$updates );
-
-                               foreach ( $updates as $k => $update ) {
-                                       // Update the row on the condition that 
it
-                                       // didn't change since we fetched it by 
putting
-                                       // the timestamp in the WHERE clause.
-                                       $success = $dbw->update( 'msg_resource',
-                                               array(
-                                                       'mr_blob' => 
$update['newBlob'],
-                                                       'mr_timestamp' => 
$dbw->timestamp() ),
-                                               array(
-                                                       'mr_resource' => 
$update['resource'],
-                                                       'mr_lang' => 
$update['lang'],
-                                                       'mr_timestamp' => 
$update['timestamp'] ),
-                                               __METHOD__
-                                       );
-
-                                       // Only requeue conflicted updates.
-                                       // If update() returned false, don't 
retry, for
-                                       // fear of getting into an infinite loop
-                                       if ( !( $success && 
$dbw->affectedRows() == 0 ) ) {
-                                               // Not conflicted
-                                               unset( $updates[$k] );
-                                       }
-                               }
-                       } while ( count( $updates ) );
-
-               } catch ( Exception $e ) {
-                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
-               }
-       }
-
-       public function clear() {
-               try {
-                       // Not using TRUNCATE, because that needs extra 
permissions,
-                       // which maybe not granted to the database user.
-                       $dbw = wfGetDB( DB_MASTER );
-                       $dbw->delete( 'msg_resource', '*', __METHOD__ );
-               } catch ( Exception $e ) {
-                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
+               $moduleNames = $this->getResourceLoader()->getModulesByMessage( 
$key );
+               foreach ( $moduleNames as $moduleName ) {
+                       // Uses a holdoff to account for database slave lag 
(for MessageCache)
+                       $this->wanCache->touchCheckKey( 
$this->wanCache->makeKey( __CLASS__, $moduleName ) );
                }
        }
 
        /**
+        * Invalidate cache keys for all known modules.
+        * Called by LocalisationCache after cache is regenerated.
+        */
+       public function clear() {
+               $cache = $this->wanCache;
+               // Disable holdoff because this invalidates all modules and 
also not needed since
+               // LocalisationCache is stored outside the database and doesn't 
have lag.
+               $cache->touchCheckKey( $cache->makeKey( __CLASS__ ), 
$cache::HOLDOFF_NONE );
+       }
+
+       /**
+        * @since 1.27
         * @return ResourceLoader
         */
        protected function getResourceLoader() {
-               // For back-compat this class supports instantiation without 
passing ResourceLoader
+               // Back-compat: This class supports instantiation without a 
ResourceLoader object.
                // Lazy-initialise this property because most callers don't 
need it.
                if ( $this->resourceloader === null ) {
-                       wfDebug( __CLASS__ . ' created without a ResourceLoader 
instance' );
+                       $this->logger->warning( __CLASS__ . ' created without a 
ResourceLoader instance' );
                        $this->resourceloader = new ResourceLoader();
                }
-
                return $this->resourceloader;
        }
 
        /**
-        * Create an update queue for updateMessage()
-        *
-        * @param string $key Message key
-        * @param array $prevUpdates Updates queue to refresh or null to build 
a fresh update queue
-        * @return array Updates queue
-        */
-       private function getUpdatesForMessage( $key, $prevUpdates = null ) {
-               $dbw = wfGetDB( DB_MASTER );
-
-               if ( is_null( $prevUpdates ) ) {
-                       $rl = $this->getResourceLoader();
-                       $moduleNames = $rl->getModulesByMessage( $key );
-                       // Fetch all blobs referencing $key
-                       $res = $dbw->select(
-                               array( 'msg_resource' ),
-                               array( 'mr_resource', 'mr_lang', 'mr_blob', 
'mr_timestamp' ),
-                               array(
-                                       'mr_resource' => $moduleNames,
-                               ),
-                               __METHOD__
-                       );
-               } else {
-                       // Refetch the blobs referenced by $prevUpdates
-
-                       // Reorganize the (resource, lang) pairs in the format
-                       // expected by makeWhereFrom2d()
-                       $twoD = array();
-
-                       foreach ( $prevUpdates as $update ) {
-                               $twoD[$update['resource']][$update['lang']] = 
true;
-                       }
-
-                       $res = $dbw->select( 'msg_resource',
-                               array( 'mr_resource', 'mr_lang', 'mr_blob', 
'mr_timestamp' ),
-                               $dbw->makeWhereFrom2d( $twoD, 'mr_resource', 
'mr_lang' ),
-                               __METHOD__
-                       );
-               }
-
-               // Build the new updates queue
-               $updates = array();
-
-               foreach ( $res as $row ) {
-                       $updates[] = array(
-                               'resource' => $row->mr_resource,
-                               'lang' => $row->mr_lang,
-                               'timestamp' => $row->mr_timestamp,
-                               'newBlob' => $this->reencodeBlob( 
$row->mr_blob, $key, $row->mr_lang )
-                       );
-               }
-
-               return $updates;
-       }
-
-       /**
+        * @since 1.27
         * @param string $key Message key
         * @param string $lang Language code
         * @return string
         */
        protected function fetchMessage( $key, $lang ) {
                $message = wfMessage( $key )->inLanguage( $lang );
+               $value = $message->plain();
                if ( !$message->exists() ) {
-                       wfDebugLog( 'resourceloader', __METHOD__ . " failed to 
find: '$key' ($lang)" );
+                       $this->logger->warning( __METHOD__ . ' failed to find 
{message} ({lang})', array(
+                               'message' => $key,
+                               'lang' => $lang,
+                       ) );
                }
-               return $message->plain();
-       }
-
-       /**
-        * Reencode a message blob with the updated value for a message
-        *
-        * @param string $blob Message blob (JSON object)
-        * @param string $key Message key
-        * @param string $lang Language code
-        * @return string Message blob with $key replaced with its new value
-        */
-       private function reencodeBlob( $blob, $key, $lang ) {
-               $decoded = FormatJson::decode( $blob, true );
-               $decoded[$key] = $this->fetchMessage( $key, $lang );
-               return FormatJson::encode( (object)$decoded );
-       }
-
-       /**
-        * Get the message blobs for a set of modules from the database.
-        * Modules whose blobs are not in the database are silently dropped.
-        *
-        * @param array $modules Array of module objects by name
-        * @param string $lang Language code
-        * @throws MWException
-        * @return array Array mapping module names to blobs
-        */
-       private function getFromDB( $modules, $lang ) {
-               if ( !count( $modules ) ) {
-                       return array();
-               }
-
-               $retval = array();
-               $dbr = wfGetDB( DB_SLAVE );
-               $res = $dbr->select( 'msg_resource',
-                       array( 'mr_blob', 'mr_resource', 'mr_timestamp' ),
-                       array( 'mr_resource' => array_keys( $modules ), 
'mr_lang' => $lang ),
-                       __METHOD__
-               );
-
-               foreach ( $res as $row ) {
-                       if ( !isset( $modules[ $row->mr_resource ] ) ) {
-                               // This shouldn't be possible
-                               throw new MWException( __METHOD__ . ' passed an 
invalid module name' );
-                       }
-                       $module = $modules[ $row->mr_resource ];
-
-                       // Update the module's blob if the list of messages 
changed
-                       $blobKeys = array_keys( FormatJson::decode( 
$row->mr_blob, true ) );
-                       $moduleMsgs = array_values( array_unique( 
$module->getMessages() ) );
-                       if ( $blobKeys !== $moduleMsgs ) {
-                               $retval[$row->mr_resource] = 
$this->updateModule( $row->mr_resource, $module, $lang );
-                       } else {
-                               $retval[$row->mr_resource] = $row->mr_blob;
-                       }
-               }
-
-               return $retval;
+               return $value;
        }
 
        /**
@@ -410,11 +236,18 @@
         */
        private function generateMessageBlob( ResourceLoaderModule $module, 
$lang ) {
                $messages = array();
-
                foreach ( $module->getMessages() as $key ) {
                        $messages[$key] = $this->fetchMessage( $key, $lang );
                }
 
-               return FormatJson::encode( (object)$messages );
+               $json = FormatJson::encode( (object)$messages );
+               if ( $json === false ) {
+                       $this->logger->warning( 'Failed to encode message blob 
for {module} ({lang})', array(
+                               'module' => $module->getName(),
+                               'lang' => $lang,
+                       ) );
+                       $json = '{}';
+               }
+               return $json;
        }
 }
diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php
index 47960ca..24df574 100644
--- a/includes/cache/MessageCache.php
+++ b/includes/cache/MessageCache.php
@@ -98,7 +98,7 @@
         * @return MessageCache
         */
        public static function singleton() {
-               if ( is_null( self::$instance ) ) {
+               if ( self::$instance === null ) {
                        global $wgUseDatabaseMessages, $wgMsgCacheExpiry;
                        self::$instance = new self(
                                wfGetMessageCacheStorage(),
diff --git a/includes/libs/objectcache/WANObjectCache.php 
b/includes/libs/objectcache/WANObjectCache.php
index 8bdafcf..01f8ccc 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -311,11 +311,9 @@
                                : false;
                        if ( $purge === false ) {
                                // Key is not set or invalid; regenerate
-                               $this->cache->add( $timeKey,
-                                       $this->makePurgeValue( $now, 
self::HOLDOFF_TTL ),
-                                       self::CHECK_KEY_TTL
-                               );
-                               $purge = array( self::FLD_TIME => $now, 
self::FLD_HOLDOFF => self::HOLDOFF_TTL );
+                               $newVal = $this->makePurgeValue( $now, 
self::HOLDOFF_TTL );
+                               $this->cache->add( $timeKey, $newVal, 
self::CHECK_KEY_TTL );
+                               $purge = self::parsePurgeValue( $newVal );
                        }
                        $purgeValues[] = $purge;
                }
diff --git a/includes/resourceloader/ResourceLoader.php 
b/includes/resourceloader/ResourceLoader.php
index 97253ab..1f3085a 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -101,11 +101,11 @@
         * requests its own information. This sacrifice of modularity yields a 
substantial
         * performance improvement.
         *
-        * @param array $modules List of module names to preload information for
+        * @param array $moduleNames List of module names to preload 
information for
         * @param ResourceLoaderContext $context Context to load the 
information within
         */
-       public function preloadModuleInfo( array $modules, 
ResourceLoaderContext $context ) {
-               if ( !count( $modules ) ) {
+       public function preloadModuleInfo( array $moduleNames, 
ResourceLoaderContext $context ) {
+               if ( !$moduleNames ) {
                        // Or else Database*::select() will explode, plus it's 
cheaper!
                        return;
                }
@@ -116,11 +116,12 @@
                // Batched version of ResourceLoaderModule::getFileDependencies
                $vary = "$skin|$lang";
                $res = $dbr->select( 'module_deps', array( 'md_module', 
'md_deps' ), array(
-                               'md_module' => $modules,
+                               'md_module' => $moduleNames,
                                'md_skin' => $vary,
                        ), __METHOD__
                );
-               // Prime in-object cache values for each module
+
+               // Prime in-object cache for file dependencies
                $modulesWithDeps = array();
                foreach ( $res as $row ) {
                        $module = $this->getModule( $row->md_module );
@@ -132,41 +133,25 @@
                        }
                }
                // Register the absence of a dependency row too
-               foreach ( array_diff( $modules, $modulesWithDeps ) as $name ) {
+               foreach ( array_diff( $moduleNames, $modulesWithDeps ) as $name 
) {
                        $module = $this->getModule( $name );
                        if ( $module ) {
                                $this->getModule( $name )->setFileDependencies( 
$context, array() );
                        }
                }
 
-               // Get message blob mtimes. Only do this for modules with 
messages
-               $modulesWithMessages = array();
-               foreach ( $modules as $name ) {
+               // Prime in-object cache for message blobs for modules with 
messages
+               $modules = array();
+               foreach ( $moduleNames as $name ) {
                        $module = $this->getModule( $name );
-                       if ( $module && count( $module->getMessages() ) ) {
-                               $modulesWithMessages[] = $name;
+                       if ( $module && $module->getMessages() ) {
+                               $modules[$name] = $module;
                        }
                }
-               $modulesWithoutMessages = array_flip( $modules ); // Will be 
trimmed down by the loop below
-               if ( count( $modulesWithMessages ) ) {
-                       $res = $dbr->select( 'msg_resource', array( 
'mr_resource', 'mr_timestamp' ), array(
-                                       'mr_resource' => $modulesWithMessages,
-                                       'mr_lang' => $lang
-                               ), __METHOD__
-                       );
-                       foreach ( $res as $row ) {
-                               $module = $this->getModule( $row->mr_resource );
-                               if ( $module ) {
-                                       $module->setMsgBlobMtime( $lang, 
wfTimestamp( TS_UNIX, $row->mr_timestamp ) );
-                                       unset( 
$modulesWithoutMessages[$row->mr_resource] );
-                               }
-                       }
-               }
-               foreach ( array_keys( $modulesWithoutMessages ) as $name ) {
-                       $module = $this->getModule( $name );
-                       if ( $module ) {
-                               $module->setMsgBlobMtime( $lang, 1 );
-                       }
+               $store = $this->getMessageBlobStore();
+               $blobs = $store->getBlobs( $modules, $lang );
+               foreach ( $blobs as $name => $blob ) {
+                       $modules[$name]->setMessageBlob( $blob, $lang );
                }
        }
 
@@ -247,10 +232,7 @@
        public function __construct( Config $config = null, LoggerInterface 
$logger = null ) {
                global $IP;
 
-               if ( !$logger ) {
-                       $logger = new NullLogger();
-               }
-               $this->setLogger( $logger );
+               $this->logger = $logger ?: new NullLogger();
 
                if ( !$config ) {
                        $this->logger->debug( __METHOD__ . ' was called without 
providing a Config instance' );
@@ -275,7 +257,7 @@
                        $this->registerTestModules();
                }
 
-               $this->setMessageBlobStore( new MessageBlobStore( $this ) );
+               $this->setMessageBlobStore( new MessageBlobStore( $this, 
$this->logger ) );
        }
 
        /**
@@ -561,6 +543,7 @@
                                /** @var ResourceLoaderModule $object */
                                $object = new $class( $info );
                                $object->setConfig( $this->getConfig() );
+                               $object->setLogger( $this->logger );
                        }
                        $object->setName( $name );
                        $this->modules[$name] = $object;
@@ -679,7 +662,7 @@
                }
 
                try {
-                       // Preload for getCombinedVersion()
+                       // Preload for getCombinedVersion() and for batch 
makeModuleResponse()
                        $this->preloadModuleInfo( array_keys( $modules ), 
$context );
                } catch ( Exception $e ) {
                        MWExceptionHandler::logException( $e );
@@ -977,19 +960,6 @@
                                $this->errors[] = 'Image generation failed';
                        }
                        return $data;
-               }
-
-               // Pre-fetch blobs
-               if ( $context->shouldIncludeMessages() ) {
-                       try {
-                               $this->blobStore->get( $this, $modules, 
$context->getLanguage() );
-                       } catch ( Exception $e ) {
-                               MWExceptionHandler::logException( $e );
-                               $this->logger->warning( 'Prefetching 
MessageBlobStore failed: {exception}', array(
-                                       'exception' => $e
-                               ) );
-                               $this->errors[] = 
self::formatExceptionNoComment( $e );
-                       }
                }
 
                foreach ( $missing as $name ) {
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php 
b/includes/resourceloader/ResourceLoaderFileModule.php
index 1421b10..0f28bfe 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -583,7 +583,7 @@
                $summary[] = array(
                        'options' => $options,
                        'fileHashes' => $this->getFileHashes( $context ),
-                       'msgBlobMtime' => $this->getMsgBlobMtime( 
$context->getLanguage() ),
+                       'messageBlob' => $this->getMessageBlob( $context ),
                );
                return $summary;
        }
diff --git a/includes/resourceloader/ResourceLoaderModule.php 
b/includes/resourceloader/ResourceLoaderModule.php
index fcda87b..113fc84 100644
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -22,10 +22,14 @@
  * @author Roan Kattouw
  */
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 /**
  * Abstraction for ResourceLoader modules, with name registration and maxage 
functionality.
  */
-abstract class ResourceLoaderModule {
+abstract class ResourceLoaderModule implements LoggerAwareInterface {
        # Type of resource
        const TYPE_SCRIPTS = 'scripts';
        const TYPE_STYLES = 'styles';
@@ -60,8 +64,8 @@
 
        // In-object cache for file dependencies
        protected $fileDeps = array();
-       // In-object cache for message blob mtime
-       protected $msgBlobMtime = array();
+       // In-object cache for message blob (keyed by language)
+       protected $msgBlobs = array();
        // In-object cache for version hash
        protected $versionHash = array();
        // In-object cache for module content
@@ -71,6 +75,11 @@
         * @var Config
         */
        protected $config;
+
+       /**
+        * @var LoggerInterface
+        */
+       protected $logger;
 
        /* Methods */
 
@@ -166,6 +175,25 @@
         */
        public function setConfig( Config $config ) {
                $this->config = $config;
+       }
+
+       /**
+        * @since 1.27
+        * @param LoggerInterface $logger
+        */
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
+       }
+
+       /**
+        * @since 1.27
+        * @return LoggerInterface
+        */
+       protected function getLogger() {
+               if ( !$this->logger ) {
+                       $this->logger = new NullLogger();
+               }
+               return $this->logger;
        }
 
        /**
@@ -453,45 +481,40 @@
        }
 
        /**
-        * Get the last modification timestamp of the messages in this module 
for a given language.
-        * @param string $lang Language code
-        * @return int UNIX timestamp
+        * Get the hash of the message blob.
+        *
+        * @since 1.27
+        * @param ResourceLoaderContext $context
+        * @return string|null JSON blob or null if module has no messages
         */
-       public function getMsgBlobMtime( $lang ) {
-               if ( !isset( $this->msgBlobMtime[$lang] ) ) {
-                       if ( !count( $this->getMessages() ) ) {
-                               return 1;
-                       }
-
-                       $dbr = wfGetDB( DB_SLAVE );
-                       $msgBlobMtime = $dbr->selectField( 'msg_resource',
-                               'mr_timestamp',
-                               array(
-                                       'mr_resource' => $this->getName(),
-                                       'mr_lang' => $lang
-                               ),
-                               __METHOD__
-                       );
-                       // If no blob was found, but the module does have 
messages, that means we need
-                       // to regenerate it. Return NOW
-                       if ( $msgBlobMtime === false ) {
-                               $msgBlobMtime = wfTimestampNow();
-                       }
-                       $this->msgBlobMtime[$lang] = wfTimestamp( TS_UNIX, 
$msgBlobMtime );
+       protected function getMessageBlob( ResourceLoaderContext $context ) {
+               if ( !$this->getMessages() ) {
+                       // Don't bother consulting MessageBlobStore
+                       return null;
                }
-               return $this->msgBlobMtime[$lang];
+               // Message blobs may only vary language, not by context keys
+               $lang = $context->getLanguage();
+               if ( !isset( $this->msgBlobs[$lang] ) ) {
+                       $this->getLogger()->warning( 'Message blob for {module} 
should have been preloaded', array(
+                               'module' => $this->getName(),
+                       ) );
+                       $store = 
$context->getResourceLoader()->getMessageBlobStore();
+                       $this->msgBlobs[$lang] = $store->getBlob( $this, $lang 
);
+               }
+               return $this->msgBlobs[$lang];
        }
 
        /**
-        * Set in-object cache for message blob time.
+        * Set in-object cache for message blobs.
         *
-        * This is used to retrieve data in batches. See 
ResourceLoader::preloadModuleInfo().
+        * Used to allow fetching of message blobs in batches. See 
ResourceLoader::preloadModuleInfo().
         *
+        * @since 1.27
+        * @param string|null $blob JSON blob or null
         * @param string $lang Language code
-        * @param int $mtime UNIX timestamp
         */
-       public function setMsgBlobMtime( $lang, $mtime ) {
-               $this->msgBlobMtime[$lang] = $mtime;
+       public function setMessageBlob( $blob, $lang ) {
+               $this->msgBlobs[$lang] = $blob;
        }
 
        /**
@@ -607,13 +630,9 @@
                }
 
                // Messages
-               $blobs = $rl->getMessageBlobStore()->get(
-                       $rl,
-                       array( $this->getName() => $this ),
-                       $context->getLanguage()
-               );
-               if ( isset( $blobs[$this->getName()] ) ) {
-                       $content['messagesBlob'] = $blobs[$this->getName()];
+               $blob = $this->getMessageBlob( $context );
+               if ( $blob ) {
+                       $content['messagesBlob'] = $blob;
                }
 
                $templates = $this->getTemplates();
@@ -746,7 +765,7 @@
         * A number of utility methods are available to help you gather data. 
These are not
         * called by default and must be included by the subclass' 
getDefinitionSummary().
         *
-        * - getMsgBlobMtime()
+        * - getMessageBlob()
         *
         * @since 1.23
         * @param ResourceLoaderContext $context
diff --git a/maintenance/cleanupRemovedModules.php 
b/maintenance/cleanupRemovedModules.php
index ae05930..68f57a3 100644
--- a/maintenance/cleanupRemovedModules.php
+++ b/maintenance/cleanupRemovedModules.php
@@ -58,20 +58,6 @@
                        wfWaitForSlaves();
                } while ( $numRows > 0 );
                $this->output( "done\n" );
-
-               $this->output( "Cleaning up msg_resource table...\n" );
-               $i = 1;
-
-               $mrRes = $dbw->tableName( 'msg_resource' );
-               do {
-                       $where = $moduleList ? "mr_resource NOT IN 
($moduleList)" : '1=1';
-                       $dbw->query( "DELETE FROM $mrRes WHERE $where LIMIT 
$limit", __METHOD__ );
-                       $numRows = $dbw->affectedRows();
-                       $this->output( "Batch $i: $numRows rows\n" );
-                       $i++;
-                       wfWaitForSlaves();
-               } while ( $numRows > 0 );
-               $this->output( "done\n" );
        }
 }
 
diff --git a/tests/phpunit/includes/OutputPageTest.php 
b/tests/phpunit/includes/OutputPageTest.php
index 5f21e07..f5ef016 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -375,7 +375,6 @@
        }
 
        public function updateModule( $name, ResourceLoaderModule $module, 
$lang ) {
-               return;
        }
 
        public function updateMessage( $key ) {
diff --git a/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php 
b/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
index 3de315a..776538a 100644
--- a/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
+++ b/tests/phpunit/includes/resourceloader/MessageBlobStoreTest.php
@@ -1,12 +1,31 @@
 <?php
 
 /**
- * @group Database
  * @group Cache
  * @covers MessageBlobStore
  */
-class MessageBlobStoreTest extends ResourceLoaderTestCase {
-       protected $tablesUsed = array( 'msg_resource' );
+class MessageBlobStoreTest extends PHPUnit_Framework_TestCase {
+
+       protected function setUp() {
+               parent::setUp();
+               // MediaWiki tests defaults $wgMainWANCache to CACHE_NONE.
+               // Use hash instead so that caching is observed
+               $this->wanCache = $this->getMockBuilder( 'WANObjectCache' )
+                       ->setConstructorArgs( array( array(
+                               'cache' => new HashBagOStuff(),
+                               'pool' => 'test',
+                               'relayer' => new EventRelayerNull( array() )
+                       ) ) )
+                       ->setMethods( array( 'makePurgeValue' ) )
+                       ->getMock();
+
+               $this->wanCache->expects( $this->any() )
+                       ->method( 'makePurgeValue' )
+                       ->will( $this->returnCallback( function ( $timestamp, 
$holdoff ) {
+                               // Disable holdoff as it messes with testing
+                               return WANObjectCache::PURGE_VAL_PREFIX . 
(float)$timestamp . ':0';
+                       } ) );
+       }
 
        protected function makeBlobStore( $methods = null, $rl = null ) {
                $blobStore = $this->getMockBuilder( 'MessageBlobStore' )
@@ -14,6 +33,8 @@
                        ->setMethods( $methods )
                        ->getMock();
 
+               $access = TestingAccessWrapper::newFromObject( $blobStore );
+               $access->wanCache = $this->wanCache;
                return $blobStore;
        }
 
@@ -67,9 +88,9 @@
                $rl = new ResourceLoader();
                $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( array( 'fetchMessage' ), $rl 
);
-               $blobStore->expects( $this->exactly( 2 ) )
+               $blobStore->expects( $this->once() )
                        ->method( 'fetchMessage' )
-                       ->will( $this->onConsecutiveCalls( 'First', 'Second' ) 
);
+                       ->will( $this->returnValue( 'First' ) );
 
                $blob = $blobStore->getBlob( $module, 'en' );
                $this->assertEquals( '{"example":"First"}', $blob, 'Generated 
blob' );
@@ -80,9 +101,9 @@
                $rl = new ResourceLoader();
                $rl->register( $module->getName(), $module );
                $blobStore = $this->makeBlobStore( array( 'fetchMessage' ), $rl 
);
-               $blobStore->expects( $this->never() )
+               $blobStore->expects( $this->once() )
                        ->method( 'fetchMessage' )
-                       ->will( $this->returnValue( 'Wrong' ) );
+                       ->will( $this->returnValue( 'Second' ) );
 
                $blob = $blobStore->getBlob( $module, 'en' );
                $this->assertEquals( '{"example":"Second"}', $blob, 'Updated 
blob' );
@@ -122,4 +143,25 @@
                $blob = $blobStore->getBlob( $module, 'en' );
                $this->assertEquals( '{"foo":"Hello","bar":"World"}', $blob, 
'Updated blob' );
        }
+
+       public function testClear() {
+               $module = $this->makeModule( array( 'example' ) );
+               $rl = new ResourceLoader();
+               $rl->register( $module->getName(), $module );
+               $blobStore = $this->makeBlobStore( array( 'fetchMessage' ), $rl 
);
+               $blobStore->expects( $this->exactly( 2 ) )
+                       ->method( 'fetchMessage' )
+                       ->will( $this->onConsecutiveCalls( 'First', 'Second' ) 
);
+
+               $blob = $blobStore->getBlob( $module, 'en' );
+               $this->assertEquals( '{"example":"First"}', $blob, 'Generated 
blob' );
+
+               $blob = $blobStore->getBlob( $module, 'en' );
+               $this->assertEquals( '{"example":"First"}', $blob, 'Cache-hit' 
);
+
+               $blobStore->clear();
+
+               $blob = $blobStore->getBlob( $module, 'en' );
+               $this->assertEquals( '{"example":"Second"}', $blob, 'Updated 
blob' );
+       }
 }
diff --git 
a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php 
b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
index c552b80..9a36d18 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
@@ -5,7 +5,7 @@
        // Version hash for a blank file module.
        // Result of ResourceLoader::makeHash(), ResourceLoaderTestModule
        // and ResourceLoaderFileModule::getDefinitionSummary().
-       protected static $blankVersion = 'wvTifjse';
+       protected static $blankVersion = 'GqV9IPpY';
 
        protected static function expandPlaceholders( $text ) {
                return strtr( $text, array(

-- 
To view, visit https://gerrit.wikimedia.org/r/257966
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id8c26f41a82597e34013f95294cdc3971a4f52ae
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.27.0-wmf.8
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Parent5446 <tylerro...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to