Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/89015
Change subject: (bug #55569) Make Claims list not use hashes.
......................................................................
(bug #55569) Make Claims list not use hashes.
Claim hashes are not unique and should only be used as a way to check for
equivalence. Thus, Claims should nto be a HashArray.
Change-Id: Idd5ffb72774a948852a6dbe2e05700b9aa94a211
---
M DataModel/Claim/ClaimListAccess.php
M DataModel/Claim/Claims.php
M DataModel/Entity/Entity.php
M tests/phpunit/Claim/ClaimAggregateTest.php
M tests/phpunit/Claim/ClaimListAccessTest.php
M tests/phpunit/Claim/ClaimsTest.php
M tests/phpunit/Entity/EntityTest.php
M tests/phpunit/Entity/ItemTest.php
M tests/phpunit/EntityDiffTest.php
9 files changed, 709 insertions(+), 214 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseDataModel
refs/changes/15/89015/1
diff --git a/DataModel/Claim/ClaimListAccess.php
b/DataModel/Claim/ClaimListAccess.php
index 9c0a20b..c4d7350 100644
--- a/DataModel/Claim/ClaimListAccess.php
+++ b/DataModel/Claim/ClaimListAccess.php
@@ -16,7 +16,8 @@
interface ClaimListAccess {
/**
- * Adds the provided claims to the list.
+ * Adds the provided claims to the list. If a claim with the same GUID
is
+ * already in the list, it is replaced.
*
* @since 0.2
*
@@ -25,7 +26,7 @@
public function addClaim( Claim $claim );
/**
- * Returns if the list contains a claim with the same hash as the
provided claim.
+ * Returns if the list contains a claim with the same GUID as the
provided claim.
*
* @since 0.2
*
@@ -36,7 +37,8 @@
public function hasClaim( Claim $claim );
/**
- * Removes the claim with the same hash as the provided claim if such a
claim exists in the list.
+ * Removes the claim with the same GUID as the provided claim if such a
claim exists in the list.
+ * If the claim is not in the list, the call has no effect.
*
* @since 0.2
*
diff --git a/DataModel/Claim/Claims.php b/DataModel/Claim/Claims.php
index b0eb29d..fbf922e 100644
--- a/DataModel/Claim/Claims.php
+++ b/DataModel/Claim/Claims.php
@@ -2,11 +2,14 @@
namespace Wikibase;
+use ArrayObject;
+use Diff\Diff;
use Diff\Differ;
use Diff\DiffOpAdd;
use Diff\DiffOpChange;
use Diff\DiffOpRemove;
use Diff\MapDiffer;
+use Hashable;
use InvalidArgumentException;
/**
@@ -14,9 +17,6 @@
* @see Claims
*
* A claim (identified using it's GUID) can only be added once.
- *
- * TODO: if it turns out we do not need efficient lookups by guid after all
- * we can switch back to extending the simpler HashableObjectStorage.
*
* @since 0.1
*
@@ -27,28 +27,35 @@
* @author Jeroen De Dauw < [email protected] >
* @author Daniel Kinzler
*/
-class Claims extends HashArray implements ClaimListAccess {
-
- /**
- * Maps claim GUIDs to their offsets.
- *
- * @since 0.1
- *
- * @var array [ element hash (string) => element offset (string|int) ]
- */
- protected $guidIndex = array();
+class Claims extends ArrayObject implements ClaimListAccess, Hashable {
/**
* Constructor.
+ *
* @see GenericArrayObject::__construct
*
* @since 0.3
*
* @param null|array $input
+ *
+ * @throws \InvalidArgumentException
*/
public function __construct( $input = null ) {
- $this->acceptDuplicates = true;
- parent::__construct( $input );
+ parent::__construct( array() );
+
+ if ( $input !== null ) {
+ if ( !is_array( $input) && !( $input instanceof
\Traversable ) ) {
+ throw new InvalidArgumentException( '$input
must be traversable' );
+ }
+
+ foreach ( $input as $claim ) {
+ if ( !( $claim instanceof Claim ) ) {
+ throw new InvalidArgumentException(
'$input must contain only Claim instances' );
+ }
+
+ $this->addClaim( $claim );
+ }
+ }
}
/**
@@ -63,6 +70,42 @@
}
/**
+ * @since 0.5
+ *
+ * @param string $guid
+ *
+ * @throws \InvalidArgumentException
+ * @return string
+ */
+ protected function getGuidKey( $guid ) {
+ if ( !is_string( $guid ) ) {
+ throw new InvalidArgumentException( 'Expected a GUID
string' );
+ }
+
+ $key = strtoupper( $guid );
+ return $key;
+ }
+
+ /**
+ * @param Claim $claim
+ *
+ * @since 0.5
+ *
+ * @throws \InvalidArgumentException
+ * @return string
+ */
+ protected function getClaimKey( Claim $claim ) {
+ $guid = $claim->getGuid();
+
+ if ( $guid === null ) {
+ throw new InvalidArgumentException( 'Can\'t handle
claims with no GUID set!' );
+ }
+
+ $key = $this->getGuidKey( $guid );
+ return $key;
+ }
+
+ /**
* @see ClaimListAccess::addClaim
*
* @since 0.1
@@ -70,7 +113,19 @@
* @param Claim $claim
*/
public function addClaim( Claim $claim ) {
- $this->append( $claim );
+ $key = $this->getClaimKey( $claim );
+ $this->offsetSet( $key, $claim );
+ }
+
+ /**
+ * @see ArrayAccess::append
+ *
+ * @since 0.5
+ *
+ * @param Claim $claim
+ */
+ public function append( $claim ) {
+ $this->addClaim( $claim );
}
/**
@@ -83,7 +138,14 @@
* @return boolean
*/
public function hasClaim( Claim $claim ) {
- return $this->hasElement( $claim );
+ $guid = $claim->getGuid();
+
+ if ( $guid === null ) {
+ return false;
+ }
+
+ $key = $this->getGuidKey( $guid );
+ return $this->offsetExists( $key );
}
/**
@@ -94,7 +156,17 @@
* @param Claim $claim
*/
public function removeClaim( Claim $claim ) {
- $this->removeElement( $claim );
+ $guid = $claim->getGuid();
+
+ if ( $guid === null ) {
+ return;
+ }
+
+ $key = $this->getGuidKey( $guid );
+
+ if ( $this->offsetExists( $key ) ) {
+ $this->offsetUnset( $key );
+ }
}
/**
@@ -107,7 +179,7 @@
* @return boolean
*/
public function hasClaimWithGuid( $claimGuid ) {
- return array_key_exists( $claimGuid, $this->guidIndex );
+ return $this->offsetExists( $claimGuid );
}
/**
@@ -118,7 +190,9 @@
* @param string $claimGuid
*/
public function removeClaimWithGuid( $claimGuid ) {
- $this->offsetUnset( $this->guidIndex[$claimGuid] );
+ if ( $this->offsetExists( $claimGuid ) ) {
+ $this->offsetUnset( $claimGuid );
+ }
}
/**
@@ -131,12 +205,75 @@
* @return Claim|null
*/
public function getClaimWithGuid( $claimGuid ) {
- if ( $this->hasClaimWithGuid( $claimGuid ) ) {
- return $this->offsetGet( $this->guidIndex[$claimGuid] );
- }
- else {
+ if ( $this->offsetExists( $claimGuid ) ) {
+ return $this->offsetGet( $claimGuid );
+ } else {
return null;
}
+ }
+
+ /**
+ * @see ArrayAccess::offsetExists
+ *
+ * @param string $guid
+ *
+ * @return bool
+ *
+ * @throws \InvalidArgumentException
+ */
+ public function offsetExists( $guid ) {
+ $key = $this->getGuidKey( $guid );
+ return parent::offsetExists( $key );
+ }
+
+ /**
+ * @see ArrayAccess::offsetGet
+ *
+ * @param string $guid
+ *
+ * @return Claim
+ *
+ * @throws \InvalidArgumentException
+ */
+ public function offsetGet( $guid ) {
+ $key = $this->getGuidKey( $guid );
+ return parent::offsetGet( $key );
+ }
+
+ /**
+ * @see ArrayAccess::offsetSet
+ *
+ * @param string $guid
+ * @param Claim $claim
+ *
+ * @throws \InvalidArgumentException
+ */
+ public function offsetSet( $guid, $claim ) {
+ if ( !is_object( $claim ) || !( $claim instanceof Claim ) ) {
+ throw new InvalidArgumentException( 'Expected a Claim
instance' );
+ }
+
+ $claimKey = $this->getClaimKey( $claim );
+
+ if ( $guid !== null ) {
+ $guidKey = $this->getGuidKey( $guid );
+
+ if ( $guidKey !== $claimKey ) {
+ throw new InvalidArgumentException( 'The key
must be the claim\'s GUID.' );
+ }
+ }
+
+ parent::offsetSet( $claimKey, $claim );
+ }
+
+ /**
+ * @see ArrayAccess::offsetUnset
+ *
+ * @param string $guid
+ */
+ public function offsetUnset( $guid ) {
+ $key = $this->getGuidKey( $guid );
+ parent::offsetUnset( $key );
}
/**
@@ -147,7 +284,9 @@
* @return string[]
*/
public function getGuids() {
- return array_keys( $this->guidIndex );
+ return array_map( function ( Claim $claim ) {
+ return $claim->getGuid();
+ }, iterator_to_array( $this ) );
}
/**
@@ -188,104 +327,71 @@
/* @var Claim $claim */
foreach ( $this as $claim ) {
- $snaks[] = $claim->getMainSnak();
+ $guid = $claim->getGuid();
+ $snaks[$guid] = $claim->getMainSnak();
}
return $snaks;
}
/**
- * @see GenericArrayObject::preSetElement
+ * Returns a map from GUIDs to claim hashes.
*
- * @since 0.3
+ * @since 0.4
*
- * @param int|string $index
- * @param Claim $claim
- *
- * @return boolean
+ * @return string[]
*/
- protected function preSetElement( $index, $claim ) {
- $shouldSet = parent::preSetElement( $index, $claim );
+ public function getHashes() {
+ $snaks = array();
- if ( $shouldSet ) {
- $this->guidIndex[$claim->getGuid()] = $index;
+ /* @var Claim $claim */
+ foreach ( $this as $claim ) {
+ $guid = $claim->getGuid();
+ $snaks[$guid] = $claim->getHash();
}
- return $shouldSet;
- }
-
- /**
- * @see ArrayObject::offsetUnset()
- *
- * @since 0.3
- *
- * @param mixed $index
- */
- public function offsetUnset( $index ) {
- if ( $this->offsetExists( $index ) ) {
- /**
- * @var Claim $claim
- */
- $claim = $this->offsetGet( $index );
-
- unset( $this->guidIndex[$claim->getGuid()] );
- parent::offsetUnset( $index );
- }
+ return $snaks;
}
/**
* @since 0.4
*
* @param Claims $claims
- * @param Differ|null $differ
+ * @param Differ|null $differ for building a diff of two GUID-to-hash
maps.
*
- * @return \Diff\Diff
+ * @return Diff
* @throws InvalidArgumentException
*/
public function getDiff( Claims $claims, Differ $differ = null ) {
- assert( $this->indicesAreUpToDate() );
- assert( $claims->indicesAreUpToDate() );
-
if ( $differ === null ) {
$differ = new MapDiffer();
}
- $sourceHashes = array();
- $targetHashes = array();
+ $sourceHashes = $this->getHashes();
+ $targetHashes = $claims->getHashes();
- /**
- * @var Claim $claim
- */
- foreach ( $this as $claim ) {
- $sourceHashes[$claim->getGuid()] = $claim->getHash();
- }
+ $diff = new Diff( array(), true );
- foreach ( $claims as $claim ) {
- $targetHashes[$claim->getGuid()] = $claim->getHash();
- }
-
- $diff = new \Diff\Diff( array(), true );
-
- foreach ( $differ->doDiff( $sourceHashes, $targetHashes ) as
$diffOp ) {
+ foreach ( $differ->doDiff( $sourceHashes, $targetHashes ) as
$guid => $diffOp ) {
if ( $diffOp instanceof DiffOpChange ) {
- $oldClaim = $this->getByElementHash(
$diffOp->getOldValue() );
- $newClaim = $claims->getByElementHash(
$diffOp->getNewValue() );
+ $oldClaim = $this->getClaimWithGuid( $guid );
+ $newClaim = $claims->getClaimWithGuid( $guid );
assert( $oldClaim instanceof Claim );
assert( $newClaim instanceof Claim );
assert( $oldClaim->getGuid() ===
$newClaim->getGuid() );
- $diff[$oldClaim->getGuid()] = new DiffOpChange(
$oldClaim, $newClaim );
+ $diff[$guid] = new DiffOpChange( $oldClaim,
$newClaim );
}
elseif ( $diffOp instanceof DiffOpAdd ) {
- $claim = $claims->getByElementHash(
$diffOp->getNewValue() );
+ $claim = $claims->getClaimWithGuid( $guid );
assert( $claim instanceof Claim );
- $diff[$claim->getGuid()] = new DiffOpAdd(
$claim );
+ $diff[$guid] = new DiffOpAdd( $claim );
}
elseif ( $diffOp instanceof DiffOpRemove ) {
- $claim = $this->getByElementHash(
$diffOp->getOldValue() );
+ $claim = $this->getClaimWithGuid( $guid );
assert( $claim instanceof Claim );
- $diff[$claim->getGuid()] = new DiffOpRemove(
$claim );
+ $diff[$guid] = new DiffOpRemove( $claim );
}
else {
throw new InvalidArgumentException( 'Invalid
DiffOp type cannot be handled' );
@@ -295,4 +401,31 @@
return $diff;
}
+ /**
+ * Returns a hash based on the value of the object.
+ * The hash is based on the hashes of the claims contained,
+ * with the order of claims considered significant.
+ *
+ * @since 0.5
+ *
+ * @return string
+ */
+ public function getHash() {
+ $hash = sha1( '' );
+
+ /* @var Claim $claim */
+ foreach ( $this as $claim ) {
+ $hash = sha1( $hash . $claim->getHash() );
+ }
+
+ return $hash;
+ }
+
+ /**
+ * Returns true if count( $this ) === 0
+ */
+ public function isEmpty() {
+ $iter = $this->getIterator();
+ return !$iter->valid();
+ }
}
diff --git a/DataModel/Entity/Entity.php b/DataModel/Entity/Entity.php
index 02d0c94..9fc87f3 100644
--- a/DataModel/Entity/Entity.php
+++ b/DataModel/Entity/Entity.php
@@ -51,7 +51,7 @@
/**
* @since 0.3
*
- * @var Claims|null
+ * @var Claim[]|null
*/
protected $claims;
@@ -715,9 +715,14 @@
* @param Claim $claim
*/
public function addClaim( Claim $claim ) {
+ if ( $claim->getGuid() === null ) {
+ throw new InvalidArgumentException( 'Can\'t add a Claim
without a GUID.' );
+ }
+
+ // TODO: ensure guid is valid for entity
+
$this->unstubClaims();
$this->claims[] = $claim;
- // TODO: ensure guid is valid for entity
}
/**
diff --git a/tests/phpunit/Claim/ClaimAggregateTest.php
b/tests/phpunit/Claim/ClaimAggregateTest.php
index 22d8540..90583f6 100644
--- a/tests/phpunit/Claim/ClaimAggregateTest.php
+++ b/tests/phpunit/Claim/ClaimAggregateTest.php
@@ -43,6 +43,10 @@
$argLists = array();
+ foreach ( $claims as $i => $claim ) {
+ $claim->setGuid( "ClaimListAccessTest\$claim-$i" );
+ }
+
/**
* @var ClaimAggregate $aggregate
*/
diff --git a/tests/phpunit/Claim/ClaimListAccessTest.php
b/tests/phpunit/Claim/ClaimListAccessTest.php
index ad746f4..a3b9a04 100644
--- a/tests/phpunit/Claim/ClaimListAccessTest.php
+++ b/tests/phpunit/Claim/ClaimListAccessTest.php
@@ -39,6 +39,10 @@
$argLists = array();
+ foreach ( $claims as $i => $claim ) {
+ $claim->setGuid( "ClaimListAccessTest\$claim-$i" );
+ }
+
/**
* @var ClaimListAccess $list
*/
diff --git a/tests/phpunit/Claim/ClaimsTest.php
b/tests/phpunit/Claim/ClaimsTest.php
index 016d08b..bf2070d 100644
--- a/tests/phpunit/Claim/ClaimsTest.php
+++ b/tests/phpunit/Claim/ClaimsTest.php
@@ -7,214 +7,488 @@
use Diff\DiffOpAdd;
use Diff\DiffOpChange;
use Diff\DiffOpRemove;
+use ReflectionClass;
use Wikibase\Claim;
use Wikibase\Claims;
-use Wikibase\EntityId;
+use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\Property;
use Wikibase\PropertyNoValueSnak;
use Wikibase\PropertySomeValueSnak;
use Wikibase\PropertyValueSnak;
use Wikibase\Reference;
use Wikibase\ReferenceList;
+use Wikibase\Snak;
use Wikibase\SnakList;
use Wikibase\Statement;
/**
* @covers Wikibase\Claims
*
- * @file
* @since 0.1
- *
- * @ingroup WikibaseLib
- * @ingroup Test
*
* @group Wikibase
* @group WikibaseDataModel
* @group WikibaseClaim
*
* @licence GNU GPL v2+
- * @author Jeroen De Dauw < [email protected] >
+ * @author Daniel Kinzler
*/
class ClaimsTest extends \PHPUnit_Framework_TestCase {
- public function getInstanceClass() {
- return '\Wikibase\Claims';
- }
+ protected $guidCounter = 0;
- public function instanceProvider() {
- $class = $this->getInstanceClass();
-
- $instances = array();
-
- foreach ( $this->getConstructorArg() as $arg ) {
- $instances[] = array( new $class( $arg ) );
+ protected function makeClaim( Snak $mainSnak, $guid = null ) {
+ if ( $guid === null ) {
+ $this->guidCounter++;
+ $guid = 'TEST$claim-' . $this->guidCounter;
}
- return $instances;
+ $claim = new Claim( $mainSnak );
+ $claim->setGuid( $guid );
+
+ return $claim;
}
- public function getElementInstances() {
- $instances = array();
+ protected function makeStatement( Snak $mainSnak, $guid = null ) {
+ if ( $guid === null ) {
+ $this->guidCounter++;
+ $guid = 'TEST$statement-' . $this->guidCounter;
+ }
- $instances[] = new Claim(
- new PropertyNoValueSnak(
- new EntityId( Property::ENTITY_TYPE, 23 ) ) );
+ $claim = new Statement( $mainSnak );
+ $claim->setGuid( $guid );
- $instances[] = new Claim(
- new PropertySomeValueSnak(
- new EntityId( Property::ENTITY_TYPE, 42 ) ) );
-
- $instances[] = new Claim(
- new PropertyValueSnak(
- new EntityId( Property::ENTITY_TYPE, 42 ),
- new StringValue( "foo" ) ) );
-
- return $instances;
+ return $claim;
}
- public function getConstructorArg() {
+ /**
+ * @dataProvider constructorProvider
+ */
+ public function testConstructor() {
+ $class = new ReflectionClass( 'Wikibase\Claims' );
+ $class->newInstanceArgs( func_get_args() );
+ }
+
+ public function constructorProvider() {
return array(
- null,
array(),
- $this->getElementInstances(),
+ array( null ),
+ array( array() ),
+ array( array( $this->makeClaim( new
PropertyNoValueSnak( new PropertyId( "P15" ) ) ) ) ),
);
}
/**
- * @dataProvider instanceProvider
- *
- * @param \Wikibase\Claims $array
+ * @dataProvider constructorErrorProvider
*/
- public function testHasClaim( Claims $array ) {
- /**
- * @var Claim $hashable
- */
- foreach ( iterator_to_array( $array ) as $hashable ) {
- $this->assertTrue( $array->hasClaim( $hashable ) );
- $array->removeClaim( $hashable );
- $this->assertFalse( $array->hasClaim( $hashable ) );
- }
+ public function testConstructorError() {
+ $this->setExpectedException( 'InvalidArgumentException' );
- $this->assertTrue( true );
+ $class = new ReflectionClass( 'Wikibase\Claims' );
+ $class->newInstanceArgs( func_get_args() );
}
- /**
- * @dataProvider instanceProvider
- *
- * @param \Wikibase\Claims $array
- */
- public function testRemoveClaim( Claims $array ) {
- $elementCount = count( $array );
-
- /**
- * @var Claim $element
- */
- foreach ( iterator_to_array( $array ) as $element ) {
- $this->assertTrue( $array->hasClaim( $element ) );
-
- $array->removeClaim( $element );
-
- $this->assertFalse( $array->hasClaim( $element ) );
- $this->assertEquals( --$elementCount, count( $array ) );
- }
-
- $elements = $this->getElementInstances();
- $element = array_shift( $elements );
-
- $array->removeClaim( $element );
- $array->removeClaim( $element );
-
- $this->assertTrue( true );
+ public function constructorErrorProvider() {
+ return array(
+ array( 17 ),
+ array( array( "foo" ) ),
+ );
}
- /**
- * @dataProvider instanceProvider
- *
- * @param \Wikibase\Claims $array
- */
- public function testAddClaim( Claims $array ) {
- $elementCount = count( $array );
+ public function testHasClaim() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
- $elements = $this->getElementInstances();
- $element = array_shift( $elements );
+ $this->assertFalse( $claims->hasClaim( $claim1 ) );
+ $this->assertFalse( $claims->hasClaim( $claim2 ) );
- ++$elementCount;
+ $claims->addClaim( $claim1 );
+ $this->assertTrue( $claims->hasClaim( $claim1 ) );
+ $this->assertFalse( $claims->hasClaim( $claim2 ) );
- $array->addClaim( $element );
+ $claims->addClaim( $claim2 );
+ $this->assertTrue( $claims->hasClaim( $claim1 ) );
+ $this->assertTrue( $claims->hasClaim( $claim2 ) );
- $this->assertEquals( $elementCount, count( $array ) );
+ // no guid
+ $claim0 = new Claim( new PropertyNoValueSnak( new PropertyId(
"P15" ) ) );
+ $this->assertFalse( $claims->hasClaim( $claim0 ) );
}
- /**
- * @dataProvider instanceProvider
- *
- * @param \Wikibase\Claims $array
- */
- public function testGetMainSnaks( Claims $array ) {
- $snaks = $array->getMainSnaks();
- $this->assertInternalType( 'array', $snaks );
- $this->assertSameSize( $array, $snaks );
+ public function testHasClaimWithGuid() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $this->assertFalse( $claims->hasClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertFalse( $claims->hasClaimWithGuid(
$claim2->getGuid() ) );
+
+ $claims->addClaim( $claim1 );
+ $this->assertTrue( $claims->hasClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertFalse( $claims->hasClaimWithGuid(
$claim2->getGuid() ) );
+
+ $claims->addClaim( $claim2 );
+ $this->assertTrue( $claims->hasClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertTrue( $claims->hasClaimWithGuid(
$claim2->getGuid() ) );
}
- public function testGetClaimsForProperty() {
- $array = new Claims( $this->getElementInstances() );
+ public function testRemoveClaim() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
- $claims = $array->getClaimsForProperty( 42 );
- $this->assertInstanceOf( 'Wikibase\Claims', $claims );
+ $claims->addClaim( $claim1 );
+ $claims->addClaim( $claim2 );
$this->assertCount( 2, $claims );
- $claims = $array->getClaimsForProperty( 23 );
- $this->assertInstanceOf( 'Wikibase\Claims', $claims );
+ $claims->removeClaim( $claim1 );
+ $this->assertFalse( $claims->hasClaim( $claim1 ) );
+ $this->assertNull( $claims->getClaimWithGuid(
$claim1->getGuid() ) );
$this->assertCount( 1, $claims );
- $claims = $array->getClaimsForProperty( 9000 );
- $this->assertInstanceOf( 'Wikibase\Claims', $claims );
+ $claims->removeClaim( $claim2 );
+ $this->assertFalse( $claims->hasClaim( $claim2 ) );
+ $this->assertNull( $claims->getClaimWithGuid(
$claim2->getGuid() ) );
+ $this->assertCount( 0, $claims );
+
+ // no guid
+ $claim0 = new Claim( new PropertyNoValueSnak( new PropertyId(
"P15" ) ) );
+ $claims->removeClaim( $claim0 );
+ }
+
+ public function testRemoveClaimWithGuid() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims->addClaim( $claim1 );
+ $claims->addClaim( $claim2 );
+ $this->assertCount( 2, $claims );
+
+ $claims->removeClaimWithGuid( $claim1->getGuid() );
+ $this->assertFalse( $claims->hasClaim( $claim1 ) );
+ $this->assertNull( $claims->getClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertCount( 1, $claims );
+
+ $claims->removeClaimWithGuid( $claim2->getGuid() );
+ $this->assertFalse( $claims->hasClaim( $claim2 ) );
+ $this->assertNull( $claims->getClaimWithGuid(
$claim2->getGuid() ) );
$this->assertCount( 0, $claims );
}
+ public function testOffsetUnset() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims->addClaim( $claim1 );
+ $claims->addClaim( $claim2 );
+ $this->assertCount( 2, $claims );
+
+ $claims->offsetUnset( $claim1->getGuid() );
+ $this->assertFalse( $claims->hasClaim( $claim1 ) );
+ $this->assertNull( $claims->getClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertCount( 1, $claims );
+
+ $claims->offsetUnset( $claim2->getGuid() );
+ $this->assertFalse( $claims->hasClaim( $claim2 ) );
+ $this->assertNull( $claims->getClaimWithGuid(
$claim2->getGuid() ) );
+ $this->assertCount( 0, $claims );
+ }
+
+ public function testGetClaimWithGuid() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims->addClaim( $claim1 );
+ $this->assertSame( $claim1, $claims->getClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertNull( $claims->getClaimWithGuid(
$claim2->getGuid() ) );
+
+ $claims->addClaim( $claim2 );
+ $this->assertSame( $claim1, $claims->getClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertSame( $claim2, $claims->getClaimWithGuid(
$claim2->getGuid() ) );
+ }
+
+ public function testOffsetGet() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims->addClaim( $claim1 );
+ $this->assertSame( $claim1, $claims->offsetGet(
$claim1->getGuid() ) );
+
+ $claims->addClaim( $claim2 );
+ $this->assertSame( $claim1, $claims->offsetGet(
$claim1->getGuid() ) );
+ $this->assertSame( $claim2, $claims->offsetGet(
$claim2->getGuid() ) );
+ }
+
+ public function testAddClaim() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims->addClaim( $claim1 );
+ $claims->addClaim( $claim2 );
+
+ $this->assertCount( 2, $claims );
+ $this->assertEquals( $claim1, $claims[$claim1->getGuid()] );
+ $this->assertEquals( $claim2, $claims[$claim2->getGuid()] );
+
+ $claims->addClaim( $claim1 );
+ $claims->addClaim( $claim2 );
+
+ $this->assertCount( 2, $claims );
+
+ $this->assertNotNull( $claims->getClaimWithGuid(
$claim1->getGuid() ) );
+ $this->assertNotNull( $claims->getClaimWithGuid(
$claim2->getGuid() ) );
+ }
+
+ public function testAppend() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims->append( $claim1 );
+ $claims->append( $claim2 );
+
+ $this->assertCount( 2, $claims );
+ $this->assertEquals( $claim1, $claims[$claim1->getGuid()] );
+ $this->assertEquals( $claim2, $claims[$claim2->getGuid()] );
+
+ $claims->append( $claim1 );
+ $claims->append( $claim2 );
+
+ $this->assertCount( 2, $claims );
+ }
+
+ public function testAppendOperator() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims[] = $claim1;
+ $claims[] = $claim2;
+
+ $this->assertCount( 2, $claims );
+ $this->assertEquals( $claim1, $claims[$claim1->getGuid()] );
+ $this->assertEquals( $claim2, $claims[$claim2->getGuid()] );
+
+ $claims[] = $claim1;
+ $claims[] = $claim2;
+
+ $this->assertCount( 2, $claims );
+ }
+
+ public function testOffsetSet() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims->offsetSet( $claim1->getGuid(), $claim1 );
+ $claims->offsetSet( $claim2->getGuid(), $claim2 );
+
+ $this->assertCount( 2, $claims );
+ $this->assertEquals( $claim1, $claims[$claim1->getGuid()] );
+ $this->assertEquals( $claim2, $claims[$claim2->getGuid()] );
+
+ $claims->offsetSet( $claim1->getGuid(), $claim1 );
+ $claims->offsetSet( $claim2->getGuid(), $claim2 );
+
+ $this->assertCount( 2, $claims );
+
+ $this->setExpectedException( 'InvalidArgumentException' );
+ $claims->offsetSet( 'spam', $claim1 );
+ }
+
+ public function testOffsetSetOperator() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claims[$claim1->getGuid()] = $claim1;
+ $claims[$claim2->getGuid()] = $claim2;
+
+ $this->assertCount( 2, $claims );
+ $this->assertEquals( $claim1, $claims[$claim1->getGuid()] );
+ $this->assertEquals( $claim2, $claims[$claim2->getGuid()] );
+
+ $claims[$claim1->getGuid()] = $claim1;
+ $claims[$claim2->getGuid()] = $claim2;
+
+ $this->assertCount( 2, $claims );
+ }
+
+ public function testGuidNormalization() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $claim1LowerGuid = strtolower( $claim1->getGuid() );
+ $claim2UpperGuid = strtoupper( $claim2->getGuid() );
+
+ $claims->addClaim( $claim1 );
+ $claims->addClaim( $claim2 );
+ $this->assertCount( 2, $claims );
+
+ $this->assertEquals( $claim1, $claims->getClaimWithGuid(
$claim1LowerGuid ) );
+ $this->assertEquals( $claim2, $claims->getClaimWithGuid(
$claim2UpperGuid ) );
+
+ $this->assertEquals( $claim1, $claims->offsetGet(
$claim1LowerGuid ) );
+ $this->assertEquals( $claim2, $claims->offsetGet(
$claim2UpperGuid ) );
+
+ $this->assertEquals( $claim1, $claims[$claim1LowerGuid] );
+ $this->assertEquals( $claim2, $claims[$claim2UpperGuid] );
+
+ $claims = new Claims();
+ $claims->offsetSet( strtoupper( $claim1LowerGuid ), $claim1 );
+ $claims->offsetSet( strtolower( $claim2UpperGuid ), $claim2 );
+ $this->assertCount( 2, $claims );
+
+ $this->assertEquals( $claim1, $claims->getClaimWithGuid(
$claim1LowerGuid ) );
+ $this->assertEquals( $claim2, $claims->getClaimWithGuid(
$claim2UpperGuid ) );
+ }
+
+ public function testGetMainSnaks() {
+ $claims = new Claims( array(
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertySomeValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P23" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P9000" ) ) ),
+ ) );
+
+ $snaks = $claims->getMainSnaks();
+ $this->assertInternalType( 'array', $snaks );
+ $this->assertSameSize( $claims, $snaks );
+
+ foreach ( $snaks as $guid => $snak ) {
+ $this->assertInstanceOf( 'Wikibase\Snak', $snak );
+ $this->assertTrue( $claims->hasClaimWithGuid( $guid ) );
+ }
+ }
+
+ public function testGetGuids() {
+ $claims = new Claims( array(
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertySomeValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P23" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P9000" ) ) ),
+ ) );
+
+ $guids = $claims->getGuids();
+ $this->assertInternalType( 'array', $guids );
+ $this->assertSameSize( $claims, $guids );
+
+ foreach ( $guids as $guid ) {
+ $this->assertInternalType( 'string', $guid );
+ $this->assertTrue( $claims->hasClaimWithGuid( $guid ) );
+ }
+ }
+
+ public function testGetHashes() {
+ $claims = new Claims( array(
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertySomeValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P23" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P9000" ) ) ),
+ ) );
+
+ $hashes = $claims->getHashes();
+ $this->assertInternalType( 'array', $hashes );
+ $this->assertSameSize( $claims, $hashes );
+
+ foreach ( $hashes as $hash ) {
+ $this->assertInternalType( 'string', $hash );
+ }
+
+ $hashSet = array_flip( $hashes );
+
+ foreach ( $claims as $claim ) {
+ $hash = $claim->getHash();
+ $this->assertArrayHasKey( $hash, $hashSet );
+ }
+ }
+
+ public function testGetClaimsForProperty() {
+ $claims = new Claims( array(
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertySomeValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P23" ) ) ),
+ ) );
+
+ $matches = $claims->getClaimsForProperty( 42 );
+ $this->assertInstanceOf( 'Wikibase\Claims', $claims );
+ $this->assertCount( 2, $matches );
+
+ $matches = $claims->getClaimsForProperty( 23 );
+ $this->assertInstanceOf( 'Wikibase\Claims', $claims );
+ $this->assertCount( 1, $matches );
+
+ $matches = $claims->getClaimsForProperty( 9000 );
+ $this->assertInstanceOf( 'Wikibase\Claims', $claims );
+ $this->assertCount( 0, $matches );
+ }
+
+ /**
+ * Attempts to add Claims with no GUID set will fail.
+ */
+ public function testNoGuidFailure() {
+ $claim = new Claim( new PropertyNoValueSnak( 42 ) );
+ $list = new Claims();
+
+ $this->setExpectedException( 'InvalidArgumentException' );
+ $list->addClaim( $claim );
+ }
+
public function testDuplicateClaims() {
- $firstClaim = new Claim( new PropertyNoValueSnak( 42 ) );
- $secondClaim = new Claim( new PropertyNoValueSnak( 42 ) );
+ $firstClaim = $this->makeClaim( new PropertyNoValueSnak( 42 ) );
+ $secondClaim = $this->makeClaim( new PropertyNoValueSnak( 42 )
);
$list = new Claims();
- $this->assertTrue( $list->addElement( $firstClaim ), 'Adding
the first element should work' );
- $this->assertTrue( $list->addElement( $secondClaim ), 'Adding a
duplicate element should work' );
+ $list->addClaim( $firstClaim );
+ $list->addClaim( $secondClaim );
- $this->assertEquals( 2, count( $list->getArrayCopy() ), 'Adding
two duplicates to an empty list should result in a count of two' );
+ $this->assertEquals( 2, count( $list ), 'Adding two duplicates
to an empty list should result in a count of two' );
- $this->assertTrue( $list->addElement( new Claim( new
PropertySomeValueSnak( 1 ) ) ) );
+ $this->assertEquals( $firstClaim, $list->getClaimWithGuid(
$firstClaim->getGuid() ) );
+ $this->assertEquals( $secondClaim, $list->getClaimWithGuid(
$secondClaim->getGuid() ) );
- $list->removeDuplicates();
+ $list->removeClaimWithGuid( $secondClaim->getGuid() );
- $this->assertEquals( 2, count( $list->getArrayCopy() ),
'Removing duplicates from a list should work' );
+ $this->assertNotNull( $list->getClaimWithGuid(
$firstClaim->getGuid() ) );
+ $this->assertNull( $list->getClaimWithGuid(
$secondClaim->getGuid() ) );
}
public function getDiffProvider() {
$argLists = array();
- $claim0 = new Claim( new PropertyNoValueSnak( 42 ) );
- $claim1 = new Claim( new PropertySomeValueSnak( 42 ) );
- $claim2 = new Claim( new PropertyValueSnak( 42, new
StringValue( 'ohi' ) ) );
- $claim3 = new Claim( new PropertyNoValueSnak( 1 ) );
- $claim4 = new Claim( new PropertyNoValueSnak( 2 ) );
+ $claim0 = $this->makeClaim( new PropertyNoValueSnak( 42 ) );
+ $claim1 = $this->makeClaim( new PropertySomeValueSnak( 42 ) );
+ $claim2 = $this->makeClaim( new PropertyValueSnak( 42, new
StringValue( 'ohi' ) ) );
+ $claim3 = $this->makeClaim( new PropertyNoValueSnak( 1 ) );
+ $claim4 = $this->makeClaim( new PropertyNoValueSnak( 2 ) );
- $statement0 = new Statement( new PropertyNoValueSnak( 5 ) );
+ $statement0 = $this->makeStatement( new PropertyNoValueSnak( 5
) );
$statement0->setRank( Statement::RANK_PREFERRED );
- $statement1 = new Statement( new PropertyNoValueSnak( 5 ) );
+ $statement1 = $this->makeStatement( new PropertyNoValueSnak( 5
) );
$statement1->setReferences( new ReferenceList( array( new
Reference(
new SnakList( array( new PropertyValueSnak( 10, new
StringValue( 'spam' ) ) ) )
) ) ) );
- $claim0->setGuid( 'claim0' );
- $claim1->setGuid( 'claim1' );
- $claim2->setGuid( 'claim2' );
- $statement1->setGuid( 'statement1' );
- $statement0->setGuid( 'statement0' );
-
+ // same GUID, changed main snak
$claim2v2 = unserialize( serialize( $claim2 ) );
$claim2v2->setMainSnak( new PropertyValueSnak( 42, new
StringValue( 'omnomnom' ) ) );
+ // different GUID, same contents, same hash
+ $claim0a = unserialize( serialize( $claim0 ) );
+ $claim0a->setGuid( 'TEST$claim0x' );
+ $claim0a->setMainSnak( new PropertyValueSnak( 99, new
StringValue( 'frob' ) ) );
+
+ // same GUID as $claim0a, same content/hash as $claim0
+ $claim0b = unserialize( serialize( $claim0 ) );
+ $claim0b->setGuid( 'TEST$claim0x' );
$source = new Claims();
$target = new Claims();
@@ -277,6 +551,11 @@
$expected = new Diff( array( $claim2->getGuid() => new
DiffOpChange( $claim2, $claim2v2 ) ), true );
$argLists[] = array( $source, $target, $expected, 'Changing the
value of a claim should result in a change op' );
+ $source = new Claims( array( $claim0, $claim0a ) );
+ $target = new Claims( array( $claim0, $claim0b ) );
+ $expected = new Diff( array( $claim0a->getGuid() => new
DiffOpChange( $claim0a, $claim0b ) ), true );
+ $argLists[] = array( $source, $target, $expected, 'It should be
possible for a claim to become the same as another claim' );
+
return $argLists;
}
@@ -302,4 +581,62 @@
$claims->getClaimsForProperty( 'foo bar' );
}
+ public function testGetHash() {
+ $claimsA = new Claims();
+ $claimsB = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+ $claim2 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P16" ) ) );
+
+ $this->assertEquals( $claimsA->getHash(), $claimsB->getHash(),
'empty list' );
+
+ $claimsA->addClaim( $claim1 );
+ $claimsB->addClaim( $claim2 );
+ $this->assertNotEquals( $claimsA->getHash(),
$claimsB->getHash(), 'different content' );
+
+ $claimsA->addClaim( $claim2 );
+ $claimsB->addClaim( $claim1 );
+ $this->assertNotEquals( $claimsA->getHash(),
$claimsB->getHash(), 'different order' );
+
+ $claimsA->removeClaim( $claim1 );
+ $claimsB->removeClaim( $claim1 );
+ $this->assertEquals( $claimsA->getHash(), $claimsB->getHash(),
'same content' );
+ }
+
+ public function testItrerator() {
+ $claims = new Claims( array(
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertySomeValueSnak( new
PropertyId( "P42" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P23" ) ) ),
+ $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P9000" ) ) ),
+ ) );
+
+ $array = iterator_to_array( $claims->getIterator() );
+
+ $this->assertSameSize( $claims, $array );
+
+ reset( $array );
+ reset( $claims );
+
+ while ( $actual = current( $array ) ) {
+ $expected = current( $claims );
+
+ $this->assertEquals( $actual, $expected );
+
+ next( $claims );
+ next( $array );
+ }
+ }
+
+ public function testIsEmpty() {
+ $claims = new Claims();
+ $claim1 = $this->makeClaim( new PropertyNoValueSnak( new
PropertyId( "P15" ) ) );
+
+ $this->assertTrue( $claims->isEmpty() );
+
+ $claims->addClaim( $claim1 );
+ $this->assertFalse( $claims->isEmpty() );
+
+ $claims->removeClaim( $claim1 );
+ $this->assertTrue( $claims->isEmpty() );
+ }
}
diff --git a/tests/phpunit/Entity/EntityTest.php
b/tests/phpunit/Entity/EntityTest.php
index 71ab54a..b30387b 100644
--- a/tests/phpunit/Entity/EntityTest.php
+++ b/tests/phpunit/Entity/EntityTest.php
@@ -886,6 +886,9 @@
$claim1 = new Claim( new PropertySomeValueSnak( 42 ) ),
);
+ $claims[0]->setGuid( 'TEST$NVS42' );
+ $claims[1]->setGuid( 'TEST$SVS42' );
+
$entity->setClaims( new Claims( $claims ) );
$this->assertSameSize( $claims, $entity->getClaims(), "added
some claims" );
diff --git a/tests/phpunit/Entity/ItemTest.php
b/tests/phpunit/Entity/ItemTest.php
index df882b5..0dba54d 100644
--- a/tests/phpunit/Entity/ItemTest.php
+++ b/tests/phpunit/Entity/ItemTest.php
@@ -97,6 +97,10 @@
) )
);
+ foreach ( $claims as $i => $claim ) {
+ $claim->setGuid( "ItemTest\$claim-$i" );
+ }
+
return $claims;
}
diff --git a/tests/phpunit/EntityDiffTest.php b/tests/phpunit/EntityDiffTest.php
index 6280237..508273b 100644
--- a/tests/phpunit/EntityDiffTest.php
+++ b/tests/phpunit/EntityDiffTest.php
@@ -85,7 +85,10 @@
$diffs[] = new EntityDiff( $diffOps );
- $claims = new \Wikibase\Claims( array( new \Wikibase\Claim( new
\Wikibase\PropertyNoValueSnak( 42 ) ) ) );
+ $claim = new \Wikibase\Claim( new
\Wikibase\PropertyNoValueSnak( 42 ) );
+ $claim->setGuid( 'EntityDiffTest$foo' );
+
+ $claims = new \Wikibase\Claims( array( $claim ) );
$diffOps['claim'] = $claims->getDiff( new \Wikibase\Claims() );
--
To view, visit https://gerrit.wikimedia.org/r/89015
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idd5ffb72774a948852a6dbe2e05700b9aa94a211
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseDataModel
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
