jenkins-bot has submitted this change and it was merged. Change subject: Add ungrouped list api parameter ......................................................................
Add ungrouped list api parameter Adds the ability to get api output without snaks being grouped by property Bug: 55795 Change-Id: I87d20b80686f8b9bf9fe189faaf1aa4b74c8aaf6 --- M repo/includes/api/GetClaims.php M repo/includes/api/GetEntities.php M repo/tests/phpunit/includes/api/EntityTestHelper.php M repo/tests/phpunit/includes/api/GetClaimsTest.php M repo/tests/phpunit/includes/api/GetEntitiesTest.php M repo/tests/phpunit/includes/api/ResultBuilderTest.php M repo/tests/phpunit/includes/api/WikibaseApiTestCase.php 7 files changed, 150 insertions(+), 67 deletions(-) Approvals: Daniel Kinzler: Looks good to me, approved Addshore: Verified jenkins-bot: Verified diff --git a/repo/includes/api/GetClaims.php b/repo/includes/api/GetClaims.php index 68d1a32..69de77d 100644 --- a/repo/includes/api/GetClaims.php +++ b/repo/includes/api/GetClaims.php @@ -8,6 +8,7 @@ use Wikibase\Entity; use Wikibase\Claims; use Wikibase\Claim; +use Wikibase\Lib\Serializers\SerializationOptions; use Wikibase\Repo\WikibaseRepo; /** @@ -17,6 +18,7 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > + * @author Adam Shorland */ class GetClaims extends ApiWikibase { @@ -45,6 +47,14 @@ if ( !$entity ) { $this->dieUsage( "No entity found matching ID $id", 'no-such-entity' ); + } + + if( $params['ungroupedlist'] ) { + $this->resultBuilder->getSerializationOptions() + ->setOption( + SerializationOptions::OPT_GROUP_BY_PROPERTIES, + array() + ); } $claims = $this->getClaims( $entity, $claimGuid ); @@ -106,7 +116,7 @@ $claims = array(); - /** @var \Wikibase\Claim $claim */ + /** @var Claim $claim */ foreach ( $claimsList as $claim ) { if ( $this->claimMatchesFilters( $claim ) ) { $claims[] = $claim; @@ -214,6 +224,10 @@ ), ApiBase::PARAM_DFLT => 'references', ), + 'ungroupedlist' => array( + ApiBase::PARAM_TYPE => 'boolean', + ApiBase::PARAM_DFLT => false, + ), ); } @@ -231,6 +245,7 @@ 'claim' => 'A GUID identifying the claim. Required unless entity is provided. The GUID is the globally unique identifier for a claim, e.g. "q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0F".', 'rank' => 'Optional filter to return only the claims that have the specified rank', 'props' => 'Some parts of the claim are returned optionally. This parameter controls which ones are returned.', + 'ungroupedlist' => 'Do not group snaks by property id', ); } @@ -256,10 +271,14 @@ */ protected function getExamples() { return array( - "api.php?action=wbgetclaims&entity=Q42" => "Get claims for item with ID Q42", - "api.php?action=wbgetclaims&entity=Q42&property=P2" => "Get claims for item with ID Q42 and property with ID P2", - "api.php?action=wbgetclaims&entity=Q42&rank=normal" => "Get claims for item with ID Q42 that are ranked as normal", - 'api.php?action=wbgetclaims&claim=Q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0F' => 'Get claim with GUID of Q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0F', + "api.php?action=wbgetclaims&entity=Q42" => + "Get claims for item with ID Q42", + "api.php?action=wbgetclaims&entity=Q42&property=P2" => + "Get claims for item with ID Q42 and property with ID P2", + "api.php?action=wbgetclaims&entity=Q42&rank=normal" => + "Get claims for item with ID Q42 that are ranked as normal", + 'api.php?action=wbgetclaims&claim=Q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0F' => + 'Get claim with GUID of Q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0F', ); } diff --git a/repo/includes/api/GetEntities.php b/repo/includes/api/GetEntities.php index 8270cc3..f27ccc3 100644 --- a/repo/includes/api/GetEntities.php +++ b/repo/includes/api/GetEntities.php @@ -127,11 +127,14 @@ if ( !empty( $params['sites'] ) && !empty( $params['titles'] ) ) { $itemByTitleHelper = $this->getItemByTitleHelper(); list( $ids, $missingItems ) = $itemByTitleHelper->getItemIds( $params['sites'], $params['titles'], $params['normalize'] ); - $this->addMissingItemstoResult( $missingItems ); + $this->addMissingItemsToResult( $missingItems ); } return $ids; } + /** + * @return ItemByTitleHelper + */ private function getItemByTitleHelper() { $siteLinkCache = StoreFactory::getStore()->newSiteLinkCache(); $siteStore = SiteSQLStore::newInstance(); @@ -146,7 +149,7 @@ /** * @param array $missingItems Array of arrays, Each internal array has a key 'site' and 'title' */ - private function addMissingItemstoResult( $missingItems ){ + private function addMissingItemsToResult( $missingItems ){ foreach( $missingItems as $missingItem ) { $this->getResultBuilder()->addMissingEntity( $missingItem ); } @@ -222,6 +225,12 @@ } else { $languages = $params['languages']; } + if( $params['ungroupedlist'] ) { + $options->setOption( + SerializationOptions::OPT_GROUP_BY_PROPERTIES, + array() + ); + } $options->setLanguages( $languages ); $options->setOption( EntitySerializer::OPT_SORT_ORDER, $params['dir'] ); $options->setOption( EntitySerializer::OPT_PARTS, $props ); @@ -283,6 +292,10 @@ ApiBase::PARAM_TYPE => 'boolean', ApiBase::PARAM_DFLT => false ), + 'ungroupedlist' => array( + ApiBase::PARAM_TYPE => 'boolean', + ApiBase::PARAM_DFLT => false, + ), ) ); } @@ -318,6 +331,7 @@ 'normalize' => array( 'Try to normalize the page title against the client site.', 'This only works if exactly one site and one page have been given.' ), + 'ungroupedlist' => array( 'Do not group snaks by property id' ), ) ); } diff --git a/repo/tests/phpunit/includes/api/EntityTestHelper.php b/repo/tests/phpunit/includes/api/EntityTestHelper.php index bb908f5..cc505c1 100644 --- a/repo/tests/phpunit/includes/api/EntityTestHelper.php +++ b/repo/tests/phpunit/includes/api/EntityTestHelper.php @@ -69,6 +69,15 @@ array( "language" => "nb", "value" => "Hovedsted og delstat og i Forbundsrepublikken Tyskland." ), array( "language" => "nn", "value" => "Hovudstad og delstat i Forbundsrepublikken Tyskland." ), ), + "claims" => array( + array( 'mainsnak' => array( + 'snaktype' => 'value', + 'property' => 'P56', + 'datavalue' => array( 'value' => 'imastring1', 'type' => 'string' ), + ), + 'type' => 'statement', + 'rank' => 'normal' ) + ), ) ), 'London' => array( @@ -267,12 +276,13 @@ /** * Remove props and langs that are not included in $props or $langs from the $entityOutput array * @param array $entityOutput Array of entity output - * @param null|array $props Props to keep in the output + * @param array $props Props to keep in the output * @param null|array $langs Languages to keep in the output * @return array Array of entity output with props and langs removed */ - protected static function stripUnwantedOutputValues( $entityOutput, $props = null, $langs = null ){ + protected static function stripUnwantedOutputValues( $entityOutput, $props = array(), $langs = null ){ $entityProps = array(); + $props[] = 'type'; // always return the type so we can demobilize foreach( $props as $prop ){ if( array_key_exists( $prop, $entityOutput ) ){ $entityProps[ $prop ] = $entityOutput[ $prop ] ; diff --git a/repo/tests/phpunit/includes/api/GetClaimsTest.php b/repo/tests/phpunit/includes/api/GetClaimsTest.php index b424ffa..7d41698 100644 --- a/repo/tests/phpunit/includes/api/GetClaimsTest.php +++ b/repo/tests/phpunit/includes/api/GetClaimsTest.php @@ -2,13 +2,19 @@ namespace Wikibase\Test\Api; -use Wikibase\ByPropertyIdArray; -use Wikibase\DataModel\Entity\PropertyId; +use DataValues\StringValue; +use UsageException; use Wikibase\Entity; use Wikibase\Claim; use Wikibase\Claims; use Wikibase\Item; +use Wikibase\Lib\Serializers\ClaimSerializer; +use Wikibase\Lib\Serializers\SerializationOptions; +use Wikibase\Lib\Serializers\SerializerFactory; use Wikibase\Property; +use Wikibase\PropertyNoValueSnak; +use Wikibase\PropertySomeValueSnak; +use Wikibase\PropertyValueSnak; use Wikibase\Repo\WikibaseRepo; use Wikibase\Statement; @@ -29,6 +35,7 @@ * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > * @author Katie Filbert < [email protected] > + * @author Adam Shorland */ class GetClaimsTest extends \ApiTestCase { @@ -43,10 +50,10 @@ $content->save( '', null, EDIT_NEW ); /** @var $claims Claim[] */ - $claims[0] = $entity->newClaim( new \Wikibase\PropertyNoValueSnak( 42 ) ); - $claims[1] = $entity->newClaim( new \Wikibase\PropertyNoValueSnak( 1 ) ); - $claims[2] = $entity->newClaim( new \Wikibase\PropertySomeValueSnak( 42 ) ); - $claims[3] = $entity->newClaim( new \Wikibase\PropertyValueSnak( 9001, new \DataValues\StringValue( 'o_O' ) ) ); + $claims[0] = $entity->newClaim( new PropertyNoValueSnak( 42 ) ); + $claims[1] = $entity->newClaim( new PropertyNoValueSnak( 1 ) ); + $claims[2] = $entity->newClaim( new PropertySomeValueSnak( 42 ) ); + $claims[3] = $entity->newClaim( new PropertyValueSnak( 9001, new StringValue( 'o_O' ) ) ); foreach( $claims as $key => $claim ){ $claim->setGuid( $entity->getId()->getPrefixedId() . '$D8404CDA-56A1-4334-AF13-A3290BCD9CL' . $key ); @@ -84,7 +91,7 @@ 'entity' => $this->getFormattedIdForEntity( $entity ), ); - $argLists[] = array( $params, $entity->getClaims() ); + $argLists[] = array( $params, $entity->getClaims(), true ); /** * @var Claim $claim @@ -94,15 +101,17 @@ 'action' => 'wbgetclaims', 'claim' => $claim->getGuid(), ); + $argLists[] = array( $params, array( $claim ), true ); - $argLists[] = array( $params, array( $claim ) ); + $params['ungroupedlist'] = true; + $argLists[] = array( $params, array( $claim ), false ); } foreach ( array( Statement::RANK_DEPRECATED, Statement::RANK_NORMAL, Statement::RANK_PREFERRED ) as $rank ) { $params = array( 'action' => 'wbgetclaims', 'entity' => $this->getFormattedIdForEntity( $entity ), - 'rank' => \Wikibase\Lib\Serializers\ClaimSerializer::serializeRank( $rank ), + 'rank' => ClaimSerializer::serializeRank( $rank ), ); $claims = array(); @@ -113,7 +122,7 @@ } } - $argLists[] = array( $params, $claims ); + $argLists[] = array( $params, $claims, true ); } } @@ -127,42 +136,36 @@ public function testValidRequests() { foreach ( $this->validRequestProvider() as $argList ) { - list( $params, $claims ) = $argList; + list( $params, $claims, $groupedByProperty ) = $argList; - $this->doTestValidRequest( $params, $claims ); + $this->doTestValidRequest( $params, $claims, $groupedByProperty ); } } /** * @param string[] $params * @param Claims|Claim[] $claims + * @param bool $groupedByProperty */ - public function doTestValidRequest( array $params, $claims ) { + public function doTestValidRequest( array $params, $claims, $groupedByProperty ) { + if ( is_array( $claims ) ) { + $claims = new Claims( $claims ); + } + $options = new SerializationOptions(); + if( !$groupedByProperty ) { + $options->setOption( SerializationOptions::OPT_GROUP_BY_PROPERTIES, array() ); + } + $serializerFactory = new SerializerFactory(); + $serializer = $serializerFactory->newSerializerForObject( $claims ); + $serializer->setOptions( $options ); + $expected = $serializer->getSerialized( $claims ); + list( $resultArray, ) = $this->doApiRequest( $params ); $this->assertInternalType( 'array', $resultArray, 'top level element is an array' ); $this->assertArrayHasKey( 'claims', $resultArray, 'top level element has a claims key' ); - if ( is_array( $claims ) ) { - $claims = new \Wikibase\Claims( $claims ); - } - - $serializerFactory = new \Wikibase\Lib\Serializers\SerializerFactory(); - $serializer = $serializerFactory->newSerializerForObject( $claims ); - $expected = $serializer->getSerialized( $claims ); - - $byPropClaims = new ByPropertyIdArray( $claims ); - $byPropClaims->buildIndex(); - - // TODO: this is a rather simplistic test. - // Would be nicer if we could deserialize the list and then use the equals method - // or to serialize the expected value and have a recursive array compare on that - foreach ( $expected as $propertyId => $claimsForProperty ) { - $this->assertEquals( - count( $claimsForProperty ), - count( $byPropClaims->getByPropertyId( new PropertyId( $propertyId ) ) ) - ); - } + $this->assertEquals( $expected, $resultArray['claims'] ); } /** @@ -177,7 +180,7 @@ try { $this->doApiRequest( $params ); $this->fail( 'Invalid claim guid did not throw an error' ); - } catch ( \UsageException $e ) { + } catch ( UsageException $e ) { $this->assertEquals( 'invalid-guid', $e->getCodeString(), 'Invalid claim guid raised correct error' ); } } diff --git a/repo/tests/phpunit/includes/api/GetEntitiesTest.php b/repo/tests/phpunit/includes/api/GetEntitiesTest.php index ea900c4..23de4b1 100644 --- a/repo/tests/phpunit/includes/api/GetEntitiesTest.php +++ b/repo/tests/phpunit/includes/api/GetEntitiesTest.php @@ -2,6 +2,10 @@ namespace Wikibase\Test\Api; +use Wikibase\Lib\Serializers\EntitySerializer; +use Wikibase\Lib\Serializers\SerializationOptions; +use Wikibase\Lib\Serializers\SerializerFactory; + /** * @covers Wikibase\Api\GetEntities * @@ -18,7 +22,6 @@ * @group WikibaseRepo * @group GetEntitiesTest * @group BreakingTheSlownessBarrier - * * @group Database * @group medium */ @@ -99,8 +102,7 @@ 'claims', 'datatype', //multiple props - 'labels|sitelinks/urls', - 'info|aliases|labels|claims' + 'labels|sitelinks/urls|info|claims', ); /** @@ -111,7 +113,6 @@ 'de', 'zh', //multiple languages - 'it|es|zh|ar', 'de|nn|nb|en|en-gb|it|es|zh|ar' ); @@ -124,8 +125,8 @@ ); /** - * These are all availible formats for the API. we need to make sure they all work - * Each format is only tested against the first set of good paramers, from then on json is always used + * These are all available formats for the API. we need to make sure they all work + * Each format is only tested against the first set of good parameters, from then on json is always used */ protected static $goodFormats = array( 'json', @@ -156,6 +157,10 @@ $testCase['p']['languages'] = $langData; $testCase['p'] = array_merge( $testCase['p'], $sortData ); $testCases[] = $testCase; + if( in_array( 'claims', explode( '|', $propData ) ) ){ + $testCase['p']['ungroupedlist'] = true; + $testCases[] = $testCase; + } } } } @@ -175,7 +180,7 @@ * This method tests all valid API requests * @dataProvider provideData */ - function testGetEntities( $params, $expected ) { + public function testGetEntities( $params, $expected ) { // -- setup any further data ----------------------------------------------- $params['ids'] = implode( '|', $this->getIdsFromHandlesAndIds( $params ) ); $params = $this->removeHandles( $params ); @@ -191,7 +196,7 @@ $this->assertEquals( $expected['count'], count( $result['entities'] ), "Request returned incorrect number of entities" ); - foreach ( $result['entities'] as $entityKey => $entity ) { + foreach ( $result['entities'] as $entity ) { if ( array_key_exists( 'missing', $expected ) && array_key_exists( 'missing', $entity ) ) { $this->assertArrayHasKey( 'missing', $entity ); $this->assertGreaterThanOrEqual( 0, $expected['missing'], @@ -265,40 +270,63 @@ } else { $expected['dir'] = 'ascending'; } + + //expect snaks to be grouped by property or not + if( !isset( $params['ungroupedlist'] ) || !$params['ungroupedlist'] ) { + $expected['groupedbyproperty'] = true; + } else { + $expected['groupedbyproperty'] = false; + } return $expected; } 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'] ) ) { - $this->assertEntityPropsInfo( $entity, $expected ); + $this->assertEntityPropsInfo( $entity ); } if ( in_array( 'datatype', $expected['props'] ) ) { $this->assertArrayHasKey( 'type', $entity, 'An entity is missing the type value' ); } if ( in_array( 'sitelinks', $expected['props'] ) ) { - $this->assertEntityPropsSitelinksBadges( $entity, $expected ); + $this->assertEntityPropsSitelinksBadges( $entity ); } if ( in_array( 'sitelinks/urls', $expected['props'] ) ) { - $this->assertEntityPropsSitelinksUrls( $entity, $expected ); + $this->assertEntityPropsSitelinksUrls( $entity ); } if ( array_key_exists( 'dir', $expected ) && array_key_exists( 'sitelinks', $entity ) ) { $this->assertEntitySitelinkSorting( $entity, $expected ); } //Assert the whole entity is as expected (claims, sitelinks, aliases, descriptions, labels) + $expectedEntityOutput = EntityTestHelper::getEntityOutput ( + EntityTestHelper::getHandle( $entity['id'] ), + $expected['props'], + $expected['languages'] + ); + if( !$expected['groupedbyproperty'] ) { + $options = new SerializationOptions(); + $options->setOption( SerializationOptions::OPT_GROUP_BY_PROPERTIES, array() ); + $factory = new SerializerFactory(); + /** @var EntitySerializer $serializer */ + $serializer = $factory->newSerializerForEntity( $entity['type'], $options ); + $expectedEntityOutput = $serializer->getSerialized( + $serializer->newFromSerialization( + $expectedEntityOutput + ) + ); + } $this->assertEntityEquals( - EntityTestHelper::getEntityOutput( - EntityTestHelper::getHandle( $entity['id'] ), - $expected['props'], - $expected['languages'] - ), + $expectedEntityOutput, $entity, false ); } - private function assertEntityPropsInfo( $entity, $expected ) { + /** + * @param array $entity + */ + private function assertEntityPropsInfo( $entity ) { $this->assertArrayHasKey( 'pageid', $entity, 'An entity is missing the pageid value' ); $this->assertInternalType( 'integer', $entity['pageid'] ); $this->assertGreaterThan( 0, $entity['pageid'] ); @@ -326,14 +354,20 @@ $this->assertArrayHasKey( 'type', $entity, 'An entity is missing the type value' ); } - private function assertEntityPropsSitelinksUrls( $entity, $expected ) { + /** + * @param array $entity + */ + private function assertEntityPropsSitelinksUrls( $entity ) { foreach ( $entity['sitelinks'] as $sitelink ) { $this->assertArrayHasKey( 'url', $sitelink ); $this->assertNotEmpty( $sitelink['url'] ); } } - private function assertEntityPropsSitelinksBadges( $entity, $expected ) { + /** + * @param array $entity + */ + private function assertEntityPropsSitelinksBadges( $entity ) { foreach ( $entity['sitelinks'] as $sitelink ) { $this->assertArrayHasKey( 'badges', $sitelink ); $this->assertInternalType( 'array', $sitelink['badges'] ); @@ -435,7 +469,7 @@ $this->doTestQueryExceptions( $params, $expected['exception'] ); } - function provideLanguageFallback() { + public function provideLanguageFallback() { return array( array( 'Guangzhou', @@ -495,7 +529,7 @@ * @dataProvider provideLanguageFallback * @todo factor this into the main test method */ - function testLanguageFallback( $handle, $languages, $expectedLabels, $expectedDescriptions ) { + public function testLanguageFallback( $handle, $languages, $expectedLabels, $expectedDescriptions ) { $id = EntityTestHelper::getId( $handle ); list($res,,) = $this->doApiRequest( diff --git a/repo/tests/phpunit/includes/api/ResultBuilderTest.php b/repo/tests/phpunit/includes/api/ResultBuilderTest.php index 91d7fae..0569fb5 100644 --- a/repo/tests/phpunit/includes/api/ResultBuilderTest.php +++ b/repo/tests/phpunit/includes/api/ResultBuilderTest.php @@ -4,7 +4,6 @@ use ApiResult; use DataValues\StringValue; -use PHPUnit_Framework_TestCase; use Wikibase\Api\ResultBuilder; use Wikibase\Claim; use Wikibase\DataModel\Entity\ItemId; @@ -30,7 +29,7 @@ * @licence GNU GPL v2+ * @author Adam Shorland */ -class ResultBuilderTest extends PHPUnit_Framework_TestCase { +class ResultBuilderTest extends \PHPUnit_Framework_TestCase { protected function getDefaultResult( $indexedMode = false ){ $apiMain = $this->getMockBuilder( 'ApiMain' )->disableOriginalConstructor()->getMockForAbstractClass(); diff --git a/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php b/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php index 4dccfa0..0475385 100644 --- a/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php +++ b/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php @@ -215,7 +215,7 @@ * @param bool $expectEmptyArrays Should we expect empty arrays or just ignore them? */ public function assertEntityEquals( $expected, $actual, $expectEmptyArrays = true ) { - if ( isset( $expected['id'] ) ) { + if ( isset( $expected['id'] ) && !empty( $expected['id'] ) ) { $this->assertEquals( $expected['id'], $actual['id'], 'id' ); } if ( isset( $expected['lastrevid'] ) ) { @@ -276,6 +276,10 @@ $this->assertGreaterThanOrEqual( 39, strlen( $data['id'][$i] ) ); } //unset stuff we dont actually want to compare + if( isset( $exp['id'] ) ) { + $this->assertArrayHasKey( 'id', $data ); + } + unset( $exp['id'] ); unset( $data['id'] ); unset( $data['hash'] ); unset( $data['qualifiers-order'] ); -- To view, visit https://gerrit.wikimedia.org/r/96268 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I87d20b80686f8b9bf9fe189faaf1aa4b74c8aaf6 Gerrit-PatchSet: 15 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Addshore <[email protected]> Gerrit-Reviewer: Addshore <[email protected]> Gerrit-Reviewer: Aude <[email protected]> Gerrit-Reviewer: Daniel Kinzler <[email protected]> Gerrit-Reviewer: Tobias Gritschacher <[email protected]> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
