Merge pull request #639 from wmde/addDuplicates

Fix regression when adding the same reference object twice

Author: adrianheine

src/ReferenceList.php

@@ -28,7 +28,7 @@
 class ReferenceList implements Comparable, Countable, IteratorAggregate, Serializable {
 
 	/**
-	 * @var Reference[]
+	 * @var Reference[] Ordered list or references, indexed by SPL object hash.
 	 */
 	private $references = array();
 
@@ -73,7 +73,7 @@ public function addReference( Reference $reference, $index = null ) {
 
 		if ( $index === null || $index >= count( $this->references ) ) {
 			// Append object to the end of the reference list.
-			$this->references[] = $reference;
+			$this->references[spl_object_hash( $reference )] = $reference;
 		} else {
 			$this->insertReferenceAtIndex( $reference, $index );
 		}
@@ -100,7 +100,11 @@ public function addNewReference( $snaks = array() /*...*/ ) {
 	 * @param int $index
 	 */
 	private function insertReferenceAtIndex( Reference $reference, $index ) {
-		array_splice( $this->references, $index, 0, array( $reference ) );
+		$this->references = array_merge(
+			array_slice( $this->references, 0, $index ),
+			array( spl_object_hash( $reference ) => $reference ),
+			array_slice( $this->references, $index )
+		);
 	}
 
 	/**
@@ -126,10 +130,14 @@ public function hasReference( Reference $reference ) {
 	 * @return int|bool
 	 */
 	public function indexOf( Reference $reference ) {
-		foreach ( $this->references as $index => $ref ) {
+		$index = 0;
+
+		foreach ( $this->references as $ref ) {
 			if ( $ref === $reference ) {
 				return $index;
 			}
+
+			$index++;
 		}
 
 		return false;
@@ -174,13 +182,11 @@ public function removeReferenceHash( $referenceHash ) {
 			return;
 		}
 
-		foreach ( $this->references as $index => $ref ) {
+		foreach ( $this->references as $splObjectHash => $ref ) {
 			if ( $ref === $reference ) {
-				unset( $this->references[$index] );
+				unset( $this->references[$splObjectHash] );
 			}
 		}
-
-		$this->references = array_values( $this->references );
 	}
 
 	/**
@@ -211,7 +217,7 @@ public function getReference( $referenceHash ) {
 	 * @return string
 	 */
 	public function serialize() {
-		return serialize( $this->references );
+		return serialize( array_values( $this->references ) );
 	}
 
 	/**
@@ -222,7 +228,7 @@ public function serialize() {
 	 * @param string $serialized
 	 */
 	public function unserialize( $serialized ) {
-		$this->references = unserialize( $serialized );
+		$this->__construct( unserialize( $serialized ) );
 	}
 
 	/**
@@ -283,7 +289,7 @@ public function count() {
 	 * @return Traversable
 	 */
 	public function getIterator() {
-		return new ArrayIterator( $this->references );
+		return new ArrayIterator( array_values( $this->references ) );
 	}
 
 }

tests/unit/ReferenceListTest.php

@@ -44,6 +44,12 @@ public function testCanConstructWithReferenceListObject() {
 		$this->assertNotNull( $copy->getReference( $reference->getHash() ) );
 	}
 
+	public function testConstructorIgnoresIdenticalObjects() {
+		$reference = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$list = new ReferenceList( array( $reference, $reference ) );
+		$this->assertCount( 1, $list );
+	}
+
 	public function testConstructorDoesNotIgnoreCopies() {
 		$reference = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
 		$list = new ReferenceList( array( $reference, clone $reference ) );
@@ -194,6 +200,14 @@ public function testAddReferenceOnNonEmptyList() {
 		$this->assertSameReferenceOrder( $expectedList, $references );
 	}
 
+	public function testAddReferenceIgnoresIdenticalObjects() {
+		$list = new ReferenceList();
+		$reference = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$list->addReference( $reference );
+		$list->addReference( $reference );
+		$this->assertCount( 1, $list );
+	}
+
 	public function testAddReferenceDoesNotIgnoreCopies() {
 		$list = new ReferenceList();
 		$reference = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
@@ -202,6 +216,14 @@ public function testAddReferenceDoesNotIgnoreCopies() {
 		$this->assertCount( 2, $list );
 	}
 
+	public function testAddReferenceAtIndexIgnoresIdenticalObjects() {
+		$list = new ReferenceList();
+		$reference = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$list->addReference( $reference, 0 );
+		$list->addReference( $reference, 0 );
+		$this->assertCount( 1, $list );
+	}
+
 	public function testAddReferenceAtIndexZero() {
 		$reference1 = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
 		$reference2 = new Reference( array( new PropertyNoValueSnak( 2 ) ) );