Addshore has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/107836


Change subject: Create ChangeOpQualifierRemove class
......................................................................

Create ChangeOpQualifierRemove class

Change-Id: I9660f0c4975fbdbcf3cec1d60a9eb3fffc637ae3
---
M repo/Wikibase.classes.php
M repo/includes/ChangeOp/ChangeOpQualifier.php
A repo/includes/ChangeOp/ChangeOpQualifierRemove.php
M repo/includes/api/RemoveQualifiers.php
A repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierRemoveTest.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
A repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceRemoveTest.php
7 files changed, 259 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/36/107836/1

diff --git a/repo/Wikibase.classes.php b/repo/Wikibase.classes.php
index 3db9c65..4a17c77 100644
--- a/repo/Wikibase.classes.php
+++ b/repo/Wikibase.classes.php
@@ -53,6 +53,7 @@
                'Wikibase\ChangeOp\ChangeOpMainSnak' => 
'includes/ChangeOp/ChangeOpMainSnak.php',
                'Wikibase\ChangeOp\ChangeOpClaim' => 
'includes/ChangeOp/ChangeOpClaim.php',
                'Wikibase\ChangeOp\ChangeOpQualifier' => 
'includes/ChangeOp/ChangeOpQualifier.php',
+               'Wikibase\ChangeOp\ChangeOpQualifierRemove' => 
'includes/ChangeOp/ChangeOpQualifierRemove.php',
                'Wikibase\ChangeOp\ChangeOpReference' => 
'includes/ChangeOp/ChangeOpReference.php',
                'Wikibase\ChangeOp\ChangeOpStatementRank' => 
'includes/ChangeOp/ChangeOpStatementRank.php',
                'Wikibase\ChangeOp\ChangeOpException' => 
'includes/ChangeOp/ChangeOpException.php',
diff --git a/repo/includes/ChangeOp/ChangeOpQualifier.php 
b/repo/includes/ChangeOp/ChangeOpQualifier.php
index 12df5a0..9b8fb98 100644
--- a/repo/includes/ChangeOp/ChangeOpQualifier.php
+++ b/repo/includes/ChangeOp/ChangeOpQualifier.php
@@ -59,12 +59,8 @@
                        throw new InvalidArgumentException( '$snakHash needs to 
be a string' );
                }
 
