Merge pull request #241 from wmde/T178651

Fix SnakDeserializer causing "Undefined index" errors

Author: mariushoch

src/DeserializerFactory.php

@@ -169,7 +169,7 @@ public function newSnakListDeserializer() {
 	 * @return Deserializer
 	 */
 	public function newSnakDeserializer() {
-		return new SnakDeserializer( $this->dataValueDeserializer, $this->newEntityIdDeserializer() );
+		return new SnakDeserializer( $this->dataValueDeserializer );
 	}
 
 	/**

src/Deserializers/SnakDeserializer.php

@@ -2,6 +2,8 @@
 
 namespace Wikibase\DataModel\Deserializers;
 
+use DataValues\DataValue;
+use DataValues\Deserializers\DataValueDeserializer;
 use DataValues\UnDeserializableValue;
 use Deserializers\Deserializer;
 use Deserializers\DispatchableDeserializer;
@@ -10,6 +12,7 @@
 use Deserializers\Exceptions\MissingAttributeException;
 use Deserializers\Exceptions\MissingTypeException;
 use Deserializers\Exceptions\UnsupportedTypeException;
+use InvalidArgumentException;
 use Wikibase\DataModel\Entity\PropertyId;
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
 use Wikibase\DataModel\Snak\PropertySomeValueSnak;
@@ -29,17 +32,8 @@ class SnakDeserializer implements DispatchableDeserializer {
 	 */
 	private $dataValueDeserializer;
 
-	/**
-	 * @var Deserializer
-	 */
-	private $entityIdDeserializer;
-
-	public function __construct(
-		Deserializer $dataValueDeserializer,
-		Deserializer $entityIdDeserializer
-	) {
+	public function __construct( Deserializer $dataValueDeserializer ) {
 		$this->dataValueDeserializer = $dataValueDeserializer;
-		$this->entityIdDeserializer = $entityIdDeserializer;
 	}
 
 	/**
@@ -114,12 +108,24 @@ private function newValueSnak( array $serialization ) {
 		);
 	}
 
+	/**
+	 * @param array $serialization
+	 *
+	 * @return DataValue
+	 */
 	private function deserializeDataValue( $serialization ) {
 		try {
 			return $this->dataValueDeserializer->deserialize( $serialization );
 		} catch ( DeserializationException $ex ) {
+			$value = isset( $serialization[DataValueDeserializer::VALUE_KEY] )
+				? $serialization[DataValueDeserializer::VALUE_KEY]
+				: null;
+			$type = isset( $serialization[DataValueDeserializer::TYPE_KEY] )
+				? $serialization[DataValueDeserializer::TYPE_KEY]
+				: null;
 			$error = isset( $serialization['error'] ) ? $serialization['error'] : $ex->getMessage();
-			return new UnDeserializableValue( $serialization['value'], $serialization['type'], $error );
+
+			return new UnDeserializableValue( $value, $type, $error );
 		}
 	}
 
@@ -130,17 +136,15 @@ private function deserializeDataValue( $serialization ) {
 	 * @return PropertyId
 	 */
 	private function deserializePropertyId( $serialization ) {
-		$propertyId = $this->entityIdDeserializer->deserialize( $serialization );
-
-		if ( !( $propertyId instanceof PropertyId ) ) {
+		try {
+			return new PropertyId( $serialization );
+		} catch ( InvalidArgumentException $ex ) {
 			throw new InvalidAttributeException(
 				'property',
 				$serialization,
 				"'$serialization' is not a valid property ID"
 			);
 		}
-
-		return $propertyId;
 	}
 
 	private function assertCanDeserialize( $serialization ) {

tests/unit/Deserializers/SnakDeserializerTest.php

@@ -24,17 +24,10 @@
 class SnakDeserializerTest extends DispatchableDeserializerTest {
 
 	protected function buildDeserializer() {
-		$entityIdDeserializerMock = $this->getMock( Deserializer::class );
-		$entityIdDeserializerMock->expects( $this->any() )
-			->method( 'deserialize' )
-			->with( $this->equalTo( 'P42' ) )
-			->will( $this->returnValue( new PropertyId( 'P42' ) ) );
-
 		return new SnakDeserializer(
 			new DataValueDeserializer( [
 				'string' => StringValue::class,
-			] ),
-			$entityIdDeserializerMock
+			] )
 		);
 	}
 
@@ -150,12 +143,7 @@ public function invalidDeserializationProvider() {
 	}
 
 	public function testDeserializePropertyIdFilterItemId() {
-		$entityIdDeserializerMock = $this->getMock( Deserializer::class );
-		$entityIdDeserializerMock->expects( $this->any() )
-			->method( 'deserialize' )
-			->with( $this->equalTo( 'Q42' ) )
-			->will( $this->returnValue( new ItemId( 'Q42' ) ) );
-		$deserializer = new SnakDeserializer( new DataValueDeserializer(), $entityIdDeserializerMock );
+		$deserializer = new SnakDeserializer( new DataValueDeserializer() );
 
 		$this->setExpectedException( InvalidAttributeException::class );
 		$deserializer->deserialize( [