jenkins-bot has submitted this change and it was merged.
Change subject: Dispatch snak formatting by data type.
......................................................................
Dispatch snak formatting by data type.
This allows DispatchingSnakFormatter to dispatch by data type
as well as snak type.
Bug: T112758
Change-Id: I7075e0cd09774f881b13301bd88209837b44a121
---
M lib/includes/formatters/DispatchingSnakFormatter.php
M lib/includes/formatters/OutputFormatSnakFormatterFactory.php
M lib/tests/phpunit/formatters/DispatchingSnakFormatterTest.php
3 files changed, 202 insertions(+), 91 deletions(-)
Approvals:
Hoo man: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/includes/formatters/DispatchingSnakFormatter.php
b/lib/includes/formatters/DispatchingSnakFormatter.php
index 5ff3bff..dc2db52 100644
--- a/lib/includes/formatters/DispatchingSnakFormatter.php
+++ b/lib/includes/formatters/DispatchingSnakFormatter.php
@@ -4,11 +4,14 @@
use InvalidArgumentException;
use ValueFormatters\FormattingException;
+use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
+use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookupException;
use Wikibase\DataModel\Snak\Snak;
+use Wikimedia\Assert\Assert;
/**
- * DispatchingSnakFormatter will format a snak by delegating the formatting to
an appropriate
- * SnakFormatter for the snak's type.
+ * DispatchingSnakFormatter will format a Snak by delegating the formatting to
an appropriate
+ * SnakFormatter based on the snak type or the associated property's data type.
*
* @license GPL 2+
* @author Daniel Kinzler
@@ -16,45 +19,81 @@
class DispatchingSnakFormatter implements SnakFormatter {
/**
- * @var SnakFormatter[] a map of snak type IDs to SnakFormatter objects
- */
- private $formatters;
-
- /**
* @var string
*/
private $format;
/**
- * @param string $format
- * @param SnakFormatter[] $formatters a map of snak type IDs to
SnakFormatter objects
- *
- * @throws \InvalidArgumentException
+ * @var PropertyDataTypeLookup
*/
- public function __construct( $format, array $formatters ) {
- if ( !is_string( $format ) ) {
- throw new InvalidArgumentException( '$format must be a
string' );
- }
+ private $dataTypeLookup;
+ /**
+ * @var SnakFormatter[]
+ */
+ private $formattersByDataType;
+
+ /**
+ * @var SnakFormatter[]
+ */
+ private $formattersBySnakType;
+
+ /**
+ * @param string $format The output format generated by this formatter.
All formatters
+ * provided to the constructor must produce the same output format.
+ * @param PropertyDataTypeLookup $dataTypeLookup
+ * @param SnakFormatter[] $formattersBySnakType An associative array
mapping snak types
+ * to SnakFormatter objects. If no formatter is defined for the a
given snak type,
+ * $formattersByDataType will be checked for a SnakFormatter for the
snak's data type.
+ * @param SnakFormatter[] $formattersByDataType An associative array
mapping data types
+ * to SnakFormatter objects. If no formatter is defined for the a
given data type,
+ * the "*" key in this array is checked for a default formatter.
+ *
+ * @throws InvalidArgumentException
+ */
+ public function __construct(
+ $format,
+ PropertyDataTypeLookup $dataTypeLookup,
+ array $formattersBySnakType,
+ array $formattersByDataType
+ ) {
+ Assert::parameterType( 'string', $format, '$format' );
+
+ $this->assertFormatterArray( $format, $formattersBySnakType );
+ $this->assertFormatterArray( $format, $formattersByDataType );
+
+ $this->format = $format;
+ $this->dataTypeLookup = $dataTypeLookup;
+ $this->formattersBySnakType = $formattersBySnakType;
+ $this->formattersByDataType = $formattersByDataType;
+ }
+
+ private function assertFormatterArray( $format, array $formatters ) {
foreach ( $formatters as $type => $formatter ) {
if ( !is_string( $type ) ) {
- throw new InvalidArgumentException(
'$formatters must map type IDs to formatters.' );
+ throw new InvalidArgumentException( 'formatter
array must map type IDs to formatters.' );
}
if ( !( $formatter instanceof SnakFormatter ) ) {
- throw new InvalidArgumentException(
'$formatters must contain instances for SnakFormatter.' );
+ throw new InvalidArgumentException( 'formatter
array must contain instances of SnakFormatter.' );
}
if ( $formatter->getFormat() !== $format ) {
throw new InvalidArgumentException( 'The
formatter supplied for ' . $type
- . ' returns ' .
$formatter->getFormat() . ', but we expect ' . $format . '.' );
+ . ' produces ' .
$formatter->getFormat() . ', but we expect ' . $format . '.' );
}
}
+ }
- $this->format = $format;
- $this->formatters = $formatters;
-
- //XXX: this should perhaps use, or be, a
OutputFormatSnakFormatterFactory
+ /**
+ * @param Snak $snak
+ *
+ * @throws PropertyDataTypeLookupException
+ * @return string 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.
}
/**
@@ -64,38 +103,30 @@
* @param Snak $snak
*
* @throws FormattingException
- * @return string
+ * @throws PropertyDataTypeLookupException
+ * @return string The formatted snak value, in the format specified by
getFormat().
*/
public function formatSnak( Snak $snak ) {
- $type = $snak->getType();
- $formatter = $this->getFormatter( $type );
+ $snakType = $snak->getType();
- if ( !$formatter ) {
- throw new FormattingException( "No formatter found for
snak type $type" );
+ if ( isset( $this->formattersBySnakType[$snakType] ) ) {
+ $formatter = $this->formattersBySnakType[$snakType];
+ return $formatter->formatSnak( $snak );
}
- $text = $formatter->formatSnak( $snak );
- return $text;
- }
+ $dataType = $this->getSnakDataType( $snak );
- /**
- * @param string $type
- *
- * @return null|SnakFormatter
- */
- public function getFormatter( $type ) {
- if ( !isset( $this->formatters[$type] ) ) {
- return null;
+ if ( isset( $this->formattersByDataType[$dataType] ) ) {
+ $formatter = $this->formattersByDataType[$dataType];
+ return $formatter->formatSnak( $snak );
}
- return $this->formatters[$type];
- }
+ if ( isset( $this->formattersByDataType['*'] ) ) {
+ $formatter = $this->formattersByDataType['*'];
+ return $formatter->formatSnak( $snak );
+ }
- /**
- * @return string[]
- */
- public function getSnakTypes() {
- return array_keys( $this->formatters );
+ throw new FormattingException( "No formatter found for snak
type $snakType and data type $dataType" );
}
/**
diff --git a/lib/includes/formatters/OutputFormatSnakFormatterFactory.php
b/lib/includes/formatters/OutputFormatSnakFormatterFactory.php
index 2bd058b..1dc158f 100644
--- a/lib/includes/formatters/OutputFormatSnakFormatterFactory.php
+++ b/lib/includes/formatters/OutputFormatSnakFormatterFactory.php
@@ -87,13 +87,23 @@
$this->dataTypeFactory
);
- $formatters = array(
+ $formattersBySnakType = array(
'novalue' => $noValueSnakFormatter,
'somevalue' => $someValueSnakFormatter,
- 'value' => $valueSnakFormatter,
+ // for 'value' snaks, rely on $formattersByDataType
);
- $snakFormatter = new DispatchingSnakFormatter( $format,
$formatters );
+ $formattersByDataType = array(
+ // TODO: get specialized SnakFormatters from factory
functions.
+ '*' => $valueSnakFormatter
+ );
+
+ $snakFormatter = new DispatchingSnakFormatter(
+ $format,
+ $this->propertyDataTypeLookup,
+ $formattersBySnakType,
+ $formattersByDataType
+ );
if ( $options->getOption( SnakFormatter::OPT_ON_ERROR ) ===
SnakFormatter::ON_ERROR_WARN ) {
$snakFormatter = new ErrorHandlingSnakFormatter(
diff --git a/lib/tests/phpunit/formatters/DispatchingSnakFormatterTest.php
b/lib/tests/phpunit/formatters/DispatchingSnakFormatterTest.php
index 12d38ee..ea148c1 100644
--- a/lib/tests/phpunit/formatters/DispatchingSnakFormatterTest.php
+++ b/lib/tests/phpunit/formatters/DispatchingSnakFormatterTest.php
@@ -3,10 +3,13 @@
namespace Wikibase\Lib\Test;
use DataValues\StringValue;
+use ValueFormatters\StringFormatter;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
use Wikibase\DataModel\Snak\PropertySomeValueSnak;
use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\DataModel\Snak\Snak;
use Wikibase\Lib\DispatchingSnakFormatter;
use Wikibase\Lib\MessageSnakFormatter;
use Wikibase\Lib\SnakFormatter;
@@ -25,12 +28,54 @@
class DispatchingSnakFormatterTest extends \PHPUnit_Framework_TestCase {
/**
+ * @param string $dataType
+ *
+ * @return PropertyDataTypeLookup
+ */
+ private function getDataTypeLookup( $dataType = 'string' ) {
+ $dataTypeLookup = $this->getMock(
'Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup' );
+
+ $dataTypeLookup->expects( $this->any() )
+ ->method( 'getDataTypeIdForProperty' )
+ ->will( $this->returnValue( $dataType ) );
+
+ return $dataTypeLookup;
+ }
+
+ /**
+ * @param string $output the return value for formatSnak
+ * @param string $format the return value for getFormat
+ *
+ * @return SnakFormatter
+ */
+ private function makeSnakFormatter( $output, $format =
SnakFormatter::FORMAT_PLAIN ) {
+ $formatter = $this->getMock( 'Wikibase\Lib\SnakFormatter' );
+
+ $formatter->expects( $this->any() )
+ ->method( 'formatSnak' )
+ ->will( $this->returnValue( $output ) );
+
+ $formatter->expects( $this->any() )
+ ->method( 'getFormat' )
+ ->will( $this->returnValue( $format ) );
+
+ return $formatter;
+ }
+
+ /**
* @dataProvider constructorErrorsProvider
*/
- public function testConstructorErrors( $format, array $formatters,
$error ) {
- $this->setExpectedException( $error );
+ public function testConstructorErrors( $format, array
$formattersBySnakType, array $formattersByDataType ) {
+ $this->setExpectedException( 'InvalidArgumentException' );
- new DispatchingSnakFormatter( $format, $formatters );
+ $dataTypeLookup = $this->getDataTypeLookup();
+
+ new DispatchingSnakFormatter(
+ $format,
+ $dataTypeLookup,
+ $formattersBySnakType,
+ $formattersByDataType
+ );
}
public function constructorErrorsProvider() {
@@ -44,69 +89,94 @@
'format must be a string' => array(
17,
array(),
- 'InvalidArgumentException'
+ array(),
),
- 'keys must be strings' => array(
+ 'snak types must be strings' => array(
SnakFormatter::FORMAT_PLAIN,
array( 17 => $formatter ),
- 'InvalidArgumentException'
+ array( 'string' => $formatter ),
),
- 'formatters must be instances of SnakFormatter' =>
array(
+ 'data types must be strings' => array(
+ SnakFormatter::FORMAT_PLAIN,
+ array(),
+ array( 17 => $formatter ),
+ ),
+ 'snak type formatters must be SnakFormatters' => array(
SnakFormatter::FORMAT_PLAIN,
array( 'novalue' => 17 ),
- 'InvalidArgumentException'
+ array( 'string' => $formatter ),
),
- 'mismatching output format' => array(
+ 'data type formatters must be SnakFormatters' => array(
+ SnakFormatter::FORMAT_PLAIN,
+ array(),
+ array( 'string' => 17 ),
+ ),
+ 'snak type formatters mismatches output format' =>
array(
SnakFormatter::FORMAT_HTML,
array( 'novalue' => $formatter ),
- 'InvalidArgumentException'
+ array( 'string' => $formatter ),
+ ),
+ 'data type formatters mismatches output format' =>
array(
+ SnakFormatter::FORMAT_HTML,
+ array(),
+ array( 'string' => $formatter ),
),
);
}
- public function testFormatSnak() {
- $novalue = wfMessage(
'wikibase-snakview-snaktypeselector-novalue' );
- $somevalue = wfMessage(
'wikibase-snakview-snaktypeselector-somevalue' );
- $value = wfMessage( 'wikibase-snakview-snaktypeselector-value'
);
+ public function provideFormatSnak() {
+ $p23 = new PropertyId( 'P23' );
- $formatter = new DispatchingSnakFormatter(
SnakFormatter::FORMAT_PLAIN, array(
- 'novalue' => new MessageSnakFormatter( 'novalue',
$novalue, SnakFormatter::FORMAT_PLAIN ),
- 'somevalue' => new MessageSnakFormatter( 'somevalue',
$somevalue, SnakFormatter::FORMAT_PLAIN ),
- 'value' => new MessageSnakFormatter( 'value', $value,
SnakFormatter::FORMAT_PLAIN ),
- ) );
-
- $novalueSnak = new PropertyNoValueSnak( new PropertyId( 'P23' )
);
- $somevalueSnak = new PropertySomeValueSnak( new PropertyId(
'P23' ) );
- $valueSnak = new PropertyValueSnak( new PropertyId( 'P23' ),
new StringValue( 'test' ) );
-
- $this->assertEquals( $novalue->text(), $formatter->formatSnak(
$novalueSnak ) );
- $this->assertEquals( $somevalue->text(),
$formatter->formatSnak( $somevalueSnak ) );
- $this->assertEquals( $value->text(), $formatter->formatSnak(
$valueSnak ) );
+ return array(
+ 'novalue' => array(
+ 'NO VALUE',
+ new PropertyNoValueSnak( $p23 ),
+ 'string'
+ ),
+ 'somevalue' => array(
+ 'SOME VALUE',
+ new PropertySomeValueSnak( $p23 ),
+ 'string'
+ ),
+ 'string value' => array(
+ 'STRING VALUE',
+ new PropertyValueSnak( $p23, new StringValue(
'dummy' ) ),
+ 'string'
+ ),
+ 'other value' => array(
+ 'OTHER VALUE',
+ new PropertyValueSnak( $p23, new StringValue(
'dummy' ) ),
+ 'url'
+ ),
+ );
}
- public function testGetSnakTypes() {
- $novalue = wfMessage(
'wikibase-snakview-snaktypeselector-novalue' );
- $somevalue = wfMessage(
'wikibase-snakview-snaktypeselector-somevalue' );
- $value = wfMessage( 'wikibase-snakview-snaktypeselector-value'
);
-
- $formatters = array(
- 'novalue' => new MessageSnakFormatter( 'novalue',
$novalue, SnakFormatter::FORMAT_PLAIN ),
- 'somevalue' => new MessageSnakFormatter( 'somevalue',
$somevalue, SnakFormatter::FORMAT_PLAIN ),
- 'value' => new MessageSnakFormatter( 'value', $value,
SnakFormatter::FORMAT_PLAIN ),
+ /**
+ * @dataProvider provideFormatSnak
+ */
+ public function testFormatSnak( $expected, Snak $snak, $dataType ) {
+ $formattersBySnakType = array(
+ 'novalue' => $this->makeSnakFormatter( 'NO VALUE' ),
+ 'somevalue' => $this->makeSnakFormatter( 'SOME VALUE' ),
);
- $formatter = new DispatchingSnakFormatter(
SnakFormatter::FORMAT_PLAIN, $formatters );
+ $formattersByDataType = array(
+ 'string' => $this->makeSnakFormatter( 'STRING VALUE' ),
+ '*' => $this->makeSnakFormatter( 'OTHER VALUE' ),
+ );
- $this->assertEquals( array_keys( $formatters ),
$formatter->getSnakTypes() );
+ $formatter = new DispatchingSnakFormatter(
+ SnakFormatter::FORMAT_PLAIN,
+ $this->getDataTypeLookup( $dataType ),
+ $formattersBySnakType,
+ $formattersByDataType
+ );
- foreach ( $formatters as $type => $expected ) {
- $actual = $formatter->getFormatter( $type );
- $this->assertSame( $formatters[$type], $actual );
- }
+ $this->assertEquals( $expected, $formatter->formatSnak( $snak )
);
}
public function testGetFormat() {
- $formatter = new DispatchingSnakFormatter( 'test', array() );
+ $formatter = new DispatchingSnakFormatter( 'test',
$this->getDataTypeLookup(), array(), array() );
$this->assertEquals( 'test', $formatter->getFormat() );
}
--
To view, visit https://gerrit.wikimedia.org/r/244485
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7075e0cd09774f881b13301bd88209837b44a121
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
