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

Reply via email to