jenkins-bot has submitted this change and it was merged.
Change subject: Drop magic escaping logic from
OutputFormatValueFormatterFactory.
......................................................................
Drop magic escaping logic from OutputFormatValueFormatterFactory.
Factory callbacks defined via DataTypeDefinitions are now required
directly to provide formatters for all output formats. This allows
us to drop some rather convoluted magic for escaping from the
factory. Magic that would have to be repeated in
OutputFormatSnakFormatterFactory
in order to allow it to also use factory callbacks from DataTypeDefinitions.
Bug: T119877
Change-Id: I04e6db7524f0903c5d088adec5a0163fd35b3d69
---
M client/WikibaseClient.datatypes.php
M lib/includes/formatters/OutputFormatValueFormatterFactory.php
M lib/includes/formatters/WikibaseValueFormatterBuilders.php
M lib/tests/phpunit/formatters/OutputFormatSnakFormatterFactoryTest.php
M lib/tests/phpunit/formatters/OutputFormatValueFormatterFactoryTest.php
M lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
M repo/WikibaseRepo.datatypes.php
7 files changed, 265 insertions(+), 293 deletions(-)
Approvals:
Thiemo Mättig (WMDE): Looks good to me, approved
jenkins-bot: Verified
Objections:
Aude: There's a problem with this change, please improve
diff --git a/client/WikibaseClient.datatypes.php
b/client/WikibaseClient.datatypes.php
index ac41d79..75add0b 100644
--- a/client/WikibaseClient.datatypes.php
+++ b/client/WikibaseClient.datatypes.php
@@ -41,7 +41,8 @@
return array(
'VT:bad' => array(
'formatter-factory-callback' => function( $format,
FormatterOptions $options ) {
- return $format === SnakFormatter::FORMAT_PLAIN
? new UnDeserializableValueFormatter( $options ) : null;
+ $factory =
WikibaseClient::getDefaultFormatterBuilders();
+ return
$factory->newUnDeserializableValueFormatter( $format, $options );
}
),
'VT:globecoordinate' => array(
@@ -64,7 +65,8 @@
),
'VT:string' => array(
'formatter-factory-callback' => function( $format,
FormatterOptions $options ) {
- return $format === SnakFormatter::FORMAT_PLAIN
? new StringFormatter( $options ) : null;
+ $factory =
WikibaseClient::getDefaultFormatterBuilders();
+ return $factory->newStringFormatter( $format,
$options );
},
),
'PT:url' => array(
diff --git a/lib/includes/formatters/OutputFormatValueFormatterFactory.php
b/lib/includes/formatters/OutputFormatValueFormatterFactory.php
index 48d7dd8..5bcf71a 100644
--- a/lib/includes/formatters/OutputFormatValueFormatterFactory.php
+++ b/lib/includes/formatters/OutputFormatValueFormatterFactory.php
@@ -138,163 +138,9 @@
public function getValueFormatter( $format, FormatterOptions $options )
{
$this->applyLanguageDefaults( $options );
- switch ( $format ) {
- case SnakFormatter::FORMAT_PLAIN:
- $formatters = $this->getPlainTextFormatters(
$options );
- break;
- case SnakFormatter::FORMAT_WIKI:
- $formatters = $this->getWikiTextFormatters(
$options );
- break;
- case SnakFormatter::FORMAT_HTML:
- $formatters = $this->getHtmlFormatters(
$options );
- break;
- case SnakFormatter::FORMAT_HTML_WIDGET:
- $formatters = $this->getWidgetFormatters(
$options );
- break;
- case SnakFormatter::FORMAT_HTML_DIFF:
- $formatters = $this->getDiffFormatters(
$options );
- break;
- default:
- throw new InvalidArgumentException(
'Unsupported format: ' . $format );
- }
+ $formatters = $this->buildDefinedFormatters( $format, $options
);
return new DispatchingValueFormatter( $formatters );
- }
-
- /**
- * Returns a full set of formatters for generating plain text output.
- *
- * @param FormatterOptions $options
- * @param string[] $skip A list of types to be skipped. Useful when the
caller already has
- * formatters for some types.
- *
- * @return ValueFormatter[] A map from prefixed type IDs to
ValueFormatter instances.
- */
- private function getPlainTextFormatters( FormatterOptions $options,
array $skip = array() ) {
- return $this->buildDefinedFormatters(
- SnakFormatter::FORMAT_PLAIN,
- $options,
- $skip
- );
- }
-
- /**
- * Returns a full set of formatters for generating wikitext output.
- * If there are formatters defined for plain text that are not defined
for wikitext,
- * the plain text formatters are used with the appropriate escaping
applied.
- *
- * @param FormatterOptions $options
- * @param string[] $skip A list of types to be skipped. Useful when the
caller already has
- * formatters for some types.
- *
- * @return ValueFormatter[] A map from prefixed type IDs to
ValueFormatter instances.
- */
- private function getWikiTextFormatters( FormatterOptions $options,
array $skip = array() ) {
- $wikiFormatters = $this->buildDefinedFormatters(
- SnakFormatter::FORMAT_WIKI,
- $options,
- $skip
- );
- $plainFormatters = $this->getPlainTextFormatters(
- $options, array_merge( $skip, array_keys(
$wikiFormatters ) )
- );
-
- $wikiFormatters = array_merge(
- $wikiFormatters,
- $this->makeEscapingFormatters( $plainFormatters,
'wfEscapeWikiText' )
- );
-
- return $wikiFormatters;
- }
-
- /**
- * Returns a full set of formatters for generating HTML output.
- * If there are formatters defined for plain text that are not defined
for HTML,
- * the plain text formatters are used with the appropriate escaping
applied.
- *
- * @param FormatterOptions $options
- * @param string[] $skip A list of types to be skipped. Useful when the
caller already has
- * formatters for some types.
- *
- * @return ValueFormatter[] A map from prefixed type IDs to
ValueFormatter instances.
- */
- private function getHtmlFormatters( FormatterOptions $options, array
$skip = array() ) {
- $htmlFormatters = $this->buildDefinedFormatters(
- SnakFormatter::FORMAT_HTML,
- $options,
- $skip
- );
- $plainFormatters = $this->getPlainTextFormatters(
- $options,
- array_merge( $skip, array_keys( $htmlFormatters ) )
- );
-
- $htmlFormatters = array_merge(
- $htmlFormatters,
- $this->makeEscapingFormatters( $plainFormatters,
'htmlspecialchars' )
- );
-
- return $htmlFormatters;
- }
-
- /**
- * Returns a full set of formatters for generating HTML widgets.
- * If there are formatters defined for HTML that are not defined for
widgets,
- * the HTML formatters are used.
- *
- * @param FormatterOptions $options
- * @param string[] $skip A list of types to be skipped. Useful when the
caller already has
- * formatters for some types.
- *
- * @return ValueFormatter[] A map from prefixed type IDs to
ValueFormatter instances.
- */
- private function getWidgetFormatters( FormatterOptions $options, array
$skip = array() ) {
- $widgetFormatters = $this->buildDefinedFormatters(
- SnakFormatter::FORMAT_HTML_WIDGET,
- $options,
- $skip
- );
- $htmlFormatters = $this->getHtmlFormatters(
- $options,
- array_merge( $skip, array_keys( $widgetFormatters ) )
- );
-
- $widgetFormatters = array_merge(
- $widgetFormatters,
- $htmlFormatters
- );
-
- return $widgetFormatters;
- }
-
- /**
- * Returns a full set of formatters for generating HTML for use in
diffs.
- * If there are formatters defined for HTML that are not defined for
diffs,
- * the HTML formatters are used.
- *
- * @param FormatterOptions $options
- * @param string[] $skip A list of types to be skipped. Useful when the
caller already has
- * formatters for some types.
- *
- * @return ValueFormatter[] A map from prefixed type IDs to
ValueFormatter instances.
- */
- private function getDiffFormatters( FormatterOptions $options, array
$skip = array() ) {
- $diffFormatters = $this->buildDefinedFormatters(
- SnakFormatter::FORMAT_HTML_DIFF,
- $options,
- $skip
- );
- $htmlFormatters = $this->getHtmlFormatters(
- $options,
- array_merge( $skip, array_keys( $diffFormatters ) )
- );
-
- $diffFormatters = array_merge(
- $diffFormatters,
- $htmlFormatters
- );
-
- return $diffFormatters;
}
/**
@@ -305,47 +151,24 @@
*
* @param string $format One of the SnakFormatter::FORMAT_... constants.
* @param FormatterOptions $options
- * @param string[] $skip A list of types to be skipped. Useful when the
caller already
- * has formatters for some types.
*
* @return ValueFormatter[] A map from prefixed type IDs to
ValueFormatter instances.
*/
- private function buildDefinedFormatters( $format, FormatterOptions
$options, array $skip = array() ) {
+ private function buildDefinedFormatters( $format, FormatterOptions
$options ) {
$formatters = array();
foreach ( $this->factoryFunctions as $type => $func ) {
- if ( $skip && in_array( $type, $skip ) ) {
- continue;
- }
-
$formatter = call_user_func( $func, $format, $options );
- if ( $formatter ) {
- $formatters[$type] = $formatter;
- }
+ Assert::postcondition(
+ $formatter instanceof ValueFormatter,
+ "Callback for $type did not return a
ValueFormatter"
+ );
+
+ $formatters[$type] = $formatter;
}
return $formatters;
- }
-
- /**
- * Wrap each entry in a list of formatters in an EscapingValueFormatter.
- * This is useful to apply escaping to the output of a set of
formatters,
- * allowing them to be used for a different format.
- *
- * @param ValueFormatter[] $formatters
- * @param string $escape The escape callback, e.g. 'htmlspecialchars'
or 'wfEscapeWikitext'.
- *
- * @return ValueFormatter[]
- */
- private function makeEscapingFormatters( array $formatters, $escape ) {
- $escapingFormatters = array();
-
- foreach ( $formatters as $key => $formatter ) {
- $escapingFormatters[$key] = new EscapingValueFormatter(
$formatter, $escape );
- }
-
- return $escapingFormatters;
}
}
diff --git a/lib/includes/formatters/WikibaseValueFormatterBuilders.php
b/lib/includes/formatters/WikibaseValueFormatterBuilders.php
index 16067d0..4737569 100644
--- a/lib/includes/formatters/WikibaseValueFormatterBuilders.php
+++ b/lib/includes/formatters/WikibaseValueFormatterBuilders.php
@@ -4,6 +4,7 @@
use DataValues\Geo\Formatters\GeoCoordinateFormatter;
use DataValues\Geo\Formatters\GlobeCoordinateFormatter;
+use InvalidArgumentException;
use Language;
use ValueFormatters\DecimalFormatter;
use ValueFormatters\FormatterOptions;
@@ -98,51 +99,92 @@
/**
* @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
+ *
+ * @return bool True if $format is one of the
SnakFormatter::FORMAT_HTML_XXX formats.
+ */
+ private function isHtmlFormat( $format ) {
+ return $format === SnakFormatter::FORMAT_HTML
+ || $format === SnakFormatter::FORMAT_HTML_DIFF
+ || $format === SnakFormatter::FORMAT_HTML_WIDGET;
+ }
+
+ /**
+ * Wraps the given formatter in an EscapingValueFormatter if necessary.
+ *
+ * @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
+ * @param ValueFormatter $formatter The plain text formatter to wrap.
+ *
+ * @return ValueFormatter
+ */
+ private function escapeValueFormatter( $format, ValueFormatter
$formatter ) {
+ if ( $this->isHtmlFormat( $format ) ) {
+ return new EscapingValueFormatter( $formatter,
'htmlspecialchars' );
+ } elseif ( $format === SnakFormatter::FORMAT_WIKI ) {
+ return new EscapingValueFormatter( $formatter,
'wfEscapeWikiText' );
+ } elseif ( $format === SnakFormatter::FORMAT_PLAIN ) {
+ return $formatter;
+ } else {
+ throw new InvalidArgumentException( 'Unsupported output
format: ' . $format );
+ }
+ }
+
+ /**
+ * @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
* @param FormatterOptions $options
*
- * @return ValueFormatter|null
+ * @return ValueFormatter
*/
public function newEntityIdFormatter( $format, FormatterOptions
$options ) {
- if ( $format === SnakFormatter::FORMAT_PLAIN ) {
- return $this->newPlainEntityIdFormatter( $options );
- } elseif ( $format === SnakFormatter::FORMAT_HTML ) {
- if ( !$this->entityTitleLookup ) {
- return new EscapingValueFormatter(
- $this->newPlainEntityIdFormatter(
$options ),
- 'htmlspecialchars'
- );
- } else {
- $labelDescriptionLookup =
$this->labelDescriptionLookupFactory->getLabelDescriptionLookup( $options );
- return new EntityIdValueFormatter(
- new EntityIdHtmlLinkFormatter(
- $labelDescriptionLookup,
- $this->entityTitleLookup,
- $this->languageNameLookup
- )
- );
- }
- } else {
- return null;
+ if ( $this->isHtmlFormat( $format ) && $this->entityTitleLookup
) {
+ $labelDescriptionLookup =
$this->labelDescriptionLookupFactory->getLabelDescriptionLookup( $options );
+
+ return new EntityIdValueFormatter(
+ new EntityIdHtmlLinkFormatter(
+ $labelDescriptionLookup,
+ $this->entityTitleLookup,
+ $this->languageNameLookup
+ )
+ );
}
+
+ $plainFormatter = $this->newPlainEntityIdFormatter( $options );
+ return $this->escapeValueFormatter( $format, $plainFormatter );
}
/**
* @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
* @param FormatterOptions $options
*
- * @return ValueFormatter|null
+ * @return ValueFormatter
+ */
+ public function newStringFormatter( $format, FormatterOptions $options
) {
+ return $this->escapeValueFormatter( $format, new
StringFormatter( $options ) );
+ }
+
+ /**
+ * @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
+ * @param FormatterOptions $options
+ *
+ * @return ValueFormatter
+ */
+ public function newUnDeserializableValueFormatter( $format,
FormatterOptions $options ) {
+ return $this->escapeValueFormatter( $format, new
UnDeserializableValueFormatter( $options ) );
+ }
+
+ /**
+ * @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
+ * @param FormatterOptions $options
+ *
+ * @return ValueFormatter
*/
public function newUrlFormatter( $format, FormatterOptions $options ) {
- if ( $format === SnakFormatter::FORMAT_PLAIN ) {
- return new StringFormatter( $options );
- } elseif ( $format === SnakFormatter::FORMAT_WIKI ) {
+ if ( $format === SnakFormatter::FORMAT_WIKI ) {
// use the string formatter without escaping!
return new StringFormatter( $options );
- } elseif ( $format === SnakFormatter::FORMAT_HTML ) {
- // use the string formatter without escaping!
+ } elseif ( $this->isHtmlFormat( $format ) ) {
return new HtmlUrlFormatter( $options );
} else {
- return null;
+ return $this->escapeValueFormatter( $format, new
StringFormatter( $options ) );
}
}
@@ -150,18 +192,14 @@
* @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
* @param FormatterOptions $options
*
- * @return ValueFormatter|null
+ * @return ValueFormatter
*/
public function newCommonsMediaFormatter( $format, FormatterOptions
$options ) {
- if ( $format === SnakFormatter::FORMAT_PLAIN ) {
- return new StringFormatter( $options );
- } elseif ( $format === SnakFormatter::FORMAT_WIKI ) {
- //TODO: wikitext image link (inline? thumbnail?
caption?...)
- return null;
- } elseif ( $format === SnakFormatter::FORMAT_HTML ) {
+ //TODO: for FORMAT_WIKI, wikitext image link (inline?
thumbnail? caption?...)
+ if ( $this->isHtmlFormat( $format ) ) {
return new CommonsLinkFormatter( $options );
} else {
- return null;
+ return $this->escapeValueFormatter( $format, new
StringFormatter( $options ) );
}
}
@@ -169,23 +207,26 @@
* @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
* @param FormatterOptions $options
*
- * @return ValueFormatter|null
+ * @return ValueFormatter
*/
public function newTimeFormatter( $format, FormatterOptions $options ) {
- if ( $format === SnakFormatter::FORMAT_PLAIN ) {
- return new MwTimeIsoFormatter( $options );
- } elseif ( $format === SnakFormatter::FORMAT_HTML ) {
- return new HtmlTimeFormatter( $options, new
MwTimeIsoFormatter( $options ) );
- } elseif ( $format === SnakFormatter::FORMAT_HTML_DIFF ) {
+ if ( $format === SnakFormatter::FORMAT_HTML_DIFF ) {
return new TimeDetailsFormatter(
$options,
new HtmlTimeFormatter( $options, new
MwTimeIsoFormatter( $options ) )
);
+ } elseif ( $this->isHtmlFormat( $format ) ) {
+ return new HtmlTimeFormatter( $options, new
MwTimeIsoFormatter( $options ) );
} else {
- return null;
+ return $this->escapeValueFormatter( $format, new
MwTimeIsoFormatter( $options ) );
}
}
+ /**
+ * @param FormatterOptions $options
+ *
+ * @return MediaWikiNumberLocalizer
+ */
private function getNumberLocalizer( FormatterOptions $options ) {
$language = Language::factory( $options->getOption(
ValueFormatter::OPT_LANG ) );
return new MediaWikiNumberLocalizer( $language );
@@ -205,23 +246,22 @@
* @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
* @param FormatterOptions $options
*
- * @return QuantityFormatter|null
+ * @return QuantityFormatter
*/
public function newQuantityFormatter( $format, FormatterOptions
$options ) {
- if ( $format === SnakFormatter::FORMAT_PLAIN ) {
- $decimalFormatter = new DecimalFormatter( $options,
$this->getNumberLocalizer( $options ) );
- $vocabularyUriFormatter =
$this->getVocabularyUriFormatter( $options );
- return new QuantityFormatter( $options,
$decimalFormatter, $vocabularyUriFormatter );
- } elseif ( $format === SnakFormatter::FORMAT_HTML ) {
- $decimalFormatter = new DecimalFormatter( $options,
$this->getNumberLocalizer( $options ) );
- $vocabularyUriFormatter =
$this->getVocabularyUriFormatter( $options );
- return new QuantityHtmlFormatter( $options,
$decimalFormatter, $vocabularyUriFormatter );
- } elseif ( $format === SnakFormatter::FORMAT_HTML_DIFF ) {
+ if ( $format === SnakFormatter::FORMAT_HTML_DIFF ) {
$localizer = $this->getNumberLocalizer( $options );
$vocabularyUriFormatter =
$this->getVocabularyUriFormatter( $options );
return new QuantityDetailsFormatter( $localizer,
$vocabularyUriFormatter, $options );
+ } elseif ( $this->isHtmlFormat( $format ) ) {
+ $decimalFormatter = new DecimalFormatter( $options,
$this->getNumberLocalizer( $options ) );
+ $vocabularyUriFormatter =
$this->getVocabularyUriFormatter( $options );
+ return new QuantityHtmlFormatter( $options,
$decimalFormatter, $vocabularyUriFormatter );
} else {
- return null;
+ $decimalFormatter = new DecimalFormatter( $options,
$this->getNumberLocalizer( $options ) );
+ $vocabularyUriFormatter =
$this->getVocabularyUriFormatter( $options );
+ $plainFormatter = new QuantityFormatter( $options,
$decimalFormatter, $vocabularyUriFormatter );
+ return $this->escapeValueFormatter( $format,
$plainFormatter );
}
}
@@ -229,21 +269,20 @@
* @param string $format The desired target format, see
SnakFormatter::FORMAT_XXX
* @param FormatterOptions $options
*
- * @return GlobeCoordinateFormatter|null
+ * @return GlobeCoordinateFormatter
*/
public function newGlobeCoordinateFormatter( $format, FormatterOptions
$options ) {
- if ( $format === SnakFormatter::FORMAT_PLAIN ) {
+ if ( $format === SnakFormatter::FORMAT_HTML_DIFF ) {
+ return new GlobeCoordinateDetailsFormatter( $options );
+ } else {
$options->setOption(
GeoCoordinateFormatter::OPT_FORMAT, GeoCoordinateFormatter::TYPE_DMS );
$options->setOption(
GeoCoordinateFormatter::OPT_SPACING_LEVEL, array(
GeoCoordinateFormatter::OPT_SPACE_LATLONG
) );
$options->setOption(
GeoCoordinateFormatter::OPT_DIRECTIONAL, true );
- return new GlobeCoordinateFormatter( $options );
- } elseif ( $format === SnakFormatter::FORMAT_HTML_DIFF ) {
- return new GlobeCoordinateDetailsFormatter( $options );
- } else {
- return null;
+ $plainFormatter = new GlobeCoordinateFormatter(
$options );
+ return $this->escapeValueFormatter( $format,
$plainFormatter );
}
}
@@ -254,12 +293,10 @@
* @return MonolingualHtmlFormatter
*/
public function newMonolingualFormatter( $format, FormatterOptions
$options ) {
- if ( $format === SnakFormatter::FORMAT_PLAIN ) {
- return new MonolingualTextFormatter( $options );
- } elseif ( $format === SnakFormatter::FORMAT_HTML ) {
+ if ( $this->isHtmlFormat( $format ) ) {
return new MonolingualHtmlFormatter( $options,
$this->languageNameLookup );
} else {
- return null;
+ return $this->escapeValueFormatter( $format, new
MonolingualTextFormatter( $options ) );
}
}
diff --git
a/lib/tests/phpunit/formatters/OutputFormatSnakFormatterFactoryTest.php
b/lib/tests/phpunit/formatters/OutputFormatSnakFormatterFactoryTest.php
index 2bc85a6..3613d86 100644
--- a/lib/tests/phpunit/formatters/OutputFormatSnakFormatterFactoryTest.php
+++ b/lib/tests/phpunit/formatters/OutputFormatSnakFormatterFactoryTest.php
@@ -7,6 +7,7 @@
use DataValues\StringValue;
use Language;
use ValueFormatters\FormatterOptions;
+use ValueFormatters\StringFormatter;
use ValueFormatters\ValueFormatter;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Snak\PropertyValueSnak;
@@ -32,9 +33,7 @@
$self = $this;
$callbacks = array(
'VT:string' => function( $format, FormatterOptions
$options ) use ( $self ) {
- return $format === SnakFormatter::FORMAT_PLAIN
- ? $self->makeMockValueFormatter()
- : null;
+ return $self->makeMockValueFormatter( $format );
},
);
$valueFormatterFactory = new OutputFormatValueFormatterFactory(
@@ -57,14 +56,14 @@
);
}
- public function makeMockValueFormatter() {
+ public function makeMockValueFormatter( $format ) {
$mock = $this->getMock( 'ValueFormatters\ValueFormatter' );
$mock->expects( $this->any() )
->method( 'format' )
->will( $this->returnCallback(
- function( DataValue $value ) {
- return strval( $value->getValue() );
+ function( DataValue $value ) use ( $format ) {
+ return strval( $value->getValue() ) . '
(' . $format . ')';
}
) );
@@ -76,12 +75,12 @@
'plain' => array(
SnakFormatter::FORMAT_PLAIN,
new StringValue( 'foo' ),
- 'foo',
+ 'foo (text/plain)',
),
'html' => array(
SnakFormatter::FORMAT_HTML,
- new StringValue( 'b<a>r' ),
- 'b<a>r',
+ new StringValue( 'foo' ),
+ 'foo (text/html)',
),
);
}
@@ -135,6 +134,7 @@
$callbacks = array(
'VT:string' => function( $format, FormatterOptions
$options ) use ( $self ) {
$self->assertSame( 'de', $options->getOption(
ValueFormatter::OPT_LANG ) );
+ return new StringFormatter( $options );
},
);
$valueFormatterFactory = new OutputFormatValueFormatterFactory(
diff --git
a/lib/tests/phpunit/formatters/OutputFormatValueFormatterFactoryTest.php
b/lib/tests/phpunit/formatters/OutputFormatValueFormatterFactoryTest.php
index 1bf2557..16bddc6 100644
--- a/lib/tests/phpunit/formatters/OutputFormatValueFormatterFactoryTest.php
+++ b/lib/tests/phpunit/formatters/OutputFormatValueFormatterFactoryTest.php
@@ -69,11 +69,10 @@
$self = $this;
$factoryCallbacks = array(
'VT:string' => function( $format, FormatterOptions
$options ) use ( $self ) {
- return $format === SnakFormatter::FORMAT_PLAIN
? new StringFormatter() : null;
+ return new StringFormatter();
},
'PT:url' => function( $format, FormatterOptions
$options ) use ( $self ) {
- return $format === SnakFormatter::FORMAT_PLAIN
|| $format === SnakFormatter::FORMAT_WIKI
- ? new StringFormatter() : null;
+ return new StringFormatter();
},
);
@@ -106,19 +105,7 @@
null,
'/^{foo&bar}$/'
),
- 'wiki escape' => array(
- SnakFormatter::FORMAT_WIKI,
- new StringValue( '{foo&bar}' ),
- null,
- '/^{foo&bar}$/'
- ),
- 'html escape' => array(
- SnakFormatter::FORMAT_HTML,
- new StringValue( '{foo&bar}' ),
- null,
- '/^{foo&bar}$/'
- ),
- 'no wiki escape for url' => array(
+ 'wikitext url' => array(
SnakFormatter::FORMAT_WIKI,
new StringValue( 'http://acme.com/?foo&bar' ),
'url',
diff --git
a/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
b/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
index 6a49c15..26d7361 100644
--- a/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
+++ b/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
@@ -9,6 +9,7 @@
use DataValues\QuantityValue;
use DataValues\StringValue;
use DataValues\TimeValue;
+use DataValues\UnDeserializableValue;
use Language;
use MediaWikiTestCase;
use Title;
@@ -117,18 +118,18 @@
}
/**
- * @param string $type
+ * @param string $functionName The factory function to call
* @param string $format
* @param FormatterOptions $options
*
* @return mixed
*/
- private function getFormatterForDataType( $type, $format, $options ) {
+ private function callFactoryFunction( $functionName, $format, $options
) {
$builders = $this->newWikibaseValueFormatterBuilders(
$this->getTitleLookup()
);
- $factory = array( $builders, 'new' . $type . 'Formatter' );
+ $factory = array( $builders, $functionName );
$formatter = call_user_func( $factory, $format, $options );
$this->assertInstanceOf( 'ValueFormatters\ValueFormatter',
$formatter );
@@ -136,16 +137,76 @@
}
/**
+ public function testFormatterForValueType_formats() {
+ $formats = array(
+ SnakFormatter::FORMAT_PLAIN,
+ SnakFormatter::FORMAT_WIKI,
+ SnakFormatter::FORMAT_HTML,
+ SnakFormatter::FORMAT_HTML_DIFF,
+ SnakFormatter::FORMAT_HTML_WIDGET
+ );
+
+ $valueTypes = array(
+ 'string',
+ 'wikibase-entityid',
+ 'monolingualtext',
+ 'time',
+ 'globecoordinate',
+ 'quantity',
+ );
+
+ $options = new FormatterOptions();
+
+ foreach ( $formats as $format ) {
+ foreach ( $valueTypes as $type ) {
+ // getFormatterForValueType asserts that a
valid formatter is returned
+ $this->getFormatterForValueType( $type,
$format, $options );
+ }
+ }
+ }
+
+ public function testNewFormatter_formats() {
+ $formats = array(
+ SnakFormatter::FORMAT_PLAIN,
+ SnakFormatter::FORMAT_WIKI,
+ SnakFormatter::FORMAT_HTML,
+ SnakFormatter::FORMAT_HTML_DIFF,
+ SnakFormatter::FORMAT_HTML_WIDGET
+ );
+
+ // Note: these are not actual data-type IDs,
+ $functionNames = array(
+ 'newStringFormatter',
+ 'newUrlFormatter',
+ 'newCommonsMediaFormatter',
+ 'newEntityIdFormatter',
+ 'newMonolingualFormatter',
+ 'newTimeFormatter',
+ 'newGlobeCoordinateFormatter',
+ 'newQuantityFormatter',
+ );
+
+ $options = new FormatterOptions();
+
+ foreach ( $formats as $format ) {
+ foreach ( $functionNames as $function ) {
+ // callFactoryFunction asserts that a valid
formatter is returned
+ $this->callFactoryFunction( $function, $format,
$options );
+ }
+ }
+ }
+
* @dataProvider provideNewFormatter
*/
public function testNewFormatter(
- $type,
+ $formatterName,
$format,
FormatterOptions $options,
DataValue $value,
$expected
) {
- $formatter = $this->getFormatterForDataType( $type, $format,
$options );
+ $functionName = 'new' . ucfirst( $formatterName ) . 'Formatter';
+ $formatter = $this->callFactoryFunction( $functionName,
$format, $options );
$text = $formatter->format( $value );
$this->assertRegExp( $expected, $text );
@@ -153,6 +214,38 @@
public function provideNewFormatter() {
return array(
+ // String
+ 'plain string' => array(
+ 'String',
+ SnakFormatter::FORMAT_PLAIN,
+ $this->newFormatterOptions(),
+ new StringValue( 'foo bar' ),
+ '@^foo bar$@'
+ ),
+ 'wikitext string' => array(
+ 'String',
+ SnakFormatter::FORMAT_WIKI,
+ $this->newFormatterOptions(),
+ new StringValue( 'foo[bar]' ),
+ '@^foo[bar]$@'
+ ),
+ 'html string' => array(
+ 'String',
+ SnakFormatter::FORMAT_HTML,
+ $this->newFormatterOptions(),
+ new StringValue( 'foo<bar>' ),
+ '@^foo<bar>$@'
+ ),
+
+ // UnDeserializableValue
+ 'plain bad value' => array(
+ 'UnDeserializableValue',
+ SnakFormatter::FORMAT_PLAIN,
+ $this->newFormatterOptions(),
+ new UnDeserializableValue( 'foo bar', 'xyzzy',
'broken' ),
+ '@invalid@'
+ ),
+
// Url
'plain url' => array(
'Url',
@@ -307,19 +400,47 @@
}
/**
+ 'wikitext escape' => array(
+ SnakFormatter::FORMAT_WIKI,
+ $this->newFormatterOptions(),
+ new StringValue( '[[foo]]' ),
+ '@^[[foo]]$@'
+ ),
+
+ 'html escape' => array(
+ SnakFormatter::FORMAT_HTML,
+ $this->newFormatterOptions(),
+ new StringValue( '<foo>' ),
+ '@^<foo>$@'
+ ),
+
+ 'html details escape' => array(
+ SnakFormatter::FORMAT_HTML_DIFF,
+ $this->newFormatterOptions(),
+ new StringValue( '<foo>' ),
+ '@^<foo>$@'
+ ),
+
+ 'html widget escape' => array(
+ SnakFormatter::FORMAT_HTML_WIDGET,
+ $this->newFormatterOptions(),
+ new StringValue( '<foo>' ),
+ '@^<foo>$@'
+ ),
+
* In case WikibaseValueFormatterBuilders doesn't have a
EntityTitleLookup it returns
* a formatter which doesn't link the entity id.
*
* @dataProvider provideNewFormatter_noTitleLookup
*/
public function testNewFormatter_noTitleLookup(
- $name,
+ $functionName,
$format,
FormatterOptions $options,
DataValue $value,
$expected
) {
- $formatter = $this->getFormatterForDataType( $name, $format,
$options );
+ $formatter = $this->callFactoryFunction( $functionName,
$format, $options );
$text = $formatter->format( $value );
$this->assertRegExp( $expected, $text );
@@ -328,14 +449,14 @@
public function provideNewFormatter_noTitleLookup() {
return array(
'plain item label' => array(
- 'EntityId',
+ 'newEntityIdFormatter',
SnakFormatter::FORMAT_PLAIN,
$this->newFormatterOptions(),
new EntityIdValue( new ItemId( 'Q5' ) ),
'@^Label for Q5$@'
),
'item link' => array(
- 'EntityId',
+ 'newEntityIdFormatter',
SnakFormatter::FORMAT_HTML,
$this->newFormatterOptions(),
new EntityIdValue( new ItemId( 'Q5' ) ),
@@ -348,12 +469,12 @@
* @dataProvider provideNewFormatter_LabelDescriptionLookupOption
*/
public function testNewFormatter_LabelDescriptionLookupOption(
- $name,
+ $functionName,
FormatterOptions $options,
DataValue $value,
$expected
) {
- $formatter = $this->getFormatterForDataType( $name,
SnakFormatter::FORMAT_HTML, $options );
+ $formatter = $this->callFactoryFunction( $functionName,
SnakFormatter::FORMAT_HTML, $options );
$text = $formatter->format( $value );
$this->assertRegExp( $expected, $text );
@@ -370,7 +491,7 @@
return array(
'language option' => array(
- 'EntityId',
+ 'newEntityIdFormatter',
new FormatterOptions( array(
ValueFormatter::OPT_LANG => 'de',
) ),
@@ -378,7 +499,7 @@
'@>Name für Q5<@'
),
'fallback option' => array(
- 'EntityId',
+ 'newEntityIdFormatter',
new FormatterOptions( array(
FormatterLabelDescriptionLookupFactory::OPT_LANGUAGE_FALLBACK_CHAIN =>
$fallbackChain,
) ),
@@ -386,7 +507,7 @@
'@>Name für Q5<@'
),
'LabelDescriptionLookup option' => array(
- 'EntityId',
+ 'newEntityIdFormatter',
new FormatterOptions( array(
FormatterLabelDescriptionLookupFactory::OPT_LABEL_DESCRIPTION_LOOKUP =>
$labelDescriptionLookup,
) ),
diff --git a/repo/WikibaseRepo.datatypes.php b/repo/WikibaseRepo.datatypes.php
index 75e5ae5..cbf5f56 100644
--- a/repo/WikibaseRepo.datatypes.php
+++ b/repo/WikibaseRepo.datatypes.php
@@ -82,7 +82,8 @@
return array(
'VT:bad' => array(
'formatter-factory-callback' => function( $format,
FormatterOptions $options ) {
- return $format === SnakFormatter::FORMAT_PLAIN
? new UnDeserializableValueFormatter( $options ) : null;
+ $factory =
WikibaseRepo::getDefaultFormatterBuilders();
+ return
$factory->newUnDeserializableValueFormatter( $format, $options );
}
),
'PT:commonsMedia' => array(
@@ -182,7 +183,8 @@
},
'parser-factory-callback' => $newStringParser,
'formatter-factory-callback' => function( $format,
FormatterOptions $options ) {
- return $format === SnakFormatter::FORMAT_PLAIN
? new StringFormatter( $options ) : null;
+ $factory =
WikibaseRepo::getDefaultFormatterBuilders();
+ return $factory->newStringFormatter( $format,
$options );
},
'rdf-builder-factory-callback' => function (
$mode,
--
To view, visit https://gerrit.wikimedia.org/r/254064
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I04e6db7524f0903c5d088adec5a0163fd35b3d69
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Adrian Lang <adrian.he...@wikimedia.de>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
