Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/84978
Change subject: Handle formatting errors gracefully
......................................................................
Handle formatting errors gracefully
Change-Id: I278acd0a4156ab78fa40341126cb05d94a664cc7
---
M lib/includes/ClaimDifferenceVisualizer.php
M repo/includes/SummaryFormatter.php
2 files changed, 79 insertions(+), 35 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/78/84978/1
diff --git a/lib/includes/ClaimDifferenceVisualizer.php
b/lib/includes/ClaimDifferenceVisualizer.php
index bdfc3b9..f65fdcf 100644
--- a/lib/includes/ClaimDifferenceVisualizer.php
+++ b/lib/includes/ClaimDifferenceVisualizer.php
@@ -8,6 +8,7 @@
use Html;
use Diff\Diff;
use RuntimeException;
+use ValueParsers\FormattingException;
use Wikibase\Lib\EntityIdLabelFormatter;
use Wikibase\Lib\SnakFormatter;
@@ -37,7 +38,7 @@
*
* @var EntityIdLabelFormatter
*/
- private $propertyFormatter;
+ private $propertyIdFormatter;
/**
* @since 0.5
@@ -51,19 +52,19 @@
*
* @since 0.4
*
- * @param EntityIdLabelFormatter $propertyFormatter
+ * @param EntityIdLabelFormatter $propertyIdFormatter
* @param SnakFormatter $snakFormatter
*
* @throws \InvalidArgumentException
*/
- public function __construct( EntityIdLabelFormatter $propertyFormatter,
SnakFormatter $snakFormatter ) {
+ public function __construct( EntityIdLabelFormatter
$propertyIdFormatter, SnakFormatter $snakFormatter ) {
if ( $snakFormatter->getFormat() !==
SnakFormatter::FORMAT_PLAIN ) {
throw new \InvalidArgumentException(
'Expected $snakFormatter to generate plain
text, not '
. $snakFormatter->getFormat() );
}
- $this->propertyFormatter = $propertyFormatter;
+ $this->propertyIdFormatter = $propertyIdFormatter;
$this->snakFormatter = $snakFormatter;
}
@@ -162,8 +163,8 @@
$valueFormatter = new DiffOpValueFormatter(
// todo: should show specific headers for both columns
$this->getSnakHeader( $mainSnakChange->getNewValue() ),
- $this->snakFormatter->formatSnak(
$mainSnakChange->getOldValue() ),
- $this->snakFormatter->formatSnak(
$mainSnakChange->getNewValue() )
+ $this->formatSnak( $mainSnakChange->getOldValue() ),
+ $this->formatSnak( $mainSnakChange->getNewValue() )
);
return $valueFormatter->generateHtml();
@@ -218,16 +219,42 @@
$newValue = null;
if ( $oldSnak instanceof Snak ) {
- $oldValue = $this->snakFormatter->formatSnak( $oldSnak
);
+ $oldValue = $this->formatSnak( $oldSnak );
}
if ( $newSnak instanceof Snak ) {
- $newValue = $this->snakFormatter->formatSnak( $newSnak
);
+ $newValue = $this->formatSnak( $newSnak );
}
$valueFormatter = new DiffOpValueFormatter( $snakHeader,
$oldValue, $newValue );
return $valueFormatter->generateHtml();
+ }
+
+ /**
+ * @param Snak $snak
+ *
+ * @return string
+ */
+ protected function formatSnak( Snak $snak ) {
+ try {
+ return $this->snakFormatter->formatSnak( $snak );
+ } catch ( FormattingException $ex ) {
+ return '?'; // XXX: or include the error message?
+ }
+ }
+
+ /**
+ * @param EntityId
+ *
+ * @return string
+ */
+ protected function formatPropertyId( EntityId $id ) {
+ try {
+ return $this->propertyIdFormatter->format( $id );
+ } catch ( FormattingException $ex ) {
+ return '?'; // XXX: or include the error message?
+ }
}
/**
@@ -246,9 +273,9 @@
// TODO: change hardcoded ": " so something like
wfMessage( 'colon-separator' ),
// but this will require further refactoring as it
would add HTML which gets escaped
$values[] =
- $this->propertyFormatter->format(
$snak->getPropertyId() ) .
+ $this->formatPropertyId( $snak->getPropertyId()
) .
': '.
- $this->snakFormatter->formatSnak( $snak );
+ $this->formatSnak( $snak );
}
return $values;
@@ -265,7 +292,7 @@
*/
protected function getSnakHeader( Snak $snak ) {
$propertyId = $snak->getPropertyId();
- $propertyLabel = $this->propertyFormatter->format( $propertyId
);
+ $propertyLabel = $this->formatPropertyId( $propertyId );
$headerText = wfMessage( 'wikibase-entity-property' ) . ' / ' .
$propertyLabel;
return $headerText;
@@ -340,23 +367,23 @@
// but this will require further refactoring as it
would add HTML which gets escaped
if ( $change instanceof DiffOpAdd ) {
$newVal =
- $this->propertyFormatter->format(
$change->getNewValue()->getPropertyId() ) .
+ $this->formatPropertyId(
$change->getNewValue()->getPropertyId() ) .
': ' .
- $this->snakFormatter->formatSnak(
$change->getNewValue() );
+ $this->formatSnak(
$change->getNewValue() );
} else if ( $change instanceof DiffOpRemove ) {
$oldVal =
- $this->propertyFormatter->format(
$change->getOldValue()->getPropertyId() ) .
+ $this->formatPropertyId(
$change->getOldValue()->getPropertyId() ) .
': ' .
- $this->snakFormatter->formatSnak(
$change->getOldValue() );
+ $this->formatSnak(
$change->getOldValue() );
} else if ( $change instanceof DiffOpChange ) {
$oldVal =
- $this->propertyFormatter->format(
$change->getOldValue()->getPropertyId() ) .
+ $this->formatPropertyId(
$change->getOldValue()->getPropertyId() ) .
': ' .
- $this->snakFormatter->formatSnak(
$change->getOldValue() );
+ $this->formatSnak(
$change->getOldValue() );
$newVal =
- $this->propertyFormatter->format(
$change->getNewValue()->getPropertyId() ) .
+ $this->formatPropertyId(
$change->getNewValue()->getPropertyId() ) .
': ' .
- $this->snakFormatter->formatSnak(
$change->getNewValue() );
+ $this->formatSnak(
$change->getNewValue() );
} else {
throw new RuntimeException( 'Diff operation of
unknown type.' );
}
diff --git a/repo/includes/SummaryFormatter.php
b/repo/includes/SummaryFormatter.php
index 9284ba3..e92d585 100644
--- a/repo/includes/SummaryFormatter.php
+++ b/repo/includes/SummaryFormatter.php
@@ -3,6 +3,8 @@
namespace Wikibase;
use DataValues\DataValue;
+use Exception;
+use InvalidArgumentException;
use Language;
use ValueFormatters\ValueFormatter;
use Wikibase\Lib\EntityIdFormatter;
@@ -48,8 +50,17 @@
* @param EntityIdFormatter $idFormatter
* @param ValueFormatter $valueFormatter
* @param SnakFormatter $snakFormatter
+ * @param \Language $language
+ *
+ * @throws \InvalidArgumentException
*/
public function __construct( EntityIdFormatter $idFormatter,
ValueFormatter $valueFormatter, SnakFormatter $snakFormatter, Language
$language ) {
+ if ( $snakFormatter->getFormat() !==
SnakFormatter::FORMAT_PLAIN ) {
+ throw new InvalidArgumentException(
+ 'Expected $snakFormatter to procude text/plain
output, not '
+ . $snakFormatter->getFormat() );
+ }
+
$this->idFormatter = $idFormatter;
$this->valueFormatter = $valueFormatter;
$this->snakFormatter = $snakFormatter;
@@ -139,23 +150,29 @@
* @return string
*/
protected function formatArg( $arg ) {
- if ( $arg instanceof Snak ) {
- return $this->snakFormatter->formatSnak( $arg );
- } elseif ( $arg instanceof EntityId ) {
- return $this->idFormatter->format( $arg );
- } elseif ( $arg instanceof DataValue ) {
- return $this->valueFormatter->format( $arg );
- } elseif ( method_exists( $arg, '__toString' ) ) {
- return strval( $arg );
- } elseif ( is_object( $arg ) ) {
- return '<' . get_class( $arg ) . '>';
- } elseif ( is_array( $arg ) ) {
- //TODO: preserve keys in assoc array
- $strings = $this->formatArgList( $arg );
- return $this->language->commaList( $strings );
- } else {
- return strval( $arg );
+ try {
+ if ( $arg instanceof Snak ) {
+ return $this->snakFormatter->formatSnak( $arg );
+ } elseif ( $arg instanceof EntityId ) {
+ return $this->idFormatter->format( $arg );
+ } elseif ( $arg instanceof DataValue ) {
+ return $this->valueFormatter->format( $arg );
+ } elseif ( method_exists( $arg, '__toString' ) ) {
+ return strval( $arg );
+ } elseif ( is_object( $arg ) ) {
+ return '<' . get_class( $arg ) . '>';
+ } elseif ( is_array( $arg ) ) {
+ //TODO: preserve keys in assoc array
+ $strings = $this->formatArgList( $arg );
+ return $this->language->commaList( $strings );
+ } else {
+ return strval( $arg );
+ }
+ } catch ( Exception $ex ) {
+ wfWarn( __METHOD__ . ': failed to render value: ' .
$ex->getMessage() );
}
+
+ return '?';
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/84978
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I278acd0a4156ab78fa40341126cb05d94a664cc7
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
