From b6fc9067b0405603d250759cb73c41b8c8627d31 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Wed, 3 Sep 2014 12:52:33 -0400 Subject: [PATCH] LocalisationCache: Process one fallback at a time 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 | 7 ++ includes/cache/LocalisationCache.php | 116 ++++++++++++------ tests/phpunit/data/localisationcache/en.json | 5 + tests/phpunit/data/localisationcache/ru.json | 4 + tests/phpunit/data/localisationcache/uk.json | 3 + .../includes/LocalisationCacheTest.php | 34 ----- .../includes/cache/LocalisationCacheTest.php | 91 ++++++++++++++ 7 files changed, 186 insertions(+), 74 deletions(-) create mode 100644 tests/phpunit/data/localisationcache/en.json create mode 100644 tests/phpunit/data/localisationcache/ru.json create mode 100644 tests/phpunit/data/localisationcache/uk.json delete mode 100644 tests/phpunit/includes/LocalisationCacheTest.php create mode 100644 tests/phpunit/includes/cache/LocalisationCacheTest.php diff --git a/docs/hooks.txt b/docs/hooks.txt index 82da289324..369b7776dc 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -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 diff --git a/includes/cache/LocalisationCache.php b/includes/cache/LocalisationCache.php index bdfe51010b..bdfc75a6ef 100644 --- a/includes/cache/LocalisationCache.php +++ b/includes/cache/LocalisationCache.php @@ -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 index 0000000000..27600cdb2c --- /dev/null +++ b/tests/phpunit/data/localisationcache/en.json @@ -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 index 0000000000..79e1444bca --- /dev/null +++ b/tests/phpunit/data/localisationcache/ru.json @@ -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 index 0000000000..f63ce5d3f5 --- /dev/null +++ b/tests/phpunit/data/localisationcache/uk.json @@ -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 index f98410a1b8..0000000000 --- a/tests/phpunit/includes/LocalisationCacheTest.php +++ /dev/null @@ -1,34 +0,0 @@ -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 index 0000000000..fc06a50126 --- /dev/null +++ b/tests/phpunit/includes/cache/LocalisationCacheTest.php @@ -0,0 +1,91 @@ +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.' + ); + } +} -- 2.20.1