Merge pull request #671 from wmde/entityIdJson2

Serialize EntityIdValue with "id" without newFromArray support

Author: Daniel Kinzler

src/Entity/EntityIdValue.php

@@ -12,6 +12,7 @@
  *
  * @license GPL-2.0+
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ * @author Thiemo Mättig
  */
 class EntityIdValue extends DataValueObject {
 
@@ -43,7 +44,7 @@ public function serialize() {
 	 *
 	 * @return float Numeric id as a whole number. Can not be int because of 32-bit PHP.
 	 */
-	protected function getNumericId() {
+	private function getNumericId() {
 		return floatval( substr( $this->entityId->getSerialization(), 1 ) );
 	}
 
@@ -121,6 +122,7 @@ public function getArrayValue() {
 		return array(
 			'entity-type' => $this->entityId->getEntityType(),
 			'numeric-id' => $this->getNumericId(),
+			'id' => $this->entityId->getSerialization(),
 		);
 	}
 
@@ -138,27 +140,33 @@ public function getArrayValue() {
 	 */
 	public static function newFromArray( $data ) {
 		if ( !is_array( $data ) ) {
-			throw new IllegalValueException( '$data must be an array; got ' . gettype( $data ) );
+			throw new IllegalValueException( '$data must be an array' );
 		}
 
-		if ( !array_key_exists( 'entity-type', $data ) ) {
-			throw new IllegalValueException( "'entity-type' field required" );
+		if ( array_key_exists( 'entity-type', $data ) && array_key_exists( 'numeric-id', $data ) ) {
+			return self::newIdFromTypeAndNumber( $data['entity-type'], $data['numeric-id'] );
+		} elseif ( array_key_exists( 'id', $data ) ) {
+			throw new IllegalValueException(
+				'Not able to parse "id" strings, use callbacks in DataValueDeserializer instead'
+			);
 		}
 
-		if ( !array_key_exists( 'numeric-id', $data ) ) {
-			throw new IllegalValueException( "'numeric-id' field required" );
-		}
+		throw new IllegalValueException( 'Either "id" or "entity-type" and "numeric-id" fields required' );
+	}
 
+	/**
+	 * @param string $entityType
+	 * @param int|float|string $numericId
+	 *
+	 * @throws IllegalValueException
+	 * @return self
+	 */
+	private static function newIdFromTypeAndNumber( $entityType, $numericId ) {
 		try {
-			$id = LegacyIdInterpreter::newIdFromTypeAndNumber(
-				$data['entity-type'],
-				$data['numeric-id']
-			);
+			return new self( LegacyIdInterpreter::newIdFromTypeAndNumber( $entityType, $numericId ) );
 		} catch ( InvalidArgumentException $ex ) {
 			throw new IllegalValueException( $ex->getMessage(), 0, $ex );
 		}
-
-		return new static( $id );
 	}
 
 }

tests/unit/Entity/EntityIdValueTest.php

@@ -2,6 +2,7 @@
 
 namespace Wikibase\DataModel\Tests\Entity;
 
+use PHPUnit_Framework_TestCase;
 use Wikibase\DataModel\Entity\EntityIdValue;
 use Wikibase\DataModel\Entity\ItemId;
 use Wikibase\DataModel\Entity\PropertyId;
@@ -15,8 +16,9 @@
  *
  * @license GPL-2.0+
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ * @author Thiemo Mättig
  */
-class EntityIdValueTest extends \PHPUnit_Framework_TestCase {
+class EntityIdValueTest extends PHPUnit_Framework_TestCase {
 
 	public function testCanConstruct() {
 		$entityId = new ItemId( 'Q123' );
@@ -102,28 +104,37 @@ public function testDeserializationCompatibility() {
 	public function testGetArrayValueCompatibility() {
 		$id = new EntityIdValue( new ItemId( 'Q31337' ) );
 
-		$this->assertEquals(
+		$this->assertSame(
 			// This is the serialization format from when the EntityIdValue was still together with EntityId.
 			array(
 				'entity-type' => 'item',
-				'numeric-id' => 31337,
+				'numeric-id' => (float)31337,
+				'id' => 'Q31337',
 			),
 			$id->getArrayValue()
 		);
 	}
 
-	public function testNewFromArrayCompatibility() {
+	/**
+	 * @dataProvider validArrayProvider
+	 */
+	public function testNewFromArrayCompatibility( array $array ) {
 		$id = new EntityIdValue( new ItemId( 'Q31337' ) );
 
-		$this->assertEquals(
-			$id,
-			EntityIdValue::newFromArray(
-				// This is the serialization format from when the EntityIdValue was still together with EntityId.
-				array(
-					'entity-type' => 'item',
-					'numeric-id' => 31337,
-				)
-			)
+		$this->assertEquals( $id, EntityIdValue::newFromArray( $array ) );
+	}
+
+	public function validArrayProvider() {
+		return array(
+			'Legacy format' => array( array(
+				'entity-type' => 'item',
+				'numeric-id' => 31337,
+			) ),
+			'Maximum compatibility' => array( array(
+				'entity-type' => 'item',
+				'numeric-id' => 31337,
+				'id' => 'Q31337',
+			) ),
 		);
 	}
 
@@ -144,6 +155,10 @@ public function invalidArrayProvider() {
 
 			array( array() ),
 
+			'newFromArray can not deserialize' => array( array(
+				'id' => 'Q42',
+			) ),
+
 			array( array(
 				'entity-type' => 'item',
 			) ),