jenkins-bot has submitted this change and it was merged.

Change subject: Fix missing options bug in GetClaims
......................................................................


Fix missing options bug in GetClaims

The problem is that the raw mode setting is false when the builder
is constructed. Using lazy initialization to create the serialization
options seems to work.

(This also fixes GetClaimsTest, which misfired on snak's datatypes
now correctly being included in the output.)

Change-Id: Ieb1dfec050a2963eab675fd831f8da8bede17db3
---
M repo/includes/api/ResultBuilder.php
M repo/tests/phpunit/includes/api/GetClaimsTest.php
2 files changed, 80 insertions(+), 41 deletions(-)

Approvals:
  Aude: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/repo/includes/api/ResultBuilder.php 
b/repo/includes/api/ResultBuilder.php
index 7cf6fd4..732562a 100644
--- a/repo/includes/api/ResultBuilder.php
+++ b/repo/includes/api/ResultBuilder.php
@@ -71,9 +71,6 @@
                $this->entityTitleLookup = $entityTitleLookup;
                $this->serializerFactory = $serializerFactory;
                $this->missingEntityCounter = -1;
-
-               $this->options = new SerializationOptions();
-               $this->options->setOption( EntitySerializer::OPT_SORT_ORDER, 
EntitySerializer::SORT_NONE );
        }
 
        /**
@@ -83,6 +80,12 @@
         * @return SerializationOptions
         */
        public function getOptions() {
+               if ( !$this->options ) {
+                       $this->options = new SerializationOptions();
+                       $this->options->setIndexTags( 
$this->result->getIsRawMode() );
+                       $this->options->setOption( 
EntitySerializer::OPT_SORT_ORDER, EntitySerializer::SORT_NONE );
+               }
+
                return $this->options;
        }
 
@@ -275,7 +278,14 @@
         *
         * @since 0.5
         */
