jenkins-bot has submitted this change and it was merged. Change subject: Make EntityIdLabelFormatter more flexible ......................................................................
Make EntityIdLabelFormatter more flexible This allows EntityIdValue instances to be formatted based on options alone. Bug: 53745 Change-Id: I8ef6bf41d59cc54d9867714cc46ec8cb909da5f2 --- M lib/includes/formatters/EntityIdLabelFormatter.php M lib/includes/formatters/WikibaseSnakFormatterBuilders.php M lib/tests/phpunit/formatters/EntityIdLabelFormatterTest.php M lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php 4 files changed, 116 insertions(+), 83 deletions(-) Approvals: Tobias Gritschacher: Looks good to me, approved jenkins-bot: Verified diff --git a/lib/includes/formatters/EntityIdLabelFormatter.php b/lib/includes/formatters/EntityIdLabelFormatter.php index 3628b9a..b80f484 100644 --- a/lib/includes/formatters/EntityIdLabelFormatter.php +++ b/lib/includes/formatters/EntityIdLabelFormatter.php @@ -8,8 +8,8 @@ use ValueFormatters\ValueFormatterBase; use Wikibase\DataModel\Entity\EntityId; use Wikibase\DataModel\Entity\EntityIdValue; -use Wikibase\Entity; use Wikibase\EntityLookup; +use Wikibase\LanguageFallbackChain; /** * @since 0.4 @@ -17,14 +17,24 @@ * @file * @ingroup WikibaseLib * - * @todo: needs an option to turn off label resolution. - * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > * @author Katie Filbert < [email protected] > + * @author Daniel Kinzler + * + * @todo: once this is no longer used directly, rename it to EntityIdValueFormatter + * @todo: add support for language fallback chains */ class EntityIdLabelFormatter extends ValueFormatterBase { + /** + * Whether we should try to find the label of the entity + */ + const OPT_RESOLVE_ID = 'resolveEntityId'; + + /** + * What we should do if we can't find the label. + */ const OPT_LABEL_FALLBACK = 'labelFallback'; const FALLBACK_PREFIXED_ID = 0; @@ -37,14 +47,10 @@ protected $entityLookup; /** - * @var EntityIdFormatter|null - */ - protected $idFormatter = null; - - /** * @since 0.4 * - * @param FormatterOptions $options + * @param FormatterOptions $options Supported options: OPT_RESOLVE_ID (boolean), + * OPT_LABEL_FALLBACK (FALLBACK_XXX) * @param EntityLookup $entityLookup * * @throws \InvalidArgumentException @@ -54,6 +60,7 @@ $this->entityLookup = $entityLookup; + $this->defaultOption( self::OPT_RESOLVE_ID, true ); $this->defaultOption( self::OPT_LABEL_FALLBACK, self::FALLBACK_PREFIXED_ID ); $fallbackOptionIsValid = in_array( @@ -66,15 +73,21 @@ ); if ( !$fallbackOptionIsValid ) { - throw new InvalidArgumentException( 'Got an invalid label fallback option' ); + throw new InvalidArgumentException( 'Bad value for OPT_LABEL_FALLBACK option' ); + } + + if ( !is_bool( $this->getOption( self::OPT_RESOLVE_ID ) ) ) { + throw new InvalidArgumentException( 'Bad value for OPT_RESOLVE_ID option: must be a boolean' ); } } /** * @param EntityIdFormatter $idFormatter + * @deprecated + * @todo remove this when it's no longer being called */ public function setIdFormatter( EntityIdFormatter $idFormatter ) { - $this->idFormatter = $idFormatter; + // noop } /** @@ -82,9 +95,10 @@ * * @since 0.4 * - * @param EntityIdValue|EntityId $value The value to format + * @param EntityId|EntityIdValue $value The value to format * * @return string + * * @throws RuntimeException * @throws InvalidArgumentException */ @@ -94,26 +108,25 @@ } if ( !( $value instanceof EntityId ) ) { - throw new InvalidArgumentException( 'Data value type mismatch. Expected an EntityId.' ); + throw new InvalidArgumentException( 'Data value type mismatch. Expected an EntityId or EntityIdValue.' ); } - $label = $this->lookupItemLabel( $value ); + if ( $this->getOption( self::OPT_RESOLVE_ID ) ) { + $label = $this->lookupItemLabel( $value ); + } else { + $label = false; + } if ( $label === false ) { - switch ( $this->getOption( 'labelFallback' ) ) { + switch ( $this->getOption( self::OPT_LABEL_FALLBACK ) ) { case self::FALLBACK_EMPTY_STRING: $label = ''; break; case self::FALLBACK_PREFIXED_ID: - if ( $this->idFormatter === null ) { - throw new RuntimeException( 'Cannot format the id using a prefix without the EntityIdFormatter being set' ); - } - - $label = $this->idFormatter->format( $value ); + $label = $value->getPrefixedId(); break; default: - // TODO: implement: return formatting error - $label = 'TODO: ERROR: label not found'; + throw new FormattingException( 'No label found for ' . $value ); } } @@ -137,16 +150,18 @@ return false; } - $languageFallbackChain = $this->getOption( self::OPT_LANG ); + /* @var LanguageFallbackChain $languageFallbackChain */ + if ( $this->options->hasOption( 'languages' ) ) { + $languageFallbackChain = $this->getOption( 'languages' ); + } else { + $languageFallbackChain = $this->getOption( self::OPT_LANG ); + } // back-compat for usages where self::OPT_LANG is a string as a language code if ( is_string( $languageFallbackChain ) ) { return $entity->getLabel( $languageFallbackChain ); } - /** - * @var Entity $entity - */ $extractedData = $languageFallbackChain->extractPreferredValue( $entity->getLabels() ); if ( $extractedData === null ) { diff --git a/lib/includes/formatters/WikibaseSnakFormatterBuilders.php b/lib/includes/formatters/WikibaseSnakFormatterBuilders.php index 4c11113..9b4dbf2 100644 --- a/lib/includes/formatters/WikibaseSnakFormatterBuilders.php +++ b/lib/includes/formatters/WikibaseSnakFormatterBuilders.php @@ -2,12 +2,16 @@ namespace Wikibase\Lib; +use Language; use ValueFormatters\FormatterOptions; use ValueFormatters\ValueFormatter; use ValueFormatters\ValueFormatterFactory; use Wikibase\Client\WikibaseClient; use Wikibase\EntityLookup; use Wikibase\Item; +use Wikibase\LanguageFallbackChain; +use Wikibase\LanguageFallbackChainFactory; +use Wikibase\LanguageWithConversion; use Wikibase\Repo\WikibaseRepo; /** @@ -32,6 +36,11 @@ * @var PropertyDataTypeLookup */ protected $propertyDataTypeLookup; + + /** + * @var Language + */ + protected $defaultLanguage; /** * This determines which value is formatted how by providing a formatter mapping @@ -89,13 +98,16 @@ /** * @param EntityLookup $lookup * @param PropertyDataTypeLookup $propertyDataTypeLookup + * @param Language $defaultLanguage */ public function __construct( EntityLookup $lookup, - PropertyDataTypeLookup $propertyDataTypeLookup + PropertyDataTypeLookup $propertyDataTypeLookup, + Language $defaultLanguage ) { $this->propertyDataTypeLookup = $propertyDataTypeLookup; $this->entityLookup = $lookup; + $this->defaultLanguage = $defaultLanguage; } /** @@ -115,24 +127,6 @@ } /** - * Returns a system message in the given target language. - * - * @param string $key - * @param string $language - * - * @return \Message - */ - private function getMessage( $key, $language = null ) { - $msg = wfMessage( $key ); - - if ( $language !== null ) { - $msg = $msg->inLanguage( $language ); - } - - return $msg; - } - - /** * Returns a DispatchingSnakFormatter for the given format, that will dispatch based on * the snak type. The instance returned by this method will cover all standard snak types. * @@ -143,9 +137,10 @@ * @return DispatchingSnakFormatter */ public function buildDispatchingSnakFormatter( SnakFormatterFactory $factory, $format, FormatterOptions $options ) { - $language = $options->hasOption( ValueFormatter::OPT_LANG ) ? $options->getOption( ValueFormatter::OPT_LANG ) : null; - $noValueSnakFormatter = new MessageSnakFormatter( 'novalue', $this->getMessage( 'wikibase-snakview-snaktypeselector-novalue', $language ), $format ); - $someValueSnakFormatter = new MessageSnakFormatter( 'somevalue', $this->getMessage( 'wikibase-snakview-snaktypeselector-somevalue', $language ), $format ); + $this->initLanguageDefaults( $options ); + + $noValueSnakFormatter = new MessageSnakFormatter( 'novalue', wfMessage( 'wikibase-snakview-snaktypeselector-novalue', $this->defaultLanguage ), $format ); + $someValueSnakFormatter = new MessageSnakFormatter( 'somevalue', wfMessage( 'wikibase-snakview-snaktypeselector-somevalue', $this->defaultLanguage ), $format ); $valueSnakFormatter = $this->buildValueSnakFormatter( $factory, $format, $options ); $formatters = array( @@ -158,6 +153,42 @@ } /** + * Initializes the options keys ValueFormatter::OPT_LANG and 'languages' if + * they are not yet set. + * + * @param FormatterOptions $options + * + * @throws \InvalidArgumentException + * @todo : Sort out how the desired language is specified. We have two language options, + * each accepting different ways of specifying the language. That's horrible. + */ + private function initLanguageDefaults( $options ) { + $languageFallbackChainFactory = new LanguageFallbackChainFactory(); + + if ( !$options->hasOption( ValueFormatter::OPT_LANG ) ) { + $options->setOption( ValueFormatter::OPT_LANG, $this->defaultLanguage->getCode() ); + } + + $lang = $options->getOption( ValueFormatter::OPT_LANG ); + if ( !is_string( $lang ) ) { + throw new \InvalidArgumentException( 'The value of OPT_LANG must be a language code. For a fallback chain, use the `languages` option.' ); + } + + if ( !$options->hasOption( 'languages' ) ) { + $fallbackMode = ( + LanguageFallbackChainFactory::FALLBACK_VARIANTS + | LanguageFallbackChainFactory::FALLBACK_OTHERS + | LanguageFallbackChainFactory::FALLBACK_SELF ); + + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguageCode( $lang, $fallbackMode ) ); + } + + if ( !( $options->getOption( 'languages' ) instanceof LanguageFallbackChain ) ) { + throw new \InvalidArgumentException( 'The value of the `languages` option must be an instance of LanguageFallbackChain.' ); + } + } + + /** * Returns a DispatchingSnakFormatter for the given format, that will dispatch based on * the snak type. The instance returned by this method will cover all standard snak types. * diff --git a/lib/tests/phpunit/formatters/EntityIdLabelFormatterTest.php b/lib/tests/phpunit/formatters/EntityIdLabelFormatterTest.php index 7a3ec4f..a5aadd7 100644 --- a/lib/tests/phpunit/formatters/EntityIdLabelFormatterTest.php +++ b/lib/tests/phpunit/formatters/EntityIdLabelFormatterTest.php @@ -8,7 +8,6 @@ use Wikibase\DataModel\Entity\EntityIdValue; use Wikibase\DataModel\Entity\ItemId; use Wikibase\DataModel\Entity\PropertyId; -use Wikibase\Lib\EntityIdFormatter; use Wikibase\Lib\EntityIdLabelFormatter; use Wikibase\Item; use Wikibase\LanguageFallbackChainFactory; @@ -29,6 +28,7 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > + * @author Daniel Kinzler */ class EntityIdLabelFormatterTest extends \PHPUnit_Framework_TestCase { @@ -39,16 +39,12 @@ $entity->setLabel( 'en', 'foo' ); $entity->setLabel( 'nl', 'bar' ); $entity->setLabel( 'zh-cn', '测试' ); - $entity->setId( 42 ); + $entity->setId( new ItemId( 'Q42' ) ); + $loader->putEntity( $entity ); return $loader; - } - - protected function newEntityIdFormatter() { - $options = new FormatterOptions(); - return new EntityIdFormatter( $options ); } /** @@ -64,11 +60,11 @@ $options = new FormatterOptions(); $options->setOption( EntityIdLabelFormatter::OPT_LANG, 'en' ); - $argLists[] = array( new ItemId( 'q42' ), 'foo', $options ); + $argLists[] = array( new ItemId( 'Q42' ), 'foo', $options ); $options = new FormatterOptions(); - $options->setOption( EntityIdLabelFormatter::OPT_LANG, $languageFallbackChainFactory->newFromLanguage( Language::factory( 'en' ) ) ); + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguage( Language::factory( 'en' ) ) ); $argLists[] = array( new ItemId( 'q42' ), 'foo', $options ); @@ -76,35 +72,35 @@ $options = new FormatterOptions(); $options->setOption( EntityIdLabelFormatter::OPT_LANG, 'nl' ); - $argLists[] = array( new ItemId( 'q42' ), 'bar', $options ); + $argLists[] = array( new ItemId( 'Q42' ), 'bar', $options ); $options = new FormatterOptions(); $options->setOption( EntityIdLabelFormatter::OPT_LANG, 'de' ); - $argLists[] = array( new ItemId( 'q42' ), 'Q42', $options ); + $argLists[] = array( new ItemId( 'Q42' ), 'Q42', $options ); $options = new FormatterOptions(); - $options->setOption( EntityIdLabelFormatter::OPT_LANG, $languageFallbackChainFactory->newFromLanguage( Language::factory( 'de' ) ) ); + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguage( Language::factory( 'de' ) ) ); $argLists[] = array( new ItemId( 'q42' ), 'foo', $options ); $options = new FormatterOptions(); - $options->setOption( EntityIdLabelFormatter::OPT_LANG, $languageFallbackChainFactory->newFromLanguage( Language::factory( 'zh' ) ) ); + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguage( Language::factory( 'zh' ) ) ); $argLists[] = array( new ItemId( 'q42' ), '测试', $options ); $options = new FormatterOptions(); - $options->setOption( EntityIdLabelFormatter::OPT_LANG, $languageFallbackChainFactory->newFromLanguage( Language::factory( 'zh-tw' ) ) ); + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguage( Language::factory( 'zh-tw' ) ) ); $argLists[] = array( new ItemId( 'q42' ), '測試', $options ); $options = new FormatterOptions(); - $options->setOption( EntityIdLabelFormatter::OPT_LANG, $languageFallbackChainFactory->newFromLanguage( + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguage( Language::factory( 'zh-tw' ), LanguageFallbackChainFactory::FALLBACK_SELF ) ); @@ -112,7 +108,7 @@ $options = new FormatterOptions(); - $options->setOption( EntityIdLabelFormatter::OPT_LANG, $languageFallbackChainFactory->newFromLanguage( + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguage( Language::factory( 'zh-tw' ), LanguageFallbackChainFactory::FALLBACK_SELF | LanguageFallbackChainFactory::FALLBACK_VARIANTS ) ); @@ -121,7 +117,7 @@ $options = new FormatterOptions(); - $options->setOption( EntityIdLabelFormatter::OPT_LANG, $languageFallbackChainFactory->newFromLanguage( + $options->setOption( 'languages', $languageFallbackChainFactory->newFromLanguage( Language::factory( 'sr' ) ) ); @@ -132,37 +128,27 @@ $options->setOption( EntityIdLabelFormatter::OPT_LANG, 'de' ); $options->setOption( EntityIdLabelFormatter::OPT_LABEL_FALLBACK, EntityIdLabelFormatter::FALLBACK_EMPTY_STRING ); - $argLists[] = array( new ItemId( 'q42' ), '', $options ); - + $argLists[] = array( new EntityIdValue( new ItemId( 'Q42' ) ), '', $options ); $options = new FormatterOptions(); $options->setOption( EntityIdLabelFormatter::OPT_LANG, 'en' ); - $options->setOption( EntityIdLabelFormatter::OPT_LABEL_FALLBACK, EntityIdLabelFormatter::FALLBACK_EMPTY_STRING ); + $options->setOption( EntityIdLabelFormatter::OPT_RESOLVE_ID, false ); - $argLists[] = array( new ItemId( 'q42' ), 'foo', $options ); + $argLists[] = array( new EntityIdValue( new ItemId( 'Q42' ) ), 'Q42', $options ); $options = new FormatterOptions(); $options->setOption( EntityIdLabelFormatter::OPT_LANG, 'en' ); - $argLists[] = array( new ItemId( 'q9001' ), 'Q9001', $options ); + $argLists[] = array( new EntityIdValue( new ItemId( 'Q9001' ) ), 'Q9001', $options ); $options = new FormatterOptions(); $options->setOption( EntityIdLabelFormatter::OPT_LANG, 'en' ); - $argLists[] = array( new PropertyId( 'p9001' ), 'P9001', $options ); + $argLists[] = array( new PropertyId( 'P9001' ), 'P9001', $options ); - $allArgLists = array(); - - foreach ( $argLists as $argList ) { - $allArgLists[] = $argList; - - $argList[0] = new EntityIdValue( $argList[0] ); - $allArgLists[] = $argList; - } - - return $allArgLists; + return $argLists; } /** @@ -174,7 +160,6 @@ */ public function testParseWithValidArguments( $entityId, $expectedString, FormatterOptions $formatterOptions ) { $formatter = new EntityIdLabelFormatter( $formatterOptions, $this->newEntityLoader() ); - $formatter->setIdFormatter( $this->newEntityIdFormatter() ); $formattedValue = $formatter->format( $entityId ); diff --git a/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php b/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php index 102f37e..80a8f70 100644 --- a/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php +++ b/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php @@ -46,7 +46,9 @@ ->method( 'getEntity' ) ->will( $this->returnValue( $entity ) ); - return new WikibaseSnakFormatterBuilders( $entityLookup, $typeLookup ); + $lang = \Language::factory( 'en' ); + + return new WikibaseSnakFormatterBuilders( $entityLookup, $typeLookup, $lang ); } /** -- To view, visit https://gerrit.wikimedia.org/r/82869 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I8ef6bf41d59cc54d9867714cc46ec8cb909da5f2 Gerrit-PatchSet: 11 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: Denny Vrandecic <[email protected]> Gerrit-Reviewer: Jeroen De Dauw <[email protected]> Gerrit-Reviewer: Tobias Gritschacher <[email protected]> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
