From 4615338a5192c218873f43491ace39caa8f07f30 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sun, 6 Oct 2013 18:31:20 +0200 Subject: [PATCH] Implement getInterfaceTextInLanguage and use API and Parser Fallback for user language and content language is easiest to resolve server-side. Also saves sending a lot of data to the client that it doesn't need. Similar to how ResourceLoader only sends 1 set of message values. Bug: 50431 Bug: 52922 Change-Id: If8317ed6522a05d5a48a210ff43c97277b950a97 --- TemplateDataBlob.php | 108 ++++++++++++++-- api/ApiTemplateData.php | 22 +++- tests/TemplateDataBlobTest.php | 221 +++++++++++++++++++++++++++++++++ 3 files changed, 338 insertions(+), 13 deletions(-) diff --git a/TemplateDataBlob.php b/TemplateDataBlob.php index 0eb9ce4f..fd185272 100644 --- a/TemplateDataBlob.php +++ b/TemplateDataBlob.php @@ -154,7 +154,7 @@ class TemplateDataBlob { // Param.label if ( isset( $paramObj->label ) ) { if ( !is_object( $paramObj->label ) && !is_string( $paramObj->label ) ) { - // TODO: Also validate that the keys are valid lang codes and the values strings. + // TODO: Also validate that the keys are valid lang codes and the values strings. return Status::newFatal( 'templatedata-invalid-type', "params.{$paramName}.label", @@ -349,15 +349,103 @@ class TemplateDataBlob { return $text; } + /** + * Get a single localized string from an InterfaceText object. + * + * Uses the preferred language passed to this function, or one of its fallbacks, + * or the site content language, or its fallbacks. + * + * @param stdClass $text An InterfaceText object + * @param string $langCode Preferred language + * @return null|string Text value from the InterfaceText object or null if no suitable + * match was found + */ + protected static function getInterfaceTextInLanguage( stdClass $text, $langCode ) { + if ( isset( $text->$langCode ) ) { + return $text->$langCode; + } + + list( $userlangs, $sitelangs ) = Language::getFallbacksIncludingSiteLanguage( $langCode ); + + foreach ( $userlangs as $lang ) { + if ( isset( $text->$lang ) ) { + return $text->$lang; + } + } + + foreach ( $sitelangs as $lang ) { + if ( isset( $text->$lang ) ) { + return $text->$lang; + } + } + + // If none of the languages are found fallback to null. Alternatively we could fallback to + // reset( $text ) which will return whatever key there is, but we should't give the user a + // "random" language with no context (e.g. could be RTL/Hebrew for an LTR/Japanese user). + return null; + } + + /** + * @return Status + */ public function getStatus() { return $this->status; } + /** + * @return object + */ public function getData() { - // Returned by reference. Data is a private member. Use clone instead? + // TODO: Returned by reference. Data is a private member. Use clone instead? return $this->data; } + /** + * Get data with all InterfaceText objects resolved to a single string to the + * appropriate language. + * + * @param string $langCode Preferred language + * @return object + */ + public function getDataInLanguage( $langCode ) { + // Deep clone, also need to clone ->params and all interfacetext objects + // within param properties. + $data = unserialize( serialize( $this->data ) ); + + // Root.description + if ( $data->description !== null ) { + $data->description = self::getInterfaceTextInLanguage( $data->description, $langCode ); + } + + foreach ( $data->params as $paramObj ) { + // Param.label + if ( $paramObj->label !== null ) { + $paramObj->label = self::getInterfaceTextInLanguage( $paramObj->label, $langCode ); + } + + // Param.description + if ( $paramObj->description !== null ) { + $paramObj->description = self::getInterfaceTextInLanguage( $paramObj->description, $langCode ); + } + } + + foreach ( $data->sets as $setObj ) { + $label = self::getInterfaceTextInLanguage( $setObj->label, $langCode ); + if ( $label === null ) { + // Contrary to other InterfaceTexts, set label is not optional. If we're here it + // means the template data from the wiki doesn't contain either the user language, + // site language or any of its fallbacks. Wikis should fix data that is in this + // condition (TODO: Disallow during saving?). For now, fallback to whatever we can + // get that does exist in the text object. + $label = reset( $setObj->label ); + } + + $setObj->label = $label; + } + + return $data; + } + /** * @return string JSON */ @@ -373,9 +461,7 @@ class TemplateDataBlob { } public function getHtml( Language $lang ) { - global $wgContLang; - $langCode = $wgContLang->getCode(); - $data = $this->data; + $data = $this->getDataInLanguage( $lang->getCode() ); $html = Html::openElement( 'div', array( 'class' => 'mw-templatedata-doc-wrap' ) ) . Html::element( @@ -387,7 +473,7 @@ class TemplateDataBlob { ) ), $data->description !== null ? - $data->description->$langCode : + $data->description : wfMessage( 'templatedata-doc-desc-empty' )->inLanguage( $lang ) ) . '' @@ -441,8 +527,8 @@ class TemplateDataBlob { $html .= '' // Label . Html::element( 'th', array(), - isset( $paramObj->label->$langCode ) ? - $paramObj->label->$langCode : + $paramObj->label !== null ? + $paramObj->label : ucfirst( $paramName ) ) // Parameters and aliases @@ -453,12 +539,12 @@ class TemplateDataBlob { . Html::element( 'td', array( 'class' => array( 'mw-templatedata-doc-muted' => ( - !isset( $paramObj->description->$langCode ) && $paramObj->deprecated === false + $paramObj->description === null && $paramObj->deprecated === false ) ) ), - $paramObj->description->$langCode !== null ? - $paramObj->description->$langCode : + $paramObj->description !== null ? + $paramObj->description : wfMessage( 'templatedata-doc-param-desc-empty' )->inLanguage( $lang ) ) // Type diff --git a/api/ApiTemplateData.php b/api/ApiTemplateData.php index dc3b5a11..1a568d88 100644 --- a/api/ApiTemplateData.php +++ b/api/ApiTemplateData.php @@ -57,6 +57,14 @@ class ApiTemplateData extends ApiBase { $params = $this->extractRequestParams(); $result = $this->getResult(); + if ( is_null( $params['lang'] ) ) { + $langCode = false; + } elseif ( !Language::isValidCode( $params['lang'] ) ) { + $this->dieUsage( 'Invalid language code for parameter lang', 'invalidlang' ); + } else { + $langCode = $params['lang']; + } + $pageSet = $this->getPageSet(); $pageSet->execute(); $titles = $pageSet->getGoodTitles(); // page_id => Title object @@ -82,13 +90,20 @@ class ApiTemplateData extends ApiBase { $rawData = $row->pp_value; $tdb = TemplateDataBlob::newFromDatabase( $rawData ); $status = $tdb->getStatus(); + if ( !$status->isOK() ) { $this->dieUsage( 'Page #' . intval( $row->pp_page ) . ' templatedata contains invalid data: ' . $status->getMessage(), 'templatedata-corrupt' ); } - $data = $tdb->getData(); + + if ( $langCode ) { + $data = $tdb->getDataInLanguage( $langCode ); + } else { + $data = $tdb->getData(); + } + $resp[$row->pp_page] = array( 'title' => strval( $titles[$row->pp_page] ), 'description' => $data->description, @@ -110,13 +125,16 @@ class ApiTemplateData extends ApiBase { 'format' => array( ApiBase::PARAM_DFLT => 'json', ApiBase::PARAM_TYPE => array( 'json', 'jsonfm' ), - ) + ), + 'lang' => null ); } public function getParamDescription() { return $this->getPageSet()->getParamDescription() + array( 'format' => 'The format of the output', + 'lang' => 'Return localized values in this language (by default all available' . + ' translations are returned)', ); } diff --git a/tests/TemplateDataBlobTest.php b/tests/TemplateDataBlobTest.php index e652e85a..4648790e 100644 --- a/tests/TemplateDataBlobTest.php +++ b/tests/TemplateDataBlobTest.php @@ -29,6 +29,7 @@ class TemplateDataBlobTest extends MediaWikiTestCase { return $string; } + public static function provideParse() { $cases = array( array( @@ -397,4 +398,224 @@ class TemplateDataBlobTest extends MediaWikiTestCase { $templateData = TemplateDataBlob::newFromDatabase( $gzJson ); $this->assertInstanceOf( 'TemplateDataBlob', $templateData ); } + + public static function provideGetDataInLanguage() { + $cases = array( + array( + 'input' => '{ + "description": { + "de": "German", + "nl": "Dutch", + "en": "English", + "de-formal": "German (formal address)" + }, + "params": {} + } + ', + 'output' => '{ + "description": "German", + "params": {}, + "sets": [] + } + ', + 'lang' => 'de', + 'msg' => 'Simple description' + ), + array( + 'input' => '{ + "description": "Hi", + "params": {} + } + ', + 'output' => '{ + "description": "Hi", + "params": {}, + "sets": [] + } + ', + 'lang' => 'fr', + 'msg' => 'Non multi-language value returned as is (expands to { "en": value } for' . + ' content-lang, "fr" falls back to "en")' + ), + array( + 'input' => '{ + "description": { + "nl": "Dutch", + "de": "German" + }, + "params": {} + } + ', + 'output' => '{ + "description": "Dutch", + "params": {}, + "sets": [] + } + ', + 'lang' => 'fr', + 'msg' => 'Try content language before giving up on user language and fallbacks' + ), + array( + 'input' => '{ + "description": { + "es": "Spanish", + "de": "German" + }, + "params": {} + } + ', + 'output' => '{ + "description": null, + "params": {}, + "sets": [] + } + ', + 'lang' => 'fr', + 'msg' => 'Description is optional, use null if no suitable fallback' + ), + array( + 'input' => '{ + "description": { + "de": "German", + "nl": "Dutch", + "en": "English" + }, + "params": {} + } + ', + 'output' => '{ + "description": "German", + "params": {}, + "sets": [] + } + ', + 'lang' => 'de-formal', + 'msg' => '"de-formal" falls back to "de"' + ), + array( + 'input' => '{ + "params": { + "foo": { + "label": { + "fr": "French", + "en": "English" + } + } + } + } + ', + 'output' => '{ + "description": null, + "params": { + "foo": { + "label": "French", + "required": false, + "description": null, + "deprecated": false, + "aliases": [], + "default": "", + "type": "unknown" + } + }, + "sets": [] + } + ', + 'lang' => 'fr', + 'msg' => 'Simple parameter label' + ), + array( + 'input' => '{ + "params": { + "foo": { + "label": { + "es": "Spanish", + "de": "German" + } + } + } + } + ', + 'output' => '{ + "description": null, + "params": { + "foo": { + "label": null, + "required": false, + "description": null, + "deprecated": false, + "aliases": [], + "default": "", + "type": "unknown" + } + }, + "sets": [] + } + ', + 'lang' => 'fr', + 'msg' => 'Parameter label is optional, use null if no matching fallback' + ), + array( + 'input' => '{ + "params": {}, + "sets": [ + { + "label": { + "es": "Spanish", + "de": "German" + }, + "params": [] + } + ] + } + ', + 'output' => '{ + "description": null, + "params": {}, + "sets": [ + { + "label": "Spanish", + "params": [] + } + ] + } + ', + 'lang' => 'fr', + 'msg' => 'Set label is not optional, choose first available key as final fallback' + ), + ); + $calls = array(); + foreach ( $cases as $case ) { + $calls[] = array( $case ); + } + return $calls; + } + + /** + * @dataProvider provideGetDataInLanguage + */ + public function testGetDataInLanguage( Array $case ) { + + // Change content-language to be non-English so we can distinguish between the + // last 'en' fallback and the content language in our tests + $this->setMwGlobals( array( + 'wgLanguageCode' => 'nl', + 'wgContLang' => Language::factory( 'nl' ), + ) ); + + if ( !isset( $case['msg'] ) ) { + $case['msg'] = is_string( $case['status'] ) ? $case['status'] : 'TemplateData assertion'; + } + + $t = TemplateDataBlob::newFromJSON( $case['input'] ); + $status = $t->getStatus(); + + $this->assertTrue( $status->isGood(), 'Status is good: ' . $case['msg'] ); + + $actual = $t->getDataInLanguage( $case['lang'] ); + $this->assertJsonStringEqualsJsonString( + $case['output'], + json_encode( $actual ), + $case['msg'] + ); + } }