Merge pull request #509 from wmde/snakOrder

Fix SnakList::orderByProperty
wmde · Jul 1, 2015 · 9494275 · 9494275
2 parents bbed7bb + 16b3cf7
commit 9494275
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 18 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -1,5 +1,8 @@
 # Wikibase DataModel release notes
 
+## Version 3.0.1 (2015-07-01)
+* Fixed out of bounds bug in `SnakList::orderByProperty`
+
 ## Version 3.0.0 (2015-06-06)
 
 #### Breaking changes

diff --git a/WikibaseDataModel.php b/WikibaseDataModel.php
@@ -12,7 +12,7 @@
 	return 1;
 }
 
-define( 'WIKIBASE_DATAMODEL_VERSION', '3.0.0' );
+define( 'WIKIBASE_DATAMODEL_VERSION', '3.0.1' );
 
 if ( defined( 'MEDIAWIKI' ) ) {
 	call_user_func( function() {

diff --git a/src/Snak/SnakList.php b/src/Snak/SnakList.php
@@ -126,16 +126,18 @@ public function orderByProperty( $order = array() ) {
 		$orderedProperties = array_unique( array_merge( $order, array_keys( $snaksByProperty ) ) );
 
 		foreach ( $orderedProperties as $property ) {
-			$snaks = $snaksByProperty[$property];
-			$this->moveSnaksToBottom( $snaks );
+			if ( array_key_exists( $property, $snaksByProperty ) ) {
+				$snaks = $snaksByProperty[$property];
+				$this->moveSnaksToBottom( $snaks );
+			}
 		}
 	}
 
 	/**
-	 * @param array $snaks to remove and re add
+	 * @param Snak[] $snaks to remove and re add
 	 */
-	private function moveSnaksToBottom( $snaks ) {
-		foreach( $snaks as $snak ) {
+	private function moveSnaksToBottom( array $snaks ) {
+		foreach ( $snaks as $snak ) {
 			$this->removeSnak( $snak );
 			$this->addSnak( $snak );
 		}
@@ -145,15 +147,15 @@ private function moveSnaksToBottom( $snaks ) {
 	 * Gets the snaks in the current object in an array
 	 * grouped by property id
 	 *
-	 * @return array
+	 * @return array[]
 	 */
 	private function getSnaksByProperty() {
 		$snaksByProperty = array();
 
-		foreach( $this as $snak ) {
+		foreach ( $this as $snak ) {
 			/** @var Snak $snak */
 			$propertyId = $snak->getPropertyId()->getSerialization();
-			if( !isset( $snaksByProperty[$propertyId] ) ) {
+			if ( !isset( $snaksByProperty[$propertyId] ) ) {
 				$snaksByProperty[$propertyId] = array();
 			}
 			$snaksByProperty[$propertyId][] = $snak;

diff --git a/tests/unit/Snak/SnakListTest.php b/tests/unit/Snak/SnakListTest.php
@@ -183,8 +183,15 @@ public function orderByPropertyProvider() {
 		 * @var array
 		 */
 		$rawArguments = array(
-			//default order
-			array( array(), array() ),
+			'Default order' => array(
+				array(),
+				array(),
+			),
+			'Unknown id in order' => array(
+				array(),
+				array(),
+				array( 'P1' )
+			),
 			array(
 				array( new PropertyNoValueSnak( $id1 ) ),
 				array( new PropertyNoValueSnak( $id1 ) ),
@@ -211,8 +218,7 @@ public function orderByPropertyProvider() {
 					new PropertyNoValueSnak( $id2 ),
 				),
 			),
-			//with additional order
-			array(
+			'With additional order' => array(
 				array(
 					new PropertyNoValueSnak( $id3 ),
 					new PropertyNoValueSnak( $id2 ),
@@ -223,7 +229,7 @@ public function orderByPropertyProvider() {
 					new PropertyNoValueSnak( $id3 ),
 					new PropertyValueSnak( $id1, new StringValue( 'a' ) ),
 				),
-				array( $id2->getSerialization() )
+				array( 'P2' )
 			),
 			array(
 				array(
@@ -234,21 +240,20 @@ public function orderByPropertyProvider() {
 					new PropertyNoValueSnak( $id1 ),
 				),
 				array(
-
 					new PropertyValueSnak( $id1, new StringValue( 'a' ) ),
 					new PropertyNoValueSnak( $id1 ),
 					new PropertyNoValueSnak( $id3 ),
 					new PropertyNoValueSnak( $id2 ),
 					new PropertyNoValueSnak( $id2 ),
 				),
-				array( $id1->getSerialization() )
+				array( 'P1' )
 			),
 		);
 
 		$arguments = array();
 
-		foreach ( $rawArguments as $rawArgument ) {
-			$arguments[] = array(
+		foreach ( $rawArguments as $key => $rawArgument ) {
+			$arguments[$key] = array(
 				new $class( $rawArgument[0] ),
 				new $class( $rawArgument[1] ),
 				array_key_exists( 2, $rawArgument ) ? $rawArgument[2] : array()