jenkins-bot has submitted this change and it was merged.

Change subject: Introduce DifferencesSnakVisualizer, improve 
ClaimDifferenceVisualizer tests
......................................................................


Introduce DifferencesSnakVisualizer, improve ClaimDifferenceVisualizer tests

Change-Id: I5885448773298e82c82e1c8263ca4590cfa705cc
---
M repo/includes/Diff/ClaimDifferenceVisualizer.php
A repo/includes/Diff/DifferencesSnakVisualizer.php
M repo/includes/Diff/EntityContentDiffView.php
M repo/includes/actions/EditEntityAction.php
M repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php
A repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
6 files changed, 468 insertions(+), 278 deletions(-)

Approvals:
  Hoo man: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/repo/includes/Diff/ClaimDifferenceVisualizer.php 
b/repo/includes/Diff/ClaimDifferenceVisualizer.php
index 0e72692..a8816a9 100644
--- a/repo/includes/Diff/ClaimDifferenceVisualizer.php
+++ b/repo/includes/Diff/ClaimDifferenceVisualizer.php
@@ -7,18 +7,11 @@
 use Diff\DiffOp\DiffOpAdd;
 use Diff\DiffOp\DiffOpChange;
 use Diff\DiffOp\DiffOpRemove;
-use Exception;
-use InvalidArgumentException;
 use Message;
 use RuntimeException;
-use ValueFormatters\FormattingException;
-use ValueFormatters\ValueFormatter;
 use Wikibase\DataModel\Claim\Claim;
-use Wikibase\DataModel\Entity\EntityId;
-use Wikibase\DataModel\Snak\Snak;
 use Wikibase\DataModel\Snak\SnakList;
 use Wikibase\Lib\Serializers\ClaimSerializer;
