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

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(-)

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



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: merged
Gerrit-Change-Id: If5ce08ebc18eabe4dade567a88bbefc9aa215ff4
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Anja Jentzsch <[email protected]>
Gerrit-Reviewer: Ataherivand <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Daniel Werner <[email protected]>
Gerrit-Reviewer: Denny Vrandecic <[email protected]>
Gerrit-Reviewer: Henning Snater <[email protected]>
Gerrit-Reviewer: Jens Ohlig <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: John Erling Blad <[email protected]>
Gerrit-Reviewer: Lydia Pintscher <[email protected]>
Gerrit-Reviewer: Markus Kroetzsch <[email protected]>
Gerrit-Reviewer: Nikola Smolenski <[email protected]>
Gerrit-Reviewer: Silke Meyer <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to