jenkins-bot has submitted this change and it was merged. Change subject: Add unit tests and fix implemention accordingly ......................................................................
Add unit tests and fix implemention accordingly * Add unit tests for all types of invalid input we check for. * Add unit tests for all types of input we expand or otherwise normalise. * Implement InterfaceText expansion/normalisation. * Fix bug that caused a string value in the root description property to be considered invalid (it only accepted an object, it should accept both). Change-Id: I5a15080f1f924451a9dde8af96ea2922011981ec --- M TemplateData.hooks.php M TemplateData.php M TemplateDataBlob.php A tests/TemplateDataBlobTest.php 4 files changed, 199 insertions(+), 7 deletions(-) Approvals: Catrope: Looks good to me, approved jenkins-bot: Verified diff --git a/TemplateData.hooks.php b/TemplateData.hooks.php index 2f478a9..d830948 100644 --- a/TemplateData.hooks.php +++ b/TemplateData.hooks.php @@ -17,6 +17,15 @@ } /** + * Register unit tests + */ + public static function onUnitTestsList( array &$files ) { + $testDir = __DIR__ . '/tests/'; + $files = array_merge( $files, glob( "$testDir/*Test.php" ) ); + return true; + } + + /** * @param Page &$page * @param User &$user * @param Content &$content diff --git a/TemplateData.php b/TemplateData.php index 44e53c7..d90eb93 100644 --- a/TemplateData.php +++ b/TemplateData.php @@ -33,6 +33,7 @@ // Register hooks $wgHooks['ParserFirstCallInit'][] = 'TemplateDataHooks::onParserFirstCallInit'; $wgHooks['PageContentSave'][] = 'TemplateDataHooks::onPageContentSave'; +$wgHooks['UnitTestsList'][] = 'TemplateDataHooks::onUnitTestsList'; // Register APIs $wgAPIModules['templatedata'] = 'ApiTemplateData'; diff --git a/TemplateDataBlob.php b/TemplateDataBlob.php index b4f4b03..c30d353 100644 --- a/TemplateDataBlob.php +++ b/TemplateDataBlob.php @@ -30,8 +30,6 @@ $ti = new self( json_decode( $json ) ); $status = $ti->parse(); - // TODO: Normalise `params.*.description` to a plain object. - if ( !$status->isOK() ) { // Don't save invalid data, clear it. $ti->data = new stdClass(); @@ -72,11 +70,12 @@ } if ( isset( $data->description ) ) { - if ( !is_object( $data->params ) ) { - return Status::newFatal( 'templatedata-invalid-type', 'params', 'object' ); + if ( !is_object( $data->description ) && !is_string( $data->description ) ) { + return Status::newFatal( 'templatedata-invalid-type', 'description', 'string|object' ); } + $data->description = self::normaliseInterfaceText( $data->description ); } else { - $data->description = ''; + $data->description = self::normaliseInterfaceText( '' ); } foreach ( $data->params as $paramName => $paramObj ) { @@ -111,8 +110,9 @@ // and the values strings. return Status::newFatal( 'templatedata-invalid-type', 'params.' . $paramName . '.description', 'string|object' ); } + $paramObj->description = self::normaliseInterfaceText( $paramObj->description ); } else { - $paramObj->description = ''; + $paramObj->description = self::normaliseInterfaceText( '' ); } if ( isset( $paramObj->deprecated ) ) { @@ -151,6 +151,20 @@ } return Status::newGood(); + } + + /** + * Normalise a InterfaceText field in the TemplateData blob. + * @return stdClass|string $text + */ + protected static function normaliseInterfaceText( $text ) { + if ( is_string( $text ) ) { + global $wgContLang; + $ret = array(); + $ret[ $wgContLang->getCode() ] = $text; + return (object) $ret; + } + return $text; } public function getStatus() { @@ -207,7 +221,7 @@ return $html; } - private function __construct( stdClass $data = null ) { + private function __construct( $data = null ) { $this->data = $data; } diff --git a/tests/TemplateDataBlobTest.php b/tests/TemplateDataBlobTest.php new file mode 100644 index 0000000..905bbfe --- /dev/null +++ b/tests/TemplateDataBlobTest.php @@ -0,0 +1,168 @@ +<?php +class TemplateDataBlobTest extends MediaWikiTestCase { + + protected function setUp() { + parent::setUp(); + + $this->setMwGlobals( array( + 'wgLanguageCode' => 'en', + 'wgContLang' => Language::factory( 'en' ), + ) ); + } + + public static function provideParse() { + return array( + array( + ' + {} + ', + ' + {} + ', + 'Empty object' + ), + array( + ' + { + "foo": "bar" + } + ', + ' + {} + ', + 'Unknown properties are stripped' + ), + array( + ' + { + "params": { + "foo": {} + } + } + ', + ' + { + "description": { + "en": "" + }, + "params": { + "foo": { + "description": { + "en": "" + }, + "default": "", + "required": false, + "deprecated": false, + "aliases": [], + "clones": [] + } + } + } + ', + 'Optional properties are added if missing' + ), + array( + ' + { + "description": "User badge MediaWiki developers.", + "params": { + "nickname": { + "description": "User name of user who owns the badge", + "default": "Base page name of the host page", + "required": false, + "aliases": [ + "1" + ] + } + } + } + ', + ' + { + "description": { + "en": "User badge MediaWiki developers." + }, + "params": { + "nickname": { + "description": { + "en": "User name of user who owns the badge" + }, + "default": "Base page name of the host page", + "required": false, + "deprecated": false, + "aliases": [ + "1" + ], + "clones": [] + } + } + } + ', + 'InterfaceText is expanded to langcode-keyed object, assuming content language' + ) + ); + } + + /** + * @dataProvider provideParse + */ + public function testParse( $input, $expected, $msg ) { + $t = TemplateDataBlob::newFromJSON( $input ); + $actual = $t->getJSON(); + $this->assertJsonStringEqualsJsonString( + $expected, + $actual, + $msg + ); + } + + public static function provideStatus() { + return array( + array( + ' + [] + ', + false, + 'Not an object' + ), + array( + ' + { + "params": {} + } + ', + true, + 'Minimal valid blob' + ), + array( + ' + { + "params": {}, + "foo": "bar" + } + ', + false, + 'Unknown properties' + ), + ); + } + + /** + * @dataProvider provideStatus + */ + public function testStatus( $inputJSON, $isGood, $msg ) { + // Make sure we don't have two errors cancelling each other out + if ( json_decode( $inputJSON ) === null ) { + throw new Exception( 'Test case provided invalid JSON.' ); + } + + $t = TemplateDataBlob::newFromJSON( $inputJSON ); + $status = $t->getStatus(); + + $this->assertEquals( + $status->isGood(), + $isGood, + $msg + ); + } +} -- To view, visit https://gerrit.wikimedia.org/r/59396 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5a15080f1f924451a9dde8af96ea2922011981ec Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/extensions/TemplateData Gerrit-Branch: master Gerrit-Owner: Krinkle <krinklem...@gmail.com> Gerrit-Reviewer: Catrope <roan.katt...@gmail.com> Gerrit-Reviewer: Krinkle <krinklem...@gmail.com> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits