Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/321668
Change subject: BinaryOptionDispatchingSnakFormatter fails on unknown properties
......................................................................
BinaryOptionDispatchingSnakFormatter fails on unknown properties
This turned out to be a blocker for what we are developing in
ArticlePlaceholder. The entire Special:AboutTopic fails hard when an
Item contains a Property that got deleted. This should not happen.
There is even a proper message rendered. Nothing "TODO" here.
This patch also fixes the broken namespace(s).
Bug: T147580
Change-Id: I57b2c74a4a5cdcb66e116ab3cbd1e0b5e9ce12f6
---
M client/includes/DataAccess/DataAccessSnakFormatterFactory.php
M lib/includes/Formatters/BinaryOptionDispatchingSnakFormatter.php
M lib/tests/phpunit/Formatters/BinaryOptionDispatchingSnakFormatterTest.php
3 files changed, 35 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/68/321668/1
diff --git a/client/includes/DataAccess/DataAccessSnakFormatterFactory.php
b/client/includes/DataAccess/DataAccessSnakFormatterFactory.php
index d5a2797..f932ecf 100644
--- a/client/includes/DataAccess/DataAccessSnakFormatterFactory.php
+++ b/client/includes/DataAccess/DataAccessSnakFormatterFactory.php
@@ -8,7 +8,7 @@
use Wikibase\Client\Usage\UsageTrackingSnakFormatter;
use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
use Wikibase\LanguageFallbackChainFactory;
-use Wikibase\Lib\BinaryOptionDispatchingSnakFormatter;
+use Wikibase\Lib\Formatters\BinaryOptionDispatchingSnakFormatter;
use Wikibase\Lib\EscapingSnakFormatter;
use Wikibase\Lib\FormatterLabelDescriptionLookupFactory;
use Wikibase\Lib\OutputFormatSnakFormatterFactory;
diff --git a/lib/includes/Formatters/BinaryOptionDispatchingSnakFormatter.php
b/lib/includes/Formatters/BinaryOptionDispatchingSnakFormatter.php
index 533a626..8b069d6 100644
--- a/lib/includes/Formatters/BinaryOptionDispatchingSnakFormatter.php
+++ b/lib/includes/Formatters/BinaryOptionDispatchingSnakFormatter.php
@@ -1,11 +1,12 @@
<?php
-namespace Wikibase\Lib;
+namespace Wikibase\Lib\Formatters;
use ValueFormatters\FormattingException;
use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookupException;
use Wikibase\DataModel\Snak\Snak;
+use Wikibase\Lib\SnakFormatter;
use Wikimedia\Assert\Assert;
/**
@@ -76,11 +77,14 @@
* @param Snak $snak
*
* @throws PropertyDataTypeLookupException
- * @return string The Snak's data type
+ * @return string|null The Snak's data type
*/
private function getSnakDataType( Snak $snak ) {
- return $this->dataTypeLookup->getDataTypeIdForProperty(
$snak->getPropertyId() );
- // @todo: wrap the PropertyDataTypeLookupException, but make
sure ErrorHandlingSnakFormatter still handles it.
+ try {
+ return $this->dataTypeLookup->getDataTypeIdForProperty(
$snak->getPropertyId() );
+ } catch ( PropertyDataTypeLookupException $ex ) {
+ return null;
+ }
}
/**
@@ -97,7 +101,7 @@
if ( $snakType === 'value' ) {
$dataType = $this->getSnakDataType( $snak );
- if ( in_array( $dataType,
$this->specialCasedPropertyDataTypes ) ) {
+ if ( $dataType !== null && in_array( $dataType,
$this->specialCasedPropertyDataTypes ) ) {
return
$this->specialCaseSnakFormatter->formatSnak( $snak );
}
}
diff --git
a/lib/tests/phpunit/Formatters/BinaryOptionDispatchingSnakFormatterTest.php
b/lib/tests/phpunit/Formatters/BinaryOptionDispatchingSnakFormatterTest.php
index f789bb2..cad859a 100644
--- a/lib/tests/phpunit/Formatters/BinaryOptionDispatchingSnakFormatterTest.php
+++ b/lib/tests/phpunit/Formatters/BinaryOptionDispatchingSnakFormatterTest.php
@@ -1,20 +1,21 @@
<?php
-namespace Wikibase\Lib\Test;
+namespace Wikibase\Lib\Tests\Formatters;
use PHPUnit_Framework_TestCase;
-use DataValues\DataValue;
+use DataValues\StringValue;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
+use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookupException;
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
use Wikibase\DataModel\Snak\PropertySomeValueSnak;
use Wikibase\DataModel\Snak\PropertyValueSnak;
use Wikibase\DataModel\Snak\Snak;
-use Wikibase\Lib\BinaryOptionDispatchingSnakFormatter;
+use Wikibase\Lib\Formatters\BinaryOptionDispatchingSnakFormatter;
use Wikibase\Lib\SnakFormatter;
/**
- * @covers Wikibase\Lib\BinaryOptionDispatchingSnakFormatterTest
+ * @covers Wikibase\Lib\Formatters\BinaryOptionDispatchingSnakFormatter
*
* @group SnakFormatters
* @group DataValueExtensions
@@ -50,6 +51,7 @@
public function formatSnakProvider() {
$pSpecial = new PropertyId( 'P1' );
$pRegular = new PropertyId( 'P2' );
+ $value = new StringValue( '' );
return [
'PropertyNoValueSnak gets fallback treatment always' =>
[
@@ -61,13 +63,17 @@
false
],
'PropertyValueSnak with special treatment' => [
- new PropertyValueSnak( $pSpecial,
$this->getMock( DataValue::class ) ),
+ new PropertyValueSnak( $pSpecial, $value ),
true
],
'PropertyValueSnak without special treatment' => [
- new PropertyValueSnak( $pRegular,
$this->getMock( DataValue::class ) ),
+ new PropertyValueSnak( $pRegular, $value ),
false
- ]
+ ],
+ 'Fallback on non-existing Properties' => [
+ new PropertyValueSnak( new PropertyId( 'P3' ),
$value ),
+ false
+ ],
];
}
@@ -83,6 +89,12 @@
$this->assertSame( 'text/whatever', $formatter->getFormat() );
}
+ /**
+ * @param int $expectedCallCount
+ * @param string $result
+ *
+ * @return SnakFormatter
+ */
private function getSnakFormatter( $expectedCallCount, $result = '' ) {
$snakFormatter = $this->getMock( SnakFormatter::class );
$snakFormatter->expects( $this->exactly( $expectedCallCount ) )
@@ -93,20 +105,23 @@
return $snakFormatter;
}
+ /**
+ * @return PropertyDataTypeLookup
+ */
private function getPropertyDataTypeLookup() {
$propertyDataTypeLookup = $this->getMock(
PropertyDataTypeLookup::class );
$propertyDataTypeLookup->expects( $this->any() )
->method( 'getDataTypeIdForProperty' )
- ->willReturnCallback( function( PropertyId $propertyId
) {
+ ->will( $this->returnCallback( function( PropertyId
$propertyId ) {
switch ( $propertyId->getSerialization() ) {
case 'P1':
return 'special';
case 'P2':
return 'something';
default:
- $this->fail( 'Unexpcted
PropertyId' );
+ throw new
PropertyDataTypeLookupException( $propertyId );
}
- } );
+ } ) );
return $propertyDataTypeLookup;
}
--
To view, visit https://gerrit.wikimedia.org/r/321668
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I57b2c74a4a5cdcb66e116ab3cbd1e0b5e9ce12f6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
