Daniel Kinzler has uploaded a new change for review. https://gerrit.wikimedia.org/r/150790
Change subject: Allow explicit keys for entities in API results. ...................................................................... Allow explicit keys for entities in API results. This adds parameters for explicitly giving a key when adding an entity to an API result. This is needed to handle redirects correctly, where the requested ID, which should be used as the key in the result, is not the actual ID of the entity. This also introduces a more compelx fallback logic for determining the key used for missing entities. Bug: 45509 Change-Id: Ica78bc830fed8bc61b797a0c6f9ec3bf21ecb370 --- M repo/includes/LinkedData/EntityDataSerializationService.php M repo/includes/api/GetEntities.php M repo/includes/api/ResultBuilder.php M repo/tests/phpunit/includes/api/ResultBuilderTest.php 4 files changed, 88 insertions(+), 19 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/90/150790/1 diff --git a/repo/includes/LinkedData/EntityDataSerializationService.php b/repo/includes/LinkedData/EntityDataSerializationService.php index df7c4a1..d778bcd 100644 --- a/repo/includes/LinkedData/EntityDataSerializationService.php +++ b/repo/includes/LinkedData/EntityDataSerializationService.php @@ -542,7 +542,7 @@ $this->entityTitleLookup, $this->serializerFactory ); - $resultBuilder->addEntityRevision( $entityRevision, $options ); + $resultBuilder->addEntityRevision( null, $entityRevision, $options ); wfProfileOut( __METHOD__ ); return $res; diff --git a/repo/includes/api/GetEntities.php b/repo/includes/api/GetEntities.php index 6dfb4d3..00ffefb 100644 --- a/repo/includes/api/GetEntities.php +++ b/repo/includes/api/GetEntities.php @@ -168,7 +168,7 @@ */ private function addMissingItemsToResult( $missingItems ){ foreach( $missingItems as $missingItem ) { - $this->getResultBuilder()->addMissingEntity( $missingItem ); + $this->getResultBuilder()->addMissingEntity( null, $missingItem ); } } @@ -199,7 +199,7 @@ foreach ( $entityIds as $entityId ) { $entityRevision = $this->entityRevisionLookup->getEntityRevision( $entityId ); if ( is_null( $entityRevision ) ) { - $this->getResultBuilder()->addMissingEntity( array( 'id' => $entityId->getSerialization() ) ); + $this->getResultBuilder()->addMissingEntity( null, array( 'id' => $entityId->getSerialization() ) ); } else { $revisionArray[] = $entityRevision; } @@ -220,7 +220,7 @@ $props = $this->getPropsFromParams( $params ); $options = $this->getSerializationOptions( $params, $props ); $siteFilterIds = $params['sitefilter']; - $this->getResultBuilder()->addEntityRevision( $entityRevision, $options, $props, $siteFilterIds ); + $this->getResultBuilder()->addEntityRevision( null, $entityRevision, $options, $props, $siteFilterIds ); wfProfileOut( __METHOD__ ); } diff --git a/repo/includes/api/ResultBuilder.php b/repo/includes/api/ResultBuilder.php index f6bda5f..d1cadf8 100644 --- a/repo/includes/api/ResultBuilder.php +++ b/repo/includes/api/ResultBuilder.php @@ -274,16 +274,23 @@ /** * Get serialized entity for the EntityRevision and add it to the result * + * @param string|null $key The key for the entity in the 'entities' structure. + * Will default to the entity's serialized ID if null. * @param EntityRevision $entityRevision * @param SerializationOptions|null $options * @param array|string $props a list of fields to include, or "all" * @param array $siteIds A list of site IDs to filter by - + * * @since 0.5 */ - public function addEntityRevision( EntityRevision $entityRevision, SerializationOptions $options = null, $props = 'all', $siteIds = array() ) { + public function addEntityRevision( $key, EntityRevision $entityRevision, SerializationOptions $options = null, $props = 'all', $siteIds = array() ) { $entity = $entityRevision->getEntity(); $entityId = $entity->getId(); + + if ( $key === null ) { + $key = $entityId->getSerialization(); + } + $record = array(); if ( $options ) { @@ -314,9 +321,9 @@ $entitySerialization = $entitySerializer->getSerialized( $entity ); if ( !empty( $siteIds ) && array_key_exists( 'sitelinks', $entitySerialization ) ) { - foreach ( $entitySerialization['sitelinks'] as $key => $sitelink ) { + foreach ( $entitySerialization['sitelinks'] as $siteId => $sitelink ) { if ( is_array( $sitelink ) && !in_array( $sitelink['site'], $siteIds ) ) { - unset( $entitySerialization['sitelinks'][$key] ); + unset( $entitySerialization['sitelinks'][$siteId] ); } } } @@ -324,7 +331,7 @@ $record = array_merge( $record, $entitySerialization ); } - $this->appendValue( array( 'entities' ), $entityId->getSerialization(), $record, 'entity' ); + $this->appendValue( array( 'entities' ), $key, $record, 'entity' ); } /** @@ -475,14 +482,25 @@ /** * Add an entry for a missing entity... * + * @param string|null $key The key under which to place the missing entity in the 'entities' + * structure. If null, defaults to the 'id' field in $missingDetails if that is set; + * otherwise, it defaults to using a unique negative number. * @param array $missingDetails array containing key value pair missing details * * @since 0.5 */ - public function addMissingEntity( $missingDetails ) { + public function addMissingEntity( $key, $missingDetails ) { + if ( $key === null && isset( $missingDetails['id'] ) ) { + $key = $missingDetails['id']; + } + + if ( $key === null ) { + $key = $this->missingEntityCounter; + } + $this->appendValue( 'entities', - $this->missingEntityCounter, + $key, array_merge( $missingDetails, array( 'missing' => "" ) ), 'entity' ); diff --git a/repo/tests/phpunit/includes/api/ResultBuilderTest.php b/repo/tests/phpunit/includes/api/ResultBuilderTest.php index b4c3388..21fb3e5 100644 --- a/repo/tests/phpunit/includes/api/ResultBuilderTest.php +++ b/repo/tests/phpunit/includes/api/ResultBuilderTest.php @@ -295,9 +295,32 @@ ) ) ); $resultBuilder = $this->getResultBuilder( $result ); - $resultBuilder->addEntityRevision( $entityRevision, new SerializationOptions(), $props ); + $resultBuilder->addEntityRevision( null, $entityRevision, new SerializationOptions(), $props ); $this->assertEquals( $expected, $result->getData() ); + } + + public function testAddEntityRevisionKey() { + $item = Item::newEmpty(); + $item->setId( new ItemId( 'Q11' ) ); + + $entityRevision = new EntityRevision( $item, 33, '20131126202923' ); + + $props = array(); + $result = $this->getDefaultResult(); + $resultBuilder = $this->getResultBuilder( $result ); + + // automatic key + $resultBuilder->addEntityRevision( null, $entityRevision, new SerializationOptions(), $props ); + + $data = $result->getData(); + $this->assertArrayHasKey( 'Q11', $data['entities'] ); + + // explicit key + $resultBuilder->addEntityRevision( 'FOO', $entityRevision, new SerializationOptions(), $props ); + + $data = $result->getData(); + $this->assertArrayHasKey( 'FOO', $data['entities'] ); } public function testAddEntityRevisionWithSiteLinksFilter() { @@ -313,7 +336,7 @@ $result = $this->getDefaultResult(); $resultBuilder = $this->getResultBuilder( $result ); - $resultBuilder->addEntityRevision( $entityRevision, $options, $props, $siteIds ); + $resultBuilder->addEntityRevision( null, $entityRevision, $options, $props, $siteIds ); $expected = array( 'entities' => array( 'Q123099' => array( @@ -352,7 +375,7 @@ $result = $this->getDefaultResult( $indexedMode ); $resultBuilder = $this->getResultBuilder( $result ); - $resultBuilder->addEntityRevision( $entityRevision, $options, $props, $siteIds ); + $resultBuilder->addEntityRevision( null, $entityRevision, $options, $props, $siteIds ); $expected = array( 'entities' => array( array( @@ -622,8 +645,13 @@ $result = $this->getDefaultResult(); $resultBuilder = $this->getResultBuilder( $result ); - foreach( $missingEntities as $missingDetails ){ - $resultBuilder->addMissingEntity( $missingDetails ); + foreach( $missingEntities as $key => $missingDetails ){ + if ( is_int( $key ) ) { + // string keys are kept for use in the result structure, integer keys aren't + $key = null; + } + + $resultBuilder->addMissingEntity( $key, $missingDetails ); } $this->assertEquals( $expected, $result->getData() ); @@ -640,7 +668,32 @@ '-1' => array( 'site' => 'enwiki', 'title' => 'Berlin', - //@todo fix bug 45509 useless missing flag + 'missing' => '', + ) + ), + ) + ), + array( + array( + array( 'id' => 'Q77' ), + ), + array( + 'entities' => array( + 'Q77' => array( + 'id' => 'Q77', + 'missing' => '', + ) + ), + ) + ), + array( + array( + 'Q77' => array( 'foo' => 'bar' ), + ), + array( + 'entities' => array( + 'Q77' => array( + 'foo' => 'bar', 'missing' => '', ) ), @@ -656,13 +709,11 @@ '-1' => array( 'site' => 'enwiki', 'title' => 'Berlin', - //@todo fix bug 45509 useless missing flag 'missing' => '', ), '-2' => array( 'site' => 'dewiki', 'title' => 'Foo', - //@todo fix bug 45509 useless missing flag 'missing' => '', ) ), -- To view, visit https://gerrit.wikimedia.org/r/150790 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ica78bc830fed8bc61b797a0c6f9ec3bf21ecb370 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits