Jeroen De Dauw has submitted this change and it was merged.
Change subject: Introduced 'qualifiers-order' parameter in ClaimSerializer
......................................................................
Introduced 'qualifiers-order' parameter in ClaimSerializer
(bug 52391) Since, in the serialized JSON structure, the qualifiers are
encapsulated
in an object indexed by property id, the 'qualifiers-order' parameter specifies
the
order of those property ids.
Change-Id: I6f8399b621dddb6c84082db5fd35286a2ecd438c
---
M .jshintignore
M lib/includes/serializers/ClaimSerializer.php
M lib/resources/wikibase.datamodel/wikibase.Claim.js
M lib/resources/wikibase.datamodel/wikibase.SnakList.js
M lib/tests/phpunit/serializers/ClaimSerializerTest.php
M lib/tests/qunit/wikibase.datamodel/Wikibase.SnakList.tests.js
M repo/tests/phpunit/includes/api/SetClaimTest.php
7 files changed, 246 insertions(+), 30 deletions(-)
Approvals:
Jeroen De Dauw: Looks good to me, approved
jenkins-bot: Verified
diff --git a/.jshintignore b/.jshintignore
index fe966a7..eea9c1c 100644
--- a/.jshintignore
+++ b/.jshintignore
@@ -22,7 +22,6 @@
./lib/tests/qunit/wikibase.datamodel/datamodel.Property.tests.js
./lib/tests/qunit/wikibase.datamodel/Wikibase.claim.tests.js
./lib/tests/qunit/wikibase.datamodel/Wikibase.reference.tests.js
-./lib/tests/qunit/wikibase.datamodel/Wikibase.SnakList.tests.js
./lib/tests/qunit/wikibase.datamodel/Wikibase.snak.tests.js
./lib/tests/qunit/wikibase.datamodel/wikibase.Statement.tests.js
./lib/tests/qunit/wikibase.RepoApi/wikibase.RepoApiError.tests.js
diff --git a/lib/includes/serializers/ClaimSerializer.php
b/lib/includes/serializers/ClaimSerializer.php
index f041c8e..ac00c7b 100644
--- a/lib/includes/serializers/ClaimSerializer.php
+++ b/lib/includes/serializers/ClaimSerializer.php
@@ -109,6 +109,12 @@
if ( $qualifiers !== array() ) {
$serialization['qualifiers'] = $qualifiers;
+
+ $serialization['qualifiers-order'] = array();
+ foreach( $qualifiers as $propertyId => $snaks ) {
+ $serialization['qualifiers-order'][] =
$propertyId;
+ }
+ $this->setIndexedTagName(
$serialization['qualifiers-order'], 'qualifiers-order' );
}
$serialization['type'] = $claim instanceof Statement ?
'statement' : 'claim';
@@ -180,11 +186,7 @@
$claim->setGuid( $serialization['id'] );
- if ( array_key_exists( 'qualifiers', $serialization ) ) {
- $snaksUnserializer = new ByPropertyListUnserializer(
$snakUnserializer );
- $snaks = $snaksUnserializer->newFromSerialization(
$serialization['qualifiers'] );
- $claim->setQualifiers( new \Wikibase\SnakList( $snaks )
);
- }
+ $claim->setQualifiers( $this->unserializeQualifiers(
$serialization, $snakUnserializer ) );
if ( $isStatement ) {
if ( !in_array( $serialization['rank'], self::$rankMap
) ) {
@@ -212,4 +214,50 @@
return $claim;
}
+ /**
+ * Deserializes qualifiers from a serialized claim.
+ *
+ * @since 0.4
+ *
+ * @param array $serialization
+ * @param SnakSerializer $snakUnserializer
+ * @return \Wikibase\SnakList
+ * @throws \MWException
+ */
+ protected function unserializeQualifiers( $serialization,
$snakUnserializer ) {
+ if ( !array_key_exists( 'qualifiers', $serialization ) ) {
+ return new \Wikibase\SnakList();
+ } else {
+ $sortedQualifiers = array();
+
+ if( !array_key_exists( 'qualifiers-order',
$serialization ) ) {
+ $sortedQualifiers =
$serialization['qualifiers'];
+
+ } else {
+ foreach( $serialization['qualifiers-order'] as
$propertyId ) {
+ if( !isset(
$serialization['qualifiers'][$propertyId] ) ) {
+ throw new MWException( 'No
snaks with property id "' . $propertyId . '" '
+ . 'found in "qualifiers"
parameter although specified in '
+ . '"qualifiers-order"' );
+ }
+
+ $sortedQualifiers[$propertyId] =
$serialization['qualifiers'][$propertyId];
+ }
+
+ $missingProperties = array_diff_key(
+ $sortedQualifiers,
+ $serialization['qualifiers']
+ );
+
+ if( count( $missingProperties ) > 0 ) {
+ throw new MWException( 'Property ids '
. implode( ', ', $missingProperties )
+ . ' have not been specified in
"qualifiers-order"' );
+ }
+ }
+
+ $snaksUnserializer = new ByPropertyListUnserializer(
$snakUnserializer );
+ return new \Wikibase\SnakList(
$snaksUnserializer->newFromSerialization( $sortedQualifiers ) );
+ }
+ }
+
}
diff --git a/lib/resources/wikibase.datamodel/wikibase.Claim.js
b/lib/resources/wikibase.datamodel/wikibase.Claim.js
index 1f0aa75..94b7199 100644
--- a/lib/resources/wikibase.datamodel/wikibase.Claim.js
+++ b/lib/resources/wikibase.datamodel/wikibase.Claim.js
@@ -139,6 +139,7 @@
if ( this._qualifiers ) {
json.qualifiers = this._qualifiers.toJSON();
+ json['qualifiers-order'] =
this._qualifiers.getPropertyOrder();
}
return json;
@@ -162,7 +163,7 @@
isStatement = json.type === 'statement';
if ( json.qualifiers !== undefined ) {
- qualifiers = wb.SnakList.newFromJSON( json.qualifiers );
+ qualifiers = wb.SnakList.newFromJSON( json.qualifiers,
json['qualifiers-order'] );
}
if ( isStatement && json.references !== undefined ) {
diff --git a/lib/resources/wikibase.datamodel/wikibase.SnakList.js
b/lib/resources/wikibase.datamodel/wikibase.SnakList.js
index 6b05b17..97384c1 100644
--- a/lib/resources/wikibase.datamodel/wikibase.SnakList.js
+++ b/lib/resources/wikibase.datamodel/wikibase.SnakList.js
@@ -152,6 +152,22 @@
},
/**
+ * Returns a list of property ids representing the order of the snaks
grouped by property.
+ *
+ * @return {string[]}
+ */
+ getPropertyOrder: function() {
+ var json = this.toJSON(),
+ propertyIds = [];
+
+ $.each( json, function( propertyId, snak ) {
+ propertyIds.push( propertyId );
+ } );
+
+ return propertyIds;
+ },
+
+ /**
* Returns a simple JSON structure representing this Snak. The
structure will be a map, having
* property IDs as keys and an array of the Snak's JSON as values.
*
@@ -190,17 +206,45 @@
/**
* Creates a new Snak Object from a given JSON structure.
*
- * @param {String} json
- * @return wb.Snak|null
+ * @param {string} json
+ * @param {string[]} [order] List of property ids defining the order of the
snaks grouped by
+ * property.
+ * @return {wikibase.SnakList|null}
*/
-SELF.newFromJSON = function( json ) {
+SELF.newFromJSON = function( json, order ) {
var snaksList = new SELF();
- $.each( json, function( propertyId, snaksPerProperty ) {
- $.each( snaksPerProperty, function( i, snakJson ) {
- snaksList.addSnak( wb.Snak.newFromJSON( snakJson ) );
+ if( !order ) {
+ // No order specified: Just loop through the json object:
+ $.each( json, function( propertyId, snaksPerProperty ) {
+ $.each( snaksPerProperty, function( i, snakJson ) {
+ snaksList.addSnak( wb.Snak.newFromJSON(
snakJson ) );
+ } );
} );
- } );
+
+ } else {
+ // Check whether all property ids that are featured by snaks
are specified in the order
+ // list:
+ $.each( json, function( propertyId, snakListJson ) {
+ if( $.inArray( propertyId, order ) === -1 ) {
+ throw new Error( 'Snak featuring the property
id ' + propertyId + ' is not present '
+ + 'within list of property ids defined
for ordering' );
+ }
+ } );
+
+ // Add all snaks grouped by property according to the order
specified via the "order"
+ // parameter:
+ for( var i = 0; i < order.length; i++ ) {
+ if( !json[order[i]] ) {
+ throw new Error( 'Trying to oder by property '
+ order[i] + ' without any snak '
+ + ' featuring this property being
present' );
+ }
+
+ for( var j = 0; j < json[order[i]].length; j++ ) {
+ snaksList.addSnak( wb.Snak.newFromJSON(
json[order[i]][j] ) );
+ }
+ }
+ }
return snaksList;
};
diff --git a/lib/tests/phpunit/serializers/ClaimSerializerTest.php
b/lib/tests/phpunit/serializers/ClaimSerializerTest.php
index c0a18d0..74695cf 100644
--- a/lib/tests/phpunit/serializers/ClaimSerializerTest.php
+++ b/lib/tests/phpunit/serializers/ClaimSerializerTest.php
@@ -1,8 +1,11 @@
<?php
namespace Wikibase\Test;
+
+use Wikibase\Claim;
use Wikibase\Statement;
use Wikibase\Lib\Serializers\ClaimSerializer;
+use Wikibase\Lib\Serializers\SnakSerializer;
/**
* Tests for the Wikibase\ClaimSerializer class.
@@ -34,6 +37,7 @@
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
+ * @author H. Snater < mediaw...@snater.com >
*/
class ClaimSerializerTest extends SerializerBaseTest {
@@ -60,13 +64,13 @@
$id = new \Wikibase\EntityId( \Wikibase\Property::ENTITY_TYPE,
42 );
- $validArgs[] = new \Wikibase\Claim( new
\Wikibase\PropertyNoValueSnak( $id ) );
+ $validArgs[] = new Claim( new \Wikibase\PropertyNoValueSnak(
$id ) );
- $validArgs[] = new \Wikibase\Claim( new
\Wikibase\PropertySomeValueSnak( $id ) );
+ $validArgs[] = new Claim( new \Wikibase\PropertySomeValueSnak(
$id ) );
$validArgs = $this->arrayWrap( $validArgs );
- $claim = new \Wikibase\Claim( new
\Wikibase\PropertyNoValueSnak( $id ) );
+ $claim = new Claim( new \Wikibase\PropertyNoValueSnak( $id ) );
$validArgs[] = array(
$claim,
@@ -80,7 +84,7 @@
),
);
- $statement = new \Wikibase\Statement( new
\Wikibase\PropertyNoValueSnak( $id ) );
+ $statement = new Statement( new \Wikibase\PropertyNoValueSnak(
$id ) );
$validArgs[] = array(
$statement,
@@ -95,6 +99,44 @@
),
);
+ $claim = new Claim(
+ new \Wikibase\PropertyNoValueSnak( $id ),
+ new \Wikibase\SnakList( array(
+ new \Wikibase\PropertyNoValueSnak( $id ),
+ new \Wikibase\PropertySomeValueSnak( $id ),
+ new \Wikibase\PropertyNoValueSnak(
+ new \Wikibase\EntityId(
\Wikibase\Property::ENTITY_TYPE, 1 )
+ ),
+ ) )
+ );
+
+ $snakSerializer = new SnakSerializer();
+
+ $validArgs[] = array(
+ $claim,
+ array(
+ 'id' => $claim->getGuid(),
+ 'mainsnak' => $snakSerializer->getSerialized(
+ new \Wikibase\PropertyNoValueSnak( $id )
+ ),
+ 'qualifiers' => array(
+ 'p42' => array(
+ $snakSerializer->getSerialized(
new \Wikibase\PropertyNoValueSnak( $id ) ),
+ $snakSerializer->getSerialized(
+ new
\Wikibase\PropertySomeValueSnak( $id )
+ ),
+ ),
+ 'p1' => array(
+ $snakSerializer->getSerialized(
new \Wikibase\PropertyNoValueSnak(
+ new \Wikibase\EntityId(
\Wikibase\Property::ENTITY_TYPE, 1 )
+ ) ),
+ ),
+ ),
+ 'qualifiers-order' => array( 'p42', 'p1' ),
+ 'type' => 'claim',
+ ),
+ );
+
return $validArgs;
}
diff --git a/lib/tests/qunit/wikibase.datamodel/Wikibase.SnakList.tests.js
b/lib/tests/qunit/wikibase.datamodel/Wikibase.SnakList.tests.js
index f75470a..4117d5c 100644
--- a/lib/tests/qunit/wikibase.datamodel/Wikibase.SnakList.tests.js
+++ b/lib/tests/qunit/wikibase.datamodel/Wikibase.SnakList.tests.js
@@ -1,12 +1,10 @@
/**
- * QUnit tests for wikibase.SnakList
- * @see https://www.mediawiki.org/wiki/Extension:Wikibase
- *
* @since 0.4
* @ingroup WikibaseLib
*
* @licence GNU GPL v2+
* @author Daniel Werner < daniel.wer...@wikimedia.de >
+ * @author H. Snater < mediaw...@snater.com >
*/
( function( wb, dv, $, QUnit ) {
@@ -15,10 +13,10 @@
QUnit.module( 'wikibase.datamodel.SnakList.js',
QUnit.newMwEnvironment() );
var snaks = [
- new wb.PropertyNoValueSnak( '9001' ),
- new wb.PropertySomeValueSnak( '42' ),
- new wb.PropertySomeValueSnak( '42' ), // two times 42!
- new wb.PropertyValueSnak( '42', new dv.StringValue(
'~=[,,_,,]:3' ) )
+ new wb.PropertyNoValueSnak( 'p9001' ),
+ new wb.PropertySomeValueSnak( 'p42' ),
+ new wb.PropertySomeValueSnak( 'p42' ), // two times 42!
+ new wb.PropertyValueSnak( 'p42', new dv.StringValue(
'~=[,,_,,]:3' ) )
];
var anotherSnak = new wb.PropertySomeValueSnak( 'p1' ),
anotherSnak2 = new wb.PropertySomeValueSnak( 'p2' );
@@ -66,10 +64,35 @@
assert.throws(
function() {
- var newList = new wb.SnakList( 'foo' );
+ return new wb.SnakList( 'foo' );
},
'Can not create SnakList with strange constructor
argument'
);
+ } );
+
+ QUnit.test( 'newFromJSON()', function( assert ) {
+ var snakList = new wb.SnakList( snaks ),
+ initialOrder = snakList.getPropertyOrder(),
+ clonedSnakList = wb.SnakList.newFromJSON(
snakList.toJSON() );
+
+ assert.ok(
+ snakList.equals( clonedSnakList ),
+ 'Cloned snak list using the JSON representation.'
+ );
+
+ var reorderedClone = wb.SnakList.newFromJSON(
snakList.toJSON(), ['p42', 'p9001'] ),
+ cloneOrder = reorderedClone.getPropertyOrder();
+
+ assert.ok(
+ snakList.equals( reorderedClone ),
+ 'Cloned snak list with applying a different property
order.'
+ );
+
+ assert.ok(
+ initialOrder[0] === cloneOrder[1] && initialOrder[1]
=== cloneOrder[0],
+ 'Verified differing property order.'
+ );
+
} );
QUnit.test( 'SnakList list operations', function( assert ) {
@@ -92,6 +115,12 @@
'New Snak list does not recognize another Snak, not in
the list as one of its own'
);
+ assert.deepEqual(
+ newSnakList.getPropertyOrder(),
+ ['p9001', 'p42'],
+ 'Verified property order.'
+ );
+
assert.ok(
newSnakList.addSnak( anotherSnak ) &&
newSnakList.length === initialLength + 1,
'Another snak added, length attribute increased by one'
@@ -100,6 +129,12 @@
assert.ok(
newSnakList.hasSnak( anotherSnak ),
'Newly added Snak recognized as one of the list\'s own
Snaks now'
+ );
+
+ assert.deepEqual(
+ newSnakList.getPropertyOrder(),
+ ['p9001', 'p42', 'p1'],
+ 'Verified property order.'
);
var clonedSnak = wb.Snak.newFromJSON( anotherSnak.toJSON() );
@@ -123,6 +158,12 @@
newSnakList.removeSnak( clonedSnak ) &&
newSnakList.length === initialLength + 1,
'Newly added Snak removed again (identified by cloned
Snak, so we test for non === ' +
'case; list length decreased by one'
+ );
+
+ assert.deepEqual(
+ newSnakList.getPropertyOrder(),
+ ['p9001', 'p42', 'p2'],
+ 'Verified property order.'
);
var i = 0;
@@ -153,7 +194,7 @@
assert.throws(
function() {
- newSnakList.addSnak( 'foo' )
+ newSnakList.addSnak( 'foo' );
},
'Can not add some strange thing to the Snak list'
);
diff --git a/repo/tests/phpunit/includes/api/SetClaimTest.php
b/repo/tests/phpunit/includes/api/SetClaimTest.php
index 3b798bb..ed0e642 100644
--- a/repo/tests/phpunit/includes/api/SetClaimTest.php
+++ b/repo/tests/phpunit/includes/api/SetClaimTest.php
@@ -105,6 +105,12 @@
$statement->getReferences()->addReference( new
\Wikibase\Reference( $snaks ) );
$statements[] = $statement;
+ $statement = clone $statement;
+ $snaks = new \Wikibase\SnakList( $this->snakProvider() );
+ $statement->setQualifiers( $snaks );
+ $statement->getReferences()->addReference( new
\Wikibase\Reference( $snaks ) );
+ $statements[] = $statement;
+
$ranks = array(
\Wikibase\Statement::RANK_DEPRECATED,
\Wikibase\Statement::RANK_NORMAL,
@@ -135,6 +141,19 @@
// Addition request
$this->makeRequest( $claim, $item->getId(), 1 );
+ // Reorder qualifiers:
+ if( count( $claim->getQualifiers() ) > 0 ) {
+ // Simply reorder the qualifiers by putting the
first qualifier to the end. This is
+ // supposed to be done in the serialized
representation since changing the actual
+ // object might apply intrinsic sorting.
+ $serializerFactory = new
\Wikibase\Lib\Serializers\SerializerFactory();
+ $serializer =
$serializerFactory->newSerializerForObject( $claim );
+ $serializedClaim = $serializer->getSerialized(
$claim );
+ $firstPropertyId = array_shift(
$serializedClaim['qualifiers-order'] );
+ array_push(
$serializedClaim['qualifiers-order'], $firstPropertyId );
+ $this->makeRequest( $serializedClaim,
$item->getId(), 1 );
+ }
+
$claim = new \Wikibase\Statement( new
\Wikibase\PropertyNoValueSnak( 9001 ) );
$claim->setGuid( $guid );
@@ -143,13 +162,26 @@
}
}
- protected function makeRequest( Claim $claim, \Wikibase\EntityId
$entityId, $claimCount ) {
+ /**
+ * @param \Wikibase\Claim|array $claim Native or serialized claim
object.
+ * @param EntityId $entityId
+ * @param $claimCount
+ */
+ protected function makeRequest( $claim, \Wikibase\EntityId $entityId,
$claimCount ) {
$serializerFactory = new
\Wikibase\Lib\Serializers\SerializerFactory();
- $serializer = $serializerFactory->newSerializerForObject(
$claim );
+
+ if( is_a( $claim, '\Wikibase\Claim' ) ) {
+ $serializer =
$serializerFactory->newSerializerForObject( $claim );
+ $serializedClaim = $serializer->getSerialized( $claim );
+ } else {
+ $serializer =
$serializerFactory->newUnserializerForClass( 'Wikibase\Claim' );
+ $serializedClaim = $claim;
+ $claim = $serializer->newFromSerialization(
$serializedClaim );
+ }
$params = array(
'action' => 'wbsetclaim',
- 'claim' => \FormatJson::encode(
$serializer->getSerialized( $claim ) ),
+ 'claim' => \FormatJson::encode( $serializedClaim ),
);
$this->makeValidRequest( $params );
@@ -162,6 +194,11 @@
$this->assertTrue( $claims->hasClaim( $claim ) );
+ $savedClaim = $claims->getClaimWithGuid( $claim->getGuid() );
+ if( count( $claim->getQualifiers() ) ) {
+ $this->assertArrayEquals(
$claim->getQualifiers()->toArray(), $savedClaim->getQualifiers()->toArray(),
true );
+ }
+
$this->assertEquals( $claimCount, $claims->count() );
}
@@ -173,6 +210,10 @@
$this->assertArrayHasKey( 'pageinfo', $resultArray, 'top level
element has a pageinfo key' );
$this->assertArrayHasKey( 'claim', $resultArray, 'top level
element has a statement key' );
+ if( isset( $resultArray['claim']['qualifiers'] ) ) {
+ $this->assertArrayHasKey( 'qualifiers-order',
$resultArray['claim'], '"qualifiers-order" key is set when returning
qualifiers' );
+ }
+
return $resultArray;
}
--
To view, visit https://gerrit.wikimedia.org/r/77862
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6f8399b621dddb6c84082db5fd35286a2ecd438c
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Henning Snater <henning.sna...@wikimedia.de>
Gerrit-Reviewer: Daniel Werner <daniel.wer...@wikimedia.de>
Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
