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

Reply via email to