Fix bad state transition on unknown snak type

Previously, if the DispatchingValueSnakRdfBuilder couldn’t find a
builder to dispatch to, it left the writer in the same state as before;
depending on the previous triples, this can crash the whole RDF dump,
because the writer should be in “object” state when it returns from this
builder. Ensure this by emitting a fake triple (it’s not great, but I
don’t think we can “rewind” the builder). Also beef up the log message
while we’re at it.

Test this by using a real RDF writer rather than a mock – this way, the
state machine in RdfWriterBase is exercised and will throw a
LogicException if the builder didn’t put the writer into a state where
it can finish.

Bug: T384625
Change-Id: I39f3d1337eaf1eacdd043c2e7271d0dc94689588

repo/includes/Rdf/DispatchingValueSnakRdfBuilder.php

@@ -57,20 +57,23 @@ public function addValue(
 		PropertyValueSnak $snak
 	) {
 		$valueType = $snak->getDataValue()->getType();
-		$builder = $this->getValueBuilder( $dataType, $valueType );
+		$builder = $this->getValueBuilder( $dataType, $valueType, $propertyValueNamespace, $propertyValueLName, $snak );
 
 		if ( $builder ) {
 			$builder->addValue( $writer, $propertyValueNamespace, $propertyValueLName, $dataType, $snakNamespace, $snak );
+		} else {
+			// emit a fake predicate+object just to ensure the writer stays in a sane state when we return (T384625)
+			$writer->a( RdfVocabulary::NS_ONTOLOGY, 'BrokenSnak' );
 		}
 	}
 
-	/**
-	 * @param string|null $dataTypeId
-	 * @param string $dataValueType
-	 *
-	 * @return null|ValueSnakRdfBuilder
-	 */
-	private function getValueBuilder( $dataTypeId, $dataValueType ) {
+	private function getValueBuilder(
+		?string $dataTypeId,
+		string $dataValueType,
+		string $propertyValueNamespace,
+		string $propertyValueLName,
+		PropertyValueSnak $snak
+	): ?ValueSnakRdfBuilder {
 		if ( $dataTypeId !== null ) {
 			if ( isset( $this->valueBuilders["PT:$dataTypeId"] ) ) {
 				return $this->valueBuilders["PT:$dataTypeId"];
@@ -84,17 +87,25 @@ private function getValueBuilder( $dataTypeId, $dataValueType ) {
 		if ( $dataTypeId !== null ) {
 			$this->logger->warning(
 				__METHOD__ . ': No RDF builder defined for data type ' .
-				'{dataTypeId} nor for value type {dataValueType}.',
+				'{dataTypeId} nor for value type {dataValueType} ' .
+				'(for predicate {$propertyValueNamespace}:{propertyValueLName}).',
 				[
 					'dataTypeId' => $dataTypeId,
 					'dataValueType' => $dataValueType,
+					'propertyValueNamespace' => $propertyValueNamespace,
+					'propertyValueLName' => $propertyValueLName,
+					'snak' => $snak,
 				]
 			);
 		} else {
 			$this->logger->warning(
-				__METHOD__ . ': No RDF builder defined for value type {dataValueType}.',
+				__METHOD__ . ': No RDF builder defined for value type {dataValueType} ' .
+				'(for predicate {$propertyValueNamespace}:{propertyValueLName}).',
 				[
 					'dataValueType' => $dataValueType,
+					'propertyValueNamespace' => $propertyValueNamespace,
+					'propertyValueLName' => $propertyValueLName,
+					'snak' => $snak,
 				]
 			);
 		}

repo/tests/phpunit/includes/Rdf/DispatchingValueSnakRdfBuilderTest.php

@@ -9,6 +9,7 @@
 use Wikibase\Repo\Rdf\DispatchingValueSnakRdfBuilder;
 use Wikibase\Repo\Rdf\ValueSnakRdfBuilder;
 use Wikimedia\Purtle\RdfWriter;
+use Wikimedia\Purtle\TurtleRdfWriter;
 
 /**
  * @covers \Wikibase\Repo\Rdf\DispatchingValueSnakRdfBuilder
@@ -49,7 +50,8 @@ public function testAddValue() {
 	}
 
 	public function testAddValue_unknownType() {
-		$writer = $this->createMock( RdfWriter::class );
+		$writer = new TurtleRdfWriter();
+		$writer->start();
 		$namespace = 'xx';
 		$lname = 'yy';
 
@@ -59,9 +61,11 @@ public function testAddValue_unknownType() {
 		$logger = new TestLogger();
 		$dispatchingBuilder = new DispatchingValueSnakRdfBuilder( [], $logger );
 
+		$writer->about( 'subject' );
 		$dispatchingBuilder->addValue( $writer, $namespace, $lname, 'foo', 'v', $snak );
 
 		$this->assertTrue( $logger->hasWarningRecords() );
+		$writer->finish(); // assert writer is in a finishable state, not with the subject left hanging
 	}
 
 }