LocalisationCache: Process one fallback at a time
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 3 Sep 2014 16:52:33 +0000 (12:52 -0400)
committerNiklas Laxström <niklas.laxstrom@gmail.com>
Thu, 4 Sep 2014 14:56:31 +0000 (16:56 +0200)
Currently LocalisationCache merges the core data for all languages in
the fallback chain, then the extension data, then merges those two, and
then gives extensions like LocalisationUpdate a chance to make final
overrides with the LocalisationCacheRecache hook.

But if LocalisationUpdate doesn't want to locally duplicate all the
messages for every language (e.g. r104041), LocalisationCacheRecache is
too late: the information as to whether a message came from the primary
language or a fallback has been lost, so when LU itself has an override
for a fallback language it can't know whether or not the existing
message should be overridden or not.

The solution is for LocalisationCache to gather the data for each
fallback language separately, call a new hook for LU to affect just that
language (LocalisationCacheRecacheFallback), and only then merge the
fallback languages together.

Bug: 68781
Change-Id: Iacfe96063fcc66c1f97ca5e5292a8fc70af988cf

docs/hooks.txt
includes/cache/LocalisationCache.php
tests/phpunit/data/localisationcache/en.json [new file with mode: 0644]
tests/phpunit/data/localisationcache/ru.json [new file with mode: 0644]
tests/phpunit/data/localisationcache/uk.json [new file with mode: 0644]
tests/phpunit/includes/LocalisationCacheTest.php [deleted file]
tests/phpunit/includes/cache/LocalisationCacheTest.php [new file with mode: 0644]

index 82da289..369b777 100644 (file)
@@ -1635,6 +1635,13 @@ $code: language code
 &$alldata: The localisation data from core and extensions
 &purgeBlobs: whether to purge/update the message blobs via MessageBlobStore::clear()
 
+'LocalisationCacheRecacheFallback': Called for each language when merging
+fallback data into the cache.
+$cache: The LocalisationCache object
+$code: language code
+&$alldata: The localisation data from core and extensions. Note some keys may
+  be omitted if they won't be merged into the final result.
+
 'LocalisationChecksBlacklist': When fetching the blacklist of
 localisation checks.
 &$blacklist: array of checks to blacklist. See the bottom of
index bdfe510..bdfc75a 100644 (file)
@@ -843,73 +843,109 @@ class LocalisationCache {
                        if ( $coreData['fallbackSequence'][$len - 1] !== 'en' ) {
                                $coreData['fallbackSequence'][] = 'en';
                        }
+               }
 
