jenkins-bot has submitted this change and it was merged.
Change subject: Implemented index in ChangeOpReference
......................................................................
Implemented index in ChangeOpReference
(requires change I28610f2f052ff1e56630673438913832d8920b9c)
ChangeOpReference supports specifying the index of a Reference within the list
of References.
Change-Id: I0941d1227c873a0d84a1d486d036644874ec1fb1
---
M repo/includes/ChangeOp/ChangeOpReference.php
M repo/includes/api/SetReference.php
M repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
3 files changed, 118 insertions(+), 11 deletions(-)
Approvals:
Addshore: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/ChangeOp/ChangeOpReference.php
b/repo/includes/ChangeOp/ChangeOpReference.php
index b0aa36a..b77b265 100644
--- a/repo/includes/ChangeOp/ChangeOpReference.php
+++ b/repo/includes/ChangeOp/ChangeOpReference.php
@@ -42,6 +42,13 @@
protected $referenceHash;
/**
+ * @since 0.5
+ *
+ * @var int|null
+ */
+ protected $index;
+
+ /**
* Constructs a new reference change operation
*
* @since 0.4
@@ -49,11 +56,11 @@
* @param string $claimGuid
* @param Reference|null $reference
* @param string $referenceHash
- * @param EntityIdFormatter $idFormatter
+ * @param int|null $index
*
* @throws \InvalidArgumentException
*/
- public function __construct( $claimGuid, $reference, $referenceHash ) {
+ public function __construct( $claimGuid, $reference, $referenceHash,
$index = null ) {
if ( !is_string( $claimGuid ) || $claimGuid === '' ) {
throw new InvalidArgumentException( '$claimGuid needs
to be a string and must not be empty' );
}
@@ -70,9 +77,14 @@
throw new InvalidArgumentException( 'Either
$referenceHash or $reference needs to be set' );
}
+ if( !is_null( $index ) && !is_integer( $index ) ) {
+ throw new InvalidArgumentException( '$index needs to be
null or an integer value' );
+ }
+
$this->claimGuid = $claimGuid;
$this->reference = $reference;
$this->referenceHash = $referenceHash;
+ $this->index = $index;
}
/**
@@ -129,7 +141,7 @@
$hash = $this->reference->getHash();
throw new ChangeOpException( "Claim has already a
reference with hash $hash" );
}
- $references->addReference( $this->reference );
+ $references->addReference( $this->reference, $this->index );
$this->updateSummary( $summary, 'add' );
}
@@ -145,11 +157,21 @@
if ( !$references->hasReferenceHash( $this->referenceHash ) ) {
throw new ChangeOpException( "Reference with hash
$this->referenceHash does not exist" );
}
- if ( $references->hasReference( $this->reference ) ) {
- throw new ChangeOpException( "Claim has already a
reference with hash {$this->reference->getHash()}" );
+
+ $currentIndex = $references->indexOf( $this->reference );
+
+ if( is_null( $this->index ) && $currentIndex !== false ) {
+ // Set index to current index to not have the reference
removed and appended but
+ // retain its position within the list of references.
+ $this->index = $currentIndex;
+ }
+
+ if ( $references->hasReference( $this->reference ) &&
$this->index === $currentIndex ) {
+ throw new ChangeOpException( "Claim has already a
reference with hash "
+ . "{$this->reference->getHash()} and index
($currentIndex) is not changed" );
}
$references->removeReferenceHash( $this->referenceHash );
- $references->addReference( $this->reference );
+ $references->addReference( $this->reference, $this->index );
$this->updateSummary( $summary, 'set' );
}
diff --git a/repo/includes/api/SetReference.php
b/repo/includes/api/SetReference.php
index 7e0f77c..33c7c18 100644
--- a/repo/includes/api/SetReference.php
+++ b/repo/includes/api/SetReference.php
@@ -189,12 +189,11 @@
$params = $this->extractRequestParams();
$claimGuid = $params['statement'];
- $idFormatter =
WikibaseRepo::getDefaultInstance()->getIdFormatter();
if ( isset( $params['reference'] ) ) {
- $changeOp = new ChangeOpReference( $claimGuid,
$reference, $params['reference'], $idFormatter );
+ $changeOp = new ChangeOpReference( $claimGuid,
$reference, $params['reference'] );
} else {
- $changeOp = new ChangeOpReference( $claimGuid,
$reference, '', $idFormatter );
+ $changeOp = new ChangeOpReference( $claimGuid,
$reference, '' );
}
return $changeOp;
diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
b/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
index a43ad8a..d364530 100644
--- a/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
+++ b/repo/tests/phpunit/includes/ChangeOp/ChangeOpReferenceTest.php
@@ -2,12 +2,14 @@
namespace Wikibase\Test;
+use LogicException;
use Wikibase\Claims;
use Wikibase\ChangeOp\ChangeOpReference;
use Wikibase\Entity;
use Wikibase\ItemContent;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Lib\ClaimGuidGenerator;
+use Wikibase\PropertyNoValueSnak;
use Wikibase\Reference;
use Wikibase\SnakList;
use InvalidArgumentException;
@@ -43,6 +45,7 @@
$args[] = array( $validClaimGuid, 'notAReference',
$validReferenceHash );
$args[] = array( $validClaimGuid, 'notAReference', '' );
$args[] = array( $validClaimGuid, null, '' );
+ $args[] = array( $validClaimGuid, $validReference,
$validReferenceHash, 'string' );
return $args;
}
@@ -52,8 +55,8 @@
*
* @expectedException InvalidArgumentException
*/
- public function testInvalidConstruct( $claimGuid, $reference,
$referenceHash ) {
- $ChangeOpQualifier = new ChangeOpReference( $claimGuid,
$reference, $referenceHash );
+ public function testInvalidConstruct( $claimGuid, $reference,
$referenceHash, $index = null ) {
+ new ChangeOpReference( $claimGuid, $reference, $referenceHash,
$index );
}
public function changeOpAddProvider() {
@@ -87,6 +90,65 @@
$claim = reset( $claims );
$references = $claim->getReferences();
$this->assertTrue( $references->hasReferenceHash(
$referenceHash ), "No reference with expected hash" );
+ }
+
+ public function changeOpAddProviderWithIndex() {
+ $snak = new PropertyNoValueSnak( 1 );
+ $args = array();
+
+ $item = $this->provideNewItemWithClaim( 'q123', $snak );
+ $claims = $item->getClaims();
+ $claim = reset( $claims );
+
+ $references = array(
+ new Reference( new SnakList( array( new
PropertyNoValueSnak( 1 ) ) ) ),
+ new Reference( new SnakList( array( new
PropertyNoValueSnak( 2 ) ) ) ),
+ );
+
+ $referenceList = $claim->getReferences();
+ $referenceList->addReference( $references[0] );
+ $referenceList->addReference( $references[1] );
+
+ $item->setClaims( new Claims( $claims ) );
+
+ $claimGuid = $claim->getGuid();
+
+ $newReference = new Reference( new SnakList( array( new
PropertyNoValueSnak( 3 ) ) ) );
+ $newReferenceIndex = 1;
+
+ $changeOp = new ChangeOpReference(
+ $claimGuid,
+ $newReference,
+ '',
+ $newReferenceIndex
+ );
+
+ $args[] = array ( $item, $changeOp, $newReference,
$newReferenceIndex );
+
+ return $args;
+ }
+
+ /**
+ * @dataProvider changeOpAddProviderWithIndex
+ *
+ * @param Entity $item
+ * @param ChangeOpReference $changeOp
+ * @param Reference $newReference
+ * @param int $expectedIndex
+ *
+ * @throws \LogicException
+ */
+ public function testApplyAddNewReferenceWithIndex(
+ $item,
+ $changeOp,
+ $newReference,
+ $expectedIndex
+ ) {
+ $this->assertTrue( $changeOp->apply( $item ), 'Applying the
ChangeOp did not return true' );
+ $claims = new Claims( $item->getClaims() );
+ $claim = reset( $claims );
+ $references = $claim->getReferences();
+ $this->assertEquals( $expectedIndex, $references->indexOf(
$newReference ) );
}
public function changeOpRemoveProvider() {
@@ -148,6 +210,30 @@
$changeOp = new ChangeOpReference( $claimGuid,
$changedReference, $referenceHash );
$args[] = array ( $item, $changeOp,
$changedReference->getHash() );
+ // Just change a reference's index:
+ $item = $this->provideNewItemWithClaim( 'q123', $snak );
+ $claims = $item->getClaims();
+ $claim = reset( $claims );
+
+ $references = array(
+ new Reference( new SnakList( array( new
PropertyNoValueSnak( 1 ) ) ) ),
+ new Reference( new SnakList( array( new
PropertyNoValueSnak( 2 ) ) ) ),
+ );
+
+ $referenceList = $claim->getReferences();
+ $referenceList->addReference( $references[0] );
+ $referenceList->addReference( $references[1] );
+
+ $claim->setReferences( $referenceList );
+ $item->setClaims( new Claims( $claims ) );
+ $changeOp = new ChangeOpReference(
+ $claim->getGuid(),
+ $references[1],
+ $references[1]->getHash(),
+ 0
+ );
+ $args[] = array ( $item, $changeOp, $references[1]->getHash() );
+
return $args;
}
--
To view, visit https://gerrit.wikimedia.org/r/88997
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0941d1227c873a0d84a1d486d036644874ec1fb1
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Henning Snater <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits