jenkins-bot has submitted this change and it was merged.
Change subject: Merge sitelinks/badges to sitelinks (api props)
......................................................................
Merge sitelinks/badges to sitelinks (api props)
Badges are part of sitelinks and should be returned
always.
The only exception is sitelinks/removed. If it's
set, badges are omitted as they are redundant then.
Some whitespace fixes included.
Change-Id: Ib35985655767422d40c5fbf37ecbfc40cd661958
---
M lib/includes/serializers/SiteLinkSerializer.php
M lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
M repo/includes/api/GetEntities.php
M repo/tests/phpunit/includes/api/GetEntitiesTest.php
4 files changed, 48 insertions(+), 67 deletions(-)
Approvals:
Addshore: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/includes/serializers/SiteLinkSerializer.php
b/lib/includes/serializers/SiteLinkSerializer.php
index 6e33ade..0c83f02 100644
--- a/lib/includes/serializers/SiteLinkSerializer.php
+++ b/lib/includes/serializers/SiteLinkSerializer.php
@@ -85,7 +85,7 @@
$serialization = array();
$includeUrls = in_array( 'sitelinks/urls',
$this->options->getProps() );
- $includeBadges = in_array( 'sitelinks/badges' ,
$this->options->getProps() );
+ $setRemoved = in_array( 'sitelinks/removed' ,
$this->options->getProps() );
foreach ( $this->sortSiteLinks( $siteLinks ) as $link ) {
$response = array(
@@ -102,7 +102,7 @@
}
}
- if ( $includeBadges ) {
+ if ( !$setRemoved ) {
$badges = array();
foreach ( $link->getBadges() as $badge ) {
@@ -116,7 +116,7 @@
$response['badges'] = $badges;
}
- if ( in_array( 'sitelinks/removed',
$this->options->getProps() ) ) {
+ if ( $setRemoved ) {
$response['removed'] = '';
}
diff --git a/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
b/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
index 459c6b7..a1aea2a 100644
--- a/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
+++ b/lib/tests/phpunit/serializers/SiteLinkSerializerTest.php
@@ -32,7 +32,6 @@
$options = new EntitySerializationOptions( $idFormatter );
$options->setIndexTags( false );
- $options->addProp( "sitelinks/badges" );
$siteLinks = array(
new SimpleSiteLink( "enwiki", "Rome", array( new
ItemId( "Q42" ) ) ),
new SimpleSiteLink( "dewiki", "Rom" ),
@@ -49,8 +48,8 @@
$options->setIndexTags( false );
$options->addProp( "sitelinks/removed" );
$siteLinks = array(
- new SimpleSiteLink( "enwiki", "" ),
- new SimpleSiteLink( "dewiki", "" ),
+ new SimpleSiteLink( "enwiki", "", array( new
ItemId( "Q42" ) ) ),
+ new SimpleSiteLink( "dewiki", "", array() ),
new SimpleSiteLink( "itwiki", "" ),
);
$expectedSerialization = array(
@@ -62,7 +61,6 @@
$options = new EntitySerializationOptions( $idFormatter );
$options->setIndexTags( true );
- $options->addProp( "sitelinks/badges" );
$siteLinks = array(
new SimpleSiteLink( "enwiki", "Rome", array( new
ItemId( "Q149" ), new ItemId( "Q49" ) ) ),
new SimpleSiteLink( "dewiki", "Rom", array( new ItemId(
"Q42" ) ) ),
@@ -72,22 +70,6 @@
array( "site" => "enwiki", "title" => "Rome", "badges"
=> array( "Q149", "Q49", "_element" => "badge" ) ),
array( "site" => "dewiki", "title" => "Rom", "badges"
=> array( "Q42", "_element" => "badge" ) ),
array( "site" => "itwiki", "title" => "Roma", "badges"
=> array( "_element" => "badge" ) ),
- "_element" => "sitelink",
- );
- $validArgs[] = array( $siteLinks, $options,
$expectedSerialization );
-
- // no badges prop
- $options = new EntitySerializationOptions( $idFormatter );
- $options->setIndexTags( true );
- $siteLinks = array(
- new SimpleSiteLink( "enwiki", "Rome", array( new
ItemId( "Q149" ), new ItemId( "Q49" ) ) ),
- new SimpleSiteLink( "dewiki", "Rom", array( new ItemId(
"Q42" ) ) ),
- new SimpleSiteLink( "itwiki", "Roma" ),
- );
- $expectedSerialization = array(
- array( "site" => "enwiki", "title" => "Rome" ),
- array( "site" => "dewiki", "title" => "Rom" ),
- array( "site" => "itwiki", "title" => "Roma" ),
"_element" => "sitelink",
);
$validArgs[] = array( $siteLinks, $options,
$expectedSerialization );
diff --git a/repo/includes/api/GetEntities.php
b/repo/includes/api/GetEntities.php
index 3cb4e97..bd417db 100644
--- a/repo/includes/api/GetEntities.php
+++ b/repo/includes/api/GetEntities.php
@@ -115,8 +115,7 @@
* @return bool
*/
protected function hasImpliedSiteLinksProp( $props ) {
- return in_array( 'sitelinks/urls', $props )
- || in_array( 'sitelinks/badges', $props );
+ return in_array( 'sitelinks/urls', $props );
}
/**
@@ -286,7 +285,7 @@
ApiBase::PARAM_ALLOW_DUPLICATES => true
),
'props' => array(
- ApiBase::PARAM_TYPE => array( 'info',
'sitelinks', 'sitelinks/urls', 'sitelinks/badges', 'aliases', 'labels',
+ ApiBase::PARAM_TYPE => array( 'info',
'sitelinks', 'sitelinks/urls', 'aliases', 'labels',
'descriptions', 'claims', 'datatype' ),
ApiBase::PARAM_DFLT =>
'info|sitelinks|aliases|labels|descriptions|claims|datatype',
ApiBase::PARAM_ISMULTI => true,
diff --git a/repo/tests/phpunit/includes/api/GetEntitiesTest.php
b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
index 41bd019..bd12370 100644
--- a/repo/tests/phpunit/includes/api/GetEntitiesTest.php
+++ b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
@@ -95,11 +95,9 @@
'descriptions',
'claims',
'datatype',
- //sub properties
- 'sitelinks/badges',
//multiple props
'labels|sitelinks/urls',
- 'info|aliases|labels|claims|sitelinks/badges'
+ 'info|aliases|labels|claims'
);
/**
@@ -145,10 +143,10 @@
$testCases = array();
// Generate test cases based on the static information provided
in arrays above
- foreach( self::$goodItems as $itemData ){
- foreach( self::$goodProps as $propData ){
- foreach( self::$goodLangs as $langData ){
- foreach( self::$goodSorts as $sortData
){
+ foreach ( self::$goodItems as $itemData ) {
+ foreach ( self::$goodProps as $propData ) {
+ foreach ( self::$goodLangs as $langData ) {
+ foreach ( self::$goodSorts as $sortData
) {
$testCase['p'] = $itemData['p'];
$testCase['e'] = $itemData['e'];
$testCase['p']['props'] =
$propData;
@@ -161,7 +159,7 @@
}
// We only want to test each format once so don't include this
in the main generation loop
- foreach( self::$goodFormats as $formatData ){
+ foreach ( self::$goodFormats as $formatData ) {
$testCase = $testCases[0];
$testCase['p']['format'] = $formatData;
$testCases[] = $testCase;
@@ -174,7 +172,7 @@
* This method tests all valid API requests
* @dataProvider provideData
*/
- function testGetEntities( $params, $expected ){
+ function testGetEntities( $params, $expected ) {
// -- setup any further data
-----------------------------------------------
$params['ids'] = implode( '|', $this->getIdsFromHandlesAndIds(
$params ) );
$params = $this->removeHandles( $params );
@@ -190,8 +188,8 @@
$this->assertEquals( $expected['count'], count(
$result['entities'] ),
"Request returned incorrect number of entities" );
- foreach( $result['entities'] as $entityKey => $entity ){
- if( array_key_exists( 'missing', $expected ) &&
array_key_exists( 'missing', $entity ) ){
+ foreach ( $result['entities'] as $entityKey => $entity ) {
+ if ( array_key_exists( 'missing', $expected ) &&
array_key_exists( 'missing', $entity ) ) {
$this->assertArrayHasKey( 'missing', $entity );
$this->assertGreaterThanOrEqual( 0,
$expected['missing'],
'Got missing entity but not expecting
any more' );
@@ -203,11 +201,11 @@
}
//Our missing counter should now be at 0, if it is not we have
seen too many or not enough missing entities
- if( array_key_exists( 'missing', $expected ) ){
+ if ( array_key_exists( 'missing', $expected ) ) {
$this->assertEquals( 0, $expected['missing'] );
}
- if( array_key_exists( 'normalized', $expected ) &&
$expected['normalized'] === true ){
+ if ( array_key_exists( 'normalized', $expected ) &&
$expected['normalized'] === true ) {
$this->assertNormalization( $result, $params );
} else {
$this->assertArrayNotHasKey( 'normalized', $result );
@@ -215,14 +213,14 @@
}
private function getIdsFromHandlesAndIds( $params ) {
- if( array_key_exists( 'ids', $params ) ){
+ if ( array_key_exists( 'ids', $params ) ) {
$ids = explode( '|', $params['ids'] );
} else {
$ids = array();
}
- if( array_key_exists( 'handles', $params ) ){
- foreach( $params['handles'] as $handle ){
+ if ( array_key_exists( 'handles', $params ) ) {
+ foreach ( $params['handles'] as $handle ) {
//For every id we use we add both the uppercase
and lowercase id to the test
//This then makes sure we only get 1 entity
when the only difference between the ids is the case
$ids[] = strtolower( EntityTestHelper::getId(
$handle ) );
@@ -233,7 +231,7 @@
}
private function removeHandles( $params ) {
- if( array_key_exists( 'handles', $params ) ){
+ if ( array_key_exists( 'handles', $params ) ) {
unset( $params['handles'] );
}
return $params;
@@ -241,19 +239,25 @@
private function calculateExpectedData( $expected, $params ) {
//expect the props in params or the default props of the api
- if( array_key_exists( 'props', $params ) ){
+ if ( array_key_exists( 'props', $params ) ) {
$expected['props'] = explode( '|', $params['props'] );
} else {
$expected['props'] = array( 'info', 'sitelinks',
'aliases', 'labels', 'descriptions', 'claims', 'datatype' );
}
+
+ //implied props
+ if ( in_array( 'sitelinks/urls' , $expected['props'] ) ) {
+ $expected['props'][] = 'sitelinks';
+ }
+
//expect the languages in params or just all languages
- if( array_key_exists( 'languages', $params ) ){
+ if ( array_key_exists( 'languages', $params ) ) {
$expected['languages'] = explode( '|',
$params['languages'] );
} else {
$expected['languages'] = null;
}
//expect order in params or expect default
- if( array_key_exists( 'dir', $params ) ){
+ if ( array_key_exists( 'dir', $params ) ) {
$expected['dir'] = $params['dir'];
} else {
$expected['dir'] = 'ascending';
@@ -263,19 +267,19 @@
private function assertEntityResult( $entity, $expected ) {
//Assert individual props of each entity (if we want them, make
sure they are there)
- if( in_array( 'info', $expected['props'] ) ){
+ if ( in_array( 'info', $expected['props'] ) ) {
$this->assertEntityPropsInfo( $entity, $expected );
}
- if( in_array( 'datatype', $expected['props'] ) ){
+ if ( in_array( 'datatype', $expected['props'] ) ) {
$this->assertArrayHasKey( 'type', $entity, 'An entity
is missing the type value' );
}
- if( in_array( 'sitelinks/urls', $expected['props'] ) ){
- $this->assertEntityPropsSitelinksUrls( $entity,
$expected );
- }
- if( in_array( 'sitelinks/badges', $expected['props'] ) ){
+ if ( in_array( 'sitelinks', $expected['props'] ) ) {
$this->assertEntityPropsSitelinksBadges( $entity,
$expected );
}
- if( array_key_exists( 'dir', $expected ) && array_key_exists(
'sitelinks', $entity ) ){
+ if ( in_array( 'sitelinks/urls', $expected['props'] ) ) {
+ $this->assertEntityPropsSitelinksUrls( $entity,
$expected );
+ }
+ if ( array_key_exists( 'dir', $expected ) && array_key_exists(
'sitelinks', $entity ) ) {
$this->assertEntitySitelinkSorting( $entity, $expected
);
}
@@ -319,22 +323,18 @@
}
private function assertEntityPropsSitelinksUrls( $entity, $expected ) {
- $expected['props'][] = 'sitelinks';
-
- foreach( $entity['sitelinks'] as $sitelink ){
+ foreach ( $entity['sitelinks'] as $sitelink ) {
$this->assertArrayHasKey( 'url', $sitelink );
$this->assertNotEmpty( $sitelink['url'] );
}
}
private function assertEntityPropsSitelinksBadges( $entity, $expected )
{
- $expected['props'][] = 'sitelinks';
-
- foreach( $entity['sitelinks'] as $sitelink ){
+ foreach ( $entity['sitelinks'] as $sitelink ) {
$this->assertArrayHasKey( 'badges', $sitelink );
$this->assertInternalType( 'array', $sitelink['badges']
);
- foreach( $sitelink['badges'] as $badge ){
+ foreach ( $sitelink['badges'] as $badge ) {
$this->assertStringStartsWith( 'Q', $badge );
$this->assertGreaterThan( 1, strlen( $badge ) );
}
@@ -343,14 +343,14 @@
private function assertEntitySitelinkSorting( $entity, $expected ) {
$last = '';
- if( $expected['dir'] == 'descending' ){
+ if ( $expected['dir'] == 'descending' ) {
$last = 'zzzzzzzz';
}
- foreach( $entity['sitelinks'] as $link ){
+ foreach ( $entity['sitelinks'] as $link ) {
$site = $link['site'];
- if( $expected['dir'] == 'ascending' ){
+ if ( $expected['dir'] == 'ascending' ) {
$this->assertTrue(
strcmp( $last, $site ) <= 0,
"Failed to assert order of sitelinks,
('{$last}' vs '{$site}') <=0"
@@ -372,7 +372,7 @@
$result['normalized']['n']['from']
);
$this->assertEquals(
- // Normalization in unit tests is actually using
Title::getPrefixedText instead of a real API call
+ // Normalization in unit tests is actually using
Title::getPrefixedText instead of a real API call
\Title::newFromText( $params['titles']
)->getPrefixedText(),
$result['normalized']['n']['to']
);
@@ -417,12 +417,12 @@
/**
* @dataProvider provideExceptionData
*/
- public function testGetEntitiesExceptions( $params, $expected ){
+ public function testGetEntitiesExceptions( $params, $expected ) {
// -- set any defaults ------------------------------------
$params['action'] = 'wbgetentities';
- if( array_key_exists( 'handles', $params ) ){
+ if ( array_key_exists( 'handles', $params ) ) {
$ids = array();
- foreach( $params['handles'] as $handle ){
+ foreach ( $params['handles'] as $handle ) {
$ids[ $handle ] = EntityTestHelper::getId(
$handle );
}
$params['ids'] = implode( '|', $ids );
--
To view, visit https://gerrit.wikimedia.org/r/87615
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib35985655767422d40c5fbf37ecbfc40cd661958
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Michał Łazowik <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Michał Łazowik <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