-use Wikibase\Lib\SnakFormatter;
 
 /**
  * Class for generating HTML for Claim Diffs.
@@ -30,23 +23,14 @@
  * @author Katie Filbert < [email protected] >
  * @author Adam Shorland
  * @author Daniel Kinzler
+ * @author Adrian Heine < [email protected] >
  */
 class ClaimDifferenceVisualizer {
 
        /**
-        * @var ValueFormatter
+        * @var DifferencesSnakVisualizer
         */
-       private $propertyIdFormatter;
-
-       /**
-        * @var SnakFormatter
-        */
-       private $snakDetailsFormatter;
-
-       /**
-        * @var SnakFormatter
-        */
-       private $snakBreadCrumbFormatter;
+       private $snakVisualizer;
 
        /**
         * @var string
@@ -56,38 +40,14 @@
        /**
         * @since 0.4
         *
-        * @param ValueFormatter $propertyIdFormatter Formatter for IDs, must 
generate HTML.
-        * @param SnakFormatter $snakDetailsFormatter detailed Formatter for 
Snaks, must generate HTML.
-        * @param SnakFormatter $snakBreadCrumbFormatter terse Formatter for 
Snaks, must generate HTML.
+        * @param DifferencesSnakVisualizer $snakVisualizer
         * @param string $languageCode
-        *
-        * @throws InvalidArgumentException
         */
        public function __construct(
-               ValueFormatter $propertyIdFormatter,
-               SnakFormatter $snakDetailsFormatter,
-               SnakFormatter $snakBreadCrumbFormatter,
+               DifferencesSnakVisualizer $snakVisualizer,
                $languageCode
        ) {
-               if ( $snakDetailsFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML
-                       && $snakDetailsFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML_DIFF
-               ) {
-                       throw new InvalidArgumentException(
-                               'Expected $snakDetailsFormatter to generate 
html, not '
-                               . $snakDetailsFormatter->getFormat() );
-               }
-
-               if ( $snakBreadCrumbFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML
-                       && $snakBreadCrumbFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML_DIFF
-               ) {
-                       throw new InvalidArgumentException(
-                               'Expected $snakBreadCrumbFormatter to generate 
html, not '
-                               . $snakBreadCrumbFormatter->getFormat() );
-               }
-
-               $this->propertyIdFormatter = $propertyIdFormatter;
-               $this->snakDetailsFormatter = $snakDetailsFormatter;
-               $this->snakBreadCrumbFormatter = $snakBreadCrumbFormatter;
+               $this->snakVisualizer = $snakVisualizer;
                $this->languageCode = $languageCode;
        }
 
@@ -96,7 +56,7 @@
         * @since 0.4
         *
         * @param ClaimDifference $claimDifference
-        * @param Claim $baseClaim
+        * @param Claim $baseClaim The new claim, if it exists; otherwise, the 
old claim
         *
         * @return string
         */
@@ -168,22 +128,22 @@
         *
         * @return string
         */
-       protected function visualizeMainSnakChange( DiffOpChange 
$mainSnakChange ) {
+       private function visualizeMainSnakChange( DiffOpChange $mainSnakChange 
) {
                $oldSnak = $mainSnakChange->getOldValue();
                $newSnak = $mainSnakChange->getNewValue();
 
                if ( $newSnak === null ) {
-                       $headerHtml = $this->getSnakLabelHeader( $oldSnak );
+                       $headerHtml = $this->snakVisualizer->getPropertyHeader( 
$oldSnak );
                } else {
-                       $headerHtml = $this->getSnakLabelHeader( $newSnak );
+                       $headerHtml = $this->snakVisualizer->getPropertyHeader( 
$newSnak );
                }
 
                $valueFormatter = new DiffOpValueFormatter(
                        // todo: should show specific headers for each column
                        $headerHtml,
                        // TODO: How to highlight the actual changes inside the 
snak?
-                       $this->formatSnakDetails( $oldSnak ),
-                       $this->formatSnakDetails( $newSnak )
+                       $this->snakVisualizer->getDetailedValue( $oldSnak ),
+                       $this->snakVisualizer->getDetailedValue( $newSnak )
                );
 
                return $valueFormatter->generateHtml();
@@ -199,9 +159,9 @@
         *
         * @return string
         */
-       protected function visualizeRankChange( DiffOpChange $rankChange, Claim 
$claim ) {
+       private function visualizeRankChange( DiffOpChange $rankChange, Claim 
$claim ) {
                $claimMainSnak = $claim->getMainSnak();
-               $claimHeader = $this->getSnakValueHeader( $claimMainSnak );
+               $claimHeader = 
$this->snakVisualizer->getPropertyAndValueHeader( $claimMainSnak );
 
                $valueFormatter = new DiffOpValueFormatter(
                        $claimHeader . ' / ' . wfMessage( 
'wikibase-diffview-rank' )->inLanguage( $this->languageCode )->parse(),
@@ -217,7 +177,7 @@
         *
         * @return null|String HTML
         */
-       protected function getRankHtml( $rank ) {
+       private function getRankHtml( $rank ) {
                if ( $rank === null ) {
                        return null;
                }
@@ -233,109 +193,6 @@
        }
 
        /**
-        * @param Snak|null $snak
-        *
-        * @return string HTML
-        */
-       protected function formatSnakDetails( Snak $snak = null ) {
-               if ( $snak === null ) {
-                       return null;
-               }
-
-               try {
-                       return $this->snakDetailsFormatter->formatSnak( $snak );
-               } catch ( Exception $ex ) {
-                       // @fixme maybe there is a way we can render something 
more useful
-                       // we are getting multiple types of exceptions and 
should handle
-                       // consistent (and shared code) with what we do in 
SnakHtmlGenerator.
-                       $messageText = wfMessage( 
'wikibase-snakformat-invalid-value' )
-                               ->inLanguage( $this->languageCode )
-                               ->parse();
-
-                       return $messageText;
-               }
-       }
-
-       /**
-        * @param EntityId $entityId
-        *
-        * @return string HTML
-        */
-       protected function formatPropertyId( EntityId $entityId ) {
-               try {
-                       return $this->propertyIdFormatter->format( $entityId );
-               } catch ( FormattingException $ex ) {
-                       return $entityId->getSerialization(); // XXX: or 
include the error message?
-               }
-       }
-
-       /**
-        * Get formatted values of SnakList in an array
-        *
-        * @since 0.4
-        *
-        * @param SnakList $snakList
-        *
-        * @return string[] A list if HTML strings
-        */
-        protected function getSnakListValues( SnakList $snakList ) {
-               $values = array();
-               $colon = wfMessage( 'colon-separator' )->inLanguage( 
$this->languageCode )->escaped();
-
-               foreach ( $snakList as $snak ) {
-                       /** @var $snak Snak */
-                       $values[] =
-                               $this->formatPropertyId( $snak->getPropertyId() 
) .
-                               $colon.
-                               $this->formatSnakDetails( $snak );
-               }
-
-               return $values;
-       }
-
-       /**
-        * Get formatted header for a snak, including the snak's property 
label, but not the snak's value.
-        *
-        * @since 0.4
-        *
-        * @param Snak|null $snak
-        *
-        * @return string HTML
-        */
-       protected function getSnakLabelHeader( Snak $snak = null ) {
-               $headerText = wfMessage( 'wikibase-entity-property' 
)->inLanguage( $this->languageCode )->parse();
-
-               if ( $snak !== null ) {
-                       $propertyId = $snak->getPropertyId();
-                       $headerText .= ' / ' . $this->formatPropertyId( 
$propertyId );
-               }
-
-               return $headerText;
-       }
-
-       /**
-        * Get formatted header for a snak, including the snak's property label 
and value.
-        *
-        * @since 0.4
-        *
-        * @param Snak $snak
-        *
-        * @return string HTML
-        */
-       protected function getSnakValueHeader( Snak $snak ) {
-               $headerText = $this->getSnakLabelHeader( $snak );
-
-               try {
-                       $headerText .= wfMessage( 'colon-separator' 
)->inLanguage( $this->languageCode )->escaped()
-                               . $this->snakBreadCrumbFormatter->formatSnak( 
$snak );
-               } catch ( Exception $ex ) {
-                       // just ignore it
-               }
-
-               return $headerText;
-       }
-
-       /**
         * Get Html for snaklist changes
         *
         * @since 0.4
@@ -347,25 +204,25 @@
         * @return string
         * @throws RuntimeException
         */
-       protected function visualizeSnakListChanges( Diff $changes, Claim 
$claim, Message $breadCrumb ) {
+       private function visualizeSnakListChanges( Diff $changes, Claim $claim, 
Message $breadCrumb ) {
                $html = '';
 
                $claimMainSnak = $claim->getMainSnak();
-               $claimHeader = $this->getSnakValueHeader( $claimMainSnak );
+               $claimHeader = 
$this->snakVisualizer->getPropertyAndValueHeader( $claimMainSnak );
 
-               $newVal = null;
-               $oldVal = null;
-
-               foreach( $changes as $change ) {
-                       if ( $change instanceof DiffOpAdd ) {
-                               $newVal = $this->getSnakListValues( 
$change->getNewValue()->getSnaks() );
-                       } else if ( $change instanceof DiffOpRemove ) {
-                               $oldVal = $this->getSnakListValues( 
$change->getOldValue()->getSnaks() );
-                       } else if ( $change instanceof DiffOpChange ) {
-                               $oldVal = $this->getSnakListValues( 
$change->getOldValue()->getSnaks() );
-                               $newVal = $this->getSnakListValues( 
$change->getNewValue()->getSnaks() );
-                       } else {
-                               throw new RuntimeException( 'Diff operation of 
unknown type.' );
+               foreach ( $changes as $change ) {
+                       $newVal = $oldVal = null;
+                       if ( $change instanceof DiffOpAdd || $change instanceof 
DiffOpChange ) {
+                               $newVal = $this->mapSnakList(
+                                       $change->getNewValue()->getSnaks(),
+                                       array( $this->snakVisualizer, 
'getPropertyAndDetailedValue' )
+                               );
+                       }
+                       if ( $change instanceof DiffOpRemove || $change 
instanceof DiffOpChange ) {
+                               $oldVal = $this->mapSnakList(
+                                       $change->getOldValue()->getSnaks(),
+                                       array( $this->snakVisualizer, 
'getPropertyAndDetailedValue' )
+                               );
                        }
 
                        $valueFormatter = new DiffOpValueFormatter(
@@ -374,11 +231,26 @@
                                $newVal
                        );
 
-                       $oldVal = $newVal = null;
                        $html .= $valueFormatter->generateHtml();
                }
 
                return $html;
+       }
+
+       /**
+        * Map SnakList
+        *
+        * @param SnakList $snakList
+        * @param callable $mapper
+        *
+        * @return mixed[]
+        */
+       private function mapSnakList( SnakList $snakList, $mapper ) {
+               $ret = array();
+               foreach ( $snakList as $snak ) {
+                       $ret[] = call_user_func( $mapper, $snak );
+               }
+               return $ret;
        }
 
        /**
@@ -392,36 +264,20 @@
         * @return string
         * @throws RuntimeException
         */
-       protected function visualizeQualifierChanges( Diff $changes, Claim 
$claim ) {
+       private function visualizeQualifierChanges( Diff $changes, Claim $claim 
) {
                $html = '';
-               $colon = wfMessage( 'colon-separator' )->inLanguage( 
$this->languageCode )->escaped();
 
                $claimMainSnak = $claim->getMainSnak();
-               $claimHeader = $this->getSnakValueHeader( $claimMainSnak );
-               $newVal = $oldVal = null;
+               $claimHeader = 
$this->snakVisualizer->getPropertyAndValueHeader( $claimMainSnak );
 
-               foreach( $changes as $change ) {
-                       if ( $change instanceof DiffOpAdd ) {
-                               $newVal =
-                                       $this->formatPropertyId( 
$change->getNewValue()->getPropertyId() ) .
-                                       $colon .
-                                       $this->formatSnakDetails( 
$change->getNewValue() );
-                       } else if ( $change instanceof DiffOpRemove ) {
-                               $oldVal =
-                                       $this->formatPropertyId( 
$change->getOldValue()->getPropertyId() ) .
-                                       $colon .
-                                       $this->formatSnakDetails( 
$change->getOldValue() );
-                       } else if ( $change instanceof DiffOpChange ) {
-                               $oldVal =
-                                       $this->formatPropertyId( 
$change->getOldValue()->getPropertyId() ) .
-                                       $colon .
-                                       $this->formatSnakDetails( 
$change->getOldValue() );
-                               $newVal =
-                                       $this->formatPropertyId( 
$change->getNewValue()->getPropertyId() ) .
-                                       $colon .
-                                       $this->formatSnakDetails( 
$change->getNewValue() );
-                       } else {
-                               throw new RuntimeException( 'Diff operation of 
unknown type.' );
+               foreach ( $changes as $change ) {
+                       $newVal = $oldVal = null;
+
+                       if ( $change instanceof DiffOpAdd || $change instanceof 
DiffOpChange ) {
+                               $newVal = 
$this->snakVisualizer->getPropertyAndDetailedValue( $change->getNewValue() );
+                       }
+                       if ( $change instanceof DiffOpRemove || $change 
instanceof DiffOpChange ) {
+                               $oldVal = 
$this->snakVisualizer->getPropertyAndDetailedValue( $change->getOldValue() );
                        }
 
                        $valueFormatter = new DiffOpValueFormatter(
@@ -430,7 +286,6 @@
                                        $newVal
                        );
 
-                       $oldVal = $newVal = null;
                        $html .= $valueFormatter->generateHtml();
                }
 
diff --git a/repo/includes/Diff/DifferencesSnakVisualizer.php 
b/repo/includes/Diff/DifferencesSnakVisualizer.php
new file mode 100644
index 0000000..40f47de
--- /dev/null
+++ b/repo/includes/Diff/DifferencesSnakVisualizer.php
@@ -0,0 +1,174 @@
+<?php
+
+namespace Wikibase\Repo\Diff;
+
+use Exception;
+use InvalidArgumentException;
+use ValueFormatters\FormattingException;
+use ValueFormatters\ValueFormatter;
+use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Snak\Snak;
+use Wikibase\Lib\SnakFormatter;
+
+/**
+ * Visualizes Snaks for difference views
+ *
+ * @author Adrian Heine < [email protected] >
+ */
+class DifferencesSnakVisualizer {
+
+       /**
+        * @var string
+        */
+       private $languageCode;
+
+       /**
+        * @var ValueFormatter
+        */
+       private $propertyIdFormatter;
+
+       /**
+        * @var SnakFormatter
+        */
+       private $snakBreadCrumbFormatter;
+
+       /**
+        * @var SnakFormatter
+        */
+       private $snakDetailsFormatter;
+
+       /**
+        * @param ValueFormatter $propertyIdFormatter Formatter for IDs, must 
generate HTML.
+        * @param SnakFormatter $snakDetailsFormatter detailed Formatter for 
Snaks, must generate HTML.
+        * @param SnakFormatter $snakBreadCrumbFormatter terse Formatter for 
Snaks, must generate HTML.
+        * @param string $languageCode
+        *
+        * @throws InvalidArgumentException
+        */
+       public function __construct(
+               ValueFormatter $propertyIdFormatter,
+               SnakFormatter $snakDetailsFormatter,
+               SnakFormatter $snakBreadCrumbFormatter,
+               $languageCode
+       ) {
+               if ( $snakDetailsFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML
+                       && $snakDetailsFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML_DIFF
+               ) {
+                       throw new InvalidArgumentException(
+                               'Expected $snakDetailsFormatter to generate 
html, not '
+                               . $snakDetailsFormatter->getFormat() );
+               }
+
+               if ( $snakBreadCrumbFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML
+                       && $snakBreadCrumbFormatter->getFormat() !== 
SnakFormatter::FORMAT_HTML_DIFF
+               ) {
+                       throw new InvalidArgumentException(
+                               'Expected $snakBreadCrumbFormatter to generate 
html, not '
+                               . $snakBreadCrumbFormatter->getFormat() );
+               }
+
+               $this->propertyIdFormatter = $propertyIdFormatter;
+               $this->snakDetailsFormatter = $snakDetailsFormatter;
+               $this->snakBreadCrumbFormatter = $snakBreadCrumbFormatter;
+               $this->languageCode = $languageCode;
+       }
+
+       /**
+        * @param Snak|null $snak
+        *
+        * @return string|null HTML
+        */
+       public function getDetailedValue( Snak $snak = null ) {
+               if ( $snak === null ) {
+                       return null;
+               }
+
+               try {
+                       return $this->snakDetailsFormatter->formatSnak( $snak );
+               } catch ( Exception $ex ) {
+                       // @fixme maybe there is a way we can render something 
more useful
+                       // we are getting multiple types of exceptions and 
should handle
+                       // consistent (and shared code) with what we do in 
SnakHtmlGenerator.
+                       $messageText = wfMessage( 
'wikibase-snakformat-invalid-value' )
+                               ->inLanguage( $this->languageCode )
+                               ->parse();
+
+                       return $messageText;
+               }
+       }
+
+       /**
+        * @param EntityId $entityId
+        *
+        * @return string HTML
+        */
+       private function formatPropertyId( EntityId $entityId ) {
+               try {
+                       return $this->propertyIdFormatter->format( $entityId );
+               } catch ( FormattingException $ex ) {
+                       return $entityId->getSerialization(); // XXX: or 
include the error message?
+               }
+       }
+
+       /**
+        * Get formatted header for a snak, including the snak's property 
label, but not the snak's value.
+        *
+        * @since 0.4
+        *
+        * @param Snak|null $snak
+        *
+        * @return string HTML
+        */
+       public function getPropertyHeader( Snak $snak = null ) {
+               $headerText = wfMessage( 'wikibase-entity-property' 
)->inLanguage( $this->languageCode )->parse();
+
+               if ( $snak !== null ) {
+                       $propertyId = $snak->getPropertyId();
+                       $headerText .= ' / ' . $this->formatPropertyId( 
$propertyId );
+               }
+
+               return $headerText;
+       }
+
+       /**
+        * Get formatted header for a snak, including the snak's property label 
and value.
+        *
+        * @since 0.4
+        *
+        * @param Snak $snak
+        *
+        * @return string HTML
+        */
+       public function getPropertyAndValueHeader( Snak $snak ) {
+               $before = $this->getPropertyHeader( $snak );
+
+               try {
+                       $after = $this->snakBreadCrumbFormatter->formatSnak( 
$snak );
+                       $result = $this->getColonSeparatedHtml( $before, $after 
);
+               } catch ( Exception $ex ) {
+                       // just ignore it
+                       $result = $before;
+               }
+
+               return $result;
+       }
+
+       /**
+        * Get a detailed formatted snak, including the snak's property label 
and value.
+        *
+        * @param Snak $snak
+        */
+       public function getPropertyAndDetailedValue( Snak $snak ) {
+               return $this->getColonSeparatedHtml(
+                       $this->formatPropertyId( $snak->getPropertyId() ),
+                       $this->getDetailedValue( $snak )
+               );
+       }
+
+       private function getColonSeparatedHtml( $before, $after ) {
+               $colon = wfMessage( 'colon-separator' )->inLanguage( 
$this->languageCode )->escaped();
+
+               return $before . $colon . $after;
+       }
+
+}
diff --git a/repo/includes/Diff/EntityContentDiffView.php 
b/repo/includes/Diff/EntityContentDiffView.php
index 6ba4dfb..095bef4 100644
--- a/repo/includes/Diff/EntityContentDiffView.php
+++ b/repo/includes/Diff/EntityContentDiffView.php
@@ -93,7 +93,15 @@
                $this->diffVisualizer = new EntityDiffVisualizer(
                        $this->getContext(),
                        new ClaimDiffer( new OrderedListDiffer( new 
ComparableComparer() ) ),
-                       new ClaimDifferenceVisualizer( 
$this->propertyNameFormatter, $this->detailedSnakFormatter, 
$this->terseSnakFormatter, $langCode ),
+                       new ClaimDifferenceVisualizer(
+                               new DifferencesSnakVisualizer(
+                                       $this->propertyNameFormatter,
+                                       $this->detailedSnakFormatter,
+                                       $this->terseSnakFormatter,
+                                       $langCode
+                               ),
+                               $langCode
+                       ),
                        $wikibaseRepo->getSiteStore(),
                        $wikibaseRepo->getEntityTitleLookup(),
                        $wikibaseRepo->getEntityRevisionLookup()
diff --git a/repo/includes/actions/EditEntityAction.php 
b/repo/includes/actions/EditEntityAction.php
index 901109d..f62e21f 100644
--- a/repo/includes/actions/EditEntityAction.php
+++ b/repo/includes/actions/EditEntityAction.php
@@ -22,6 +22,7 @@
 use Wikibase\Repo\Content\EntityContentDiff;
 use Wikibase\Repo\Diff\ClaimDiffer;
 use Wikibase\Repo\Diff\ClaimDifferenceVisualizer;
+use Wikibase\Repo\Diff\DifferencesSnakVisualizer;
 use Wikibase\Repo\Diff\EntityDiffVisualizer;
 use Wikibase\Repo\WikibaseRepo;
 
@@ -74,9 +75,12 @@
                        $this->getContext(),
                        new ClaimDiffer( new OrderedListDiffer( new 
ComparableComparer() ) ),
                        new ClaimDifferenceVisualizer(
-                               $propertyIdFormatter,
-                               $snakDetailsFormatter,
-                               $snakBreadCrumbFormatter,
+                               new DifferencesSnakVisualizer(
+                                       $propertyIdFormatter,
+                                       $snakDetailsFormatter,
+                                       $snakBreadCrumbFormatter,
+                                       $languageCode
+                               ),
                                $languageCode
                        ),
                        $wikibaseRepo->getSiteStore(),
diff --git a/repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php 
b/repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php
index 1691c3b..d158ee5 100644
--- a/repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php
+++ b/repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php
@@ -7,6 +7,7 @@
 use Diff\DiffOp\DiffOpAdd;
 use Diff\DiffOp\DiffOpChange;
 use Diff\DiffOp\DiffOpRemove;
+use MediaWikiTestCase;
 use Wikibase\DataModel\Claim\Claim;
 use Wikibase\DataModel\Entity\PropertyId;
 use Wikibase\DataModel\Reference;
@@ -14,9 +15,9 @@
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
 use Wikibase\DataModel\Snak\PropertySomeValueSnak;
 use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\DataModel\Snak\Snak;
 use Wikibase\DataModel\Snak\SnakList;
 use Wikibase\DataModel\Statement\Statement;
-use Wikibase\Lib\SnakFormatter;
 use Wikibase\Repo\Diff\ClaimDifference;
 use Wikibase\Repo\Diff\ClaimDifferenceVisualizer;
 
@@ -30,71 +31,57 @@
  * @licence GNU GPL v2+
  * @author Adam Shorland
  */
-class ClaimDifferenceVisualizerTest extends \MediaWikiTestCase {
+class ClaimDifferenceVisualizerTest extends MediaWikiTestCase {
 
-       public function newSnakFormatter( $format = SnakFormatter::FORMAT_HTML 
){
-               $instance = $this->getMock( 'Wikibase\Lib\SnakFormatter' );
-               $instance->expects( $this->any() )
-                       ->method( 'getFormat' )
-                       ->will( $this->returnValue( $format ) );
-               $instance->expects( $this->any() )
-                       ->method( 'canFormatSnak' )
-                       ->will( $this->returnValue( true ) );
-               $instance->expects( $this->any() )
-                       ->method( 'formatSnak' )
-                       ->will( $this->returnValue( '<i>SNAK</i>' ) );
-               return $instance;
-       }
-
-       public function newEntityIdLabelFormatter(){
-               $instance = $this
-                       ->getMockBuilder( 'Wikibase\Lib\EntityIdLabelFormatter' 
)
+       public function newDifferencesSnakVisualizer() {
+               $instance = $this->getMockBuilder( 
'Wikibase\Repo\Diff\DifferencesSnakVisualizer' )
                        ->disableOriginalConstructor()
                        ->getMock();
 
                $instance->expects( $this->any() )
-                       ->method( 'format' )
-                       ->will( $this->returnValue( '<a>PID</a>' ) );
+                       ->method( 'getPropertyAndDetailedValue' )
+                       ->will( $this->returnCallback( function( 
PropertyValueSnak $snak ) {
+                               return 
$snak->getPropertyId()->getSerialization() . ': ' . 
$snak->getDataValue()->getValue()
+                                       . ' (DETAILED)';
+                       } ) );
+
+               $instance->expects( $this->any() )
+                       ->method( 'getDetailedValue' )
+                       ->will( $this->returnCallback( function( 
PropertyValueSnak $snak = null ) {
+                               return $snak === null ? null : 
$snak->getDataValue()->getValue() . ' (DETAILED)';
+                       } ) );
+
+               $instance->expects( $this->any() )
+                       ->method( 'getPropertyHeader' )
+                       ->will( $this->returnCallback( function( Snak $snak ) {
+                               return 'property / ' . 
$snak->getPropertyId()->getSerialization();
+                       } ) );
+
+
+               $instance->expects( $this->any() )
+                       ->method( 'getPropertyAndValueHeader' )
+                       ->will( $this->returnCallback( function( 
PropertyValueSnak $snak ) {
+                               return 'property / ' . 
$snak->getPropertyId()->getSerialization() . ': ' .
+                                       $snak->getDataValue()->getValue();
+                       } ) );
 
                return $instance;
        }
 
-       public function newClaimDifferenceVisualizer(){
+       public function newClaimDifferenceVisualizer() {
                return new ClaimDifferenceVisualizer(
-                       $this->newEntityIdLabelFormatter(),
-                       $this->newSnakFormatter(),
-                       $this->newSnakFormatter(),
+                       $this->newDifferencesSnakVisualizer(),
                        'en'
                );
        }
 
-       public function testConstruction(){
+       public function testConstruction() {
                $instance = $this->newClaimDifferenceVisualizer();
                $this->assertInstanceOf( 
'Wikibase\Repo\Diff\ClaimDifferenceVisualizer', $instance );
        }
 
-       public function testConstructionWithBadDetailsFormatter(){
-               $this->setExpectedException( 'InvalidArgumentException' );
-               new ClaimDifferenceVisualizer(
-                       $this->newEntityIdLabelFormatter(),
-                       $this->newSnakFormatter( 'qwertyuiop' ),
-                       $this->newSnakFormatter(),
-                       'en'
-               );
-       }
-
-       public function testConstructionWithBadTerseFormatter(){
-               $this->setExpectedException( 'InvalidArgumentException' );
-               new ClaimDifferenceVisualizer(
-                       $this->newEntityIdLabelFormatter(),
-                       $this->newSnakFormatter(),
-                       $this->newSnakFormatter( 'qwertyuiop' ),
-                       'en'
-               );
-       }
-
        //TODO come up with a better way of testing this.... EWW at all the 
html...
-       public function provideDifferenceAndClaim(){
+       public function provideDifferenceAndClaim() {
                return array(
                        //0 no change
                        array(
@@ -111,24 +98,24 @@
                                        )
                                ),
                                new Claim( new PropertyValueSnak( new 
PropertyId( 'P1' ), new StringValue( 'foo' ) ) ),
-                               '<tr><td colspan="2" 
class="diff-lineno">property / <a>PID</a></td><td colspan="2" 
class="diff-lineno">property / <a>PID</a></td></tr>'.
+                               '<tr><td colspan="2" 
class="diff-lineno">property / P1</td><td colspan="2" 
class="diff-lineno">property / P1</td></tr>'.
                                '<tr><td class="diff-marker">-</td><td 
class="diff-deletedline">'.
-                               '<div><del class="diffchange 
diffchange-inline"><span><i>SNAK</i></span></del></div></td>'.
+                               '<div><del class="diffchange 
diffchange-inline"><span>bar (DETAILED)</span></del></div></td>'.
                                '<td class="diff-marker">+</td><td 
class="diff-addedline">'.
-                               '<div><ins class="diffchange 
diffchange-inline"><span><i>SNAK</i></span></ins></div></td></tr>'
+                               '<div><ins class="diffchange 
diffchange-inline"><span>foo (DETAILED)</span></ins></div></td></tr>'
                        ),
                        //2 +qualifiers
                        array(
                                new ClaimDifference(
                                        null,
                                        new Diff( array(
-                                               new DiffOpAdd( new 
PropertySomeValueSnak( 44 ) ),
+                                               new DiffOpAdd( new 
PropertyValueSnak( 44, new StringValue( 'v' ) ) ),
                                        ) )
                                ),
                                new Claim( new PropertyValueSnak( new 
PropertyId( 'P1' ), new StringValue( 'foo' ) ) ),
-                               '<tr><td colspan="2" 
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property / 
<a>PID</a>: <i>SNAK</i> / qualifier</td></tr>'.
+                               '<tr><td colspan="2" 
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property / P1: foo 
/ qualifier</td></tr>'.
                                '<tr><td colspan="2">&nbsp;</td><td 
class="diff-marker">+</td><td class="diff-addedline">'.
-                               '<div><ins class="diffchange 
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></ins></div></td></tr>'
+                               '<div><ins class="diffchange 
diffchange-inline"><span>P44: v (DETAILED)</span></ins></div></td></tr>'
                        ),
                        //3 +references
                        array(
@@ -136,13 +123,13 @@
                                        null,
                                        null,
                                        new Diff( array(
-                                               new DiffOpRemove( new 
Reference( new SnakList( array( new PropertyNoValueSnak( 50 ) ) ) ) ),
+                                               new DiffOpRemove( new 
Reference( new SnakList( array( new PropertyValueSnak( 50, new StringValue( 'v' 
) ) ) ) ) ),
                                        ) )
                                ),
                                new Claim( new PropertyValueSnak( new 
PropertyId( 'P1' ), new StringValue( 'foo' ) ) ),
-                               '<tr><td colspan="2" 
class="diff-lineno">property / <a>PID</a>: <i>SNAK</i> / reference</td><td 
colspan="2" class="diff-lineno"></td></tr>'.
+                               '<tr><td colspan="2" 
class="diff-lineno">property / P1: foo / reference</td><td colspan="2" 
class="diff-lineno"></td></tr>'.
                                '<tr><td class="diff-marker">-</td><td 
class="diff-deletedline">'.
-                               '<div><del class="diffchange 
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></del></div></td><td 
colspan="2">&nbsp;</td></tr>'
+                               '<div><del class="diffchange 
diffchange-inline"><span>P50: v (DETAILED)</span></del></div></td><td 
colspan="2">&nbsp;</td></tr>'
                        ),
                        //4 ranks
                        array(
@@ -153,7 +140,7 @@
                                        new DiffOpChange( 
Statement::RANK_NORMAL, Statement::RANK_PREFERRED )
                                ),
                                new Statement( new Claim( new 
PropertyValueSnak( new PropertyId( 'P1' ), new StringValue( 'foo' ) ) ) ),
-                               '<tr><td colspan="2" 
class="diff-lineno">property / <a>PID</a>: <i>SNAK</i> / rank</td><td 
colspan="2" class="diff-lineno">property / <a>PID</a>: <i>SNAK</i> / 
rank</td></tr>'.
+                               '<tr><td colspan="2" 
class="diff-lineno">property / P1: foo / rank</td><td colspan="2" 
class="diff-lineno">property / P1: foo / rank</td></tr>'.
                                '<tr><td class="diff-marker">-</td><td 
class="diff-deletedline">'.
                                '<div><del class="diffchange 
diffchange-inline"><span>Normal rank</span></del></div></td>'.
                                '<td class="diff-marker">+</td><td 
class="diff-addedline">'.
@@ -175,33 +162,33 @@
                $expect =
                        // main snak
                        '<tr><td colspan="2" class="diff-lineno"></td>'.
-                       '<td colspan="2" class="diff-lineno">property / 
<a>PID</a></td></tr>'.
+                       '<td colspan="2" class="diff-lineno">property / 
P12</td></tr>'.
                        '<tr><td colspan="2">&nbsp;</td><td 
class="diff-marker">+</td><td class="diff-addedline">'.
-                       '<div><ins class="diffchange 
diffchange-inline"><span><i>SNAK</i></span></ins></div></td></tr>'.
+                       '<div><ins class="diffchange 
diffchange-inline"><span>foo (DETAILED)</span></ins></div></td></tr>'.
 
                        // rank
                        '<tr><td colspan="2" class="diff-lineno"></td>'.
-                       '<td colspan="2" class="diff-lineno">property / 
<a>PID</a>: <i>SNAK</i> / rank</td></tr>'.
+                       '<td colspan="2" class="diff-lineno">property / P12: 
foo / rank</td></tr>'.
                        '<tr><td colspan="2">&nbsp;</td><td 
class="diff-marker">+</td><td class="diff-addedline">'.
                        '<div><ins class="diffchange 
diffchange-inline"><span>Normal rank</span></ins></div></td></tr>'.
 
                        // qualifier
                        '<tr><td colspan="2" class="diff-lineno"></td>'.
-                       '<td colspan="2" class="diff-lineno">property / 
<a>PID</a>: <i>SNAK</i> / qualifier</td></tr>'.
+                       '<td colspan="2" class="diff-lineno">property / P12: 
foo / qualifier</td></tr>'.
                        '<tr><td colspan="2">&nbsp;</td><td 
class="diff-marker">+</td><td class="diff-addedline">'.
-                       '<div><ins class="diffchange 
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></ins></div></td></tr>'.
+                       '<div><ins class="diffchange 
diffchange-inline"><span>P50: v (DETAILED)</span></ins></div></td></tr>'.
 
                        // reference
                        '<tr><td colspan="2" class="diff-lineno"></td>'.
-                       '<td colspan="2" class="diff-lineno">property / 
<a>PID</a>: <i>SNAK</i> / reference</td></tr>'.
+                       '<td colspan="2" class="diff-lineno">property / P12: 
foo / reference</td></tr>'.
                        '<tr><td colspan="2">&nbsp;</td><td 
class="diff-marker">+</td><td class="diff-addedline">'.
-                       '<div><ins class="diffchange 
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></ins></div></td></tr>';
+                       '<div><ins class="diffchange 
diffchange-inline"><span>P44: referencevalue 
(DETAILED)</span></ins></div></td></tr>';
 
                $visualizer = $this->newClaimDifferenceVisualizer();
                $claim = new Statement(
                        new Claim(
                                new PropertyValueSnak( new PropertyId( 'P12' ), 
new StringValue( 'foo' ) ),
-                               new SnakList( array( new PropertyNoValueSnak( 
50 ) ) )
+                               new SnakList( array( new PropertyValueSnak( 50, 
new StringValue( 'v' ) ) ) )
                        ),
                        new ReferenceList( array(
                                new Reference(
@@ -216,38 +203,38 @@
        public function testVisualizeRemovedClaim(){
                $expect =
                        // main snak
-                       '<tr><td colspan="2" class="diff-lineno">property / 
<a>PID</a></td>'.
+                       '<tr><td colspan="2" class="diff-lineno">property / 
P12</td>'.
                        '<td colspan="2" class="diff-lineno"></td></tr>'.
                        '<tr><td class="diff-marker">-</td><td 
class="diff-deletedline">'.
-                       '<div><del class="diffchange 
diffchange-inline"><span><i>SNAK</i></span></del></div>'.
+                       '<div><del class="diffchange 
diffchange-inline"><span>foo (DETAILED)</span></del></div>'.
                        '</td><td colspan="2">&nbsp;</td></tr>'.
 
                        // rank
-                       '<tr><td colspan="2" class="diff-lineno">property / 
<a>PID</a>: <i>SNAK</i> / rank</td>'.
+                       '<tr><td colspan="2" class="diff-lineno">property / 
P12: foo / rank</td>'.
                        '<td colspan="2" class="diff-lineno"></td></tr>'.
                        '<tr><td class="diff-marker">-</td><td 
class="diff-deletedline">'.
                        '<div><del class="diffchange 
diffchange-inline"><span>Normal rank</span></del></div>'
                        .'</td><td colspan="2">&nbsp;</td></tr>'.
 
                        // qualifier
-                       '<tr><td colspan="2" class="diff-lineno">property / 
<a>PID</a>: <i>SNAK</i> / qualifier</td>'.
+                       '<tr><td colspan="2" class="diff-lineno">property / 
P12: foo / qualifier</td>'.
                        '<td colspan="2" class="diff-lineno"></td></tr>'.
                        '<tr><td class="diff-marker">-</td><td 
class="diff-deletedline">'.
-                       '<div><del class="diffchange 
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></del></div>'.
+                       '<div><del class="diffchange 
diffchange-inline"><span>P50: v (DETAILED)</span></del></div>'.
                        '</td><td colspan="2">&nbsp;</td></tr>'.
 
                        // reference
-                       '<tr><td colspan="2" class="diff-lineno">property / 
<a>PID</a>: <i>SNAK</i> / reference</td>'.
+                       '<tr><td colspan="2" class="diff-lineno">property / 
P12: foo / reference</td>'.
                        '<td colspan="2" class="diff-lineno"></td></tr>'.
                        '<tr><td class="diff-marker">-</td><td 
class="diff-deletedline">'.
-                       '<div><del class="diffchange 
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></del></div>'.
+                       '<div><del class="diffchange 
diffchange-inline"><span>P44: referencevalue (DETAILED)</span></del></div>'.
                        '</td><td colspan="2">&nbsp;</td></tr>';
 
                $visualizer = $this->newClaimDifferenceVisualizer();
                $claim = new Statement(
                        new Claim(
                                new PropertyValueSnak( new PropertyId( 'P12' ), 
new StringValue( 'foo' ) ),
-                               new SnakList( array( new PropertyNoValueSnak( 
50 ) ) )
+                               new SnakList( array( new PropertyValueSnak( 50, 
new StringValue( 'v' ) ) ) )
                        ),
                        new ReferenceList( array(
                                new Reference(
diff --git a/repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php 
b/repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
new file mode 100644
index 0000000..7558920
--- /dev/null
+++ b/repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
@@ -0,0 +1,162 @@
+<?php
+
+namespace Wikibase\Test;
+
+use DataValues\StringValue;
+use MediaWikiTestCase;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Snak\PropertyNoValueSnak;
+use Wikibase\DataModel\Snak\PropertySomeValueSnak;
+use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\Lib\SnakFormatter;
+use Wikibase\Repo\Diff\DifferencesSnakVisualizer;
+
+/**
+ * @covers Wikibase\Repo\Diff\DifferencesSnakVisualizer
+ *
+ * @group Wikibase
+ * @group WikibaseRepo
+ * @group WikibaseClaim
+ *
+ * @licence GNU GPL v2+
+ * @author Adrian Heine < [email protected] >
+ */
+class DifferencesSnakVisualizerTest extends MediaWikiTestCase {
+
+       public function newSnakFormatter( $returnValue = '<i>SNAK</i>', $format 
= SnakFormatter::FORMAT_HTML ) {
+               $instance = $this->getMock( 'Wikibase\Lib\SnakFormatter' );
+               $instance->expects( $this->any() )
+                       ->method( 'getFormat' )
+                       ->will( $this->returnValue( $format ) );
+               $instance->expects( $this->any() )
+                       ->method( 'canFormatSnak' )
+                       ->will( $this->returnValue( true ) );
+               $instance->expects( $this->any() )
+                       ->method( 'formatSnak' )
+                       ->will( $this->returnValue( $returnValue ) );
+               return $instance;
+       }
+
+       public function newEntityIdLabelFormatter() {
+               $instance = $this
+                       ->getMockBuilder( 'Wikibase\Lib\EntityIdLabelFormatter' 
)
+                       ->disableOriginalConstructor()
+                       ->getMock();
+
+               $instance->expects( $this->any() )
+                       ->method( 'format' )
+                       ->will( $this->returnValue( '<a>PID</a>' ) );
+
+               return $instance;
+       }
+
+       public function newDifferencesSnakVisualizer(){
+               return new DifferencesSnakVisualizer(
+                       $this->newEntityIdLabelFormatter(),
+                       $this->newSnakFormatter( '<i>DETAILED SNAK</i>' ),
+                       $this->newSnakFormatter(),
+                       'en'
+               );
+       }
+
+       public function testConstruction(){
+               $instance = $this->newDifferencesSnakVisualizer();
+               $this->assertInstanceOf( 
'Wikibase\Repo\Diff\DifferencesSnakVisualizer', $instance );
+       }
+
+       public function testConstructionWithBadDetailsFormatter() {
+               $this->setExpectedException( 'InvalidArgumentException' );
+               new DifferencesSnakVisualizer(
+                       $this->newEntityIdLabelFormatter(),
+                       $this->newSnakFormatter( '', 'qwertyuiop' ),
+                       $this->newSnakFormatter(),
+                       'en'
+               );
+       }
+
+       public function testConstructionWithBadTerseFormatter() {
+               $this->setExpectedException( 'InvalidArgumentException' );
+               new DifferencesSnakVisualizer(
+                       $this->newEntityIdLabelFormatter(),
+                       $this->newSnakFormatter(),
+                       $this->newSnakFormatter( '', 'qwertyuiop' ),
+                       'en'
+               );
+       }
+
+       /**
+        * @dataProvider provideGetPropertyAndDetailedValue
+        */
+       public function testGetPropertyAndDetailedValue( $snak, $expected ) {
+               $snakVisualizer = $this->newDifferencesSnakVisualizer();
+               $result = $snakVisualizer->getPropertyAndDetailedValue( $snak );
+               $this->assertEquals($result, $expected );
+       }
+
+       public function provideGetPropertyAndDetailedValue() {
+               $expected = '<a>PID</a>: <i>DETAILED SNAK</i>';
+               return array(
+                       array( new PropertySomeValueSnak( new PropertyId( 'P1' 
) ), $expected ),
+                       array( new PropertyNoValueSnak( new PropertyId( 'P1' ) 
), $expected ),
+                       array( new PropertyValueSnak( new PropertyId( 'P1' ), 
new StringValue( '' ) ), $expected ),
+                       //array( null, '' ),
+               );
+       }
+
+       /**
+        * @dataProvider provideGetDetailedValue
+        */
+       public function testGetDetailedValue( $snak, $expected ) {
+               $snakVisualizer = $this->newDifferencesSnakVisualizer();
+               $result = $snakVisualizer->getDetailedValue( $snak );
+               $this->assertEquals($result, $expected );
+       }
+
+       public function provideGetDetailedValue() {
+               $expected = '<i>DETAILED SNAK</i>';
+               return array(
+                       array( new PropertySomeValueSnak( new PropertyId( 'P1' 
) ), $expected ),
+                       array( new PropertyNoValueSnak( new PropertyId( 'P1' ) 
), $expected ),
+                       array( new PropertyValueSnak( new PropertyId( 'P1' ), 
new StringValue( '' ) ), $expected ),
+                       array( null, '' ),
+               );
+       }
+
+       /**
+        * @dataProvider provideGetPropertyAndValueHeader
+        */
+       public function testGetPropertyAndValueHeader( $snak, $expected ) {
+               $snakVisualizer = $this->newDifferencesSnakVisualizer();
+               $result = $snakVisualizer->getPropertyAndValueHeader( $snak );
+               $this->assertEquals($result, $expected );
+       }
+
+       public function provideGetPropertyAndValueHeader() {
+               $expected = 'property / <a>PID</a>: <i>SNAK</i>';
+               return array(
+                       array( new PropertySomeValueSnak( new PropertyId( 'P1' 
) ), $expected ),
+                       array( new PropertyNoValueSnak( new PropertyId( 'P1' ) 
), $expected ),
+                       array( new PropertyValueSnak( new PropertyId( 'P1' ), 
new StringValue( '' ) ), $expected ),
+                       //array( null, '' ),
+               );
+       }
+
+       /**
+        * @dataProvider provideGetPropertyHeader
+        */
+       public function testGetPropertyHeader( $snak, $expected ) {
+               $snakVisualizer = $this->newDifferencesSnakVisualizer();
+               $result = $snakVisualizer->getPropertyHeader( $snak );
+               $this->assertEquals($result, $expected );
+       }
+
+       public function provideGetPropertyHeader() {
+               $expected  = 'property / <a>PID</a>';
+               return array(
+                       array( new PropertySomeValueSnak( new PropertyId( 'P1' 
) ), $expected ),
+                       array( new PropertyNoValueSnak( new PropertyId( 'P1' ) 
), $expected ),
+                       array( new PropertyValueSnak( new PropertyId( 'P1' ), 
new StringValue( '' ) ), $expected ),
+                       array( null, 'property' ),
+               );
+       }
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/188552
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5885448773298e82c82e1c8263ca4590cfa705cc
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Adrian Lang <[email protected]>
Gerrit-Reviewer: Adrian Lang <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[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

Reply via email to