Jeroen De Dauw has uploaded a new change for review.
https://gerrit.wikimedia.org/r/67526
Change subject: Got rid of MWException in Claims and improved its tests
......................................................................
Got rid of MWException in Claims and improved its tests
Change-Id: Ie53aa8a666e3bc6cf5bdfa016af60cf6f479034c
---
M DataModel/DataModel/Claim/Claims.php
M DataModel/tests/phpunit/Claim/ClaimsTest.php
2 files changed, 79 insertions(+), 54 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/26/67526/1
diff --git a/DataModel/DataModel/Claim/Claims.php
b/DataModel/DataModel/Claim/Claims.php
index 0842a6d..660c8ea 100644
--- a/DataModel/DataModel/Claim/Claims.php
+++ b/DataModel/DataModel/Claim/Claims.php
@@ -3,7 +3,11 @@
namespace Wikibase;
use Diff\Differ;
-use MWException;
+use Diff\DiffOpAdd;
+use Diff\DiffOpChange;
+use Diff\DiffOpRemove;
+use Diff\MapDiffer;
+use InvalidArgumentException;
/**
* Implementation of the Claims interface.
@@ -168,12 +172,12 @@
*
* @param int $propertyId
*
- * @throws \MWException if $propertyId isn't valid
+ * @throws InvalidArgumentException
* @return Claims
*/
public function getClaimsForProperty( $propertyId ) {
if ( !is_int( $propertyId ) ) {
- throw new MWException( "ID must be an int." );
+ throw new InvalidArgumentException( '$propertyId must
be an integer' );
}
$claimsByProp = new ByPropertyIdArray( $this );
@@ -251,14 +255,14 @@
* @param Differ|null $differ
*
* @return \Diff\Diff
- * @throws MWException
+ * @throws InvalidArgumentException
*/
public function getDiff( Claims $claims, Differ $differ = null ) {
assert( $this->indicesAreUpToDate() );
assert( $claims->indicesAreUpToDate() );
if ( $differ === null ) {
- $differ = new \Diff\MapDiffer();
+ $differ = new MapDiffer();
}
$sourceHashes = array();
@@ -278,7 +282,7 @@
$diff = new \Diff\Diff( array(), true );
foreach ( $differ->doDiff( $sourceHashes, $targetHashes ) as
$diffOp ) {
- if ( $diffOp instanceof \Diff\DiffOpChange ) {
+ if ( $diffOp instanceof DiffOpChange ) {
$oldClaim = $this->getByElementHash(
$diffOp->getOldValue() );
$newClaim = $claims->getByElementHash(
$diffOp->getNewValue() );
@@ -286,20 +290,20 @@
assert( $newClaim instanceof Claim );
assert( $oldClaim->getGuid() ===
$newClaim->getGuid() );
- $diff[$oldClaim->getGuid()] = new
\Diff\DiffOpChange( $oldClaim, $newClaim );
+ $diff[$oldClaim->getGuid()] = new DiffOpChange(
$oldClaim, $newClaim );
}
- elseif ( $diffOp instanceof \Diff\DiffOpAdd ) {
+ elseif ( $diffOp instanceof DiffOpAdd ) {
$claim = $claims->getByElementHash(
$diffOp->getNewValue() );
assert( $claim instanceof Claim );
- $diff[$claim->getGuid()] = new \Diff\DiffOpAdd(
$claim );
+ $diff[$claim->getGuid()] = new DiffOpAdd(
$claim );
}
- elseif ( $diffOp instanceof \Diff\DiffOpRemove ) {
+ elseif ( $diffOp instanceof DiffOpRemove ) {
$claim = $this->getByElementHash(
$diffOp->getOldValue() );
assert( $claim instanceof Claim );
- $diff[$claim->getGuid()] = new
\Diff\DiffOpRemove( $claim );
+ $diff[$claim->getGuid()] = new DiffOpRemove(
$claim );
}
else {
- throw new MWException( 'Invalid DiffOp type
cannot be handled' );
+ throw new InvalidArgumentException( 'Invalid
DiffOp type cannot be handled' );
}
}
diff --git a/DataModel/tests/phpunit/Claim/ClaimsTest.php
b/DataModel/tests/phpunit/Claim/ClaimsTest.php
index 8a895b7..35e7961 100644
--- a/DataModel/tests/phpunit/Claim/ClaimsTest.php
+++ b/DataModel/tests/phpunit/Claim/ClaimsTest.php
@@ -2,11 +2,25 @@
namespace Wikibase\Test;
+use DataValues\StringValue;
+use Diff\Diff;
+use Diff\DiffOpAdd;
+use Diff\DiffOpChange;
+use Diff\DiffOpRemove;
use Wikibase\Claim;
use Wikibase\Claims;
+use Wikibase\EntityId;
+use Wikibase\Property;
+use Wikibase\PropertyNoValueSnak;
+use Wikibase\PropertySomeValueSnak;
+use Wikibase\PropertyValueSnak;
+use Wikibase\Reference;
+use Wikibase\ReferenceList;
+use Wikibase\SnakList;
+use Wikibase\Statement;
/**
- * Tests for the Wikibase\Claims class.
+ * @covers Wikibase\Claims
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -57,18 +71,18 @@
public function getElementInstances() {
$instances = array();
- $instances[] = new \Wikibase\Claim(
- new \Wikibase\PropertyNoValueSnak(
- new \Wikibase\EntityId(
\Wikibase\Property::ENTITY_TYPE, 23 ) ) );
+ $instances[] = new Claim(
+ new PropertyNoValueSnak(
+ new EntityId( Property::ENTITY_TYPE, 23 ) ) );
- $instances[] = new \Wikibase\Claim(
- new \Wikibase\PropertySomeValueSnak(
- new \Wikibase\EntityId(
\Wikibase\Property::ENTITY_TYPE, 42 ) ) );
+ $instances[] = new Claim(
+ new PropertySomeValueSnak(
+ new EntityId( Property::ENTITY_TYPE, 42 ) ) );
- $instances[] = new \Wikibase\Claim(
- new \Wikibase\PropertyValueSnak(
- new \Wikibase\EntityId(
\Wikibase\Property::ENTITY_TYPE, 42 ),
- new \DataValues\StringValue( "foo" ) ) );
+ $instances[] = new Claim(
+ new PropertyValueSnak(
+ new EntityId( Property::ENTITY_TYPE, 42 ),
+ new StringValue( "foo" ) ) );
return $instances;
}
@@ -174,8 +188,8 @@
}
public function testDuplicateClaims() {
- $firstClaim = new Claim( new \Wikibase\PropertyNoValueSnak( 42
) );
- $secondClaim = new Claim( new \Wikibase\PropertyNoValueSnak( 42
) );
+ $firstClaim = new Claim( new PropertyNoValueSnak( 42 ) );
+ $secondClaim = new Claim( new PropertyNoValueSnak( 42 ) );
$list = new Claims();
$this->assertTrue( $list->addElement( $firstClaim ), 'Adding
the first element should work' );
@@ -183,7 +197,7 @@
$this->assertEquals( 2, count( $list->getArrayCopy() ), 'Adding
two duplicates to an empty list should result in a count of two' );
- $this->assertTrue( $list->addElement( new Claim( new
\Wikibase\PropertySomeValueSnak( 1 ) ) ) );
+ $this->assertTrue( $list->addElement( new Claim( new
PropertySomeValueSnak( 1 ) ) ) );
$list->removeDuplicates();
@@ -193,18 +207,18 @@
public function getDiffProvider() {
$argLists = array();
- $claim0 = new Claim( new \Wikibase\PropertyNoValueSnak( 42 ) );
- $claim1 = new Claim( new \Wikibase\PropertySomeValueSnak( 42 )
);
- $claim2 = new Claim( new \Wikibase\PropertyValueSnak( 42, new
\DataValues\StringValue( 'ohi' ) ) );
- $claim3 = new Claim( new \Wikibase\PropertyNoValueSnak( 1 ) );
- $claim4 = new Claim( new \Wikibase\PropertyNoValueSnak( 2 ) );
+ $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 ) );
- $statement0 = new \Wikibase\Statement( new
\Wikibase\PropertyNoValueSnak( 5 ) );
- $statement0->setRank( \Wikibase\Statement::RANK_PREFERRED );
+ $statement0 = new Statement( new PropertyNoValueSnak( 5 ) );
+ $statement0->setRank( Statement::RANK_PREFERRED );
- $statement1 = new \Wikibase\Statement( new
\Wikibase\PropertyNoValueSnak( 5 ) );
- $statement1->setReferences( new \Wikibase\ReferenceList( array(
new \Wikibase\Reference(
- new \Wikibase\SnakList( array( new
\Wikibase\PropertyValueSnak( 10, new \DataValues\StringValue( 'spam' ) ) ) )
+ $statement1 = new Statement( new PropertyNoValueSnak( 5 ) );
+ $statement1->setReferences( new ReferenceList( array( new
Reference(
+ new SnakList( array( new PropertyValueSnak( 10, new
StringValue( 'spam' ) ) ) )
) ) ) );
$claim0->setGuid( 'claim0' );
@@ -214,68 +228,68 @@
$statement0->setGuid( 'statement0' );
$claim2v2 = unserialize( serialize( $claim2 ) );
- $claim2v2->setMainSnak( new \Wikibase\PropertyValueSnak( 42,
new \DataValues\StringValue( 'omnomnom' ) ) );
+ $claim2v2->setMainSnak( new PropertyValueSnak( 42, new
StringValue( 'omnomnom' ) ) );
$source = new Claims();
$target = new Claims();
- $expected = new \Diff\Diff( array(), true );
+ $expected = new Diff( array(), true );
$argLists[] = array( $source, $target, $expected, 'Two empty
lists should result in an empty diff' );
$source = new Claims();
$target = new Claims( array( $claim0 ) );
- $expected = new \Diff\Diff( array( $claim0->getGuid() => new
\Diff\DiffOpAdd( $claim0 ) ), true );
+ $expected = new Diff( array( $claim0->getGuid() => new
DiffOpAdd( $claim0 ) ), true );
$argLists[] = array( $source, $target, $expected, 'List with no
entries to list with one should result in one add op' );
$source = new Claims( array( $claim0 ) );
$target = new Claims();
- $expected = new \Diff\Diff( array( $claim0->getGuid() => new
\Diff\DiffOpRemove( $claim0 ) ), true );
+ $expected = new Diff( array( $claim0->getGuid() => new
DiffOpRemove( $claim0 ) ), true );
$argLists[] = array( $source, $target, $expected, 'List with
one entry to an empty list should result in one remove op' );
$source = new Claims( array( $claim0, $claim3, $claim2 ) );
$target = new Claims( array( $claim0, $claim2, $claim3 ) );
- $expected = new \Diff\Diff( array(), true );
+ $expected = new Diff( array(), true );
$argLists[] = array( $source, $target, $expected, 'Two
identical lists should result in an empty diff' );
$source = new Claims( array( $claim0 ) );
$target = new Claims( array( $claim1 ) );
- $expected = new \Diff\Diff( array(
- $claim1->getGuid() => new \Diff\DiffOpAdd( $claim1 ),
- $claim0->getGuid() => new \Diff\DiffOpRemove( $claim0 )
+ $expected = new Diff( array(
+ $claim1->getGuid() => new DiffOpAdd( $claim1 ),
+ $claim0->getGuid() => new DiffOpRemove( $claim0 )
), true );
$argLists[] = array( $source, $target, $expected, 'Two lists
with each a single different entry should result into one add and one remove
op' );
$source = new Claims( array( $claim2, $claim3, $claim0, $claim4
) );
$target = new Claims( array( $claim2, $claim1, $claim3, $claim4
) );
- $expected = new \Diff\Diff( array(
- $claim1->getGuid() => new \Diff\DiffOpAdd( $claim1 ),
- $claim0->getGuid() => new \Diff\DiffOpRemove( $claim0 )
+ $expected = new Diff( array(
+ $claim1->getGuid() => new DiffOpAdd( $claim1 ),
+ $claim0->getGuid() => new DiffOpRemove( $claim0 )
), true );
$argLists[] = array( $source, $target, $expected, 'Two lists
with identical items except for one change should result in one add and one
remove op' );
$source = new Claims( array( $claim0, $claim0, $claim3,
$claim2, $claim2, $claim2, $statement0 ) );
$target = new Claims( array( $claim0, $claim0, $claim2,
$claim3, $claim2, $claim2, $statement0 ) );
- $expected = new \Diff\Diff( array(), true );
+ $expected = new Diff( array(), true );
$argLists[] = array( $source, $target, $expected, 'Two
identical lists with duplicate items should result in an empty diff' );
$source = new Claims( array( $statement0, $statement1, $claim0
) );
$target = new Claims( array( $claim1, $claim1, $claim0,
$statement1 ) );
- $expected = new \Diff\Diff( array(
- $claim1->getGuid() => new \Diff\DiffOpAdd( $claim1 ),
- $statement0->getGuid() => new \Diff\DiffOpRemove(
$statement0 ),
+ $expected = new Diff( array(
+ $claim1->getGuid() => new DiffOpAdd( $claim1 ),
+ $statement0->getGuid() => new DiffOpRemove( $statement0
),
), true );
$argLists[] = array( $source, $target, $expected, 'Two lists
with duplicate items and a different entry should result into one add and one
remove op' );
$source = new Claims( array( $claim0, $claim3, $claim2 ) );
$target = new Claims( array( $claim0, $claim2v2, $claim3 ) );
- $expected = new \Diff\Diff( array( $claim2->getGuid() => new
\Diff\DiffOpChange( $claim2, $claim2v2 ) ), true );
+ $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' );
return $argLists;
@@ -286,14 +300,21 @@
*
* @param \Wikibase\Claims $source
* @param \Wikibase\Claims $target
- * @param \Diff\Diff $expected
+ * @param Diff $expected
* @param string $message
*/
- public function testGetDiff( Claims $source, Claims $target, \Diff\Diff
$expected, $message ) {
+ public function testGetDiff( Claims $source, Claims $target, Diff
$expected, $message ) {
$actual = $source->getDiff( $target );
// Note: this makes order of inner arrays relevant, and this
order is not guaranteed by the interface
$this->assertEquals( $expected->getOperations(),
$actual->getOperations(), $message );
}
+ public function
testCallingGetClaimsForPropertyWithInvalidArgumentCausesException() {
+ $claims = new Claims();
+
+ $this->setExpectedException( 'InvalidArgumentException' );
+ $claims->getClaimsForProperty( 'foo bar' );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/67526
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie53aa8a666e3bc6cf5bdfa016af60cf6f479034c
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <jeroended...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
