Merge pull request #356 from wmde/hashArray

Fix HashArray accepting anything

Author: JeroenDeDauw

src/HashArray.php

@@ -70,11 +70,17 @@ abstract public function getObjectType();
 	 * @param array|Traversable|null $input
 	 * @param int $flags
 	 * @param string $iterator_class
+	 *
+	 * @throws InvalidArgumentException
 	 */
 	public function __construct( $input = null, $flags = 0, $iterator_class = 'ArrayIterator' ) {
 		parent::__construct( array(), $flags, $iterator_class );
 
 		if ( $input !== null ) {
+			if ( !is_array( $input ) && !( $input instanceof Traversable ) ) {
+				throw new InvalidArgumentException( '$input must be an array or Traversable' );
+			}
+
 			foreach ( $input as $offset => $value ) {
 				$this->offsetSet( $offset, $value );
 			}

tests/unit/Snak/SnakListTest.php

@@ -3,6 +3,7 @@
 namespace Wikibase\DataModel\Tests\Snak;
 
 use DataValues\StringValue;
+use InvalidArgumentException;
 use Wikibase\DataModel\Entity\PropertyId;
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
 use Wikibase\DataModel\Snak\PropertyValueSnak;
@@ -26,6 +27,7 @@
  * @licence GNU GPL v2+
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  * @author Adam Shorland
+ * @author Thiemo Mättig
  */
 class SnakListTest extends HashArrayTest {
 
@@ -70,6 +72,29 @@ public function constructorProvider() {
 		);
 	}
 
+	/**
+	 * @dataProvider invalidConstructorArgumentsProvider
+	 * @param mixed $input
+	 * @expectedException InvalidArgumentException
+	 */
+	public function testGivenInvalidConstructorArguments_constructorThrowsException( $input ) {
+		new SnakList( $input );
+	}
+
+	public function invalidConstructorArgumentsProvider() {
+		$id1 = new PropertyId( 'P1' );
+
+		return array(
+			array( false ),
+			array( 1 ),
+			array( 0.1 ),
+			array( 'string' ),
+			array( $id1 ),
+			array( new PropertyNoValueSnak( $id1 ) ),
+			array( new PropertyValueSnak( $id1, new StringValue( 'a' ) ) ),
+		);
+	}
+
 	/**
 	 * @dataProvider instanceProvider
 	 * @param SnakList $array
@@ -223,11 +248,12 @@ public function orderByPropertyProvider() {
 
 		$arguments = array();
 
-		foreach( $rawArguments as $rawArgument ) {
-			if( !array_key_exists( 2, $rawArgument ) ){
-				$rawArgument[2] = array();
-			}
-			$arguments[] = array( new $class( $rawArgument[0] ), new $class( $rawArgument[1] ), $rawArgument[2] );
+		foreach ( $rawArguments as $rawArgument ) {
+			$arguments[] = array(
+				new $class( $rawArgument[0] ),
+				new $class( $rawArgument[1] ),
+				array_key_exists( 2, $rawArgument ) ? $rawArgument[2] : array()
+			);
 		}
 
 		return $arguments;
@@ -237,7 +263,7 @@ public function orderByPropertyProvider() {
 	 * @dataProvider orderByPropertyProvider
 	 * @param SnakList $snakList
 	 * @param SnakList $expected
-	 * @param array $order
+	 * @param string[] $order
 	 */
 	public function testOrderByProperty( SnakList $snakList, SnakList $expected, $order = array() ) {
 		$initialSnakList = new SnakList( array_values( iterator_to_array( $snakList ) ) );
@@ -250,7 +276,7 @@ public function testOrderByProperty( SnakList $snakList, SnakList $expected, $or
 
 		$this->assertEquals( $expected, $orderedSnakList );
 
-		if( $orderedSnakList->equals( $initialSnakList ) ) {
+		if ( $orderedSnakList->equals( $initialSnakList ) ) {
 			$this->assertTrue( $initialSnakList->getHash() === $snakList->getHash() );
 		} else {
 			$this->assertFalse( $initialSnakList->getHash() === $snakList->getHash() );

tests/unit/Statement/StatementTest.php

@@ -117,7 +117,7 @@ public function instanceProvider() {
 		$instance = clone $baseInstance;
 
 		$instance->setReferences( new ReferenceList( array(
-			new Reference( new SnakList(
+			new Reference( array(
 				new PropertyValueSnak( new PropertyId( 'P1' ), new StringValue( 'a' ) )
 			) )
 		) ) );
@@ -145,15 +145,14 @@ public function testGetReferences( Statement $statement ) {
 	 */
 	public function testSetReferences( Statement $statement ) {
 		$references = new ReferenceList( array(
-			new Reference( new SnakList(
+			new Reference( array(
 				new PropertyValueSnak(
 					new PropertyId( 'P1' ),
 					new StringValue( 'a' )
 				)
 			) )
 		) );
 
-
 		$statement->setReferences( $references );
 
 		$this->assertEquals( $references, $statement->getReferences() );