-       public function addEntityRevision( $key, 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();
 
@@ -285,12 +295,9 @@
 
                $record = array();
 
+               $serializerOptions = $this->getOptions();
                if ( $options ) {
-                       $serializerOptions = new SerializationOptions();
-                       $serializerOptions->merge( $this->options );
                        $serializerOptions->merge( $options );
-               } else {
-                       $serializerOptions = $this->options;
                }
 
                //if there are no props defined only return type and id..
@@ -348,7 +355,7 @@
         * @param array|string $path where the data is located
         */
        public function addLabels( array $labels, $path ) {
-               $labelSerializer = 
$this->serializerFactory->newLabelSerializer( $this->options );
+               $labelSerializer = 
$this->serializerFactory->newLabelSerializer( $this->getOptions() );
 
                $values = $labelSerializer->getSerialized( $labels );
                $this->setList( $path, 'labels', $values, 'label' );
@@ -363,7 +370,7 @@
         * @param array|string $path where the data is located
         */
        public function addDescriptions( array $descriptions, $path ) {
-               $descriptionSerializer = 
$this->serializerFactory->newDescriptionSerializer( $this->options );
+               $descriptionSerializer = 
$this->serializerFactory->newDescriptionSerializer( $this->getOptions() );
 
                $values = $descriptionSerializer->getSerialized( $descriptions 
);
                $this->setList( $path, 'descriptions', $values, 'description' );
@@ -378,7 +385,7 @@
         * @param array|string $path where the data is located
         */
        public function addAliases( array $aliases, $path ) {
-               $aliasSerializer = 
$this->serializerFactory->newAliasSerializer( $this->options );
+               $aliasSerializer = 
$this->serializerFactory->newAliasSerializer( $this->getOptions() );
                $values = $aliasSerializer->getSerialized( $aliases );
                $this->setList( $path, 'aliases', $values, 'alias' );
        }
@@ -393,8 +400,7 @@
         * @param array|null $options
         */
        public function addSiteLinks( array $siteLinks, $path, $options = null 
) {
-               $serializerOptions = new SerializationOptions();
-               $serializerOptions->merge( $this->options );
+               $serializerOptions = $this->getOptions();
 
                if ( is_array( $options ) ) {
                        if ( in_array( EntitySerializer::SORT_ASC, $options ) ) 
{
@@ -429,7 +435,7 @@
         * @param array|string $path where the data is located
         */
        public function addClaims( array $claims, $path ) {
-               $claimsSerializer = 
$this->serializerFactory->newClaimsSerializer( $this->options );
+               $claimsSerializer = 
$this->serializerFactory->newClaimsSerializer( $this->getOptions() );
 
                $values = $claimsSerializer->getSerialized( new Claims( $claims 
) );
                $this->setList( $path, 'claims', $values, 'claim' );
@@ -443,7 +449,7 @@
         * @since 0.5
         */
        public function addClaim( Claim $claim ) {
-               $serializer = $this->serializerFactory->newClaimSerializer( 
$this->options );
+               $serializer = $this->serializerFactory->newClaimSerializer( 
$this->getOptions() );
 
                //TODO: this is currently only used to add a Claim as the top 
level structure,
                //      with a null path and a fixed name. Would be nice to 
also allow claims
@@ -461,7 +467,7 @@
         * @since 0.5
         */
        public function addReference( Reference $reference ) {
-               $serializer = $this->serializerFactory->newReferenceSerializer( 
$this->options );
+               $serializer = $this->serializerFactory->newReferenceSerializer( 
$this->getOptions() );
 
                //TODO: this is currently only used to add a Reference as the 
top level structure,
                //      with a null path and a fixed name. Would be nice to 
also allow references
diff --git a/repo/tests/phpunit/includes/api/GetClaimsTest.php 
b/repo/tests/phpunit/includes/api/GetClaimsTest.php
index 505f4ae..f99ce69 100644
--- a/repo/tests/phpunit/includes/api/GetClaimsTest.php
+++ b/repo/tests/phpunit/includes/api/GetClaimsTest.php
@@ -14,6 +14,7 @@
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
 use Wikibase\DataModel\Snak\PropertySomeValueSnak;
 use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\Lib\PropertyDataTypeLookup;
 use Wikibase\Lib\Serializers\ClaimSerializer;
 use Wikibase\Lib\Serializers\SerializationOptions;
 use Wikibase\Lib\Serializers\SerializerFactory;
@@ -40,42 +41,70 @@
 
        /**
         * @param Entity $entity
-        *
-        * @return Entity
+        * @param int $flags
         */
-       protected function addClaimsAndSave( Entity $entity ) {
-               wfSuppressWarnings(); // We are referencing properties that 
don't exist. Not relevant here.
-
-               $store = WikibaseRepo::getDefaultInstance()->getEntityStore();
-               $store->saveEntity( $entity, '', $GLOBALS['wgUser'], EDIT_NEW );
-
-               /** @var $claims Claim[] */
-               $claims[0] = $entity->newClaim( new PropertyNoValueSnak( new 
PropertyId( 'P42' ) ) );
-               $claims[1] = $entity->newClaim( new PropertyNoValueSnak( new 
PropertyId( 'P404' ) ) );
-               $claims[2] = $entity->newClaim( new PropertySomeValueSnak( new 
PropertyId( 'P42' ) ) );
-               $claims[3] = $entity->newClaim( new PropertyValueSnak( new 
PropertyId( 'P9001' ), new StringValue( 'o_O' ) ) );
-
-               foreach( $claims as $key => $claim ){
-                       $claim->setGuid( $entity->getId()->getPrefixedId() . 
'$D8404CDA-56A1-4334-AF13-A3290BCD9CL' . $key );
-                       $entity->addClaim( $claim );
+       private function save( Entity $entity, $flags = null ) {
+               if ( $flags === null ) {
+                       $flags = $entity->getId() ? EDIT_UPDATE : EDIT_NEW;
                }
 
-               $store->saveEntity( $entity, '', $GLOBALS['wgUser'], 
EDIT_UPDATE );
-               wfRestoreWarnings();
+               $store = WikibaseRepo::getDefaultInstance()->getEntityStore();
 
-               return $entity;
+               $rev = $store->saveEntity( $entity, '', $GLOBALS['wgUser'], 
$flags );
+
+               $entity->setId( $rev->getEntity()->getId() );
+       }
+
+       /**
+        * @param Item $item
+        */
+       private function addStatements( Item $item, PropertyId $property ) {
+               if ( !$item->getId() ) {
+                       $this->save( $item );
+               }
+
+               /** @var $statements Statement[] */
+               $statements[0] = $item->newClaim( new PropertyNoValueSnak( 
$property ) );
+               $statements[1] = $item->newClaim( new PropertyNoValueSnak( 
$property ) );
+               $statements[2] = $item->newClaim( new PropertySomeValueSnak( 
$property ) );
+               $statements[3] = $item->newClaim( new PropertyValueSnak( 
$property, new StringValue( 'o_O' ) ) );
+
+               foreach ( $statements as $key => $statement ) {
+                       $statement->setGuid( $item->getId()->getPrefixedId() . 
'$D8404CDA-56A1-4334-AF13-A3290BCD9CL' . $key );
+                       $item->addClaim( $statement );
+               }
        }
 
        /**
         * @return Entity[]
         */
-       protected function getNewEntities() {
+       private function getNewEntities() {
                $property = Property::newFromType( 'string' );
+               $this->save( $property );
+
+               $propertyId = $property->getId();
+
+               $item = Item::newEmpty();
+               $this->addStatements( $item, $propertyId );
+               $this->save( $item );
 
                return array(
-                       $this->addClaimsAndSave( Item::newEmpty() ),
-                       $this->addClaimsAndSave( $property ),
+                       $property,
+                       $item,
                );
+       }
+
+       /**
+        * @return PropertyDataTypeLookup
+        */
+       private function getDataTypeLookup() {
+               $lookup = $this->getMock( 'Wikibase\Lib\PropertyDataTypeLookup' 
);
+
+               $lookup->expects( $this->any() )
+                       ->method( 'getDataTypeIdForProperty' )
+                       ->will( $this->returnValue( 'string' ) );
+
+               return $lookup;
        }
 
        public function validRequestProvider() {
@@ -148,7 +177,8 @@
                if( !$groupedByProperty ) {
                        $options->setOption( 
SerializationOptions::OPT_GROUP_BY_PROPERTIES, array() );
                }
-               $serializerFactory = new SerializerFactory();
+
+               $serializerFactory = new SerializerFactory( null, 
$this->getDataTypeLookup() );
                $serializer = $serializerFactory->newSerializerForObject( 
$claims );
                $serializer->setOptions( $options );
                $expected = $serializer->getSerialized( $claims );
@@ -191,7 +221,9 @@
        public function testGetInvalidIds( $entity, $property ) {
                if ( !$entity ) {
                        $item = Item::newEmpty();
-                       $this->addClaimsAndSave( $item );
+                       $this->addStatements( $item, new PropertyId( 'P13' ) );
+
+                       $this->save( $item );
                        $entity = $item->getId()->getSerialization();
                }
 
@@ -215,4 +247,5 @@
                        array( 'whatTheFuck', 'P42' ),
                );
        }
+
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/161212
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ieb1dfec050a2963eab675fd831f8da8bede17db3
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: mw1.24-wmf19
Gerrit-Owner: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to