Jeroen De Dauw has uploaded a new change for review.
https://gerrit.wikimedia.org/r/65795
Change subject: Fixed callback usage in ClaimDiffer and added tests
......................................................................
Fixed callback usage in ClaimDiffer and added tests
Change-Id: If5ce08ebc18eabe4dade567a88bbefc9aa215ff4
---
M DataModel/DataModel/Entity/Entity.php
M lib/includes/ClaimDiffer.php
M lib/tests/phpunit/claim/ClaimDifferTest.php
M repo/includes/EntityContentDiffView.php
M repo/includes/actions/EditEntityAction.php
M repo/includes/api/SetClaim.php
M repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
7 files changed, 132 insertions(+), 20 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/95/65795/1
diff --git a/DataModel/DataModel/Entity/Entity.php
b/DataModel/DataModel/Entity/Entity.php
index 76d0e7e..5e89d1f 100644
--- a/DataModel/DataModel/Entity/Entity.php
+++ b/DataModel/DataModel/Entity/Entity.php
@@ -870,7 +870,7 @@
function( Claim $firstClaim, Claim $secondClaim ) {
return $firstClaim->getHash() ===
$secondClaim->getHash();
}
- ) );
+ ) );
$claims = array();
diff --git a/lib/includes/ClaimDiffer.php b/lib/includes/ClaimDiffer.php
index 4aad00f..c839f32 100644
--- a/lib/includes/ClaimDiffer.php
+++ b/lib/includes/ClaimDiffer.php
@@ -3,7 +3,7 @@
namespace Wikibase;
use Diff\Diff;
-use Diff\ListDiffer;
+use Diff\Differ;
use Diff\DiffOpChange;
/**
@@ -37,7 +37,7 @@
/**
* @since 0.4
*
- * @var ListDiffer
+ * @var Differ
*/
private $listDiffer;
@@ -46,9 +46,9 @@
*
* @since 0.4
*
- * @param ListDiffer $listDiffer
+ * @param Differ $listDiffer
*/
- public function __construct( ListDiffer $listDiffer ) {
+ public function __construct( Differ $listDiffer ) {
$this->listDiffer = $listDiffer;
}
@@ -71,13 +71,12 @@
$mainSnakChange = new DiffOpChange(
$oldClaim->getMainSnak(), $newClaim->getMainSnak() );
}
- $this->listDiffer->setComparisonCallback( function( \Comparable
$old, \Comparable $new ) {
- return $old->equals( $new );
- } );
+ $oldQualifiers = iterator_to_array( $oldClaim->getQualifiers()
);
+ $newQualifiers = iterator_to_array( $newClaim->getQualifiers()
);
$qualifierChanges = new Diff( $this->listDiffer->doDiff(
- iterator_to_array( $oldClaim->getQualifiers() ),
- iterator_to_array( $newClaim->getQualifiers() )
+ $oldQualifiers,
+ $newQualifiers
), false );
if ( $oldClaim instanceof Statement && $newClaim instanceof
Statement ) {
diff --git a/lib/tests/phpunit/claim/ClaimDifferTest.php
b/lib/tests/phpunit/claim/ClaimDifferTest.php
index 7c3739d..90a08f7 100644
--- a/lib/tests/phpunit/claim/ClaimDifferTest.php
+++ b/lib/tests/phpunit/claim/ClaimDifferTest.php
@@ -2,9 +2,18 @@
namespace Wikibase\Test;
+use Diff\CallbackListDiffer;
+use Diff\Diff;
+use Diff\DiffOpAdd;
+use Diff\DiffOpChange;
+use Diff\DiffOpRemove;
use Wikibase\ClaimDiffer;
use Wikibase\ClaimDifference;
use Wikibase\Claim;
+use Wikibase\PropertyNoValueSnak;
+use Wikibase\ReferenceList;
+use Wikibase\SnakList;
+use Wikibase\Statement;
/**
* Tests for the Wikibase\ClaimDiffer class.
@@ -42,11 +51,79 @@
public function diffClaimsProvider() {
$argLists = array();
- $simpleClaim = new Claim( new \Wikibase\PropertyNoValueSnak( 42
) );
+ $noValueForP42 = new Statement( new PropertyNoValueSnak( 42 ) );
+ $noValueForP43 = new Statement( new PropertyNoValueSnak( 43 ) );
- $argLists[] = array( $simpleClaim, $simpleClaim, new
ClaimDifference() );
+ $argLists[] = array(
+ $noValueForP42,
+ $noValueForP42,
+ new ClaimDifference()
+ );
- // TODO: more tests
+ $argLists[] = array(
+ $noValueForP42,
+ $noValueForP43,
+ new ClaimDifference( new DiffOpChange( new
PropertyNoValueSnak( 42 ), new PropertyNoValueSnak( 43 ) ) )
+ );
+
+ $qualifiers = new SnakList( array( new PropertyNoValueSnak( 1 )
) );
+ $withQualifiers = clone $noValueForP42;
+ $withQualifiers->setQualifiers( $qualifiers );
+
+ $argLists[] = array(
+ $noValueForP42,
+ $withQualifiers,
+ new ClaimDifference(
+ null,
+ new Diff( array(
+ new DiffOpAdd( new PropertyNoValueSnak(
1 ) )
+ ), false )
+ )
+ );
+
+ $references = new ReferenceList( array( new
PropertyNoValueSnak( 2 ) ) );
+ $withReferences = clone $noValueForP42;
+ $withReferences->setReferences( $references );
+
+ $argLists[] = array(
+ $noValueForP42,
+ $withReferences,
+ new ClaimDifference(
+ null,
+ null,
+ new Diff( array(
+ new DiffOpAdd( new PropertyNoValueSnak(
2 ) )
+ ), false )
+ )
+ );
+
+ $argLists[] = array(
+ $withQualifiers,
+ $withReferences,
+ new ClaimDifference(
+ null,
+ new Diff( array(
+ new DiffOpRemove( new
PropertyNoValueSnak( 1 ) )
+ ), false ),
+ new Diff( array(
+ new DiffOpAdd( new PropertyNoValueSnak(
2 ) )
+ ), false )
+ )
+ );
+
+ $noValueForP42Preferred = clone $noValueForP42;
+ $noValueForP42Preferred->setRank( Statement::RANK_PREFERRED );
+
+ $argLists[] = array(
+ $noValueForP42,
+ $noValueForP42Preferred,
+ new ClaimDifference(
+ null,
+ null,
+ null,
+ new DiffOpChange( Statement::RANK_NORMAL,
Statement::RANK_PREFERRED )
+ )
+ );
return $argLists;
}
@@ -59,11 +136,23 @@
* @param ClaimDifference $expected
*/
public function testDiffClaims( Claim $oldClaim, Claim $newClaim,
ClaimDifference $expected ) {
- $differ = new ClaimDiffer( new \Diff\ListDiffer() );
+ $comparer = function( \Comparable $old, \Comparable $new ) {
+ return $old->equals( $new );
+ };
+
+ $differ = new ClaimDiffer( new CallbackListDiffer( $comparer )
);
$actual = $differ->diffClaims( $oldClaim, $newClaim );
$this->assertInstanceOf( 'Wikibase\ClaimDifference', $actual );
- $this->assertTrue( $expected->equals( $actual ), 'Expected
equals actual' );
+
+ if ( !$expected->equals( $actual ) ) {
+ q($expected, $actual);
+ }
+
+ $this->assertTrue(
+ $expected->equals( $actual ),
+ 'Diffing the claims results in the correct
ClaimDifference'
+ );
}
}
diff --git a/repo/includes/EntityContentDiffView.php
b/repo/includes/EntityContentDiffView.php
index 9db84f3..61fbdc6 100644
--- a/repo/includes/EntityContentDiffView.php
+++ b/repo/includes/EntityContentDiffView.php
@@ -1,6 +1,7 @@
<?php
namespace Wikibase;
+use Diff\CallbackListDiffer;
use Diff\ListDiffer;
use Content, Html;
@@ -135,10 +136,14 @@
$diff = $old->getEntity()->getDiff( $new->getEntity() );
$langCode = $this->getContext()->getLanguage()->getCode();
+ $comparer = function( \Comparable $old, \Comparable $new ) {
+ return $old->equals( $new );
+ };
+
// TODO: derp inject the EntityDiffVisualizer
$diffVisualizer = new EntityDiffVisualizer(
$this->getContext(),
- new ClaimDiffer( new ListDiffer() ),
+ new ClaimDiffer( new CallbackListDiffer( $comparer ) ),
new ClaimDifferenceVisualizer(
new WikiPageEntityLookup(),
$langCode,
diff --git a/repo/includes/actions/EditEntityAction.php
b/repo/includes/actions/EditEntityAction.php
index 7c1e667..1312945 100644
--- a/repo/includes/actions/EditEntityAction.php
+++ b/repo/includes/actions/EditEntityAction.php
@@ -1,6 +1,7 @@
<?php
namespace Wikibase;
+use Diff\CallbackListDiffer;
use Html, Linker, Skin, Status, Revision;
use Wikibase\Repo\WikibaseRepo;
@@ -431,10 +432,14 @@
$langCode = $this->getContext()->getLanguage()->getCode();
+ $comparer = function( \Comparable $old, \Comparable $new ) {
+ return $old->equals( $new );
+ };
+
// TODO: derp inject the EntityDiffVisualizer
$diffVisualizer = new EntityDiffVisualizer(
$this->getContext(),
- new ClaimDiffer( new \Diff\ListDiffer() ),
+ new ClaimDiffer( new CallbackListDiffer( $comparer ) ),
new ClaimDifferenceVisualizer(
new WikiPageEntityLookup(),
$langCode,
diff --git a/repo/includes/api/SetClaim.php b/repo/includes/api/SetClaim.php
index 1bfc8e2..70a94db 100644
--- a/repo/includes/api/SetClaim.php
+++ b/repo/includes/api/SetClaim.php
@@ -2,6 +2,7 @@
namespace Wikibase\Api;
+use Diff\CallbackListDiffer;
use MWException;
use ApiBase;
use Diff\ListDiffer;
@@ -53,7 +54,11 @@
public function execute() {
$claim = $this->getClaimFromRequest();
- $claimDiffer = new ClaimDiffer( new ListDiffer() );
+ $comparer = function( \Comparable $old, \Comparable $new ) {
+ return $old->equals( $new );
+ };
+
+ $claimDiffer = new ClaimDiffer( new CallbackListDiffer(
$comparer ) );
$claimSummaryBuilder = new ClaimSummaryBuilder(
$this->getModuleName(),
$claimDiffer,
diff --git a/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
b/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
index 6a3e1d9..33cc675 100644
--- a/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
+++ b/repo/tests/phpunit/includes/ClaimSummaryBuilderTest.php
@@ -2,6 +2,7 @@
namespace Wikibase\Test;
+use Diff\CallbackListDiffer;
use Diff\ListDiffer;
use Wikibase\ClaimDiffer;
use Wikibase\Claim;
@@ -137,9 +138,13 @@
->method( 'format' )
->will( $this->returnValue( 'foo' ) );
+ $comparer = function( \Comparable $old, \Comparable $new ) {
+ return $old->equals( $new );
+ };
+
$claimSummaryBuilder = new ClaimSummaryBuilder(
'wbsetclaim',
- new ClaimDiffer( new ListDiffer() ),
+ new ClaimDiffer( new CallbackListDiffer( $comparer ) ),
$idFormatter
);
@@ -168,9 +173,13 @@
->method( 'format' )
->will( $this->returnValue( 'foo' ) );
+ $comparer = function( \Comparable $old, \Comparable $new ) {
+ return $old->equals( $new );
+ };
+
$claimSummaryBuilder = new ClaimSummaryBuilder(
'wbsetclaim',
- new ClaimDiffer( new ListDiffer() ),
+ new ClaimDiffer( new CallbackListDiffer( $comparer ) ),
$idFormatter
);
--
To view, visit https://gerrit.wikimedia.org/r/65795
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If5ce08ebc18eabe4dade567a88bbefc9aa215ff4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