-               if ( !( $snak instanceof Snak ) && !is_null( $snak ) ) {
-                       throw new InvalidArgumentException( '$snak needs to be 
an instance of Snak or null' );
-               }
-
-               if ( $snakHash === '' && $snak === null ) {
-                       throw new InvalidArgumentException( 'Either $snakHash 
or $snak needs to be set' );
+               if ( !( $snak instanceof Snak ) ) {
+                       throw new InvalidArgumentException( '$snak needs to be 
an instance of Snak' );
                }
 
                $this->claimGuid = $claimGuid;
@@ -74,7 +70,6 @@
 
        /**
         * @see ChangeOp::apply()
-        * - the qualifier gets removed when $snakHash is set and $snak is not 
set
         * - a new qualifier gets added when $snakHash is empty and $snak is set
         * - the qualifier gets set to $snak when $snakHash and $snak are set
         */
@@ -91,11 +86,7 @@
                if ( $this->snakHash === '' ) {
                        $this->addQualifier( $qualifiers, $summary );
                } else {
-                       if ( $this->snak != null ) {
-                               $this->setQualifier( $qualifiers, $summary );
-                       } else {
-                               $this->removeQualifier( $qualifiers, $summary );
-                       }
+                       $this->setQualifier( $qualifiers, $summary );
                }
 
                $claim->setQualifiers( $qualifiers );
@@ -139,23 +130,6 @@
                $qualifiers->removeSnakHash( $this->snakHash );
                $qualifiers->addSnak( $this->snak );
                $this->updateSummary( $summary, 'update', '', 
$this->getSnakSummaryArgs( $this->snak ) );
-       }
-
-       /**
-        * @since 0.4
-        *
-        * @param Snaks $qualifiers
-        * @param Summary $summary
-        *
-        * @throws ChangeOpException
-        */
-       protected function removeQualifier( Snaks $qualifiers, Summary $summary 
= null ) {
-               if ( !$qualifiers->hasSnakHash( $this->snakHash ) ) {
-                       throw new ChangeOpException( "Qualifier with hash 
$this->snakHash does not exist" );
-               }
-               $removedQualifier = $qualifiers->getSnak( $this->snakHash );
-               $qualifiers->removeSnakHash( $this->snakHash );
-               $this->updateSummary( $summary, 'remove', '', 
$this->getSnakSummaryArgs( $removedQualifier ) );
        }
 
        /**
diff --git a/repo/includes/ChangeOp/ChangeOpQualifierRemove.php 
b/repo/includes/ChangeOp/ChangeOpQualifierRemove.php
new file mode 100644
index 0000000..c846461
--- /dev/null
+++ b/repo/includes/ChangeOp/ChangeOpQualifierRemove.php
@@ -0,0 +1,94 @@
+<?php
+
+namespace Wikibase\ChangeOp;
+
+use InvalidArgumentException;
+use Wikibase\DataModel\Claim\Claims;
+use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Snak\Snak;
+use Wikibase\DataModel\Snak\Snaks;
+use Wikibase\Summary;
+
+/**
+ * Class for qualifier removal change operation
+ *
+ * @since 0.5
+ * @licence GNU GPL v2+
+ * @author Adam Shorland
+ */
+class ChangeOpQualifierRemove extends ChangeOpBase {
+
+       /**
+        * Constructs a new qualifier removal change operation
+        *
+        * @since 0.5
+        *
+        * @param string $claimGuid
+        * @param string $snakHash
+        *
+        * @throws InvalidArgumentException
+        */
+       public function __construct( $claimGuid, $snakHash ) {
+               if ( !is_string( $claimGuid ) || $claimGuid === '' ) {
+                       throw new InvalidArgumentException( '$claimGuid needs 
to be a string and must not be empty' );
+               }
+
+               if ( !is_string( $snakHash ) || $snakHash === ''  ) {
+                       throw new InvalidArgumentException( '$snakHash needs to 
be a string and must not be empty' );
+               }
+
+               $this->claimGuid = $claimGuid;
+               $this->snakHash = $snakHash;
+       }
+
+       /**
+        * @see ChangeOp::apply()
+        */
+       public function apply( Entity $entity, Summary $summary = null ) {
+               $claims = new Claims( $entity->getClaims() );
+
+               if( !$claims->hasClaimWithGuid( $this->claimGuid ) ) {
+                       throw new ChangeOpException( "Entity does not have 
claim with GUID $this->claimGuid" );
+               }
+
+               $claim = $claims->getClaimWithGuid( $this->claimGuid );
+               $qualifiers = $claim->getQualifiers();
+
+               $this->removeQualifier( $qualifiers, $summary );
+
+               $claim->setQualifiers( $qualifiers );
+               $entity->setClaims( $claims );
+
+               return true;
+       }
+
+       /**
+        * @since 0.4
+        *
+        * @param Snaks $qualifiers
+        * @param Summary $summary
+        *
+        * @throws ChangeOpException
+        */
+       protected function removeQualifier( Snaks $qualifiers, Summary $summary 
= null ) {
+               if ( !$qualifiers->hasSnakHash( $this->snakHash ) ) {
+                       throw new ChangeOpException( "Qualifier with hash 
$this->snakHash does not exist" );
+               }
+               $removedQualifier = $qualifiers->getSnak( $this->snakHash );
+               $qualifiers->removeSnakHash( $this->snakHash );
+               $this->updateSummary( $summary, 'remove', '', 
$this->getSnakSummaryArgs( $removedQualifier ) );
+       }
+
+       /**
+        * @since 0.4
+        *
+        * @param Snak $snak
+        *
+        * @return array
+        */
+       protected function getSnakSummaryArgs( Snak $snak ) {
+               $propertyId = $snak->getPropertyId();
+               return array( array( $propertyId->getPrefixedId() => $snak ) );
+       }
+
+}
\ No newline at end of file
diff --git a/repo/includes/api/RemoveQualifiers.php 
b/repo/includes/api/RemoveQualifiers.php
index 2eef257..06ae42e 100644
--- a/repo/includes/api/RemoveQualifiers.php
+++ b/repo/includes/api/RemoveQualifiers.php
@@ -4,10 +4,10 @@
 
 use ApiBase;
 use Wikibase\ChangeOp\ChangeOp;
-use Wikibase\Claim;
-use Wikibase\ChangeOp\ChangeOpQualifier;
+use Wikibase\ChangeOp\ChangeOpQualifierRemove;
 use Wikibase\ChangeOp\ChangeOps;
 use Wikibase\ChangeOp\ChangeOpException;
+use Wikibase\DataModel\Claim\Claim;
 
 /**
  * API module for removing qualifiers from a claim.
@@ -81,7 +81,7 @@
                $changeOps = array();
 
                foreach ( $qualifierHashes as $qualifierHash ) {
-                       $changeOps[] = new ChangeOpQualifier( $claimGuid, null, 
$qualifierHash );
+                       $changeOps[] = new ChangeOpQualifierRemove( $claimGuid, 
$qualifierHash );
                }
 
                return $changeOps;
diff --git 
a/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierRemoveTest.php 
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierRemoveTest.php
new file mode 100644
index 0000000..32e4b32
--- /dev/null
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierRemoveTest.php
@@ -0,0 +1,74 @@
+<?php
+
+namespace Wikibase\Test;
+
+use DataValues\StringValue;
+use Wikibase\ChangeOp\ChangeOpQualifierRemove;
+use Wikibase\DataModel\Claim\Claim;
+use Wikibase\DataModel\Claim\Claims;
+use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\ItemContent;
+
+/**
+ * @covers Wikibase\ChangeOp\ChangeOpQualifierRemove
+ *
+ * @since 0.5
+ *
+ * @group Wikibase
+ * @group WikibaseRepo
+ * @group ChangeOp
+ *
+ * @licence GNU GPL v2+
+ * @author Adam Shorland
+ */
+class ChangeOpReferenceRemoveTest extends \PHPUnit_Framework_TestCase {
+
+       public function changeOpRemoveProvider() {
+               $snak = new PropertyValueSnak( 2754236, new StringValue( 'test' 
) );
+               $args = array();
+
+               $item = $this->provideNewItemWithClaim( 'q345', $snak );
+               $claims = $item->getClaims();
+               /** @var Claim $claim */
+               $claim = reset( $claims );
+               $claimGuid = $claim->getGuid();
+               $newQualifier = new PropertyValueSnak( 78462378, new 
StringValue( 'newQualifier' ) );
+               $qualifiers = $claim->getQualifiers();
+               $qualifiers->addSnak( $newQualifier );
+               $claim->setQualifiers( $qualifiers );
+               $item->setClaims( new Claims( $claims ) );
+               $snakHash = $newQualifier->getHash();
+               $changeOp = new ChangeOpQualifierRemove( $claimGuid, null, 
$snakHash );
+               $args[] = array ( $item, $changeOp, $snakHash );
+
+               return $args;
+       }
+
+       /**
+        * @dataProvider changeOpRemoveProvider
+        *
+        * @param Entity $item
+        * @param ChangeOpQualifierRemove $changeOp
+        * @param string $snakHash
+        */
+       public function testApplyRemoveQualifier( $item, $changeOp, $snakHash ) 
{
+               $this->assertTrue( $changeOp->apply( $item ), "Applying the 
ChangeOp did not return true" );
+               $claims = new Claims( $item->getClaims() );
+               /** @var Claim $claim */
+               $claim = reset( $claims );
+               $qualifiers = $claim->getQualifiers();
+               $this->assertFalse( $qualifiers->hasSnakHash( $snakHash ), 
"Qualifier still exists" );
+       }
+
+       protected function provideNewItemWithClaim( $itemId, $snak ) {
+               $entity = ItemContent::newFromArray( array( 'entity' => $itemId 
) )->getEntity();
+               $claim = $entity->newClaim( $snak );
+               $claim->setGuid( $entity->getId()->getPrefixedId() . 
'$D8404CDA-25E4-4334-AG03-A3290BCD9CQP' );
+               $claims = new Claims();
+               $claims->addClaim( $claim );
+               $entity->setClaims( $claims );
+               return $entity;
+       }
+
+} 
\ No newline at end of file
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php 
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
index f17516c..71e2b39 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpQualifierTest.php
@@ -4,6 +4,7 @@
 
 use DataValues\StringValue;
 use Wikibase\ChangeOp\ChangeOpQualifier;
+use Wikibase\DataModel\Claim\Claim;
 use Wikibase\DataModel\Claim\Claims;
 use Wikibase\DataModel\Entity\Entity;
 use Wikibase\DataModel\Snak\PropertyValueSnak;
@@ -50,7 +51,7 @@
         * @expectedException InvalidArgumentException
         */
        public function testInvalidConstruct( $claimGuid, $snak, $snakHash ) {
-               $ChangeOpQualifier = new ChangeOpQualifier( $claimGuid, $snak, 
$snakHash );
+               new ChangeOpQualifier( $claimGuid, $snak, $snakHash );
        }
 
        public function changeOpAddProvider() {
@@ -79,44 +80,10 @@
        public function testApplyAddNewQualifier( $item, $changeOp, $snakHash ) 
{
                $this->assertTrue( $changeOp->apply( $item ), "Applying the 
ChangeOp did not return true" );
                $claims = new Claims( $item->getClaims() );
+               /** @var Claim $claim */
                $claim = reset( $claims );
                $qualifiers = $claim->getQualifiers();
                $this->assertTrue( $qualifiers->hasSnakHash( $snakHash ), "No 
qualifier with expected hash" );
-       }
-
-       public function changeOpRemoveProvider() {
-               $snak = new PropertyValueSnak( 2754236, new StringValue( 'test' 
) );
-               $args = array();
-
-               $item = $this->provideNewItemWithClaim( 'q345', $snak );
-               $claims = $item->getClaims();
-               $claim = reset( $claims );
-               $claimGuid = $claim->getGuid();
-               $newQualifier = new PropertyValueSnak( 78462378, new 
StringValue( 'newQualifier' ) );
-               $qualifiers = $claim->getQualifiers();
-               $qualifiers->addSnak( $newQualifier );
-               $claim->setQualifiers( $qualifiers );
-               $item->setClaims( new Claims( $claims ) );
-               $snakHash = $newQualifier->getHash();
-               $changeOp = new ChangeOpQualifier( $claimGuid, null, $snakHash 
);
-               $args[] = array ( $item, $changeOp, $snakHash );
-
-               return $args;
-       }
-
-       /**
-        * @dataProvider changeOpRemoveProvider
-        *
-        * @param Entity $item
-        * @param ChangeOpQualifier $changeOp
-        * @param string $snakHash
-        */
-       public function testApplyRemoveQualifier( $item, $changeOp, $snakHash ) 
{
-               $this->assertTrue( $changeOp->apply( $item ), "Applying the 
ChangeOp did not return true" );
-               $claims = new Claims( $item->getClaims() );
-               $claim = reset( $claims );
-               $qualifiers = $claim->getQualifiers();
-               $this->assertFalse( $qualifiers->hasSnakHash( $snakHash ), 
"Qualifier still exists" );
        }
 
        public function changeOpSetProvider() {
@@ -125,6 +92,7 @@
 
                $item = $this->provideNewItemWithClaim( 'q123', $snak );
                $claims = $item->getClaims();
+               /** @var Claim $claim */
                $claim = reset( $claims );
                $claimGuid = $claim->getGuid();
                $newQualifier = new PropertyValueSnak( 78462378, new 
StringValue( 'newQualifier' ) );
@@ -150,6 +118,7 @@
        public function testApplySetQualifier( $item, $changeOp, $snakHash ) {
                $this->assertTrue( $changeOp->apply( $item ), "Applying the 
ChangeOp did not return true" );
                $claims = new Claims( $item->getClaims() );
+               /** @var Claim $claim */
                $claim = reset( $claims );
                $qualifiers = $claim->getQualifiers();
                $this->assertTrue( $qualifiers->hasSnakHash( $snakHash ), "No 
qualifier with expected hash" );
diff --git 
a/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceRemoveTest.php 
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceRemoveTest.php
new file mode 100644
index 0000000..787f1fc
--- /dev/null
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceRemoveTest.php
@@ -0,0 +1,79 @@
+<?php
+
+namespace Wikibase\Test;
+
+use DataValues\StringValue;
+use Wikibase\ChangeOp\ChangeOpReferenceRemove;
+use Wikibase\DataModel\Claim\Claims;
+use Wikibase\DataModel\Claim\Statement;
+use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Reference;
+use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\DataModel\Snak\SnakList;
+use Wikibase\ItemContent;
+
+/**
+ * @covers Wikibase\ChangeOp\ChangeOpReferenceRemove
+ *
+ * @since 0.5
+ *
+ * @group Wikibase
+ * @group WikibaseRepo
+ * @group ChangeOp
+ *
+ * @licence GNU GPL v2+
+ * @author Adam Shorland
+ */
+class ChangeOpQualifierRemoveTest extends \PHPUnit_Framework_TestCase {
+
+       public function changeOpRemoveProvider() {
+               $snak = new PropertyValueSnak( 2754236, new StringValue( 'test' 
) );
+               $args = array();
+
+               $item = $this->provideNewItemWithClaim( 'q345', $snak );
+               $claims = $item->getClaims();
+               /** @var Statement $claim */
+               $claim = reset( $claims );
+               $claimGuid = $claim->getGuid();
+               $snaks = new SnakList();
+               $snaks[] = new PropertyValueSnak( 78462378, new StringValue( 
'newQualifier' ) );
+               $newReference = new Reference( $snaks );
+               $references = $claim->getReferences();
+               $references->addReference( $newReference );
+               $claim->setReferences( $references );
+               $item->setClaims( new Claims( $claims ) );
+               $referenceHash = $newReference->getHash();
+               $changeOp = new ChangeOpReferenceRemove( $claimGuid, 
$referenceHash );
+               $args[] = array ( $item, $changeOp, $referenceHash );
+
+               return $args;
+       }
+
+       /**
+        * @dataProvider changeOpRemoveProvider
+        *
+        * @param Entity $item
+        * @param ChangeOpReferenceRemove $changeOp
+        * @param string $referenceHash
+        */
+       public function testApplyRemoveReference( $item, $changeOp, 
$referenceHash ) {
+               $this->assertTrue( $changeOp->apply( $item ), "Applying the 
ChangeOp did not return true" );
+               $claims = $item->getClaims();
+               /** @var Statement $claim */
+               $claim = reset( $claims );
+               $references = $claim->getReferences();
+               $this->assertFalse( $references->hasReferenceHash( 
$referenceHash ), "Reference still exists" );
+       }
+
+       protected function provideNewItemWithClaim( $itemId, $snak ) {
+               $entity = ItemContent::newFromArray( array( 'entity' => $itemId 
) )->getEntity();
+               $claim = $entity->newClaim( $snak );
+               $claim->setGuid( $entity->getId()->getPrefixedId() . 
'$D8494TYA-25E4-4334-AG03-A3290BCT9CQP' );
+               $claims = new Claims();
+               $claims->addClaim( $claim );
+               $entity->setClaims( $claims );
+
+               return $entity;
+       }
+
+} 
\ No newline at end of file

-- 
To view, visit https://gerrit.wikimedia.org/r/107836
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9660f0c4975fbdbcf3cec1d60a9eb3fffc637ae3
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to