Ignore empty references when added to ReferenceList

Author: JeroenDeDauw

RELEASE-NOTES.md

@@ -12,7 +12,9 @@
 
 ## Version 2.6.0 (dev)
 
+* Added `Reference::isEmpty`
 * Empty strings are now detected as invalid in the `SiteLink` constructor
+* Empty References are now ignored when added to `ReferenceList`
 * The `ReferenceList` constructor now throws an `InvalidArgumentException` when getting a non-iterable input
 * The `SnakList` constructor now throws an `InvalidArgumentException` when getting a non-iterable input
 * The `AliasGroup::equals` and `Term::equals` methods no longer incorrectly return true for fallback objects

src/Reference.php

@@ -67,6 +67,15 @@ public function count() {
 		return count( $this->snaks );
 	}
 
+	/**
+	 * @since 2.6
+	 *
+	 * @return bool
+	 */
+	public function isEmpty() {
+		return $this->snaks->isEmpty();
+	}
+
 	/**
 	 * @see Hashable::getHash
 	 *

src/ReferenceList.php

@@ -70,6 +70,17 @@ public function addReference( Reference $reference, $index = null ) {
 		}
 	}
 
+	/**
+	 * @see SplObjectStorage::attach
+	 * @param Reference $reference
+	 * @param mixed $data
+	 */
+	public function attach( $reference, $data = null ) {
+		if ( !$reference->isEmpty() ) {
+			parent::attach( $reference, $data );
+		}
+	}
+
 	// @codingStandardsIgnoreStart
 	/**
 	 * @since 1.1

tests/unit/ReferenceListTest.php

@@ -8,6 +8,7 @@
 use Wikibase\DataModel\Reference;
 use Wikibase\DataModel\ReferenceList;
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
+use Wikibase\DataModel\Snak\PropertyValueSnak;
 use Wikibase\DataModel\Snak\SnakList;
 
 /**
@@ -141,41 +142,73 @@ public function testRemoveReference( ReferenceList $array ) {
 		$this->assertTrue( true );
 	}
 
-	/**
-	 * @dataProvider instanceProvider
-	 *
-	 * @param ReferenceList $array
-	 */
-	public function testAddReference( ReferenceList $array ) {
-		// Append object to the end:
-		$elementCount = count( $array );
+	public function testAddReferenceOnEmptyList() {
+		$reference = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
 
-		$elements = $this->getElementInstances();
-		$element = array_shift( $elements );
-		$array->addReference( $element );
+		$references = new ReferenceList();
+		$references->addReference( $reference );
 
-		$this->assertEquals( ++$elementCount, count( $array ) );
+		$this->assertCount( 1, $references );
 
-		// Insert object at the beginning:
-		$elements = $this->getElementInstances();
-		$element = array_shift( $elements );
-		$array->addReference( $element, 0 );
+		$expectedList = new ReferenceList( array( $reference ) );
+		$this->assertSameReferenceOrder( $expectedList, $references );
+	}
+
+	private function assertSameReferenceOrder( ReferenceList $expectedList, ReferenceList $references ) {
+		$this->assertEquals(
+			iterator_to_array( $expectedList ),
+			iterator_to_array( $references )
+		);
+	}
 
-		$array->rewind();
+	public function testAddReferenceOnNonEmptyList() {
+		$reference1 = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$reference2 = new Reference( array( new PropertyNoValueSnak( 2 ) ) );
+		$reference3 = new Reference( array( new PropertyNoValueSnak( 3 ) ) );
 
-		$this->assertEquals( ++$elementCount, count( $array ) );
-		$this->assertEquals( $array->current(), $element, 'Inserted object at the beginning' );
+		$references = new ReferenceList( array( $reference1, $reference2 ) );
+		$references->addReference( $reference3 );
 
-		// Insert object at another index:
-		$elements = $this->getElementInstances();
-		$element = array_shift( $elements );
-		$array->addReference( $element, 1 );
+		$this->assertCount( 3, $references );
+
+		$expectedList = new ReferenceList( array( $reference1, $reference2, $reference3 ) );
+		$this->assertSameReferenceOrder( $expectedList, $references );
+	}
+
+	public function testAddReferenceAtIndexZero() {
+		$reference1 = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$reference2 = new Reference( array( new PropertyNoValueSnak( 2 ) ) );
+		$reference3 = new Reference( array( new PropertyNoValueSnak( 3 ) ) );
+
+		$references = new ReferenceList( array( $reference1, $reference2 ) );
+		$references->addReference( $reference3, 0 );
+
+		$expectedList = new ReferenceList( array( $reference3, $reference1, $reference2 ) );
+		$this->assertSameReferenceOrder( $expectedList, $references );
+	}
+
+	public function testGivenEmptyReference_addReferenceDoesNotAdd() {
+		$reference1 = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$reference2 = new Reference( array( new PropertyNoValueSnak( 2 ) ) );
+		$emptyReference = new Reference( array() );
+
+		$references = new ReferenceList( array( $reference1, $reference2 ) );
+		$references->addReference( $emptyReference );
+
+		$expectedList = new ReferenceList( array( $reference1, $reference2 ) );
+		$this->assertSameReferenceOrder( $expectedList, $references );
+	}
+
+	public function testGivenEmptyReferenceAndIndex_addReferenceDoesNotAdd() {
+		$reference1 = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$reference2 = new Reference( array( new PropertyNoValueSnak( 2 ) ) );
+		$emptyReference = new Reference( array() );
 
-		$array->rewind();
-		$array->next();
+		$references = new ReferenceList( array( $reference1, $reference2 ) );
+		$references->addReference( $emptyReference, 0 );
 
-		$this->assertEquals( ++$elementCount, count( $array ) );
-		$this->assertEquals( $array->current(), $element, 'Inserted object at index 1' );
+		$expectedList = new ReferenceList( array( $reference1, $reference2 ) );
+		$this->assertSameReferenceOrder( $expectedList, $references );
 	}
 
 	/**

tests/unit/ReferenceTest.php

@@ -280,4 +280,18 @@ public function invalidConstructorArgumentsProvider() {
 		);
 	}
 
+	public function testIsEmpty() {
+		$emptyReference = new Reference();
+		$this->assertTrue( $emptyReference->isEmpty() );
+
+		$referenceWithSnak = new Reference( array( new PropertyNoValueSnak( 1 ) ) );
+		$this->assertFalse( $referenceWithSnak->isEmpty() );
+
+		$referenceWithSnaks = new Reference( array(
+			new PropertyNoValueSnak( 1 ),
+			new PropertyNoValueSnak( 2 )
+		) );
+		$this->assertFalse( $referenceWithSnaks->isEmpty() );
+	}
+
 }