-                       # Load the fallback localisation item by item and merge it
-                       foreach ( $coreData['fallbackSequence'] as $fbCode ) {
-                               # Load the secondary localisation from the source file to
-                               # avoid infinite cycles on cyclic fallbacks
-                               $fbData = $this->readSourceFilesAndRegisterDeps( $fbCode, $deps );
-                               if ( $fbData === false ) {
-                                       continue;
-                               }
+               $codeSequence = array_merge( array( $code ), $coreData['fallbackSequence'] );
 
-                               foreach ( self::$allKeys as $key ) {
-                                       if ( !isset( $fbData[$key] ) ) {
-                                               continue;
-                                       }
+               wfProfileIn( __METHOD__ . '-fallbacks' );
+
+               # Load non-JSON localisation data for extensions
+               $extensionData = array_combine(
+                       $codeSequence,
+                       array_fill( 0, count( $codeSequence ), $initialData ) );
+               foreach ( $wgExtensionMessagesFiles as $extension => $fileName ) {
+                       if ( isset( $wgMessagesDirs[$extension] ) ) {
+                               # This extension has JSON message data; skip the PHP shim
+                               continue;
+                       }
+
+                       $data = $this->readPHPFile( $fileName, 'extension' );
+                       $used = false;
 
-                                       if ( is_null( $coreData[$key] ) || $this->isMergeableKey( $key ) ) {
-                                               $this->mergeItem( $key, $coreData[$key], $fbData[$key] );
+                       foreach ( $data as $key => $item ) {
+                               foreach ( $codeSequence as $csCode ) {
+                                       if ( isset( $item[$csCode] ) ) {
+                                               $this->mergeItem( $key, $extensionData[$csCode][$key], $item[$csCode] );
+                                               $used = true;
                                        }
                                }
                        }
-               }
 
-               $codeSequence = array_merge( array( $code ), $coreData['fallbackSequence'] );
+                       if ( $used ) {
+                               $deps[] = new FileDependency( $fileName );
+                       }
+               }
 
-               # Load core messages and the extension localisations.
-               wfProfileIn( __METHOD__ . '-extensions' );
+               # Load the localisation data for each fallback, then merge it into the full array
                $allData = $initialData;
-               foreach ( $wgMessagesDirs as $dirs ) {
-                       foreach ( (array)$dirs as $dir ) {
-                               foreach ( $codeSequence as $csCode ) {
+               foreach ( $codeSequence as $csCode ) {
+                       $csData = $initialData;
+
+                       # Load core messages and the extension localisations.
+                       foreach ( $wgMessagesDirs as $dirs ) {
+                               foreach ( (array)$dirs as $dir ) {
                                        $fileName = "$dir/$csCode.json";
                                        $data = $this->readJSONFile( $fileName );
 
                                        foreach ( $data as $key => $item ) {
-                                               $this->mergeItem( $key, $allData[$key], $item );
+                                               $this->mergeItem( $key, $csData[$key], $item );
                                        }
 
                                        $deps[] = new FileDependency( $fileName );
                                }
                        }
-               }
 
-               foreach ( $wgExtensionMessagesFiles as $extension => $fileName ) {
-                       if ( isset( $wgMessagesDirs[$extension] ) ) {
-                               # Already loaded the JSON files for this extension; skip the PHP shim
-                               continue;
+                       # Merge non-JSON extension data
+                       if ( isset( $extensionData[$csCode] ) ) {
+                               foreach ( $extensionData[$csCode] as $key => $item ) {
+                                       $this->mergeItem( $key, $csData[$key], $item );
+                               }
                        }
 
-                       $data = $this->readPHPFile( $fileName, 'extension' );
-                       $used = false;
-
-                       foreach ( $data as $key => $item ) {
-                               if ( $this->mergeExtensionItem( $codeSequence, $key, $allData[$key], $item ) ) {
-                                       $used = true;
+                       if ( $csCode === $code ) {
+                               # Merge core data into extension data
+                               foreach ( $coreData as $key => $item ) {
+                                       $this->mergeItem( $key, $csData[$key], $item );
+                               }
+                       } else {
+                               # Load the secondary localisation from the source file to
+                               # avoid infinite cycles on cyclic fallbacks
+                               $fbData = $this->readSourceFilesAndRegisterDeps( $csCode, $deps );
+                               if ( $fbData !== false ) {
+                                       # Only merge the keys that make sense to merge
+                                       foreach ( self::$allKeys as $key ) {
+                                               if ( !isset( $fbData[$key] ) ) {
+                                                       continue;
+                                               }
+
+                                               if ( is_null( $coreData[$key] ) || $this->isMergeableKey( $key ) ) {
+                                                       $this->mergeItem( $key, $csData[$key], $fbData[$key] );
+                                               }
+                                       }
                                }
                        }
 
-                       if ( $used ) {
-                               $deps[] = new FileDependency( $fileName );
+                       # Allow extensions an opportunity to adjust the data for this
+                       # fallback
+                       wfRunHooks( 'LocalisationCacheRecacheFallback', array( $this, $csCode, &$csData ) );
+
+                       # Merge the data for this fallback into the final array
+                       if ( $csCode === $code ) {
+                               $allData = $csData;
+                       } else {
+                               foreach ( self::$allKeys as $key ) {
+                                       if ( !isset( $csData[$key] ) ) {
+                                               continue;
+                                       }
+
+                                       if ( is_null( $allData[$key] ) || $this->isMergeableKey( $key ) ) {
+                                               $this->mergeItem( $key, $allData[$key], $csData[$key] );
+                                       }
+                               }
                        }
                }
 
-               # Merge core data into extension data
-               foreach ( $coreData as $key => $item ) {
-                       $this->mergeItem( $key, $allData[$key], $item );
-               }
-               wfProfileOut( __METHOD__ . '-extensions' );
+               wfProfileOut( __METHOD__ . '-fallbacks' );
 
                # Add cache dependencies for any referenced globals
                $deps['wgExtensionMessagesFiles'] = new GlobalDependency( 'wgExtensionMessagesFiles' );
diff --git a/tests/phpunit/data/localisationcache/en.json b/tests/phpunit/data/localisationcache/en.json
new file mode 100644 (file)
index 0000000..27600cd
--- /dev/null
@@ -0,0 +1,5 @@
+{
+       "present-uk": "en",
+       "present-ru": "en",
+       "present-en": "en"
+}
diff --git a/tests/phpunit/data/localisationcache/ru.json b/tests/phpunit/data/localisationcache/ru.json
new file mode 100644 (file)
index 0000000..79e1444
--- /dev/null
@@ -0,0 +1,4 @@
+{
+       "present-uk": "ru",
+       "present-ru": "ru"
+}
diff --git a/tests/phpunit/data/localisationcache/uk.json b/tests/phpunit/data/localisationcache/uk.json
new file mode 100644 (file)
index 0000000..f63ce5d
--- /dev/null
@@ -0,0 +1,3 @@
+{
+       "present-uk": "uk"
+}
diff --git a/tests/phpunit/includes/LocalisationCacheTest.php b/tests/phpunit/includes/LocalisationCacheTest.php
deleted file mode 100644 (file)
index f98410a..0000000
+++ /dev/null
@@ -1,34 +0,0 @@
-<?php
-
-/**
- * @covers LocalisationCache
- */
-class LocalisationCacheTest extends MediaWikiTestCase {
-       public function testPuralRulesFallback() {
-               $cache = Language::getLocalisationCache();
-
-               $this->assertEquals(
-                       $cache->getItem( 'ar', 'pluralRules' ),
-                       $cache->getItem( 'arz', 'pluralRules' ),
-                       'arz plural rules (undefined) fallback to ar (defined)'
-               );
-
-               $this->assertEquals(
-                       $cache->getItem( 'ar', 'compiledPluralRules' ),
-                       $cache->getItem( 'arz', 'compiledPluralRules' ),
-                       'arz compiled plural rules (undefined) fallback to ar (defined)'
-               );
-
-               $this->assertNotEquals(
-                       $cache->getItem( 'ksh', 'pluralRules' ),
-                       $cache->getItem( 'de', 'pluralRules' ),
-                       'ksh plural rules (defined) dont fallback to de (defined)'
-               );
-
-               $this->assertNotEquals(
-                       $cache->getItem( 'ksh', 'compiledPluralRules' ),
-                       $cache->getItem( 'de', 'compiledPluralRules' ),
-                       'ksh compiled plural rules (defined) dont fallback to de (defined)'
-               );
-       }
-}
diff --git a/tests/phpunit/includes/cache/LocalisationCacheTest.php b/tests/phpunit/includes/cache/LocalisationCacheTest.php
new file mode 100644 (file)
index 0000000..fc06a50
--- /dev/null
@@ -0,0 +1,91 @@
+<?php
+/**
+ * @group Database
+ * @group Cache
+ * @covers LocalisationCache
+ * @author Niklas Laxström
+ */
+class LocalisationCacheTest extends MediaWikiTestCase {
+       protected function setUp() {
+               global $IP;
+
+               parent::setUp();
+               $this->setMwGlobals( array(
+                       'wgMessagesDirs' => array( "$IP/tests/phpunit/data/localisationcache" ),
+                       'wgExtensionMessagesFiles' => array(),
+                       'wgHooks' => array(),
+               ) );
+       }
+
+       public function testPuralRulesFallback() {
+               $cache = new LocalisationCache( array( 'store' => 'detect' ) );
+
+               $this->assertEquals(
+                       $cache->getItem( 'ar', 'pluralRules' ),
+                       $cache->getItem( 'arz', 'pluralRules' ),
+                       'arz plural rules (undefined) fallback to ar (defined)'
+               );
+
+               $this->assertEquals(
+                       $cache->getItem( 'ar', 'compiledPluralRules' ),
+                       $cache->getItem( 'arz', 'compiledPluralRules' ),
+                       'arz compiled plural rules (undefined) fallback to ar (defined)'
+               );
+
+               $this->assertNotEquals(
+                       $cache->getItem( 'ksh', 'pluralRules' ),
+                       $cache->getItem( 'de', 'pluralRules' ),
+                       'ksh plural rules (defined) dont fallback to de (defined)'
+               );
+
+               $this->assertNotEquals(
+                       $cache->getItem( 'ksh', 'compiledPluralRules' ),
+                       $cache->getItem( 'de', 'compiledPluralRules' ),
+                       'ksh compiled plural rules (defined) dont fallback to de (defined)'
+               );
+       }
+
+       public function testRecacheFallbacks() {
+               $lc = new LocalisationCache( array( 'store' => 'detect' ) );
+               $lc->recache( 'uk' );
+               $this->assertEquals(
+                       array(
+                               'present-uk' => 'uk',
+                               'present-ru' => 'ru',
+                               'present-en' => 'en',
+                       ),
+                       $lc->getItem( 'uk', 'messages' ),
+                       'Fallbacks are only used to fill missing data'
+               );
+       }
+
+       public function testRecacheFallbacksWithHooks() {
+               global $wgHooks;
+
+               // Use hook to provide updates for messages. This is what the
+               // LocalisationUpdate extension does. See bug 68781.
+               $wgHooks['LocalisationCacheRecacheFallback'][] = function (
+                       LocalisationCache $lc,
+                       $code,
+                       array &$cache
+               ) {
+                       if ( $code === 'ru' ) {
+                               $cache['messages']['present-uk'] = 'ru-override';
+                               $cache['messages']['present-ru'] = 'ru-override';
+                               $cache['messages']['present-en'] = 'ru-override';
+                       }
+               };
+
+               $lc = new LocalisationCache( array( 'store' => 'detect' ) );
+               $lc->recache( 'uk' );
+               $this->assertEquals(
+                       array(
+                               'present-uk' => 'uk',
+                               'present-ru' => 'ru-override',
+                               'present-en' => 'ru-override',
+                       ),
+                       $lc->getItem( 'uk', 'messages' ),
+                       'Updates provided by hooks follow the normal fallback order.'
+               );
+       }
+}