Jeroen De Dauw has submitted this change and it was merged.
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
6 files changed, 190 insertions(+), 68 deletions(-)
Approvals:
Jeroen De Dauw: Verified; Looks good to me, approved
diff --git a/repo/Wikibase.classes.php b/repo/Wikibase.classes.php
index f88f0d4..f006399 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\ChangeOpReferenceRemove' =>
'includes/ChangeOp/ChangeOpReferenceRemove.php',
'Wikibase\ChangeOp\ChangeOpStatementRank' =>
'includes/ChangeOp/ChangeOpStatementRank.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..ff9b31d
--- /dev/null
+++ b/repo/includes/ChangeOp/ChangeOpQualifierRemove.php
@@ -0,0 +1,104 @@
+<?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 {
+
+ /**
+ * @since 0.5
+ *
+ * @var string
+ */
+ protected $claimGuid;
+
+ /**
+ * @since 0.5
+ *
+ * @var string
+ */
+ protected $snakHash;
+
+ /**
+ * 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;
+ }
+
+ /**
+ * @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 ) );
+ }
+
+ /**
+ * @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..099fce4
--- /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 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 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, $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" );
--
To view, visit https://gerrit.wikimedia.org/r/107836
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9660f0c4975fbdbcf3cec1d60a9eb3fffc637ae3
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[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