Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/204511
Change subject: Major update of ChangeOpsClaim[Remove] to get rid of Claims
......................................................................
Major update of ChangeOpsClaim[Remove] to get rid of Claims
This is a major update of 2 ChangeOp classes. This removes the last
remaining usages of the deprecated Claims::removeClaimWithGuid() method.
Plus a few minor updates that move our code a bit forward towards
statements and away from Claim and Claims.
Bug: T78281
Change-Id: Id8361ad7f17536365610157d27504d1e0b8d718e
---
M repo/includes/ChangeOp/ChangeOpBase.php
M repo/includes/ChangeOp/ChangeOpClaim.php
M repo/includes/ChangeOp/ChangeOpClaimRemove.php
M repo/includes/ChangeOp/ClaimChangeOpFactory.php
M repo/tests/phpunit/includes/ChangeOp/ClaimChangeOpFactoryTest.php
5 files changed, 156 insertions(+), 106 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/11/204511/1
diff --git a/repo/includes/ChangeOp/ChangeOpBase.php
b/repo/includes/ChangeOp/ChangeOpBase.php
index 64ab61b..f03abb0 100644
--- a/repo/includes/ChangeOp/ChangeOpBase.php
+++ b/repo/includes/ChangeOp/ChangeOpBase.php
@@ -19,18 +19,14 @@
/**
* @since 0.4
*
- * @param Summary $summary
+ * @param Summary|null $summary
* @param string $action
* @param string $language
* @param string|array $args
*
* @throws InvalidArgumentException
*/
- protected function updateSummary( $summary, $action, $language = '',
$args = '' ) {
- if ( $summary !== null && !$summary instanceof Summary ) {
- throw new InvalidArgumentException( '$summary needs to
be an instance of Summary or null' );
- }
-
+ protected function updateSummary( Summary $summary = null, $action,
$language = '', $args = '' ) {
if ( $summary !== null ) {
$summary->setAction( $action );
$summary->setLanguage( $language );
diff --git a/repo/includes/ChangeOp/ChangeOpClaim.php
b/repo/includes/ChangeOp/ChangeOpClaim.php
index 8e0a79e..dac770c 100644
--- a/repo/includes/ChangeOp/ChangeOpClaim.php
+++ b/repo/includes/ChangeOp/ChangeOpClaim.php
@@ -3,14 +3,15 @@
namespace Wikibase\ChangeOp;
use InvalidArgumentException;
-use OutOfBoundsException;
use ValueValidators\Result;
-use Wikibase\DataModel\ByPropertyIdArray;
use Wikibase\DataModel\Claim\Claim;
use Wikibase\DataModel\Claim\ClaimGuidParser;
-use Wikibase\DataModel\Claim\Claims;
use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\Property;
+use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\DataModel\StatementListProvider;
use Wikibase\Lib\ClaimGuidGenerator;
use Wikibase\Lib\ClaimGuidValidator;
use Wikibase\Summary;
@@ -23,6 +24,7 @@
* @licence GNU GPL v2+
* @author Adam Shorland
* @author H. Snater < [email protected] >
+ * @author Thiemo Mättig
*/
class ChangeOpClaim extends ChangeOpBase {
@@ -74,8 +76,8 @@
SnakValidator $snakValidator,
$index = null
) {
- if( !is_null( $index ) && !is_integer( $index ) ) {
- throw new InvalidArgumentException( '$index needs to be
null or an integer value' );
+ if ( !is_int( $index ) && $index === null ) {
+ throw new InvalidArgumentException( '$index must be an
integer or null' );
}
$this->claim = $claim;
@@ -87,7 +89,13 @@
}
/**
- * @see ChangeOp::apply()
+ * @see ChangeOp::apply
+ *
+ * @param Entity $entity
+ * @param Summary|null $summary
+ *
+ * @throws ChangeOpException
+ * @return bool
*/
public function apply( Entity $entity, Summary $summary = null ) {
if ( $this->claim->getGuid() === null ){
@@ -110,69 +118,54 @@
/**
* @param Entity $entity
* @param Summary|null $summary
+ *
+ * @throws InvalidArgumentException
*/
private function applyClaimToEntity( Entity $entity, Summary $summary =
null ) {
- $statements = $entity->getStatements();
- $claims = new Claims( iterator_to_array( $statements ) );
-
- if( !$claims->hasClaimWithGuid( $this->claim->getGuid() ) ) {
- $newClaims = $this->addClaim( $claims, $summary );
- } else {
- $newClaims = $this->setClaim( $claims, $summary );
+ if ( !( $entity instanceof StatementListProvider ) ) {
+ throw new InvalidArgumentException( '$entity must be a
StatementListProvider' );
}
- $entity->setStatements( new StatementList( $newClaims ) );
+ $statements = $this->removeStatement( $entity, $summary );
+ $statements = $this->addStatement( $statements );
+ $this->setStatements( $entity, $statements );
}
/**
- * @param Claims $claims
- * @param Summary $summary
+ * @param StatementListProvider $entity
+ * @param Summary|null $summary
*
- * @throws ChangeOpException
- * @return Claim[]
+ * @return Statement[]
*/
- protected function addClaim( Claims $claims, Summary $summary = null ) {
- $this->updateSummary( $summary, 'create' );
+ private function removeStatement( StatementListProvider $entity,
Summary $summary = null ) {
+ $guid = $this->claim->getGuid();
+ $statements = array();
+ $oldStatement = null;
+ $index = 0;
- $indexedClaimList = new ByPropertyIdArray( (array)$claims );
- $indexedClaimList->buildIndex();
+ /** @var Statement $statement */
+ foreach ( $entity->getStatements() as $statement ) {
+ if ( $statement->getGuid() === $guid && $oldStatement
=== null ) {
+ $oldStatement = $statement;
- try {
- $indexedClaimList->addObjectAtIndex( $this->claim,
$this->index );
- }
- catch ( OutOfBoundsException $e ) {
- if ( $this->index < 0 ) {
- throw new ChangeOpException( 'Can not add claim
at given index: '. $this->index );
+ if ( $this->index === null ) {
+ $this->index = $index;
+ }
} else {
- // XXX: hack below to retry adding the object
at a new index
- // If we fail with the user supplied index and
the index is greater than 0
- // presume the user wants to have the index at
the end of the list
- $this->addObjectAtEndOfList( $indexedClaimList
);
+ $statements[] = $statement;
}
+
+ $index++;
}
- return $indexedClaimList->toFlatArray();
- }
-
- /**
- * @param Claims $claims
- * @param Summary $summary
- *
- * @return Claim[]
- */
- protected function setClaim( Claims $claims, Summary $summary = null ) {
- $this->updateSummary( $summary, 'update' );
-
- $claimGuid = $this->claim->getGuid();
- $oldClaim = $claims->getClaimWithGuid( $claimGuid );
- $this->checkMainSnakUpdate( $oldClaim );
-
- if ( $this->index === null ) {
- $this->index = $claims->indexOf( $this->claim );
+ if ( $oldStatement !== null ) {
+ $this->checkMainSnakUpdate( $oldStatement );
+ $this->updateSummary( $summary, 'update' );
+ } else {
+ $this->updateSummary( $summary, 'create' );
}
- $claims->removeClaimWithGuid( $claimGuid );
- return $this->addClaim( $claims );
+ return $statements;
}
/**
@@ -181,29 +174,58 @@
* This checks that the main snaks of the old and the new claim
* refer to the same property.
*
- * @param Claim $oldClaim
+ * @param Statement $oldStatement
*
* @throws ChangeOpException If the main snak update is illegal.
*/
- protected function checkMainSnakUpdate( Claim $oldClaim ) {
+ private function checkMainSnakUpdate( Statement $oldStatement ) {
$newMainSnak = $this->claim->getMainSnak();
- $oldPropertyId = $oldClaim->getMainSnak()->getPropertyId();
+ $oldPropertyId = $oldStatement->getMainSnak()->getPropertyId();
if ( !$oldPropertyId->equals( $newMainSnak->getPropertyId() ) )
{
- $claimGuid = $this->claim->getGuid();
- throw new ChangeOpException( "Claim with GUID
$claimGuid uses property "
+ $guid = $this->claim->getGuid();
+ throw new ChangeOpException( "Claim with GUID $guid
uses property "
. $oldPropertyId . ", can't change to "
. $newMainSnak->getPropertyId() );
}
}
/**
- * @see Bug 58394
- * @param ByPropertyIdArray $indexedClaimList
+ * @param Statement[] $statements
+ *
+ * @throws ChangeOpException
+ * @return Statement[]
*/
- private function addObjectAtEndOfList( $indexedClaimList ) {
- $newIndex = $indexedClaimList->count() + 1;
- $indexedClaimList->addObjectAtIndex( $this->claim, $newIndex );
+ private function addStatement( array $statements ) {
+ // If we fail with the user supplied index and the index is
greater than or equal 0
+ // presume the user wants to have the index at the end of the
list.
+ if ( $this->index < 0 ) {
+ throw new ChangeOpException( 'Can not add claim at
given index: '. $this->index );
+ } elseif ( $this->index !== null && $this->index < count(
$statements ) ) {
+ array_splice( $statements, $this->index, 0,
$this->claim );
+ } else {
+ $statements[] = $this->claim;
+ }
+
+ return $statements;
+ }
+
+ /**
+ * @param Entity $entity
+ * @param Statement[] $statements
+ *
+ * @throws InvalidArgumentException
+ */
+ private function setStatements( Entity $entity, array $statements ) {
+ $statementList = new StatementList( $statements );
+
+ if ( $entity instanceof Item ) {
+ $entity->setStatements( $statementList );
+ } elseif ( $entity instanceof Property ) {
+ $entity->setStatements( $statementList );
+ } else {
+ throw new InvalidArgumentException( '$entity must be an
Item or Property' );
+ }
}
/**
diff --git a/repo/includes/ChangeOp/ChangeOpClaimRemove.php
b/repo/includes/ChangeOp/ChangeOpClaimRemove.php
index 7bff58f..cb68c8f 100644
--- a/repo/includes/ChangeOp/ChangeOpClaimRemove.php
+++ b/repo/includes/ChangeOp/ChangeOpClaimRemove.php
@@ -4,35 +4,35 @@
use InvalidArgumentException;
use ValueValidators\Result;
-use Wikibase\DataModel\Claim\Claims;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Snak\Snak;
+use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\DataModel\StatementListProvider;
use Wikibase\Summary;
/**
- * Class for claim remove operation
+ * Class for statement remove operation.
*
* @since 0.5
* @licence GNU GPL v2+
* @author Adam Shorland
+ * @author Thiemo Mättig
*/
class ChangeOpClaimRemove extends ChangeOpBase {
/**
- * @since 0.5
- *
* @var string
*/
- protected $claimGuid;
+ private $guid;
/**
* @return string
*/
public function getClaimGuid() {
- return $this->claimGuid;
+ return $this->guid;
}
/**
@@ -40,62 +40,92 @@
*
* @since 0.5
*
- * @param string $claimGuid
+ * @param string $guid
*
* @throws InvalidArgumentException
*/
- public function __construct( $claimGuid ) {
- if ( !is_string( $claimGuid ) || $claimGuid === '' ) {
- throw new InvalidArgumentException( '$claimGuid needs
to be a string and must not be empty' );
+ public function __construct( $guid ) {
+ if ( !is_string( $guid ) || $guid === '' ) {
+ throw new InvalidArgumentException( '$guid must be a
non-empty string' );
}
- $this->claimGuid = $claimGuid;
+ $this->guid = $guid;
}
/**
- * @see ChangeOp::apply()
+ * @see ChangeOp::apply
+ *
+ * @param Entity $entity
+ * @param Summary|null $summary
+ *
+ * @throws InvalidArgumentException
+ * @return bool
*/
public function apply( Entity $entity, Summary $summary = null ) {
- if ( $entity instanceof Item || $entity instanceof Property ) {
- $claims = new Claims( $entity->getStatements() );
- $this->removeClaim( $claims, $summary );
- $entity->setStatements( new StatementList( $claims ) );
+ if ( !( $entity instanceof StatementListProvider ) ) {
+ throw new InvalidArgumentException( '$entity must be a
StatementListProvider' );
}
- else {
- throw new InvalidArgumentException( 'This code only
works with items and properties' );
- }
+
+ $statements = $this->removeStatement( $entity, $summary );
+ $this->setStatements( $entity, $statements );
return true;
}
/**
- * @since 0.4
- *
- * @param Claims $claims
- * @param Summary $summary
+ * @param StatementListProvider $entity
+ * @param Summary|null $summary
*
* @throws ChangeOpException
+ * @return Statement[]
*/
- protected function removeClaim( Claims $claims, Summary $summary = null
) {
- $claim = $claims->getClaimWithGuid( $this->claimGuid );
+ private function removeStatement( StatementListProvider $entity,
Summary $summary = null ) {
+ $statements = array();
+ $removedStatement = null;
- if ( $claim === null ) {
- throw new ChangeOpException( "Entity does not have
claim with GUID $this->claimGuid" );
+ /** @var Statement $statement */
+ foreach ( $entity->getStatements() as $statement ) {
+ if ( $statement->getGuid() === $this->guid &&
$removedStatement === null ) {
+ $removedStatement = $statement;
+ } else {
+ $statements[] = $statement;
+ }
}
- $removedSnak = $claim->getMainSnak();
- $claims->removeClaimWithGuid( $this->claimGuid );
- $this->updateSummary( $summary, 'remove', '',
$this->getClaimSummaryArgs( $removedSnak ) );
+ if ( $removedStatement === null ) {
+ throw new ChangeOpException( "Entity does not have
statement with GUID $this->guid" );
+ }
+
+ $removedSnak = $removedStatement->getMainSnak();
+ $this->updateSummary( $summary, 'remove', '',
$this->getSummaryArgs( $removedSnak ) );
+
+ return $statements;
}
/**
- * @since 0.4
+ * @param Entity $entity
+ * @param Statement[] $statements
*
+ * @throws InvalidArgumentException
+ */
+ private function setStatements( Entity $entity, array $statements ) {
+ $statementList = new StatementList( $statements );
+
+ if ( $entity instanceof Item ) {
+ $entity->setStatements( $statementList );
+ } elseif ( $entity instanceof Property ) {
+ $entity->setStatements( $statementList );
+ } else {
+ throw new InvalidArgumentException( '$entity must be an
Item or Property' );
+ }
+ }
+
+ /**
* @param Snak $mainSnak
*
* @return array
*/
- protected function getClaimSummaryArgs( Snak $mainSnak ) {
+ private function getSummaryArgs( Snak $mainSnak ) {
$propertyId = $mainSnak->getPropertyId();
return array( array( $propertyId->getSerialization() =>
$mainSnak ) );
}
@@ -116,4 +146,4 @@
return parent::validate( $entity );
}
-}
\ No newline at end of file
+}
diff --git a/repo/includes/ChangeOp/ClaimChangeOpFactory.php
b/repo/includes/ChangeOp/ClaimChangeOpFactory.php
index b6c58d0..026de21 100644
--- a/repo/includes/ChangeOp/ClaimChangeOpFactory.php
+++ b/repo/includes/ChangeOp/ClaimChangeOpFactory.php
@@ -6,6 +6,7 @@
use Wikibase\DataModel\Claim\Claim;
use Wikibase\DataModel\Claim\ClaimGuidParser;
use Wikibase\DataModel\Snak\Snak;
+use Wikibase\DataModel\Statement\Statement;
use Wikibase\Lib\ClaimGuidGenerator;
use Wikibase\Lib\ClaimGuidValidator;
use Wikibase\Validators\SnakValidator;
@@ -58,15 +59,15 @@
}
/**
- * @param Claim $claim
+ * @param Statement $statement
* @param int|null $index
*
* @throws InvalidArgumentException
* @return ChangeOp
*/
- public function newAddClaimOp( Claim $claim, $index = null ) {
+ public function newAddClaimOp( Statement $statement, $index = null ) {
return new ChangeOpClaim(
- $claim,
+ $statement,
$this->guidGenerator,
$this->guidValidator,
$this->guidParser,
diff --git a/repo/tests/phpunit/includes/ChangeOp/ClaimChangeOpFactoryTest.php
b/repo/tests/phpunit/includes/ChangeOp/ClaimChangeOpFactoryTest.php
index cac049e..600229a 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ClaimChangeOpFactoryTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ClaimChangeOpFactoryTest.php
@@ -6,6 +6,7 @@
use Wikibase\DataModel\Claim\Claim;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
+use Wikibase\DataModel\Statement\Statement;
/**
* @covers Wikibase\ChangeOp\ClaimChangeOpFactory
@@ -36,9 +37,9 @@
public function testNewAddClaimOp() {
$snak = new PropertyNoValueSnak( new PropertyId( 'P7' ) );
- $claim = new Claim( $snak );
+ $statement = new Statement( new Claim( $snak ) );
- $op = $this->newChangeOpFactory()->newAddClaimOp( $claim );
+ $op = $this->newChangeOpFactory()->newAddClaimOp( $statement );
$this->assertInstanceOf( 'Wikibase\ChangeOp\ChangeOp', $op );
}
--
To view, visit https://gerrit.wikimedia.org/r/204511
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id8361ad7f17536365610157d27504d1e0b8d718e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
