jenkins-bot has submitted this change and it was merged.
Change subject: Fix property parser function, when property label not resolved
......................................................................
Fix property parser function, when property label not resolved
Change-Id: I7e650f0b142ec8464607e093afa5d27b6b4be126
---
M client/includes/parserhooks/PropertyParserFunction.php
M client/includes/parserhooks/PropertyParserFunctionRenderer.php
M
client/tests/phpunit/includes/parserhooks/PropertyParserFunctionRendererTest.php
M lib/WikibaseLib.classes.php
A lib/includes/PropertyLabelNotResolvedException.php
M lib/tests/phpunit/MockPropertyLabelResolver.php
6 files changed, 156 insertions(+), 83 deletions(-)
Approvals:
Hoo man: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/includes/parserhooks/PropertyParserFunction.php
b/client/includes/parserhooks/PropertyParserFunction.php
index a143ad9..4e9214b 100644
--- a/client/includes/parserhooks/PropertyParserFunction.php
+++ b/client/includes/parserhooks/PropertyParserFunction.php
@@ -2,18 +2,20 @@
namespace Wikibase;
+use InvalidArgumentException;
+use Language;
+use Parser;
+use Status;
use ValueFormatters\FormatterOptions;
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\SimpleSiteLink;
+use Wikibase\Lib\PropertyLabelNotResolvedException;
use Wikibase\Lib\SnakFormatter;
/**
* Handler of the {{#property}} parser function.
*
* @since 0.4
- *
- * @file
- * @ingroup WikibaseClient
*
* @licence GNU GPL v2+
* @author Katie Filbert < [email protected] >
@@ -24,7 +26,7 @@
class PropertyParserFunction {
/**
- * @var \Parser
+ * @var Parser
*/
protected $parser;
@@ -44,14 +46,12 @@
protected $propertyLabelResolver;
/**
- * Constructor.
- *
- * @param \Parser $parser
+ * @param Parser $parser
* @param EntityId $entityId
- * @param EntityLookup $entityLookup
- * @param PropertyLabelResolver $propertyLabelResolver
+ * @param EntityLookup $entityLookup
+ * @param PropertyLabelResolver $propertyLabelResolver
*/
- public function __construct( \Parser $parser, EntityId $entityId,
+ public function __construct( Parser $parser, EntityId $entityId,
EntityLookup $entityLookup, PropertyLabelResolver
$propertyLabelResolver
) {
$this->parser = $parser;
@@ -63,12 +63,12 @@
/**
* Check whether variants are used in this parser run.
*
- * @param \Parser $parser
+ * @param Parser $parser
* @return bool
*/
public function isParserUsingVariants() {
$parserOptions = $this->parser->getOptions();
- return $this->parser->OutputType() === \Parser::OT_HTML &&
!$parserOptions->getInterfaceMessage()
+ return $this->parser->OutputType() === Parser::OT_HTML &&
!$parserOptions->getInterfaceMessage()
&& !$parserOptions->getDisableContentConversion();
}
@@ -94,10 +94,10 @@
/**
* Build a PropertyParserFunctionRenderer object for a given language.
*
- * @param \Language $language
+ * @param Language $language
* @return PropertyParserFunctionRenderer
*/
- public function getRenderer( \Language $language ) {
+ public function getRenderer( Language $language ) {
wfProfileIn( __METHOD__ );
$wikibaseClient = WikibaseClient::getDefaultInstance();
@@ -125,20 +125,25 @@
/**
* @param string $propertyLabel property label or ID (pXXX)
- * @param \Language $language
+ * @param Language $language
*
* @return string
*/
- public function renderInLanguage( $propertyLabel, \Language $language )
{
-
+ public function renderInLanguage( $propertyLabel, Language $language ) {
$renderer = $this->getRenderer( $language );
- $status = $renderer->renderForEntityId( $this->entityId,
$propertyLabel );
+ try {
+ $status = $renderer->renderForEntityId(
$this->entityId, $propertyLabel );
+ } catch ( PropertyLabelNotResolvedException $ex ) {
+ $status = $this->getStatusForException( $propertyLabel,
$ex->getMessage() );
+ } catch ( InvalidArgumentException $ex ) {
+ $status = $this->getStatusForException( $propertyLabel,
$ex->getMessage() );
+ }
if ( !$status->isGood() ) {
// stuff the error messages into the ParserOutput, so
we can render them later somewhere
-
$errors = $this->parser->getOutput()->getExtensionData(
'wikibase-property-render-errors' );
+
if ( $errors === null ) {
$errors = array();
}
@@ -147,9 +152,25 @@
$errors[] = $status->getWikiText();
$this->parser->getOutput()->setExtensionData(
'wikibase-property-render-errors', $errors );
+
+ return '';
}
- return $status->isOK() ? $status->getValue() : '';
+ return $status->getValue();
+ }
+
+ /**
+ * @param string $propertyLabel
+ * @param string $message
+ *
+ * @return Status
+ */
+ private function getStatusForException( $propertyLabel, $message ) {
+ return Status::newFatal(
+ 'wikibase-property-render-error',
+ $propertyLabel,
+ $message
+ );
}
/**
@@ -162,7 +183,7 @@
$textArray = array();
foreach ( $variants as $variantCode ) {
- $variantLanguage = \Language::factory( $variantCode );
+ $variantLanguage = Language::factory( $variantCode );
$variantText = $this->renderInLanguage( $propertyLabel,
$variantLanguage );
// LanguageConverter doesn't handle empty strings
correctly, and it's more difficult
// to fix the issue there, as it's using empty string
as a special value.
@@ -200,12 +221,12 @@
/**
* @since 0.4
*
- * @param \Parser &$parser
+ * @param Parser &$parser
* @param string $propertyLabel property label or ID (pXXX)
*
* @return array
*/
- public static function render( \Parser $parser, $propertyLabel ) {
+ public static function render( Parser $parser, $propertyLabel ) {
wfProfileIn( __METHOD__ );
$siteId = Settings::get( 'siteGlobalID' );
diff --git a/client/includes/parserhooks/PropertyParserFunctionRenderer.php
b/client/includes/parserhooks/PropertyParserFunctionRenderer.php
index 8873563..7ca7647 100644
--- a/client/includes/parserhooks/PropertyParserFunctionRenderer.php
+++ b/client/includes/parserhooks/PropertyParserFunctionRenderer.php
@@ -2,9 +2,14 @@
namespace Wikibase;
+use Exception;
+use InvalidArgumentException;
+use Language;
+use Status;
use ValueParsers\ParseException;
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\Lib\PropertyLabelNotResolvedException;
use Wikibase\Lib\SnakFormatter;
/**
@@ -23,7 +28,7 @@
*/
class PropertyParserFunctionRenderer {
- /* @var \Language */
+ /* @var Language */
protected $language;
/* @var EntityLookup */
@@ -39,13 +44,13 @@
protected $snaksFormatter;
/**
- * @param \Language $language
+ * @param Language $language
* @param EntityLookup $entityLookup
* @param PropertyLabelResolver $propertyLabelResolver
* @param ParserErrorMessageFormatter $errorFormatter
- * @param Lib\SnakFormatter $snaksFormatter
+ * @param SnakFormatter $snaksFormatter
*/
- public function __construct( \Language $language,
+ public function __construct( Language $language,
EntityLookup $entityLookup, PropertyLabelResolver
$propertyLabelResolver,
ParserErrorMessageFormatter $errorFormatter, SnakFormatter
$snaksFormatter ) {
$this->language = $language;
@@ -76,18 +81,19 @@
/**
* @param string $string
* @return PropertyId
+ * @throws InvalidArgumentException
+ * @throws PropertyLabelNotResolvedException
*/
private function getPropertyIdFromIdSerializationOrLabel( $string ) {
$idParser =
WikibaseClient::getDefaultInstance()->getEntityIdParser();
try {
$propertyId = $idParser->parse( $string );
- }
- catch ( ParseException $ex ) {
- $propertyId = null;
- }
- if ( $propertyId === null ) {
+ if ( ! ( $propertyId instanceof PropertyId ) ) {
+ throw new InvalidArgumentException( 'Not a
valid property id' );
+ }
+ } catch ( ParseException $ex ) {
//XXX: It might become useful to give the
PropertyLabelResolver a hint as to which
// properties may become relevant during the
present request, namely the ones
// used by the Item linked to the current page.
This could be done with
@@ -97,11 +103,11 @@
$propertyIds =
$this->propertyLabelResolver->getPropertyIdsForLabels( array( $string ) );
- if ( empty( $propertyIds ) ) {
- return new Claims();
- } else {
- $propertyId = $propertyIds[$string];
+ if ( $propertyIds === null || empty( $propertyIds ) ) {
+ throw new PropertyLabelNotResolvedException(
$this->language->getCode(), $string );
}
+
+ $propertyId = $propertyIds[$string];
}
return $propertyId;
@@ -129,37 +135,31 @@
/**
* @param EntityId $entityId
- * @param string $propertyLabel
+ * @param string $propertyLabel
*
- * @return \Status a status object wrapping a wikitext string
+ * @return Status a status object wrapping a wikitext string
*/
public function renderForEntityId( EntityId $entityId, $propertyLabel )
{
wfProfileIn( __METHOD__ );
- try {
- $entity = $this->entityLookup->getEntity( $entityId );
+ $entity = $this->entityLookup->getEntity( $entityId );
- if ( !$entity ) {
- wfProfileOut( __METHOD__ );
- return \Status::newGood( '' );
- }
-
- $claims = $this->getClaimsForProperty( $entity,
$propertyLabel );
-
- if ( $claims->isEmpty() ) {
- wfProfileOut( __METHOD__ );
- return \Status::newGood( '' );
- }
-
- $snakList = $claims->getMainSnaks();
- $text = $this->formatSnakList( $snakList,
$propertyLabel );
- $status = \Status::newGood( $text );
- } catch ( \Exception $ex ) {
- wfDebugLog( __CLASS__, __FUNCTION__ . ': ' .
$ex->getMessage() );
-
- $status = \Status::newFatal(
'wikibase-property-render-error', $propertyLabel, $ex->getMessage() );
+ if ( !$entity ) {
+ wfProfileOut( __METHOD__ );
+ return Status::newGood( '' );
}
+ $claims = $this->getClaimsForProperty( $entity, $propertyLabel
);
+
+ if ( $claims->isEmpty() ) {
+ wfProfileOut( __METHOD__ );
+ return Status::newGood( '' );
+ }
+
+ $snakList = $claims->getMainSnaks();
+ $text = $this->formatSnakList( $snakList, $propertyLabel );
+ $status = Status::newGood( $text );
+
wfProfileOut( __METHOD__ );
return $status;
}
diff --git
a/client/tests/phpunit/includes/parserhooks/PropertyParserFunctionRendererTest.php
b/client/tests/phpunit/includes/parserhooks/PropertyParserFunctionRendererTest.php
index 3a260df..b487257 100644
---
a/client/tests/phpunit/includes/parserhooks/PropertyParserFunctionRendererTest.php
+++
b/client/tests/phpunit/includes/parserhooks/PropertyParserFunctionRendererTest.php
@@ -3,11 +3,13 @@
namespace Wikibase\Test;
use DataValues\StringValue;
+use Language;
use Wikibase\Claim;
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\Item;
+use Wikibase\Lib\PropertyLabelNotResolvedException;
use Wikibase\ParserErrorMessageFormatter;
use Wikibase\Property;
use Wikibase\PropertyParserFunctionRenderer;
@@ -16,11 +18,7 @@
/**
* @covers Wikibase\PropertyParserFunctionRenderer
*
- * @file
* @since 0.4
- *
- * @ingroup WikibaseClient
- * @ingroup Test
*
* @group Wikibase
* @group WikibaseClient
@@ -32,11 +30,8 @@
class PropertyParserFunctionRendererTest extends \PHPUnit_Framework_TestCase {
private function getDefaultInstance() {
- $wikibaseClient = WikibaseClient::getDefaultInstance();
-
- $targetLanguage = \Language::factory( 'en' );
+ $targetLanguage = Language::factory( 'en' );
$errorFormatter = new ParserErrorMessageFormatter(
$targetLanguage );
- $dataTypeFactory = $wikibaseClient->getDataTypeFactory();
$mockRepo = $this->newMockRepository();
$mockResolver = new MockPropertyLabelResolver(
$targetLanguage->getCode(), $mockRepo );
@@ -88,21 +83,6 @@
return $entityLookup;
}
- public static function provideRenderForEntityId() {
- return array(
- array(
- 'p1337',
- '(a kitten), (a kitten)',
- 'Congratulations, you just killed a kitten'
- ),
- array(
- 'kitten',
- '(a kitten), (a kitten)',
- 'Congratulations, you just killed a kitten'
- ),
- );
- }
-
/**
* @dataProvider provideRenderForEntityId
*/
@@ -126,4 +106,45 @@
);
}
+ public function provideRenderForEntityId() {
+ return array(
+ array(
+ 'p1337',
+ '(a kitten), (a kitten)',
+ 'Congratulations, you just killed a kitten'
+ ),
+ array(
+ 'P1337',
+ '(a kitten), (a kitten)',
+ 'Congratulations, you just killed a kitten'
+ ),
+ array(
+ 'kitten',
+ '(a kitten), (a kitten)',
+ 'Congratulations, you just killed a kitten'
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider invalidRenderForEntityIdProvider
+ * @expectedException Wikibase\Lib\PropertyLabelNotResolvedException
+ */
+ public function testInvalidRenderForEntityId( $name, $message ) {
+ $parserFunction = $this->getDefaultInstance();
+
+ $status = $parserFunction->renderForEntityId(
+ new ItemId( 'Q42' ),
+ $name
+ );
+
+ $this->assertFalse( $status->isOK(), $message );
+ }
+
+ public function invalidRenderForEntityIdProvider() {
+ return array(
+ array( 'Kitten', 'invalid label, property by label
lookup is case-sensitive' )
+ );
+ }
+
}
diff --git a/lib/WikibaseLib.classes.php b/lib/WikibaseLib.classes.php
index a255136..f48a838 100644
--- a/lib/WikibaseLib.classes.php
+++ b/lib/WikibaseLib.classes.php
@@ -58,6 +58,7 @@
'Wikibase\ReferencedEntitiesFinder' =>
'includes/ReferencedEntitiesFinder.php',
'Wikibase\ReferencedUrlFinder' =>
'includes/ReferencedUrlFinder.php',
'Wikibase\Lib\PropertyDataTypeLookup' =>
'includes/PropertyDataTypeLookup.php',
+ 'Wikibase\Lib\PropertyLabelNotResolvedException' =>
'includes/PropertyLabelNotResolvedException.php',
'Wikibase\Lib\PropertyNotFoundException' =>
'includes/PropertyNotFoundException.php',
'Wikibase\Settings' => 'includes/Settings.php',
'Wikibase\SettingsArray' => 'includes/SettingsArray.php',
diff --git a/lib/includes/PropertyLabelNotResolvedException.php
b/lib/includes/PropertyLabelNotResolvedException.php
new file mode 100644
index 0000000..9833f9c
--- /dev/null
+++ b/lib/includes/PropertyLabelNotResolvedException.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Wikibase\Lib;
+
+/**
+ * @since 0.5
+ *
+ * @licence GNU GPL v2+
+ * @author Katie Filbert < [email protected] >
+ */
+class PropertyLabelNotResolvedException extends \RuntimeException {
+
+ protected $label;
+
+ protected $lang;
+
+ public function __construct( $label, $lang, $message = null, \Exception
$previous = null ) {
+ $this->label = $label;
+ $this->lang = $lang;
+
+ if ( $message === null ) {
+ $message = "Property not found for label '$label' and
language '$lang'";
+ }
+
+ parent::__construct( $message, 0, $previous );
+ }
+
+}
diff --git a/lib/tests/phpunit/MockPropertyLabelResolver.php
b/lib/tests/phpunit/MockPropertyLabelResolver.php
index a14a40d..abfd51b 100644
--- a/lib/tests/phpunit/MockPropertyLabelResolver.php
+++ b/lib/tests/phpunit/MockPropertyLabelResolver.php
@@ -41,7 +41,9 @@
foreach ( $labels as $label ) {
$prop = $this->repo->getPropertyByLabel( $label,
$this->lang );
- $ids[$label] = $prop->getId();
+ if ( $prop !== null ) {
+ $ids[$label] = $prop->getId();
+ }
}
return $ids;
--
To view, visit https://gerrit.wikimedia.org/r/96814
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7e650f0b142ec8464607e093afa5d27b6b4be126
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: mw1.23-wmf5
Gerrit-Owner: Aude <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
