jenkins-bot has submitted this change and it was merged.
Change subject: ChangeOpClaim now uses the claim guids
......................................................................
ChangeOpClaim now uses the claim guids
Change-Id: I117f0b10ee4e1782bbaaad77248e226773b23a64
---
M repo/includes/changeop/ChangeOpClaim.php
M repo/tests/phpunit/includes/changeop/ChangeOpClaimTest.php
2 files changed, 122 insertions(+), 37 deletions(-)
Approvals:
Tobias Gritschacher: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/changeop/ChangeOpClaim.php
b/repo/includes/changeop/ChangeOpClaim.php
index f5cd0a8..5a7667b 100644
--- a/repo/includes/changeop/ChangeOpClaim.php
+++ b/repo/includes/changeop/ChangeOpClaim.php
@@ -4,6 +4,7 @@
use InvalidArgumentException;
use Wikibase\Lib\ClaimGuidGenerator;
+use Wikibase\Lib\ClaimGuidValidator;
/**
* Class for claim change operation
@@ -32,14 +33,22 @@
protected $action;
/**
+ * @since 0.5
+ *
+ * @var ClaimGuidGenerator
+ */
+ protected $guidGenerator;
+
+ /**
* @since 0.4
*
* @param Claim $claim
* @param string $action should be add or remove
*
- * @throws InvalidArgumentException
+ * @param ClaimGuidGenerator $guidGenerator
+ * @throws \InvalidArgumentException
*/
- public function __construct( $claim, $action ) {
+ public function __construct( $claim, $action, ClaimGuidGenerator
$guidGenerator ) {
if ( !$claim instanceof Claim ) {
throw new InvalidArgumentException( '$claim needs to be
an instance of Claim' );
}
@@ -50,6 +59,7 @@
$this->claim = $claim;
$this->action = $action;
+ $this->guidGenerator = $guidGenerator;
}
/**
@@ -65,16 +75,36 @@
* @throws ChangeOpException
*/
public function apply( Entity $entity, Summary $summary = null ) {
+
if ( $this->action === "add" ) {
- $guidGenerator = new ClaimGuidGenerator(
$entity->getId() );
- $this->claim->setGuid( $guidGenerator->newGuid() );
+
+ $guidValidator = new ClaimGuidValidator();
+
+ if( $this->claim->getGuid() === null ){
+ $this->claim->setGuid(
$this->guidGenerator->newGuid() );
+ }
+ $guid = $this->claim->getGuid();
+
+ if ( $guidValidator->validate( $guid ) === false ) {
+ throw new ChangeOpException( "Claim does not
have a valid GUID" );
+ } else if ( strtoupper(
$entity->getId()->getPrefixedId() ) !== substr( $guid, 0, strpos( $guid, '$' )
) ){
+ throw new ChangeOpException( "Claim GUID
invalid for given entity" );
+ }
+
$entity->addClaim( $this->claim );
$this->updateSummary( $summary, 'add' );
+
} elseif ( $this->action === "remove" ) {
+
+ $guid = $this->claim->getGuid();
$claims = new Claims ( $entity->getClaims() );
- $claims->removeClaim( $this->claim );
+ if( !$claims->hasClaimWithGuid( $guid ) ){
+ throw new ChangeOpException( 'Cannot remove a
claim that does not exist on given entity' );
+ }
+ $claims->removeClaimWithGuid( $guid );
$entity->setClaims( $claims );
$this->updateSummary( $summary, 'remove' );
+
} else {
throw new ChangeOpException( "Unknown action for change
op: $this->action" );
}
diff --git a/repo/tests/phpunit/includes/changeop/ChangeOpClaimTest.php
b/repo/tests/phpunit/includes/changeop/ChangeOpClaimTest.php
index ed9e8c7..413d566 100644
--- a/repo/tests/phpunit/includes/changeop/ChangeOpClaimTest.php
+++ b/repo/tests/phpunit/includes/changeop/ChangeOpClaimTest.php
@@ -5,10 +5,12 @@
use Wikibase\ChangeOpClaim;
use Wikibase\Claim;
use Wikibase\Claims;
+use Wikibase\DataModel\Entity\ItemId;
use Wikibase\Entity;
-use Wikibase\EntityId;
+use Wikibase\Item;
use Wikibase\ItemContent;
use InvalidArgumentException;
+use Wikibase\Lib\ClaimGuidGenerator;
use Wikibase\PropertyNoValueSnak;
use Wikibase\PropertySomeValueSnak;
use Wikibase\SnakObject;
@@ -32,10 +34,12 @@
class ChangeOpClaimTest extends \PHPUnit_Framework_TestCase {
public function invalidConstructorProvider() {
+ $validGuidGenerator = new ClaimGuidGenerator(
ItemId::newFromNumber( 42 ) );
+
$args = array();
- $args[] = array( 42, 'add' );
- $args[] = array( 'en', 'remove' );
- $args[] = array( array(), 'remove' );
+ $args[] = array( 42, 'add', $validGuidGenerator );
+ $args[] = array( 'en', 'remove', $validGuidGenerator );
+ $args[] = array( array(), 'remove', $validGuidGenerator );
return $args;
}
@@ -45,51 +49,76 @@
*
* @param Claim $claim
* @param string $action
+ * @param ClaimGuidGenerator $guidGenerator
*/
- public function testInvalidConstruct( $claim, $action ) {
- $changeOp = new ChangeOpClaim( $claim, $action );
+ public function testInvalidConstruct( $claim, $action, $guidGenerator) {
+ $changeOp = new ChangeOpClaim( $claim, $action, $guidGenerator);
}
- public function changeOpClaimProvider() {
- $noValueClaim = new Claim( new PropertyNoValueSnak( 43 ) );
+ public function provideTestApply() {
+ $itemEmpty = Item::newEmpty();
+ $itemEmpty->setId( ItemId::newFromNumber( 888 ) );
+ $item777 = self::provideNewItemWithClaim( 777, new
PropertyNoValueSnak( 45 ) );
+ $item666 = self::provideNewItemWithClaim( 666, new
PropertySomeValueSnak( 44 ) );
- $differentEntity = ItemContent::newEmpty()->getEntity();
- $differentEntity->setId( new EntityId( 'item', 777 ) );
- $oldNoValueClaim = $differentEntity->newClaim( new
PropertyNoValueSnak( 43 ) );
+ $claims[0] = new Claim( new PropertyNoValueSnak( 43 ) );
+ $item777Claims = $item777->getClaims();
+ $claims[777] = clone $item777Claims[0];
+ $item666Claims = $item666->getClaims();
+ $claims[666] = clone $item666Claims[0];
- $entity = ItemContent::newEmpty()->getEntity();
- $entity->setId( new EntityId( 'item', 555 ) );
- $someValueClaim = new Claim( new PropertySomeValueSnak( 44 ) );
- $newNoValueClaim = $entity->newClaim( new PropertyNoValueSnak(
43 ) );
$args = array();
-
- $args[] = array ( $entity, clone $noValueClaim , 'add' , array(
clone $noValueClaim ) );
- $args[] = array ( $entity, clone $someValueClaim , 'add' ,
array( clone $noValueClaim, clone $someValueClaim ) );
- $args[] = array ( $entity, clone $noValueClaim , 'remove' ,
array( clone $someValueClaim ) );
- $args[] = array ( $entity, clone $someValueClaim , 'remove' ,
array( ) );
- $args[] = array ( $entity, clone $oldNoValueClaim , 'add' ,
array( clone $newNoValueClaim ) );
- $args[] = array ( $entity, clone $newNoValueClaim , 'remove' ,
array( ) );
+ //test adding claims with guids from other items
+ $args[] = array ( $itemEmpty, clone $claims[666] , 'add' ,
false );
+ $args[] = array ( $itemEmpty, clone $claims[777] , 'add' ,
false );
+ $args[] = array ( $item666, clone $claims[777] , 'add' , false
);
+ $args[] = array ( $item777, clone $claims[666] , 'add' , false
);
+ //test adding claims with from this item
+ $args[] = array ( $item777, clone $claims[777] , 'remove' ,
array( ) );
+ $args[] = array ( $item777, clone $claims[777] , 'add' , array(
clone $claims[777] ) );
+ $args[] = array ( $item666, clone $claims[666] , 'remove' ,
array( ) );
+ $args[] = array ( $item666, clone $claims[666] , 'add' , array(
clone $claims[666] ) );
+ //test adding claims with no guid
+ $args[] = array ( $itemEmpty, clone $claims[0] , 'add' , array(
clone $claims[0] ) );
+ $args[] = array ( $item777, clone $claims[0] , 'add' , array(
clone $claims[777], clone $claims[0] ) );
+ $args[] = array ( $item666, clone $claims[0] , 'add' , array(
clone $claims[666], clone $claims[0] ) );
+ //test removing claims with good guids that exist
+ $args[] = array ( $item777, clone $claims[777] , 'remove' ,
array( clone $claims[0] ) );
+ $args[] = array ( $item666, clone $claims[666] , 'remove' ,
array( clone $claims[0] ) );
+ //test removing claims with good guids that dont exist
+ $args[] = array ( $item777, clone $claims[777] , 'remove' ,
false );
+ $args[] = array ( $item666, clone $claims[666] , 'remove' ,
false );
+ //test removing claim with no guid
+ $args[] = array ( $itemEmpty, clone $claims[0] , 'remove' ,
false );
return $args;
}
/**
- * @dataProvider changeOpClaimProvider
+ * @dataProvider provideTestApply
*
* @param Entity $entity
- * @param $claim
- * @param $action
- * @param Claim[] $expectedClaims
- * @internal param \Wikibase\ChangeOpClaim $changeOpClaim
+ * @param Claim $claim
+ * @param string $action
+ * @param Claim[]|bool $expected
*/
- public function testApply( $entity, $claim, $action, $expectedClaims ) {
- $changeOpClaim = new ChangeOpClaim( $claim, $action );
+ public function testApply( $entity, $claim, $action, $expected ) {
+ if( $expected === false ){
+ $this->setExpectedException(
'\Wikibase\ChangeOpException' );
+ }
+
+ $changeOpClaim = new ChangeOpClaim( $claim, $action, new
ClaimGuidGenerator( $entity->getId() ) );
$changeOpClaim->apply( $entity );
+
+ if( $expected === false ){
+ $this->fail( 'Failed to throw a ChangeOpException' );
+ }
+
$entityClaims = new Claims( $entity->getClaims() );
- foreach( $expectedClaims as $expectedClaim ){
+ foreach( $expected as $expectedClaim ){
$this->assertTrue( $entityClaims->hasClaim(
$expectedClaim ) );
}
- $this->assertEquals( count( $expectedClaims ),
$entityClaims->count() );
+ $this->assertEquals( count( $expected ), $entityClaims->count()
);
}
/**
@@ -98,8 +127,34 @@
public function testApplyWithInvalidAction() {
$item = ItemContent::newEmpty();
$entity = $item->getEntity();
- $changeOpClaim = new ChangeOpClaim( new Claim( new
PropertyNoValueSnak( 43 ) ) , 'invalidAction' );
+
+ $changeOpClaim = new ChangeOpClaim(
+ new Claim( new PropertyNoValueSnak( 43 ) ) ,
+ 'invalidAction',
+ new ClaimGuidGenerator( ItemId::newFromNumber( 42 ) ) );
+
$changeOpClaim->apply( $entity );
}
+
+ /**
+ * @param integer $itemId
+ * @param $snak
+ * @return Item
+ */
+ protected function provideNewItemWithClaim( $itemId, $snak ) {
+ $entity = Item::newEmpty();
+ $entity->setId( ItemId::newFromNumber( $itemId ) );
+
+ $claim = $entity->newClaim( $snak );
+ $guidGenerator = new ClaimGuidGenerator( $entity->getId() );
+ $claim->setGuid( $guidGenerator->newGuid() );
+
+ $claims = new Claims();
+ $claims->addClaim( $claim );
+ $entity->setClaims( $claims );
+
+ return $entity;
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/82223
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I117f0b10ee4e1782bbaaad77248e226773b23a64
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
