From d76e670c7694773c0779cc51e684ed70f2cd4f3b Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Tue, 28 Aug 2012 15:54:40 +1000 Subject: [PATCH] Fixed fallback behaviour for plural rules Fallbacks didn't work at all for the new plural rule system. I fixed it by moving the getPluralRules() and getCompiledPluralRules() calls near to the readPHPFile() calls, before merging is done. Then I factored out the resulting code to readSourceFilesAndRegisterDeps(). * Removed pluralRules from mergeableMapKeys, it isn't one * Added compiledPluralRules to allKeys so that it will be merged * When a language is not present in the CLDR XML files, return null from getPluralRules() and getCompiledPluralRules() so that the fallback rules won't be overridden with an empty array. Normalised it back to an empty array in the unlikely event that there is no plural data in the fallback sequence at all, even in English. * Fixed private function, "protected" is the way to say private here. Change-Id: I3a008ef7f6ed7aaa15ad25ad796f7a2b8f827fa2 --- includes/LocalisationCache.php | 62 +++++++++++++------ .../includes/LocalisationCacheTest.php | 31 ++++++++++ 2 files changed, 75 insertions(+), 18 deletions(-) create mode 100644 tests/phpunit/includes/LocalisationCacheTest.php diff --git a/includes/LocalisationCache.php b/includes/LocalisationCache.php index c1ac848490..d8e5d3a368 100644 --- a/includes/LocalisationCache.php +++ b/includes/LocalisationCache.php @@ -110,7 +110,7 @@ class LocalisationCache { 'dateFormats', 'datePreferences', 'datePreferenceMigrationMap', 'defaultDateFormat', 'extraUserToggles', 'specialPageAliases', 'imageFiles', 'preloadedMessages', 'namespaceGenderAliases', - 'digitGroupingPattern', 'pluralRules' + 'digitGroupingPattern', 'pluralRules', 'compiledPluralRules', ); /** @@ -118,7 +118,7 @@ class LocalisationCache { * by a fallback sequence. */ static public $mergeableMapKeys = array( 'messages', 'namespaceNames', - 'dateFormats', 'imageFiles', 'preloadedMessages', 'pluralRules' + 'dateFormats', 'imageFiles', 'preloadedMessages' ); /** @@ -498,6 +498,9 @@ class LocalisationCache { */ public function getCompiledPluralRules( $code ) { $rules = $this->getPluralRules( $code ); + if ( $rules === null ) { + return null; + } try { $compiledRules = CLDRPluralRuleEvaluator::compile( $rules ); } catch( CLDRPluralRuleError $e ) { @@ -524,17 +527,18 @@ class LocalisationCache { } } if ( !isset( $this->pluralRules[$code] ) ) { - return array(); + return null; } else { return $this->pluralRules[$code]; } } + /** * Load a plural XML file with the given filename, compile the relevant * rules, and save the compiled rules in a process-local cache. */ - private function loadPluralFile( $fileName ) { + protected function loadPluralFile( $fileName ) { $doc = new DOMDocument; $doc->load( $fileName ); $rulesets = $doc->getElementsByTagName( "pluralRules" ); @@ -551,6 +555,30 @@ class LocalisationCache { } } + /** + * Read the data from the source files for a given language, and register + * the relevant dependencies in the $deps array. If the localisation + * exists, the data array is returned, otherwise false is returned. + */ + protected function readSourceFilesAndRegisterDeps( $code, &$deps ) { + $fileName = Language::getMessagesFileName( $code ); + if ( !file_exists( $fileName ) ) { + return false; + } + + $deps[] = new FileDependency( $fileName ); + $data = $this->readPHPFile( $fileName, 'core' ); + + # Load CLDR plural rules for JavaScript + $data['pluralRules'] = $this->getPluralRules( $code ); + # And for PHP + $data['compiledPluralRules'] = $this->getCompiledPluralRules( $code ); + + $deps['plurals'] = new FileDependency( __DIR__ . "/../languages/data/plurals.xml" ); + $deps['plurals-mw'] = new FileDependency( __DIR__ . "/../languages/data/plurals-mediawiki.xml" ); + return $data; + } + /** * Merge two localisation values, a primary and a fallback, overwriting the * primary value in place. @@ -649,13 +677,11 @@ class LocalisationCache { $deps = array(); # Load the primary localisation from the source file - $fileName = Language::getMessagesFileName( $code ); - if ( !file_exists( $fileName ) ) { + $data = $this->readSourceFilesAndRegisterDeps( $code, $deps ); + if ( $data === false ) { wfDebug( __METHOD__ . ": no localisation file for $code, using fallback to en\n" ); $coreData['fallback'] = 'en'; } else { - $deps[] = new FileDependency( $fileName ); - $data = $this->readPHPFile( $fileName, 'core' ); wfDebug( __METHOD__ . ": got localisation for $code from source\n" ); # Merge primary localisation @@ -684,15 +710,11 @@ class LocalisationCache { foreach ( $coreData['fallbackSequence'] as $fbCode ) { # Load the secondary localisation from the source file to # avoid infinite cycles on cyclic fallbacks - $fbFilename = Language::getMessagesFileName( $fbCode ); - - if ( !file_exists( $fbFilename ) ) { + $fbData = $this->readSourceFilesAndRegisterDeps( $fbCode, $deps ); + if ( $fbData === false ) { continue; } - $deps[] = new FileDependency( $fbFilename ); - $fbData = $this->readPHPFile( $fbFilename, 'core' ); - foreach ( self::$allKeys as $key ) { if ( !isset( $fbData[$key] ) ) { continue; @@ -749,15 +771,19 @@ class LocalisationCache { # Decouple the reference to prevent accidental damage unset( $page ); + # If there were no plural rules, return an empty array + if ( $allData['pluralRules'] === null ) { + $allData['pluralRules'] = array(); + } + if ( $allData['compiledPluralRules'] === null ) { + $allData['compiledPluralRules'] = array(); + } + # Set the list keys $allData['list'] = array(); foreach ( self::$splitKeys as $key ) { $allData['list'][$key] = array_keys( $allData[$key] ); } - # Load CLDR plural rules for JavaScript - $allData['pluralRules'] = $this->getPluralRules( $code ); - # And for PHP - $allData['compiledPluralRules'] = $this->getCompiledPluralRules( $code ); # Run hooks wfRunHooks( 'LocalisationCacheRecache', array( $this, $code, &$allData ) ); diff --git a/tests/phpunit/includes/LocalisationCacheTest.php b/tests/phpunit/includes/LocalisationCacheTest.php new file mode 100644 index 0000000000..356db87c68 --- /dev/null +++ b/tests/phpunit/includes/LocalisationCacheTest.php @@ -0,0 +1,31 @@ +assertEquals( + $cache->getItem( 'ru', 'pluralRules' ), + $cache->getItem( 'os', 'pluralRules' ), + 'os plural rules (undefined) fallback to ru (defined)' + ); + + $this->assertEquals( + $cache->getItem( 'ru', 'compiledPluralRules' ), + $cache->getItem( 'os', 'compiledPluralRules' ), + 'os compiled plural rules (undefined) fallback to ru (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)' + ); + } +} -- 2.20.1