Browse files

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
  • Loading branch information...
1 parent 9e27ff2 commit 399d443e19f9e69836f2a1a8f57f5b680231a3ee @jeblad jeblad committed Aug 9, 2012
View
7 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,
View
66 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,21 +207,21 @@ 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,
);
}
}
if ( $value !== array() ) {
- if (!$this->usekeys) {
+ if ( !$this->getUsekeys() ) {
$this->getResult()->setIndexedTagName( $value, $tag );
}
$this->getResult()->addValue( $path, $name, $value );
@@ -272,21 +246,21 @@ 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,
);
}
}
if ( $value !== array() ) {
- if (!$this->usekeys) {
+ if ( !$this->getUsekeys() ) {
$this->getResult()->setIndexedTagName( $value, $tag );
}
$this->getResult()->addValue( $path, $name, $value );
View
2 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
View
1 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 ) {
View
1 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' );
View
55 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;
+ }
+
}
View
1 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,

0 comments on commit 399d443

Please sign in to comment.