From 399d443e19f9e69836f2a1a8f57f5b680231a3ee Mon Sep 17 00:00:00 2001 From: jeblad Date: Thu, 9 Aug 2012 16:23:11 +0200 Subject: [PATCH] Removed url args for usekeys/nousekeys, and also setting through config This is now completely under control from the format, and only one form is returned and that will be with keys unless it is not compatible with the format. There are tests to check if the keys in the format is according to some of the actual values. Patchset: Rebase... Change-Id: I4eaa36de9c85c7de590092bd4d0fb6e4b51d7911 --- repo/Wikibase.hooks.php | 7 +- repo/includes/api/Api.php | 66 ++++++------------- repo/includes/api/ApiGetItems.php | 2 - repo/includes/api/ApiModifyItem.php | 1 - repo/includes/api/ApiSetItem.php | 1 - .../phpunit/includes/api/ApiGetItemsTest.php | 55 ++++++++++++++++ .../includes/api/ApiSetAliasesTest.php | 1 - 7 files changed, 76 insertions(+), 57 deletions(-) diff --git a/repo/Wikibase.hooks.php b/repo/Wikibase.hooks.php index 845c2dc8aa..dee97bf990 100644 --- a/repo/Wikibase.hooks.php +++ b/repo/Wikibase.hooks.php @@ -396,10 +396,6 @@ public static function onWikibaseDefaultSettings( array &$settings ) { // set to true will always delete empty items 'apiDeleteEmpty' => false, - // Defaults to turn off use of keys - // set to true will always return the key form - 'apiUseKeys' => true, - // Set API in debug mode // do not turn on in production! 'apiInDebug' => false, @@ -411,7 +407,7 @@ public static function onWikibaseDefaultSettings( array &$settings ) { 'apiDebugWithRights' => false, 'apiDebugWithTokens' => false, - // Which formats to use with keys when there are a "usekeys" in the URL + // Which formats to inject keys into // undefined entries are interpreted as false 'formatsWithKeys' => array( 'json' => true, @@ -424,7 +420,6 @@ public static function onWikibaseDefaultSettings( array &$settings ) { 'xmlfm' => false, 'yaml' => true, 'yamlfm' => true, - 'raw' => true, 'rawfm' => true, 'txtfm' => true, 'dbg' => true, diff --git a/repo/includes/api/Api.php b/repo/includes/api/Api.php index f20bc457d6..1d58dfe34f 100644 --- a/repo/includes/api/Api.php +++ b/repo/includes/api/Api.php @@ -16,27 +16,19 @@ * @author John Erling Blad < jeblad@gmail.com > */ abstract class Api extends \ApiBase { + /** - * Var to keep the set status for later use - * @var bool how to handle the keys - */ - protected $usekeys = false; - - /** - * Sets the usekeys-state for later use (and misuse) + * Figure out the usekeys-state * - * @param $params array parameters requested in subclass + * @return bool true if the keys should be present */ - protected function setUsekeys( array $params ) { - $usekeys = Settings::get( 'apiUseKeys' ) || ( isset( $params['usekeys'] ) ? $params['usekeys'] : false ); - - if ( $usekeys ) { - $format = $this->getMain()->getRequest()->getVal( 'format' ); + protected function getUsekeys() { + static $withkeys = false; + if ( $withkeys === false ) { $withkeys = Settings::get( 'formatsWithKeys' ); - $usekeys = isset( $format ) && isset( $withkeys[$format] ) ? $withkeys[$format] : false; } - - $this->usekeys = $usekeys; + $format = $this->getMain()->getRequest()->getVal( 'format' ); + return ( isset( $format ) && isset( $withkeys[$format] ) ) ? $withkeys[$format] : false ; } /** @@ -52,41 +44,23 @@ public function getPossibleErrors() { * @see ApiBase::getParamDescription() */ public function getParamDescription() { - $descriptions = array( + return array( 'gettoken' => array( 'If set, a new "modifyitem" token will be returned if the request completes.', 'The remaining of the call must be valid, otherwise an error can be returned without the token included.' ), ); - if ( Settings::get( 'apiUseKeys' ) ) { - $descriptions['nousekeys'] = array( 'Turn off use the keys. The use of keys are only used in formats that supports them,', - 'otherwise fall back to the ordinary style which is to use keys.' - ); - } - else { - $descriptions['usekeys'] = array( 'Turn on use the keys. The use of keys are only used in formats that supports them,', - 'otherwise fall back to the ordinary style which is to use keys.' - ); - } - return $descriptions; } /** * @see ApiBase::getAllowedParams() */ public function getAllowedParams() { - $allowedParams = array( + return array( 'gettoken' => array( ApiBase::PARAM_TYPE => 'boolean', ApiBase::PARAM_DFLT => false ), ); - if ( Settings::get( 'apiUseKeys' ) ) { - $allowedParams['nousekeys'] = array( \ApiBase::PARAM_TYPE => 'boolean' ); - } - else { - $allowedParams['usekeys'] = array( \ApiBase::PARAM_TYPE => 'boolean' ); - } - return $allowedParams; } /** @@ -131,7 +105,7 @@ protected function addTokenToResult( $token, $path=null, $name = 'itemtoken' ) { protected function addAliasesToResult( array $aliases, $path, $name = 'aliases', $tag = 'alias' ) { $value = array(); - if ( $this->usekeys ) { + if ( $this->getUsekeys() ) { foreach ( $aliases as $languageCode => $alarr ) { $arr = array(); foreach ( $alarr as $alias ) { @@ -155,7 +129,7 @@ protected function addAliasesToResult( array $aliases, $path, $name = 'aliases', } if ( $value !== array() ) { - if (!$this->usekeys) { + if ( !$this->getUsekeys() ) { $this->getResult()->setIndexedTagName( $value, $tag ); } $this->getResult()->addValue( $path, $name, $value ); @@ -202,12 +176,12 @@ protected function addSiteLinksToResult( array $siteLinks, $path, $name = 'sitel } } - $key = $this->usekeys ? $link->getSiteID() : $idx++; + $key = $this->getUsekeys() ? $link->getSiteID() : $idx++; $value[$key] = $response; } if ( $value !== array() ) { - if ( !$this->usekeys ) { + if ( !$this->getUsekeys() ) { $this->getResult()->setIndexedTagName( $value, $tag ); } @@ -233,13 +207,13 @@ protected function addDescriptionsToResult( array $descriptions, $path, $name = foreach ( $descriptions as $languageCode => $description ) { if ( $description === '' ) { - $value[$this->usekeys ? $languageCode : $idx++] = array( + $value[$this->getUsekeys() ? $languageCode : $idx++] = array( 'language' => $languageCode, 'removed' => '', ); } else { - $value[$this->usekeys ? $languageCode : $idx++] = array( + $value[$this->getUsekeys() ? $languageCode : $idx++] = array( 'language' => $languageCode, 'value' => $description, ); @@ -247,7 +221,7 @@ protected function addDescriptionsToResult( array $descriptions, $path, $name = } if ( $value !== array() ) { - if (!$this->usekeys) { + if ( !$this->getUsekeys() ) { $this->getResult()->setIndexedTagName( $value, $tag ); } $this->getResult()->addValue( $path, $name, $value ); @@ -272,13 +246,13 @@ protected function addLabelsToResult( array $labels, $path, $name = 'labels', $t foreach ( $labels as $languageCode => $label ) { if ( $label === '' ) { - $value[$this->usekeys ? $languageCode : $idx++] = array( + $value[$this->getUsekeys() ? $languageCode : $idx++] = array( 'language' => $languageCode, 'removed' => '', ); } else { - $value[$this->usekeys ? $languageCode : $idx++] = array( + $value[$this->getUsekeys() ? $languageCode : $idx++] = array( 'language' => $languageCode, 'value' => $label, ); @@ -286,7 +260,7 @@ protected function addLabelsToResult( array $labels, $path, $name = 'labels', $t } if ( $value !== array() ) { - if (!$this->usekeys) { + if ( !$this->getUsekeys() ) { $this->getResult()->setIndexedTagName( $value, $tag ); } $this->getResult()->addValue( $path, $name, $value ); diff --git a/repo/includes/api/ApiGetItems.php b/repo/includes/api/ApiGetItems.php index 171cbfb997..49d7beee82 100644 --- a/repo/includes/api/ApiGetItems.php +++ b/repo/includes/api/ApiGetItems.php @@ -77,8 +77,6 @@ public function execute() { $languages = $params['languages']; - $this->setUsekeys( $params ); - // This really needs a more generic solution as similar tricks will be // done to other props as well, for example variants for the language // attributes. It would also be nice to write something like */urls for diff --git a/repo/includes/api/ApiModifyItem.php b/repo/includes/api/ApiModifyItem.php index 205cc91a73..f58a001b03 100644 --- a/repo/includes/api/ApiModifyItem.php +++ b/repo/includes/api/ApiModifyItem.php @@ -175,7 +175,6 @@ public function execute() { } // FIXME: we can (?) do this before we do permission checks as long as we don't save - $this->setUsekeys( $params ); $success = $this->modifyItem( $itemContent, $params ); if ( !$success ) { diff --git a/repo/includes/api/ApiSetItem.php b/repo/includes/api/ApiSetItem.php index 24ec499742..4bc130fd25 100644 --- a/repo/includes/api/ApiSetItem.php +++ b/repo/includes/api/ApiSetItem.php @@ -194,7 +194,6 @@ protected function modifyItem( ItemContent &$itemContent, array $params ) { $res = $this->getResult(); - $this->setUsekeys( $params ); $this->addLabelsToResult( $item->getLabels(), 'item' ); $this->addDescriptionsToResult( $item->getDescriptions(), 'item' ); $this->addAliasesToResult( $item->getAllAliases(), 'item' ); diff --git a/repo/tests/phpunit/includes/api/ApiGetItemsTest.php b/repo/tests/phpunit/includes/api/ApiGetItemsTest.php index c316b0e9e7..0d6830217b 100644 --- a/repo/tests/phpunit/includes/api/ApiGetItemsTest.php +++ b/repo/tests/phpunit/includes/api/ApiGetItemsTest.php @@ -340,5 +340,60 @@ function testSitelinkUrls( $handle ) { $this->assertArrayHasKey( 'url', $link ); } } + + /** + * @dataProvider providerGetItemFormat + */ + function testGetItemFormat( $format, $usekeys ) { + $this->createItems(); + + $item = $this->getItemOutput( 'Berlin' ); + $id = $item['id']; + + list($res,,) = $this->doApiRequest( + array( + 'action' => 'wbgetitems', + 'format' => $format, + 'ids' => $id ) + ); + + $this->assertSuccess( $res, 'items', $id ); + if ( $usekeys ) { + foreach ( array( 'sitelinks' => 'site', 'alias' => false, 'labels' => 'language', 'descriptions' => 'language' ) as $prop => $field) { + if ( array_key_exists( $prop, $res['items'][$id] ) ) { + foreach ( $res['items'][$id][$prop] as $key => $value ) { + $this->assertTrue( is_string( $key ) ); + if ( $field !== false ) { + $this->assertArrayHasKey( $field, $value ); + $this->assertTrue( is_string( $value[$field] ) ); + $this->assertEquals( $key, $value[$field] ); + } + } + } + } + } + else { + foreach ( array( 'sitelinks' => 'site', 'alias' => false, 'labels' => 'language', 'descriptions' => 'language' ) as $prop => $field) { + if ( array_key_exists( $prop, $res['items'][$id] ) ) { + foreach ( $res['items'][$id][$prop] as $key => $value ) { + $this->assertTrue( is_integer( $key ) ); + if ( $field !== false ) { + $this->assertArrayHasKey( $field, $value ); + $this->assertTrue( is_string( $value[$field] ) ); + } + } + } + } + } + } + + function providerGetItemFormat() { + $calls = array(); + foreach ( Settings::get( 'formatsWithKeys' ) as $format => $usekeys) { + $calls[] = array( $format, $usekeys ); + } + return $calls; + } + } diff --git a/repo/tests/phpunit/includes/api/ApiSetAliasesTest.php b/repo/tests/phpunit/includes/api/ApiSetAliasesTest.php index 406dd1817b..176c2920d6 100644 --- a/repo/tests/phpunit/includes/api/ApiSetAliasesTest.php +++ b/repo/tests/phpunit/includes/api/ApiSetAliasesTest.php @@ -80,7 +80,6 @@ public function testSetAliases( $handle, $langCode, $op, $value, $expected ) { // update the item ---------------------------------------------------------------- $req = array( 'token' => $this->getItemToken(), - 'usekeys' => true, 'id' => $id, 'action' => 'wbsetaliases', 'language' => $langCode,