Merge "[MCR] Render multi-slot diffs"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 21 Aug 2018 14:04:56 +0000 (14:04 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 21 Aug 2018 14:04:56 +0000 (14:04 +0000)
15 files changed:
RELEASE-NOTES-1.32
autoload.php
docs/hooks.txt
includes/content/ContentHandler.php
includes/diff/DifferenceEngine.php
includes/diff/DifferenceEngineSlotDiffRenderer.php [new file with mode: 0644]
includes/diff/SlotDiffRenderer.php [new file with mode: 0644]
includes/diff/TextSlotDiffRenderer.php [new file with mode: 0644]
resources/src/mediawiki.diff.styles/diff.css
tests/common/TestsAutoLoader.php
tests/phpunit/includes/content/ContentHandlerTest.php
tests/phpunit/includes/diff/CustomDifferenceEngine.php [new file with mode: 0644]
tests/phpunit/includes/diff/DifferenceEngineSlotDiffRendererTest.php [new file with mode: 0644]
tests/phpunit/includes/diff/DifferenceEngineTest.php
tests/phpunit/includes/diff/TextSlotDiffRendererTest.php [new file with mode: 0644]

index cc75699..3a1cc88 100644 (file)
@@ -71,6 +71,10 @@ production.
   additional links to the subtitle of a history page.
 * The 'GetLinkColours' hook now receives an additional $title parameter,
   the Title object of the page being parsed, on which the links will be shown.
+* (T194731) DifferenceEngine supports multiple slots. Added SlotDiffRenderer to
+  render diffs between two Content objects, and DifferenceEngine::setRevisions()
+  to render diffs between two custom (potentially multi-content) revisions.
+  Added GetSlotDiffRenderer hook which works like GetDifferenceEngine for slots.
 
 === External library changes in 1.32 ===
 * …
@@ -343,12 +347,20 @@ because of Phabricator reports.
   Set $wgShowExceptionDetails and/or $wgShowHostnames instead.
 * The $wgShowDBErrorBacktrace global is deprecated and nonfunctional.
   Set $wgShowExceptionDetails instead.
-* Public access to the DifferenceEngine properties mOldid, mNewid, mOldPage,
-  mNewPage, mOldContent, mNewContent, mRevisionsLoaded, mTextLoaded and
-  mCacheHit is deprecated. Use getOldid() / getNewid() for the first two,
-  do your own lookup for page/content. mNewRev / mOldRev remains public.
+* Public access to the DifferenceEngine properties mOldid, mNewid, mOldRev,
+  mNewRev, mOldPage, mNewPage, mOldContent, mNewContent, mRevisionsLoaded,
+  mTextLoaded and mCacheHit is deprecated. Use getOldid() / getNewid() /
+  getOldRevision() / getNewRevision() for the first four (note that the
+  revision ones return a RevisionRecord, not a Revision), do your own lookup
+  for page/content.
 * The $wgExternalDiffEngine value 'wikidiff2' is deprecated. To use wikidiff2
   just enable the PHP extension, and it will be autodetected.
+* (T194731) DifferenceEngine properties mOldContent and mNewContent and methods
+  setContent(), generateContentDiffBody(), generateTextDiffBody() and textDiff()
+  are deprecated. To interact with a single slot, use a SlotDiffRenderer (and
+  subclass it to customize diff rendering); to diff custom (e.g. unsaved)
+  content, use setRevisions(). Subclassing DifferenceEngine should only be done
+  to customize page-level diff properties (such as the navigation header).
 * The wfUseMW function, soft-deprecated in 1.26, is now hard deprecated.
 * All MagicWord static methods are now deprecated.  Use the MagicWordFactory
   methods instead.
index a372dd1..8679827 100644 (file)
@@ -405,6 +405,7 @@ $wgAutoloadLocalClasses = [
        'DiffOpCopy' => __DIR__ . '/includes/diff/DairikiDiff.php',
        'DiffOpDelete' => __DIR__ . '/includes/diff/DairikiDiff.php',
        'DifferenceEngine' => __DIR__ . '/includes/diff/DifferenceEngine.php',
+       'DifferenceEngineSlotDiffRenderer' => __DIR__ . '/includes/diff/DifferenceEngineSlotDiffRenderer.php',
        'Digit2Html' => __DIR__ . '/maintenance/language/digit2html.php',
        'DjVuHandler' => __DIR__ . '/includes/media/DjVuHandler.php',
        'DjVuImage' => __DIR__ . '/includes/media/DjVuImage.php',
@@ -1343,6 +1344,7 @@ $wgAutoloadLocalClasses = [
        'SkinFallbackTemplate' => __DIR__ . '/includes/skins/SkinFallbackTemplate.php',
        'SkinTemplate' => __DIR__ . '/includes/skins/SkinTemplate.php',
        'SlideshowImageGallery' => __DIR__ . '/includes/gallery/SlideshowImageGallery.php',
+       'SlotDiffRenderer' => __DIR__ . '/includes/diff/SlotDiffRenderer.php',
        'SpecialActiveUsers' => __DIR__ . '/includes/specials/SpecialActiveusers.php',
        'SpecialAllMessages' => __DIR__ . '/includes/specials/SpecialAllMessages.php',
        'SpecialAllMyUploads' => __DIR__ . '/includes/specials/SpecialMyRedirectPages.php',
@@ -1475,6 +1477,7 @@ $wgAutoloadLocalClasses = [
        'TextContent' => __DIR__ . '/includes/content/TextContent.php',
        'TextContentHandler' => __DIR__ . '/includes/content/TextContentHandler.php',
        'TextPassDumper' => __DIR__ . '/maintenance/dumpTextPass.php',
+       'TextSlotDiffRenderer' => __DIR__ . '/includes/diff/TextSlotDiffRenderer.php',
        'TextStatsOutput' => __DIR__ . '/maintenance/language/StatOutputs.php',
        'TgConverter' => __DIR__ . '/languages/classes/LanguageTg.php',
        'ThrottledError' => __DIR__ . '/includes/exception/ThrottledError.php',
index 9a634ad..9cb22d2 100644 (file)
@@ -1783,6 +1783,12 @@ $relativeTo: MWTimestamp object of the relative (user-adjusted) timestamp
 $user: User whose preferences are being used to make timestamp
 $lang: Language that will be used to render the timestamp
 
+'GetSlotDiffRenderer': Replace or wrap the standard SlotDiffRenderer for some
+content type.
+$contentHandler: ContentHandler for which the slot diff renderer is fetched.
+&$slotDiffRenderer: SlotDiffRenderer to change or replace.
+$context: IContextSource
+
 'getUserPermissionsErrors': Add a permissions error when permissions errors are
 checked for. Use instead of userCan for most cases. Return false if the user
 can't do it, and populate $result with the reason in the form of
index 0a451b2..b3286a9 100644 (file)
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Search\ParserOutputSearchDataExtractor;
 
@@ -613,6 +614,8 @@ abstract class ContentHandler {
 
        /**
         * Factory for creating an appropriate DifferenceEngine for this content model.
+        * Since 1.32, this is only used for page-level diffs; to diff two content objects,
+        * use getSlotDiffRenderer.
         *
         * The DifferenceEngine subclass to use is selected in getDiffEngineClass(). The
         * GetDifferenceEngine hook will receive the DifferenceEngine object and can replace or
@@ -622,6 +625,9 @@ abstract class ContentHandler {
         * the DifferenceEngine instance from the hook. If the owner of the content type wants to
         * decorare the instance, overriding this method is a safer approach.)
         *
+        * @todo This is page-level functionality so it should not belong to ContentHandler.
+        *   Move it to a better place once one exists (e.g. PageTypeHandler).
+        *
         * @since 1.21
         *
         * @param IContextSource $context Context to use, anything else will be ignored.
@@ -644,6 +650,60 @@ abstract class ContentHandler {
                return $differenceEngine;
        }
 
+       /**
+        * Get an appropriate SlotDiffRenderer for this content model.
+        * @since 1.32
+        * @param IContextSource $context
+        * @return SlotDiffRenderer
+        */
+       final public function getSlotDiffRenderer( IContextSource $context ) {
+               $slotDiffRenderer = $this->getSlotDiffRendererInternal( $context );
+               if ( get_class( $slotDiffRenderer ) === TextSlotDiffRenderer::class ) {
+                       //  To keep B/C, when SlotDiffRenderer is not overridden for a given content type
+                       // but DifferenceEngine is, use that instead.
+                       $differenceEngine = $this->createDifferenceEngine( $context );
+                       if ( get_class( $differenceEngine ) !== DifferenceEngine::class ) {
+                               // TODO turn this into a deprecation warning in a later release
+                               LoggerFactory::getInstance( 'diff' )->notice(
+                                       'Falling back to DifferenceEngineSlotDiffRenderer', [
+                                               'modelID' => $this->getModelID(),
+                                               'DifferenceEngine' => get_class( $differenceEngine ),
+                                       ] );
+                               $slotDiffRenderer = new DifferenceEngineSlotDiffRenderer( $differenceEngine );
+                       }
+               }
+               Hooks::run( 'GetSlotDiffRenderer', [ $this, &$slotDiffRenderer, $context ] );
+               return $slotDiffRenderer;
+       }
+
+       /**
+        * Return the SlotDiffRenderer appropriate for this content handler.
+        * @param IContextSource $context
+        * @return SlotDiffRenderer
+        */
+       protected function getSlotDiffRendererInternal( IContextSource $context ) {
+               $contentLanguage = MediaWikiServices::getInstance()->getContentLanguage();
+               $statsdDataFactory = MediaWikiServices::getInstance()->getStatsdDataFactory();
+               $slotDiffRenderer = new TextSlotDiffRenderer();
+               $slotDiffRenderer->setStatsdDataFactory( $statsdDataFactory );
+               // XXX using the page language would be better, but it's unclear how that should be injected
+               $slotDiffRenderer->setLanguage( $contentLanguage );
+               $slotDiffRenderer->setWikiDiff2MovedParagraphDetectionCutoff(
+                       $context->getConfig()->get( 'WikiDiff2MovedParagraphDetectionCutoff' )
+               );
+
+               $engine = DifferenceEngine::getEngine();
+               if ( $engine === false ) {
+                       $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_PHP );
+               } elseif ( $engine === 'wikidiff2' ) {
+                       $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2 );
+               } else {
+                       $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_EXTERNAL, $engine );
+               }
+
+               return $slotDiffRenderer;
+       }
+
        /**
         * Get the language in which the content of the given page is written.
         *
index 5138655..2ceda21 100644 (file)
  * @file
  * @ingroup DifferenceEngine
  */
-use MediaWiki\MediaWikiServices;
-use MediaWiki\Shell\Shell;
+
+use MediaWiki\Storage\RevisionRecord;
 
 /**
- * @todo document
+ * DifferenceEngine is responsible for rendering the difference between two revisions as HTML.
+ * This includes interpreting URL parameters, retrieving revision data, checking access permissions,
+ * selecting and invoking the diff generator class for the individual slots, doing post-processing
+ * on the generated diff, adding the rest of the HTML (such as headers) and writing the whole thing
+ * to OutputPage.
+ *
+ * DifferenceEngine can be subclassed by extensions, by customizing
+ * ContentHandler::createDifferenceEngine; the content handler will be selected based on the
+ * content model of the main slot (of the new revision, when the two are different).
+ * That might change after PageTypeHandler gets introduced.
+ *
+ * In the past, the class was also used for slot-level diff generation, and extensions might still
+ * subclass it and add such functionality. When that is the case (sepcifically, when a
+ * ContentHandler returns a standard SlotDiffRenderer but a nonstandard DifferenceEngine)
+ * DifferenceEngineSlotDiffRenderer will be used to convert the old behavior into the new one.
+ *
  * @ingroup DifferenceEngine
+ *
+ * @todo This class is huge and poorly defined. It should be split into a controller responsible
+ * for interpreting query parameters, retrieving data and checking permissions; and a HTML renderer.
  */
 class DifferenceEngine extends ContextSource {
 
@@ -48,26 +66,55 @@ class DifferenceEngine extends ContextSource {
        private $mOldTags;
        private $mNewTags;
 
-       /** @var Content|null */
-       protected $mOldContent;
-
-       /** @var Content|null */
-       protected $mNewContent;
+       /**
+        * Old revision (left pane).
+        * Allowed to be an unsaved revision, unlikely that's ever needed though.
+        * Null when the old revision does not exist; this can happen when using
+        * diff=prev on the first revision.
+        * Since 1.32 public access is deprecated.
+        * @var Revision|null
+        */
+       protected $mOldRev;
 
-       /** @var Language */
-       protected $mDiffLang;
+       /**
+        * New revision (right pane).
+        * Note that this might be an unsaved revision (e.g. for edit preview).
+        * Null only in case of load failure; diff methods will just return an error message in that case.
+        * Since 1.32 public access is deprecated.
+        * @var Revision|null
+        */
+       protected $mNewRev;
 
-       /** @var Title */
+       /**
+        * Title of $mOldRev or null if the old revision does not exist or does not belong to a page.
+        * Since 1.32 public access is deprecated and the property can be null.
+        * @var Title|null
+        */
        protected $mOldPage;
 
-       /** @var Title */
+       /**
+        * Title of $mNewRev or null if the new revision does not exist or does not belong to a page.
+        * Since 1.32 public access is deprecated and the property can be null.
+        * @var Title|null
+        */
        protected $mNewPage;
 
-       /** @var Revision|null */
-       public $mOldRev;
+       /**
+        * @var Content|null
+        * @deprecated since 1.32, content slots are now handled by the corresponding SlotDiffRenderer.
+        *   This property is set to the content of the main slot, but not actually used for the main diff.
+        */
+       private $mOldContent;
+
+       /**
+        * @var Content|null
+        * @deprecated since 1.32, content slots are now handled by the corresponding SlotDiffRenderer.
+        *   This property is set to the content of the main slot, but not actually used for the main diff.
+        */
+       private $mNewContent;
 
-       /** @var Revision|null */
-       public $mNewRev;
+       /** @var Language */
+       protected $mDiffLang;
 
        /** @var bool Have the revisions IDs been loaded */
        private $mRevisionsIdsLoaded = false;
@@ -80,7 +127,10 @@ class DifferenceEngine extends ContextSource {
 
        /**
         * Was the content overridden via setContent()?
-        * If the content was overridden, most internal state (e.g. mOldid or mOldRev) should be ignored.
+        * If the content was overridden, most internal state (e.g. mOldid or mOldRev) should be ignored
+        * and only mOldContent and mNewContent is reliable.
+        * (Note that setRevisions() does not set this flag as in that case all properties are
+        * overriden and remain consistent with each other, so no special handling is needed.)
         * @var bool
         */
        protected $isContentOverridden = false;
@@ -109,6 +159,17 @@ class DifferenceEngine extends ContextSource {
        /** @var bool Refresh the diff cache */
        protected $mRefreshCache = false;
 
+       /** @var SlotDiffRenderer[] DifferenceEngine classes for the slots, keyed by role name. */
+       protected $slotDiffRenderers = null;
+
+       /**
+        * Temporary hack for B/C while slot diff related methods of DifferenceEngine are being
+        * deprecated. When true, we are inside a DifferenceEngineSlotDiffRenderer and
+        * $slotDiffRenderers should not be used.
+        * @var bool
+        */
+       protected $isSlotDiffRenderer = false;
+
        /**#@-*/
 
        /**
@@ -124,6 +185,8 @@ class DifferenceEngine extends ContextSource {
        ) {
                $this->deprecatePublicProperty( 'mOldid', '1.32', __CLASS__ );
                $this->deprecatePublicProperty( 'mNewid', '1.32', __CLASS__ );
+               $this->deprecatePublicProperty( 'mOldRev', '1.32', __CLASS__ );
+               $this->deprecatePublicProperty( 'mNewRev', '1.32', __CLASS__ );
                $this->deprecatePublicProperty( 'mOldPage', '1.32', __CLASS__ );
                $this->deprecatePublicProperty( 'mNewPage', '1.32', __CLASS__ );
                $this->deprecatePublicProperty( 'mOldContent', '1.32', __CLASS__ );
@@ -145,6 +208,81 @@ class DifferenceEngine extends ContextSource {
        }
 
        /**
+        * @return SlotDiffRenderer[] Diff renderers for each slot, keyed by role name.
+        *   Includes slots only present in one of the revisions.
+        */
+       protected function getSlotDiffRenderers() {
+               if ( $this->isSlotDiffRenderer ) {
+                       throw new LogicException( __METHOD__ . ' called in slot diff renderer mode' );
+               }
+
+               if ( $this->slotDiffRenderers === null ) {
+                       if ( !$this->loadRevisionData() ) {
+                               return [];
+                       }
+
+                       $slotContents = $this->getSlotContents();
+                       $this->slotDiffRenderers = array_map( function ( $contents ) {
+                               /** @var $content Content */
+                               $content = $contents['new'] ?: $contents['old'];
+                               return $content->getContentHandler()->getSlotDiffRenderer( $this->getContext() );
+                       }, $slotContents );
+               }
+               return $this->slotDiffRenderers;
+       }
+
+       /**
+        * Mark this DifferenceEngine as a slot renderer (as opposed to a page renderer).
+        * This is used in legacy mode when the DifferenceEngine is wrapped in a
+        * DifferenceEngineSlotDiffRenderer.
+        * @internal For use by DifferenceEngineSlotDiffRenderer only.
+        */
+       public function markAsSlotDiffRenderer() {
+               $this->isSlotDiffRenderer = true;
+       }
+
+       /**
+        * Get the old and new content objects for all slots.
+        * This method does not do any permission checks.
+        * @return array [ role => [ 'old' => SlotRecord, 'new' => SlotRecord ], ... ]
+        */
+       protected function getSlotContents() {
+               if ( $this->isContentOverridden ) {
+                       return [
+                               'main' => [
+                                       'old' => $this->mOldContent,
+                                       'new' => $this->mNewContent,
+                               ]
+                       ];
+               }
+
+               $oldRev = $this->mOldRev->getRevisionRecord();
+               $newRev = $this->mNewRev->getRevisionRecord();
+               // The order here will determine the visual order of the diff. The current logic is
+               // changed first, then added, then deleted. This is ad hoc and should not be relied on
+               // - in the future we may want the ordering to depend on the page type.
+               $roles = array_merge( $newRev->getSlotRoles(), $oldRev->getSlotRoles() );
+               $oldSlots = $oldRev->getSlots()->getSlots();
+               $newSlots = $newRev->getSlots()->getSlots();
+
+               $slots = [];
+               foreach ( $roles as $role ) {
+                       $slots[$role] = [
+                               'old' => isset( $oldSlots[$role] ) ? $oldSlots[$role]->getContent() : null,
+                               'new' => isset( $newSlots[$role] ) ? $newSlots[$role]->getContent() : null,
+                       ];
+               }
+               // move main slot to front
+               if ( isset( $slots['main'] ) ) {
+                       $slots = [ 'main' => $slots['main'] ] + $slots;
+               }
+               return $slots;
+       }
+
+       /**
+        * Set reduced line numbers mode.
+        * When set, line X is not displayed when X is 1, for example to increase readability and
+        * conserve space with many small diffs.
         * @param bool $value
         */
        public function setReducedLineNumbers( $value = true ) {
@@ -190,6 +328,25 @@ class DifferenceEngine extends ContextSource {
                return $this->mNewid;
        }
 
+       /**
+        * Get the left side of the diff.
+        * Could be null when the first revision of the page is diffed to 'prev' (or in the case of
+        * load failure).
+        * @return RevisionRecord|null
+        */
+       public function getOldRevision() {
+               return $this->mOldRev ? $this->mOldRev->getRevisionRecord() : null;
+       }
+
+       /**
+        * Get the right side of the diff.
+        * Should not be null but can still happen in the case of load failure.
+        * @return RevisionRecord|null
+        */
+       public function getNewRevision() {
+               return $this->mNewRev ? $this->mNewRev->getRevisionRecord() : null;
+       }
+
        /**
         * Look up a special:Undelete link to the given deleted revision id,
         * as a workaround for being unable to load deleted diffs in currently.
@@ -280,8 +437,11 @@ class DifferenceEngine extends ContextSource {
                }
 
                $user = $this->getUser();
-               $permErrors = $this->mNewPage->getUserPermissionsErrors( 'read', $user );
-               if ( $this->mOldPage ) { # mOldPage might not be set, see below.
+               $permErrors = [];
+               if ( $this->mNewPage ) {
+                       $permErrors = $this->mNewPage->getUserPermissionsErrors( 'read', $user );
+               }
+               if ( $this->mOldPage ) {
                        $permErrors = wfMergeErrorArrays( $permErrors,
                                $this->mOldPage->getUserPermissionsErrors( 'read', $user ) );
                }
@@ -311,7 +471,9 @@ class DifferenceEngine extends ContextSource {
                # a diff between a version V and its previous version V' AND the version V
                # is the first version of that article. In that case, V' does not exist.
                if ( $this->mOldRev === false ) {
-                       $out->setPageTitle( $this->msg( 'difference-title', $this->mNewPage->getPrefixedText() ) );
+                       if ( $this->mNewPage ) {
+                               $out->setPageTitle( $this->msg( 'difference-title', $this->mNewPage->getPrefixedText() ) );
+                       }
                        $samePage = true;
                        $oldHeader = '';
                        // Allow extensions to change the $oldHeader variable
@@ -319,7 +481,10 @@ class DifferenceEngine extends ContextSource {
                } else {
                        Hooks::run( 'DiffViewHeader', [ $this, $this->mOldRev, $this->mNewRev ] );
 
-                       if ( $this->mNewPage->equals( $this->mOldPage ) ) {
+                       if ( !$this->mOldPage || !$this->mNewPage ) {
+                               // XXX say something to the user?
+                               $samePage = false;
+                       } elseif ( $this->mNewPage->equals( $this->mOldPage ) ) {
                                $out->setPageTitle( $this->msg( 'difference-title', $this->mNewPage->getPrefixedText() ) );
                                $samePage = true;
                        } else {
@@ -329,7 +494,7 @@ class DifferenceEngine extends ContextSource {
                                $samePage = false;
                        }
 
-                       if ( $samePage && $this->mNewPage->quickUserCan( 'edit', $user ) ) {
+                       if ( $samePage && $this->mNewPage && $this->mNewPage->quickUserCan( 'edit', $user ) ) {
                                if ( $this->mNewRev->isCurrent() && $this->mNewPage->userCan( 'rollback', $user ) ) {
                                        $rollbackLink = Linker::generateRollback( $this->mNewRev, $this->getContext() );
                                        if ( $rollbackLink ) {
@@ -356,7 +521,7 @@ class DifferenceEngine extends ContextSource {
                        }
 
                        # Make "previous revision link"
-                       if ( $samePage && $this->mOldRev->getPrevious() ) {
+                       if ( $samePage && $this->mOldPage && $this->mOldRev->getPrevious() ) {
                                $prevlink = Linker::linkKnown(
                                        $this->mOldPage,
                                        $this->msg( 'previousdiff' )->escaped(),
@@ -409,7 +574,7 @@ class DifferenceEngine extends ContextSource {
 
                # Make "next revision link"
                # Skip next link on the top revision
-               if ( $samePage && !$this->mNewRev->isCurrent() ) {
+               if ( $samePage && $this->mNewPage && !$this->mNewRev->isCurrent() ) {
                        $nextlink = Linker::linkKnown(
                                $this->mNewPage,
                                $this->msg( 'nextdiff' )->escaped(),
@@ -517,7 +682,7 @@ class DifferenceEngine extends ContextSource {
                if ( $this->mMarkPatrolledLink === null ) {
                        $linkInfo = $this->getMarkPatrolledLinkInfo();
                        // If false, there is no patrol link needed/allowed
-                       if ( !$linkInfo ) {
+                       if ( !$linkInfo || !$this->mNewPage ) {
                                $this->mMarkPatrolledLink = '';
                        } else {
                                $this->mMarkPatrolledLink = ' <span class="patrollink" data-mw="interface">[' .
@@ -553,7 +718,7 @@ class DifferenceEngine extends ContextSource {
                // Prepare a change patrol link, if applicable
                if (
                        // Is patrolling enabled and the user allowed to?
-                       $wgUseRCPatrol && $this->mNewPage->quickUserCan( 'patrol', $user ) &&
+                       $wgUseRCPatrol && $this->mNewPage && $this->mNewPage->quickUserCan( 'patrol', $user ) &&
                        // Only do this if the revision isn't more than 6 hours older
                        // than the Max RC age (6h because the RC might not be cleaned out regularly)
                        RecentChange::isInRCLifespan( $this->mNewRev->getTimestamp(), 21600 )
@@ -626,6 +791,12 @@ class DifferenceEngine extends ContextSource {
                # Page content may be handled by a hooked call instead...
                if ( Hooks::run( 'ArticleContentOnDiff', [ $this, $out ] ) ) {
                        $this->loadNewText();
+                       if ( !$this->mNewPage ) {
+                               // New revision is unsaved; bail out.
+                               // TODO in theory rendering the new revision is a meaningful thing to do
+                               // even if it's unsaved, but a lot of untangling is required to do it safely.
+                       }
+
                        $out->setRevisionId( $this->mNewid );
                        $out->setRevisionTimestamp( $this->mNewRev->getTimestamp() );
                        $out->setArticleFlag( true );
@@ -714,7 +885,12 @@ class DifferenceEngine extends ContextSource {
         * Add style sheets for diff display.
         */
        public function showDiffStyle() {
-               $this->getOutput()->addModuleStyles( 'mediawiki.diff.styles' );
+               if ( !$this->isSlotDiffRenderer ) {
+                       $this->getOutput()->addModuleStyles( 'mediawiki.diff.styles' );
+                       foreach ( $this->getSlotDiffRenderers() as $slotDiffRenderer ) {
+                               $slotDiffRenderer->addModules( $this->getOutput() );
+                       }
+               }
        }
 
        /**
@@ -751,25 +927,28 @@ class DifferenceEngine extends ContextSource {
        public function getDiffBody() {
                $this->mCacheHit = true;
                // Check if the diff should be hidden from this user
-               if ( !$this->loadRevisionData() ) {
-                       return false;
-               } elseif ( $this->mOldRev &&
-                       !$this->mOldRev->userCan( Revision::DELETED_TEXT, $this->getUser() )
-               ) {
-                       return false;
-               } elseif ( $this->mNewRev &&
-                       !$this->mNewRev->userCan( Revision::DELETED_TEXT, $this->getUser() )
-               ) {
-                       return false;
-               }
-               // Short-circuit
-               if ( $this->mOldRev === false || ( $this->mOldRev && $this->mNewRev
-                       && $this->mOldRev->getId() == $this->mNewRev->getId() )
-               ) {
-                       if ( Hooks::run( 'DifferenceEngineShowEmptyOldContent', [ $this ] ) ) {
-                               return '';
+               if ( !$this->isContentOverridden ) {
+                       if ( !$this->loadRevisionData() ) {
+                               return false;
+                       } elseif ( $this->mOldRev &&
+                               !$this->mOldRev->userCan( Revision::DELETED_TEXT, $this->getUser() )
+                       ) {
+                               return false;
+                       } elseif ( $this->mNewRev &&
+                               !$this->mNewRev->userCan( Revision::DELETED_TEXT, $this->getUser() )
+                       ) {
+                               return false;
+                       }
+                       // Short-circuit
+                       if ( $this->mOldRev === false || ( $this->mOldRev && $this->mNewRev &&
+                               $this->mOldRev->getId() && $this->mOldRev->getId() == $this->mNewRev->getId() )
+                       ) {
+                               if ( Hooks::run( 'DifferenceEngineShowEmptyOldContent', [ $this ] ) ) {
+                                       return '';
+                               }
                        }
                }
+
                // Cacheable?
                $key = false;
                $cache = ObjectCache::getMainWANInstance();
@@ -800,7 +979,20 @@ class DifferenceEngine extends ContextSource {
                        return false;
                }
 
-               $difftext = $this->generateContentDiffBody( $this->mOldContent, $this->mNewContent );
+               $difftext = '';
+               // We've checked for revdelete at the beginning of this method; it's OK to ignore
+               // read permissions here.
+               $slotContents = $this->getSlotContents();
+               foreach ( $this->getSlotDiffRenderers() as $role => $slotDiffRenderer ) {
+                       $slotDiff = $slotDiffRenderer->getDiff( $slotContents[$role]['old'],
+                               $slotContents[$role]['new'] );
+                       if ( $slotDiff && $role !== 'main' ) {
+                               // TODO use human-readable role name at least
+                               $slotTitle = $role;
+                               $difftext .= $this->getSlotHeader( $slotTitle );
+                       }
+                       $difftext .= $slotDiff;
+               }
 
                // Avoid PHP 7.1 warning from passing $this by reference
                $diffEngine = $this;
@@ -822,6 +1014,21 @@ class DifferenceEngine extends ContextSource {
                return $difftext;
        }
 
+       /**
+        * Get a slot header for inclusion in a diff body (as a table row).
+        *
+        * @param string $headerText The text of the header
+        * @return string
+        *
+        */
+       protected function getSlotHeader( $headerText ) {
+               // The old revision is missing on oldid=<first>&diff=prev; only 2 columns in that case.
+               $columnCount = $this->mOldRev ? 4 : 2;
+               $userLang = $this->getLanguage()->getHtmlCode();
+               return Html::rawElement( 'tr', [ 'class' => 'mw-diff-slot-header', 'lang' => $userLang ],
+                       Html::element( 'th', [ 'colspan' => $columnCount ], $headerText ) );
+       }
+
        /**
         * Returns the cache key for diff body text or content.
         *
@@ -867,98 +1074,112 @@ class DifferenceEngine extends ContextSource {
                        $params[] = $this->getConfig()->get( 'WikiDiff2MovedParagraphDetectionCutoff' );
                }
 
+               if ( !$this->isSlotDiffRenderer ) {
+                       foreach ( $this->getSlotDiffRenderers() as $slotDiffRenderer ) {
+                               $params = array_merge( $params, $slotDiffRenderer->getExtraCacheKeys() );
+                       }
+               }
+
+               return $params;
+       }
+
+       /**
+        * Implements DifferenceEngineSlotDiffRenderer::getExtraCacheKeys(). Only used when
+        * DifferenceEngine is wrapped in DifferenceEngineSlotDiffRenderer.
+        * @return array
+        * @internal for use by DifferenceEngineSlotDiffRenderer only
+        * @deprecated
+        */
+       public function getExtraCacheKeys() {
+               // This method is called when the DifferenceEngine is used for a slot diff. We only care
+               // about special things, not the revision IDs, which are added to the cache key by the
+               // page-level DifferenceEngine, and which might not have a valid value for this object.
+               $this->mOldid = 123456789;
+               $this->mNewid = 987654321;
+
+               // This will repeat a bunch of unnecessary key fields for each slot. Not nice but harmless.
+               $cacheString = $this->getDiffBodyCacheKey();
+               if ( $cacheString ) {
+                       return [ $cacheString ];
+               }
+
+               $params = $this->getDiffBodyCacheKeyParams();
+
+               // Try to get rid of the standard keys to keep the cache key human-readable:
+               // call the getDiffBodyCacheKeyParams implementation of the base class, and if
+               // the child class includes the same keys, drop them.
+               // Uses an obscure PHP feature where static calls to non-static methods are allowed
+               // as long as we are already in a non-static method of the same class, and the call context
+               // ($this) will be inherited.
+               // phpcs:ignore Squiz.Classes.SelfMemberReference.NotUsed
+               $standardParams = DifferenceEngine::getDiffBodyCacheKeyParams();
+               if ( array_slice( $params, 0, count( $standardParams ) ) === $standardParams ) {
+                       $params = array_slice( $params, count( $standardParams ) );
+               }
+
                return $params;
        }
 
        /**
         * Generate a diff, no caching.
         *
-        * This implementation uses generateTextDiffBody() to generate a diff based on the default
-        * serialization of the given Content objects. This will fail if $old or $new are not
-        * instances of TextContent.
-        *
-        * Subclasses may override this to provide a different rendering for the diff,
-        * perhaps taking advantage of the content's native form. This is required for all content
-        * models that are not text based.
-        *
         * @since 1.21
         *
         * @param Content $old Old content
         * @param Content $new New content
         *
-        * @throws MWException If old or new content is not an instance of TextContent.
+        * @throws Exception If old or new content is not an instance of TextContent.
         * @return bool|string
+        *
+        * @deprecated since 1.32, use a SlotDiffRenderer instead.
         */
        public function generateContentDiffBody( Content $old, Content $new ) {
-               if ( !( $old instanceof TextContent ) ) {
-                       throw new MWException( "Diff not implemented for " . get_class( $old ) . "; " .
-                               "override generateContentDiffBody to fix this." );
-               }
-
-               if ( !( $new instanceof TextContent ) ) {
-                       throw new MWException( "Diff not implemented for " . get_class( $new ) . "; "
-                               . "override generateContentDiffBody to fix this." );
-               }
-
-               $otext = $old->serialize();
-               $ntext = $new->serialize();
-
-               return $this->generateTextDiffBody( $otext, $ntext );
+               $slotDiffRenderer = $new->getContentHandler()->getSlotDiffRenderer( $this->getContext() );
+               if (
+                       $slotDiffRenderer instanceof DifferenceEngineSlotDiffRenderer
+                       && $this->isSlotDiffRenderer
+               ) {
+                       // Oops, we are just about to enter an infinite loop (the slot-level DifferenceEngine
+                       // called a DifferenceEngineSlotDiffRenderer that wraps the same DifferenceEngine class).
+                       // This will happen when a content model has no custom slot diff renderer, it does have
+                       // a custom difference engine, but that does not override this method.
+                       throw new Exception( get_class( $this ) . ': could not maintain backwards compatibility. '
+                               . 'Please use a SlotDiffRenderer.' );
+               }
+               return $slotDiffRenderer->getDiff( $old, $new ) . $this->getDebugString();
        }
 
        /**
         * Generate a diff, no caching
         *
-        * @todo move this to TextDifferenceEngine, make DifferenceEngine abstract. At some point.
-        *
         * @param string $otext Old text, must be already segmented
         * @param string $ntext New text, must be already segmented
         *
+        * @throws Exception If content handling for text content is configured in a way
+        *   that makes maintaining B/C hard.
         * @return bool|string
+        *
+        * @deprecated since 1.32, use a TextSlotDiffRenderer instead.
         */
        public function generateTextDiffBody( $otext, $ntext ) {
-               $diff = function () use ( $otext, $ntext ) {
-                       $time = microtime( true );
-
-                       $result = $this->textDiff( $otext, $ntext );
-
-                       $time = intval( ( microtime( true ) - $time ) * 1000 );
-                       MediaWikiServices::getInstance()->getStatsdDataFactory()->timing( 'diff_time', $time );
-                       // Log requests slower than 99th percentile
-                       if ( $time > 100 && $this->mOldPage && $this->mNewPage ) {
-                               wfDebugLog( 'diff',
-                                       "$time ms diff: {$this->mOldid} -> {$this->mNewid} {$this->mNewPage}" );
-                       }
-
-                       return $result;
-               };
-
-               /**
-                * @param Status $status
-                * @throws FatalError
-                */
-               $error = function ( $status ) {
-                       throw new FatalError( $status->getWikiText() );
-               };
-
-               // Use PoolCounter if the diff looks like it can be expensive
-               if ( strlen( $otext ) + strlen( $ntext ) > 20000 ) {
-                       $work = new PoolCounterWorkViaCallback( 'diff',
-                               md5( $otext ) . md5( $ntext ),
-                               [ 'doWork' => $diff, 'error' => $error ]
-                       );
-                       return $work->execute();
-               }
-
-               return $diff();
+               $slotDiffRenderer = ContentHandler::getForModelID( CONTENT_MODEL_TEXT )
+                       ->getSlotDiffRenderer( $this->getContext() );
+               if ( !( $slotDiffRenderer instanceof TextSlotDiffRenderer ) ) {
+                       // Someone used the GetSlotDiffRenderer hook to replace the renderer.
+                       // This is too unlikely to happen to bother handling properly.
+                       throw new Exception( 'The slot diff renderer for text content should be a '
+                               . 'TextSlotDiffRenderer subclass' );
+               }
+               return $slotDiffRenderer->getTextDiff( $otext, $ntext ) . $this->getDebugString();
        }
 
        /**
         * Process $wgExternalDiffEngine and get a sane, usable engine
         *
         * @return bool|string 'wikidiff2', path to an executable, or false
+        * @internal For use by this class and TextSlotDiffRenderer only.
         */
-       private function getEngine() {
+       public static function getEngine() {
                global $wgExternalDiffEngine;
                // We use the global here instead of Config because we write to the value,
                // and Config is not mutable.
@@ -989,91 +1210,23 @@ class DifferenceEngine extends ContextSource {
         *
         * @param string $otext Old text, must be already segmented
         * @param string $ntext New text, must be already segmented
+        *
+        * @throws Exception If content handling for text content is configured in a way
+        *   that makes maintaining B/C hard.
         * @return bool|string
+        *
+        * @deprecated since 1.32, use a TextSlotDiffRenderer instead.
         */
        protected function textDiff( $otext, $ntext ) {
-               $otext = str_replace( "\r\n", "\n", $otext );
-               $ntext = str_replace( "\r\n", "\n", $ntext );
-
-               $engine = $this->getEngine();
-
-               // Better external diff engine, the 2 may some day be dropped
-               // This one does the escaping and segmenting itself
-               if ( $engine === 'wikidiff2' ) {
-                       $wikidiff2Version = phpversion( 'wikidiff2' );
-                       if (
-                               $wikidiff2Version !== false &&
-                               version_compare( $wikidiff2Version, '1.5.0', '>=' )
-                       ) {
-                               $text = wikidiff2_do_diff(
-                                       $otext,
-                                       $ntext,
-                                       2,
-                                       $this->getConfig()->get( 'WikiDiff2MovedParagraphDetectionCutoff' )
-                               );
-                       } else {
-                               // Don't pass the 4th parameter for compatibility with older versions of wikidiff2
-                               $text = wikidiff2_do_diff(
-                                       $otext,
-                                       $ntext,
-                                       2
-                               );
-
-                               // Log a warning in case the configuration value is set to not silently ignore it
-                               if ( $this->getConfig()->get( 'WikiDiff2MovedParagraphDetectionCutoff' ) > 0 ) {
-                                       wfLogWarning( '$wgWikiDiff2MovedParagraphDetectionCutoff is set but has no
-                                               effect since the used version of WikiDiff2 does not support it.' );
-                               }
-                       }
-
-                       $text .= $this->debug( 'wikidiff2' );
-
-                       return $text;
-               } elseif ( $engine !== false ) {
-                       # Diff via the shell
-                       $tmpDir = wfTempDir();
-                       $tempName1 = tempnam( $tmpDir, 'diff_' );
-                       $tempName2 = tempnam( $tmpDir, 'diff_' );
-
-                       $tempFile1 = fopen( $tempName1, "w" );
-                       if ( !$tempFile1 ) {
-                               return false;
-                       }
-                       $tempFile2 = fopen( $tempName2, "w" );
-                       if ( !$tempFile2 ) {
-                               return false;
-                       }
-                       fwrite( $tempFile1, $otext );
-                       fwrite( $tempFile2, $ntext );
-                       fclose( $tempFile1 );
-                       fclose( $tempFile2 );
-                       $cmd = [ $engine, $tempName1, $tempName2 ];
-                       $result = Shell::command( $cmd )
-                               ->execute();
-                       $exitCode = $result->getExitCode();
-                       if ( $exitCode !== 0 ) {
-                               throw new Exception( "External diff command returned code {$exitCode}. Stderr: "
-                                       . wfEscapeWikiText( $result->getStderr() )
-                               );
-                       }
-                       $difftext = $result->getStdout();
-                       $difftext .= $this->debug( "external $engine" );
-                       unlink( $tempName1 );
-                       unlink( $tempName2 );
-
-                       return $difftext;
-               }
-
-               # Native PHP diff
-               $contLang = MediaWikiServices::getInstance()->getContentLanguage();
-               $ota = explode( "\n", $contLang->segmentForDiff( $otext ) );
-               $nta = explode( "\n", $contLang->segmentForDiff( $ntext ) );
-               $diffs = new Diff( $ota, $nta );
-               $formatter = new TableDiffFormatter();
-               $difftext = $contLang->unsegmentForDiff( $formatter->format( $diffs ) );
-               $difftext .= $this->debug( 'native PHP' );
-
-               return $difftext;
+               $slotDiffRenderer = ContentHandler::getForModelID( CONTENT_MODEL_TEXT )
+                       ->getSlotDiffRenderer( $this->getContext() );
+               if ( !( $slotDiffRenderer instanceof TextSlotDiffRenderer ) ) {
+                       // Someone used the GetSlotDiffRenderer hook to replace the renderer.
+                       // This is too unlikely to happen to bother handling properly.
+                       throw new Exception( 'The slot diff renderer for text content should be a '
+                               . 'TextSlotDiffRenderer subclass' );
+               }
+               return $slotDiffRenderer->getTextDiff( $otext, $ntext ) . $this->getDebugString();
        }
 
        /**
@@ -1100,6 +1253,17 @@ class DifferenceEngine extends ContextSource {
                        " -->\n";
        }
 
+       private function getDebugString() {
+               $engine = self::getEngine();
+               if ( $engine === 'wikidiff2' ) {
+                       return $this->debug( 'wikidiff2' );
+               } elseif ( $engine === false ) {
+                       return $this->debug( 'native PHP' );
+               } else {
+                       return $this->debug( "external $engine" );
+               }
+       }
+
        /**
         * Localise diff output
         *
@@ -1170,10 +1334,12 @@ class DifferenceEngine extends ContextSource {
         * @return string
         */
        public function getMultiNotice() {
-               if ( !is_object( $this->mOldRev ) || !is_object( $this->mNewRev ) ) {
-                       return '';
-               } elseif ( !$this->mOldPage->equals( $this->mNewPage ) ) {
-                       // Comparing two different pages? Count would be meaningless.
+               // The notice only make sense if we are diffing two saved revisions of the same page.
+               if (
+                       !$this->mOldRev || !$this->mNewRev
+                       || !$this->mOldPage || !$this->mNewPage
+                       || !$this->mOldPage->equals( $this->mNewPage )
+               ) {
                        return '';
                }
 
@@ -1353,6 +1519,7 @@ class DifferenceEngine extends ContextSource {
         * @param Content $oldContent
         * @param Content $newContent
         * @since 1.21
+        * @deprecated since 1.32, use setRevisions or ContentHandler::getSlotDiffRenderer.
         */
        public function setContent( Content $oldContent, Content $newContent ) {
                $this->mOldContent = $oldContent;
@@ -1361,6 +1528,38 @@ class DifferenceEngine extends ContextSource {
                $this->mTextLoaded = 2;
                $this->mRevisionsLoaded = true;
                $this->isContentOverridden = true;
+               $this->slotDiffRenderers = null;
+       }
+
+       /**
+        * Use specified text instead of loading from the database.
+        * @param RevisionRecord|null $oldRevision
+        * @param RevisionRecord $newRevision
+        */
+       public function setRevisions(
+               RevisionRecord $oldRevision = null, RevisionRecord $newRevision
+       ) {
+               if ( $oldRevision ) {
+                       $this->mOldRev = new Revision( $oldRevision );
+                       $this->mOldid = $oldRevision->getId();
+                       $this->mOldPage = Title::newFromLinkTarget( $oldRevision->getPageAsLinkTarget() );
+                       // This method is meant for edit diffs and such so there is no reason to provide a
+                       // revision that's not readable to the user, but check it just in case.
+                       $this->mOldContent = $oldRevision ? $oldRevision->getContent( 'main',
+                               RevisionRecord::FOR_THIS_USER, $this->getUser() ) : null;
+               } else {
+                       $this->mOldRev = $this->mOldid = $this->mOldPage = null;
+               }
+               $this->mNewRev = new Revision( $newRevision );
+               $this->mNewid = $newRevision->getId();
+               $this->mNewPage = Title::newFromLinkTarget( $newRevision->getPageAsLinkTarget() );
+               $this->mNewContent = $newRevision->getContent( 'main',
+                       RevisionRecord::FOR_THIS_USER, $this->getUser() );
+
+               $this->mRevisionsIdsLoaded = $this->mRevisionsLoaded = true;
+               $this->mTextLoaded = !!$oldRevision + 1;
+               $this->isContentOverridden = false;
+               $this->slotDiffRenderers = null;
        }
 
        /**
@@ -1469,7 +1668,11 @@ class DifferenceEngine extends ContextSource {
 
                // Update the new revision ID in case it was 0 (makes life easier doing UI stuff)
                $this->mNewid = $this->mNewRev->getId();
-               $this->mNewPage = $this->mNewRev->getTitle();
+               if ( $this->mNewid ) {
+                       $this->mNewPage = $this->mNewRev->getTitle();
+               } else {
+                       $this->mNewPage = null;
+               }
 
                // Load the old revision object
                $this->mOldRev = false;
@@ -1491,8 +1694,10 @@ class DifferenceEngine extends ContextSource {
                        return false;
                }
 
-               if ( $this->mOldRev ) {
+               if ( $this->mOldRev && $this->mOldRev->getId() ) {
                        $this->mOldPage = $this->mOldRev->getTitle();
+               } else {
+                       $this->mOldPage = null;
                }
 
                // Load tags information for both revisions
diff --git a/includes/diff/DifferenceEngineSlotDiffRenderer.php b/includes/diff/DifferenceEngineSlotDiffRenderer.php
new file mode 100644 (file)
index 0000000..0c632b9
--- /dev/null
@@ -0,0 +1,79 @@
+<?php
+/**
+ * Adapter for turning a DifferenceEngine into a SlotDiffRenderer.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup DifferenceEngine
+ */
+
+/**
+ * B/C adapter for turning a DifferenceEngine into a SlotDiffRenderer.
+ * Before SlotDiffRenderer was introduced, getDiff() functionality was provided by DifferenceEngine
+ * subclasses. Convert such a subclass into a SlotDiffRenderer.
+ * @deprecated
+ * @ingroup DifferenceEngine
+ */
+class DifferenceEngineSlotDiffRenderer extends SlotDiffRenderer {
+
+       /** @var DifferenceEngine */
+       private $differenceEngine;
+
+       public function __construct( DifferenceEngine $differenceEngine ) {
+               $this->differenceEngine = clone $differenceEngine;
+
+               // Set state to loaded. This should not matter to any of the methods invoked by
+               // the adapter, but just in case a load does get triggered somehow, make sure it's a no-op.
+               $fakeContent = ContentHandler::getForModelID( CONTENT_MODEL_WIKITEXT )->makeEmptyContent();
+               $this->differenceEngine->setContent( $fakeContent, $fakeContent );
+
+               $this->differenceEngine->markAsSlotDiffRenderer();
+       }
+
+       /** @inheritDoc */
+       public function getDiff( Content $oldContent = null, Content $newContent = null ) {
+               if ( !$oldContent && !$newContent ) {
+                       throw new InvalidArgumentException( '$oldContent and $newContent cannot both be null' );
+               }
+               if ( !$oldContent || !$newContent ) {
+                       $someContent = $newContent ?: $oldContent;
+                       $emptyContent = $someContent->getContentHandler()->makeEmptyContent();
+                       $oldContent = $oldContent ?: $emptyContent;
+                       $newContent = $newContent ?: $emptyContent;
+               }
+               return $this->differenceEngine->generateContentDiffBody( $oldContent, $newContent );
+       }
+
+       public function addModules( OutputPage $output ) {
+               $oldContext = null;
+               if ( $output !== $this->differenceEngine->getOutput() ) {
+                       $oldContext = $this->differenceEngine->getContext();
+                       $newContext = new DerivativeContext( $oldContext );
+                       $newContext->setOutput( $output );
+                       $this->differenceEngine->setContext( $newContext );
+               }
+               $this->differenceEngine->showDiffStyle();
+               if ( $oldContext ) {
+                       $this->differenceEngine->setContext( $oldContext );
+               }
+       }
+
+       public function getExtraCacheKeys() {
+               return $this->differenceEngine->getExtraCacheKeys();
+       }
+
+}
diff --git a/includes/diff/SlotDiffRenderer.php b/includes/diff/SlotDiffRenderer.php
new file mode 100644 (file)
index 0000000..f20b416
--- /dev/null
@@ -0,0 +1,65 @@
+<?php
+/**
+ * Renders a diff for a single slot.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup DifferenceEngine
+ */
+
+/**
+ * Renders a diff for a single slot (that is, a diff between two content objects).
+ *
+ * Callers should obtain this class by invoking ContentHandler::getSlotDiffRendererClass
+ * on the content handler of the new content object (ie. the one shown on the right side
+ * of the diff), or of the old one if the new one does not exist.
+ *
+ * The default implementation just does a text diff on the native text representation.
+ * Content handler extensions can subclass this to provide a more appropriate diff method by
+ * overriding ContentHandler::getSlotDiffRendererClass. Other extensions that want to interfere
+ * with diff generation in some way can use the GetSlotDiffRenderer hook.
+ *
+ * @ingroup DifferenceEngine
+ */
+abstract class SlotDiffRenderer {
+
+       /**
+        * Get a diff between two content objects. One of them might be null (meaning a slot was
+        * created or removed), but both cannot be. $newContent (or if it's null then $oldContent)
+        * must have the same content model that was used to obtain this diff renderer.
+        * @param Content|null $oldContent
+        * @param Content|null $newContent
+        * @return string
+        */
+       abstract public function getDiff( Content $oldContent = null, Content $newContent = null );
+
+       /**
+        * Add modules needed for correct styling/behavior of the diff.
+        * @param OutputPage $output
+        */
+       public function addModules( OutputPage $output ) {
+       }
+
+       /**
+        * Return any extra keys to split the diff cache by.
+        * @return array
+        */
+       public function getExtraCacheKeys() {
+               return [];
+       }
+
+}
diff --git a/includes/diff/TextSlotDiffRenderer.php b/includes/diff/TextSlotDiffRenderer.php
new file mode 100644 (file)
index 0000000..baedcf0
--- /dev/null
@@ -0,0 +1,287 @@
+<?php
+/**
+ * Renders a slot diff by doing a text diff on the native representation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup DifferenceEngine
+ */
+
+use MediaWiki\Shell\Shell;
+use Wikimedia\Assert\Assert;
+
+/**
+ * Renders a slot diff by doing a text diff on the native representation.
+ *
+ * If you want to use this without content objects (to call getTextDiff() on some
+ * non-content-related texts), obtain an instance with
+ *     ContentHandler::getForModelID( CONTENT_MODEL_TEXT )
+ *         ->getSlotDiffRenderer( RequestContext::getMain() )
+ *
+ * @ingroup DifferenceEngine
+ */
+class TextSlotDiffRenderer extends SlotDiffRenderer {
+
+       /** Use the PHP diff implementation (DiffEngine). */
+       const ENGINE_PHP = 'php';
+
+       /** Use the wikidiff2 PHP module. */
+       const ENGINE_WIKIDIFF2 = 'wikidiff2';
+
+       /** Use an external executable. */
+       const ENGINE_EXTERNAL = 'external';
+
+       /** @var IBufferingStatsdDataFactory|null */
+       private $statsdDataFactory;
+
+       /** @var Language|null The language this content is in. */
+       private $language;
+
+       /**
+        * Number of paragraph moves the algorithm should attempt to detect.
+        * Only used with the wikidiff2 engine.
+        * @var int
+        * @see $wgWikiDiff2MovedParagraphDetectionCutoff
+        */
+       private $wikiDiff2MovedParagraphDetectionCutoff = 0;
+
+       /** @var string One of the ENGINE_* constants. */
+       private $engine = self::ENGINE_PHP;
+
+       /** @var string Path to an executable to be used as the diff engine. */
+       private $externalEngine;
+
+       /**
+        * Convenience helper to use getTextDiff without an instance.
+        * @param string $oldText
+        * @param string $newText
+        * @return string
+        */
+       public static function diff( $oldText, $newText ) {
+               /** @var $slotDiffRenderer TextSlotDiffRenderer */
+               $slotDiffRenderer = ContentHandler::getForModelID( CONTENT_MODEL_TEXT )
+                       ->getSlotDiffRenderer( RequestContext::getMain() );
+               return $slotDiffRenderer->getTextDiff( $oldText, $newText );
+       }
+
+       public function setStatsdDataFactory( IBufferingStatsdDataFactory $statsdDataFactory ) {
+               $this->statsdDataFactory = $statsdDataFactory;
+       }
+
+       public function setLanguage( Language $language ) {
+               $this->language = $language;
+       }
+       /**
+        * @param int $cutoff
+        * @see $wgWikiDiff2MovedParagraphDetectionCutoff
+        */
+       public function setWikiDiff2MovedParagraphDetectionCutoff( $cutoff ) {
+               Assert::parameterType( 'integer', $cutoff, '$cutoff' );
+               $this->wikiDiff2MovedParagraphDetectionCutoff = $cutoff;
+       }
+
+       /**
+        * Set which diff engine to use.
+        * @param string $type One of the ENGINE_* constants.
+        * @param string|null $executable Path to an external exectable, only when type is ENGINE_EXTERNAL.
+        */
+       public function setEngine( $type, $executable = null ) {
+               $engines = [ self::ENGINE_PHP, self::ENGINE_WIKIDIFF2, self::ENGINE_EXTERNAL ];
+               Assert::parameter( in_array( $type, $engines, true ), '$type',
+                       'must be one of the TextSlotDiffRenderer::ENGINE_* constants' );
+               if ( $type === self::ENGINE_EXTERNAL ) {
+                       Assert::parameter( is_string( $executable ) && is_executable( $executable ), '$executable',
+                               'must be a path to a valid executable' );
+               } else {
+                       Assert::parameter( is_null( $executable ), '$executable',
+                               'must not be set unless $type is ENGINE_EXTERNAL' );
+               }
+               $this->engine = $type;
+               $this->externalEngine = $executable;
+       }
+
+       /** @inheritDoc */
+       public function getDiff( Content $oldContent = null, Content $newContent = null ) {
+               if ( !$oldContent && !$newContent ) {
+                       throw new InvalidArgumentException( '$oldContent and $newContent cannot both be null' );
+               } elseif ( $oldContent && !( $oldContent instanceof TextContent ) ) {
+                       throw new InvalidArgumentException( __CLASS__ . ' does not handle ' . get_class( $oldContent ) );
+               } elseif ( $newContent && !( $newContent instanceof TextContent ) ) {
+                       throw new InvalidArgumentException( __CLASS__ . ' does not handle ' . get_class( $newContent ) );
+               }
+
+               if ( !$oldContent ) {
+                       $oldContent = $newContent->getContentHandler()->makeEmptyContent();
+               } elseif ( !$newContent ) {
+                       $newContent = $oldContent->getContentHandler()->makeEmptyContent();
+               }
+
+               $oldText = $oldContent->serialize();
+               $newText = $newContent->serialize();
+
+               return $this->getTextDiff( $oldText, $newText );
+       }
+
+       /**
+        * Diff the text representations of two content objects (or just two pieces of text in general).
+        * @param string $oldText
+        * @param string $newText
+        * @return string
+        */
+       public function getTextDiff( $oldText, $newText ) {
+               Assert::parameterType( 'string', $oldText, '$oldText' );
+               Assert::parameterType( 'string', $newText, '$newText' );
+
+               $diff = function () use ( $oldText, $newText ) {
+                       $time = microtime( true );
+
+                       $result = $this->getTextDiffInternal( $oldText, $newText );
+
+                       $time = intval( ( microtime( true ) - $time ) * 1000 );
+                       if ( $this->statsdDataFactory ) {
+                               $this->statsdDataFactory->timing( 'diff_time', $time );
+                       }
+
+                       // TODO reimplement this using T142313
+                       /*
+                       // Log requests slower than 99th percentile
+                       if ( $time > 100 && $this->mOldPage && $this->mNewPage ) {
+                               wfDebugLog( 'diff',
+                                       "$time ms diff: {$this->mOldid} -> {$this->mNewid} {$this->mNewPage}" );
+                       }
+                       */
+
+                       return $result;
+               };
+
+               /**
+                * @param Status $status
+                * @throws FatalError
+                */
+               $error = function ( $status ) {
+                       throw new FatalError( $status->getWikiText() );
+               };
+
+               // Use PoolCounter if the diff looks like it can be expensive
+               if ( strlen( $oldText ) + strlen( $newText ) > 20000 ) {
+                       $work = new PoolCounterWorkViaCallback( 'diff',
+                               md5( $oldText ) . md5( $newText ),
+                               [ 'doWork' => $diff, 'error' => $error ]
+                       );
+                       return $work->execute();
+               }
+
+               return $diff();
+       }
+
+       /**
+        * Diff the text representations of two content objects (or just two pieces of text in general).
+        * This does the actual diffing, getTextDiff() wraps it with logging and resource limiting.
+        * @param string $oldText
+        * @param string $newText
+        * @return string
+        * @throws Exception
+        */
+       protected function getTextDiffInternal( $oldText, $newText ) {
+               // TODO move most of this into three parallel implementations of a text diff generator
+               // class, choose which one to use via dependecy injection
+
+               $oldText = str_replace( "\r\n", "\n", $oldText );
+               $newText = str_replace( "\r\n", "\n", $newText );
+
+               // Better external diff engine, the 2 may some day be dropped
+               // This one does the escaping and segmenting itself
+               if ( $this->engine === self::ENGINE_WIKIDIFF2 ) {
+                       $wikidiff2Version = phpversion( 'wikidiff2' );
+                       if (
+                               $wikidiff2Version !== false &&
+                               version_compare( $wikidiff2Version, '1.5.0', '>=' )
+                       ) {
+                               $text = wikidiff2_do_diff(
+                                       $oldText,
+                                       $newText,
+                                       2,
+                                       $this->wikiDiff2MovedParagraphDetectionCutoff
+                               );
+                       } else {
+                               // Don't pass the 4th parameter for compatibility with older versions of wikidiff2
+                               $text = wikidiff2_do_diff(
+                                       $oldText,
+                                       $newText,
+                                       2
+                               );
+
+                               // Log a warning in case the configuration value is set to not silently ignore it
+                               if ( $this->wikiDiff2MovedParagraphDetectionCutoff > 0 ) {
+                                       wfLogWarning( '$wgWikiDiff2MovedParagraphDetectionCutoff is set but has no
+                                               effect since the used version of WikiDiff2 does not support it.' );
+                               }
+                       }
+
+                       return $text;
+               } elseif ( $this->engine === self::ENGINE_EXTERNAL ) {
+                       # Diff via the shell
+                       $tmpDir = wfTempDir();
+                       $tempName1 = tempnam( $tmpDir, 'diff_' );
+                       $tempName2 = tempnam( $tmpDir, 'diff_' );
+
+                       $tempFile1 = fopen( $tempName1, "w" );
+                       if ( !$tempFile1 ) {
+                               return false;
+                       }
+                       $tempFile2 = fopen( $tempName2, "w" );
+                       if ( !$tempFile2 ) {
+                               return false;
+                       }
+                       fwrite( $tempFile1, $oldText );
+                       fwrite( $tempFile2, $newText );
+                       fclose( $tempFile1 );
+                       fclose( $tempFile2 );
+                       $cmd = [ $this->externalEngine, $tempName1, $tempName2 ];
+                       $result = Shell::command( $cmd )
+                               ->execute();
+                       $exitCode = $result->getExitCode();
+                       if ( $exitCode !== 0 ) {
+                               throw new Exception( "External diff command returned code {$exitCode}. Stderr: "
+                                                                        . wfEscapeWikiText( $result->getStderr() )
+                               );
+                       }
+                       $difftext = $result->getStdout();
+                       unlink( $tempName1 );
+                       unlink( $tempName2 );
+
+                       return $difftext;
+               } elseif ( $this->engine === self::ENGINE_PHP ) {
+                       if ( $this->language ) {
+                               $oldText = $this->language->segmentForDiff( $oldText );
+                               $newText = $this->language->segmentForDiff( $newText );
+                       }
+                       $ota = explode( "\n", $oldText );
+                       $nta = explode( "\n", $newText );
+                       $diffs = new Diff( $ota, $nta );
+                       $formatter = new TableDiffFormatter();
+                       $difftext = $formatter->format( $diffs );
+                       if ( $this->language ) {
+                               $difftext = $this->language->unsegmentForDiff( $difftext );
+                       }
+
+                       return $difftext;
+               }
+               throw new LogicException( 'Invalid engine: ' . $this->engine );
+       }
+
+}
index c469222..2053843 100644 (file)
@@ -126,6 +126,10 @@ td.diff-marker {
        unicode-bidi: embed;
 }
 
+.mw-diff-slot-header {
+       text-align: center;
+}
+
 /*!
  * Wikidiff2 rendering for moved paragraphs
  */
index 0cb042a..9626312 100644 (file)
@@ -114,6 +114,7 @@ $wgAutoloadClasses += [
        'TestDeprecatedSubclass' => "$testDir/phpunit/includes/debug/TestDeprecatedSubclass.php",
 
        # tests/phpunit/includes/diff
+       'CustomDifferenceEngine' => "$testDir/phpunit/includes/diff/CustomDifferenceEngine.php",
        'FakeDiffOp' => "$testDir/phpunit/includes/diff/FakeDiffOp.php",
 
        # tests/phpunit/includes/externalstore
index 323a63d..43edf60 100644 (file)
@@ -1,5 +1,6 @@
 <?php
 use MediaWiki\MediaWikiServices;
+use Wikimedia\TestingAccessWrapper;
 
 /**
  * @group ContentHandler
@@ -485,4 +486,129 @@ class ContentHandlerTest extends MediaWikiTestCase {
                } );
                $this->assertContains( 'Ferrari', ContentHandler::getContentModels() );
        }
+
+       /**
+        * @covers ContentHandler::getSlotDiffRenderer
+        */
+       public function testGetSlotDiffRenderer_default() {
+               $this->mergeMwGlobalArrayValue( 'wgHooks', [
+                       'GetSlotDiffRenderer' => [],
+               ] );
+
+               // test default renderer
+               $contentHandler = new WikitextContentHandler( CONTENT_MODEL_WIKITEXT );
+               $slotDiffRenderer = $contentHandler->getSlotDiffRenderer( RequestContext::getMain() );
+               $this->assertInstanceOf( TextSlotDiffRenderer::class, $slotDiffRenderer );
+       }
+
+       /**
+        * @covers ContentHandler::getSlotDiffRenderer
+        */
+       public function testGetSlotDiffRenderer_bc() {
+               $this->mergeMwGlobalArrayValue( 'wgHooks', [
+                       'GetSlotDiffRenderer' => [],
+               ] );
+
+               // test B/C renderer
+               $customDifferenceEngine = $this->getMockBuilder( DifferenceEngine::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               // hack to track object identity across cloning
+               $customDifferenceEngine->objectId = 12345;
+               $customContentHandler = $this->getMockBuilder( ContentHandler::class )
+                       ->setConstructorArgs( [ 'foo', [] ] )
+                       ->setMethods( [ 'createDifferenceEngine' ] )
+                       ->getMockForAbstractClass();
+               $customContentHandler->expects( $this->any() )
+                       ->method( 'createDifferenceEngine' )
+                       ->willReturn( $customDifferenceEngine );
+               /** @var $customContentHandler ContentHandler */
+               $slotDiffRenderer = $customContentHandler->getSlotDiffRenderer( RequestContext::getMain() );
+               $this->assertInstanceOf( DifferenceEngineSlotDiffRenderer::class, $slotDiffRenderer );
+               $this->assertSame(
+                       $customDifferenceEngine->objectId,
+                       TestingAccessWrapper::newFromObject( $slotDiffRenderer )->differenceEngine->objectId
+               );
+       }
+
+       /**
+        * @covers ContentHandler::getSlotDiffRenderer
+        */
+       public function testGetSlotDiffRenderer_nobc() {
+               $this->mergeMwGlobalArrayValue( 'wgHooks', [
+                       'GetSlotDiffRenderer' => [],
+               ] );
+
+               // test that B/C renderer does not get used when getSlotDiffRendererInternal is overridden
+               $customDifferenceEngine = $this->getMockBuilder( DifferenceEngine::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $customSlotDiffRenderer = $this->getMockBuilder( SlotDiffRenderer::class )
+                       ->disableOriginalConstructor()
+                       ->getMockForAbstractClass();
+               $customContentHandler2 = $this->getMockBuilder( ContentHandler::class )
+                       ->setConstructorArgs( [ 'bar', [] ] )
+                       ->setMethods( [ 'createDifferenceEngine', 'getSlotDiffRendererInternal' ] )
+                       ->getMockForAbstractClass();
+               $customContentHandler2->expects( $this->any() )
+                       ->method( 'createDifferenceEngine' )
+                       ->willReturn( $customDifferenceEngine );
+               $customContentHandler2->expects( $this->any() )
+                       ->method( 'getSlotDiffRendererInternal' )
+                       ->willReturn( $customSlotDiffRenderer );
+               /** @var $customContentHandler2 ContentHandler */
+               $slotDiffRenderer = $customContentHandler2->getSlotDiffRenderer( RequestContext::getMain() );
+               $this->assertSame( $customSlotDiffRenderer, $slotDiffRenderer );
+       }
+
+       /**
+        * @covers ContentHandler::getSlotDiffRenderer
+        */
+       public function testGetSlotDiffRenderer_hook() {
+               $this->mergeMwGlobalArrayValue( 'wgHooks', [
+                       'GetSlotDiffRenderer' => [],
+               ] );
+
+               // test that the hook handler takes precedence
+               $customDifferenceEngine = $this->getMockBuilder( DifferenceEngine::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $customContentHandler = $this->getMockBuilder( ContentHandler::class )
+                       ->setConstructorArgs( [ 'foo', [] ] )
+                       ->setMethods( [ 'createDifferenceEngine' ] )
+                       ->getMockForAbstractClass();
+               $customContentHandler->expects( $this->any() )
+                       ->method( 'createDifferenceEngine' )
+                       ->willReturn( $customDifferenceEngine );
+               /** @var $customContentHandler ContentHandler */
+
+               $customSlotDiffRenderer = $this->getMockBuilder( SlotDiffRenderer::class )
+                       ->disableOriginalConstructor()
+                       ->getMockForAbstractClass();
+               $customContentHandler2 = $this->getMockBuilder( ContentHandler::class )
+                       ->setConstructorArgs( [ 'bar', [] ] )
+                       ->setMethods( [ 'createDifferenceEngine', 'getSlotDiffRendererInternal' ] )
+                       ->getMockForAbstractClass();
+               $customContentHandler2->expects( $this->any() )
+                       ->method( 'createDifferenceEngine' )
+                       ->willReturn( $customDifferenceEngine );
+               $customContentHandler2->expects( $this->any() )
+                       ->method( 'getSlotDiffRendererInternal' )
+                       ->willReturn( $customSlotDiffRenderer );
+               /** @var $customContentHandler2 ContentHandler */
+
+               $customSlotDiffRenderer2 = $this->getMockBuilder( SlotDiffRenderer::class )
+                       ->disableOriginalConstructor()
+                       ->getMockForAbstractClass();
+               $this->setTemporaryHook( 'GetSlotDiffRenderer',
+                       function ( $handler, &$slotDiffRenderer ) use ( $customSlotDiffRenderer2 ) {
+                               $slotDiffRenderer = $customSlotDiffRenderer2;
+                       } );
+
+               $slotDiffRenderer = $customContentHandler->getSlotDiffRenderer( RequestContext::getMain() );
+               $this->assertSame( $customSlotDiffRenderer2, $slotDiffRenderer );
+               $slotDiffRenderer = $customContentHandler2->getSlotDiffRenderer( RequestContext::getMain() );
+               $this->assertSame( $customSlotDiffRenderer2, $slotDiffRenderer );
+       }
+
 }
diff --git a/tests/phpunit/includes/diff/CustomDifferenceEngine.php b/tests/phpunit/includes/diff/CustomDifferenceEngine.php
new file mode 100644 (file)
index 0000000..c760d02
--- /dev/null
@@ -0,0 +1,23 @@
+<?php
+
+class CustomDifferenceEngine extends DifferenceEngine {
+
+       public function __construct() {
+               parent::__construct();
+       }
+
+       public function generateContentDiffBody( Content $old, Content $new ) {
+               return $old->getNativeData() . '|' . $new->getNativeData();
+       }
+
+       public function showDiffStyle() {
+               $this->getOutput()->addModules( 'foo' );
+       }
+
+       public function getDiffBodyCacheKeyParams() {
+               $params = parent::getDiffBodyCacheKeyParams();
+               $params[] = 'foo';
+               return $params;
+       }
+
+}
diff --git a/tests/phpunit/includes/diff/DifferenceEngineSlotDiffRendererTest.php b/tests/phpunit/includes/diff/DifferenceEngineSlotDiffRendererTest.php
new file mode 100644 (file)
index 0000000..fe129b7
--- /dev/null
@@ -0,0 +1,44 @@
+<?php
+
+/**
+ * @covers DifferenceEngineSlotDiffRenderer
+ */
+class DifferenceEngineSlotDiffRendererTest extends \PHPUnit\Framework\TestCase {
+
+       public function testGetDiff() {
+               $differenceEngine = new CustomDifferenceEngine();
+               $slotDiffRenderer = new DifferenceEngineSlotDiffRenderer( $differenceEngine );
+               $oldContent = ContentHandler::makeContent( 'xxx', null, CONTENT_MODEL_TEXT );
+               $newContent = ContentHandler::makeContent( 'yyy', null, CONTENT_MODEL_TEXT );
+
+               $diff = $slotDiffRenderer->getDiff( $oldContent, $newContent );
+               $this->assertEquals( 'xxx|yyy', $diff );
+
+               $diff = $slotDiffRenderer->getDiff( null, $newContent );
+               $this->assertEquals( '|yyy', $diff );
+
+               $diff = $slotDiffRenderer->getDiff( $oldContent, null );
+               $this->assertEquals( 'xxx|', $diff );
+       }
+
+       public function testAddModules() {
+               $output = $this->getMockBuilder( OutputPage::class )
+                       ->disableOriginalConstructor()
+                       ->setMethods( [ 'addModules' ] )
+                       ->getMock();
+               $output->expects( $this->once() )
+                       ->method( 'addModules' )
+                       ->with( 'foo' );
+               $differenceEngine = new CustomDifferenceEngine();
+               $slotDiffRenderer = new DifferenceEngineSlotDiffRenderer( $differenceEngine );
+               $slotDiffRenderer->addModules( $output );
+       }
+
+       public function testGetExtraCacheKeys() {
+               $differenceEngine = new CustomDifferenceEngine();
+               $slotDiffRenderer = new DifferenceEngineSlotDiffRenderer( $differenceEngine );
+               $extraCacheKeys = $slotDiffRenderer->getExtraCacheKeys();
+               $this->assertSame( [ 'foo' ], $extraCacheKeys );
+       }
+
+}
index 57aeb20..40f6807 100644 (file)
@@ -1,5 +1,8 @@
 <?php
 
+use MediaWiki\Storage\MutableRevisionRecord;
+use MediaWiki\Storage\RevisionRecord;
+use MediaWiki\Storage\SlotRecord;
 use Wikimedia\TestingAccessWrapper;
 
 /**
@@ -145,4 +148,207 @@ class DifferenceEngineTest extends MediaWikiTestCase {
                $this->assertEquals( $expected, $diffEngine->addLocalisedTitleTooltips( $input ) );
        }
 
+       /**
+        * @dataProvider provideGenerateContentDiffBody
+        */
+       public function testGenerateContentDiffBody(
+               Content $oldContent, Content $newContent, $expectedDiff
+       ) {
+               // Set $wgExternalDiffEngine to something bogus to try to force use of
+               // the PHP engine rather than wikidiff2.
+               $this->setMwGlobals( [
+                       'wgExternalDiffEngine' => '/dev/null',
+               ] );
+
+               $differenceEngine = new DifferenceEngine();
+               $diff = $differenceEngine->generateContentDiffBody( $oldContent, $newContent );
+               $this->assertSame( $expectedDiff, $this->getPlainDiff( $diff ) );
+       }
+
+       public function provideGenerateContentDiffBody() {
+               $this->mergeMwGlobalArrayValue( 'wgContentHandlers', [
+                       'testing-nontext' => DummyNonTextContentHandler::class,
+               ] );
+               $content1 = ContentHandler::makeContent( 'xxx', null, CONTENT_MODEL_TEXT );
+               $content2 = ContentHandler::makeContent( 'yyy', null, CONTENT_MODEL_TEXT );
+
+               return [
+                       'self-diff' => [ $content1, $content1, '' ],
+                       'text diff' => [ $content1, $content2, '-xxx+yyy' ],
+               ];
+       }
+
+       public function testGenerateTextDiffBody() {
+               // Set $wgExternalDiffEngine to something bogus to try to force use of
+               // the PHP engine rather than wikidiff2.
+               $this->setMwGlobals( [
+                       'wgExternalDiffEngine' => '/dev/null',
+               ] );
+
+               $oldText = "aaa\nbbb\nccc";
+               $newText = "aaa\nxxx\nccc";
+               $expectedDiff = " aaa aaa\n-bbb+xxx\n ccc ccc";
+
+               $differenceEngine = new DifferenceEngine();
+               $diff = $differenceEngine->generateTextDiffBody( $oldText, $newText );
+               $this->assertSame( $expectedDiff, $this->getPlainDiff( $diff ) );
+       }
+
+       public function testSetContent() {
+               // Set $wgExternalDiffEngine to something bogus to try to force use of
+               // the PHP engine rather than wikidiff2.
+               $this->setMwGlobals( [
+                       'wgExternalDiffEngine' => '/dev/null',
+               ] );
+
+               $oldContent = ContentHandler::makeContent( 'xxx', null, CONTENT_MODEL_TEXT );
+               $newContent = ContentHandler::makeContent( 'yyy', null, CONTENT_MODEL_TEXT );
+
+               $differenceEngine = new DifferenceEngine();
+               $differenceEngine->setContent( $oldContent, $newContent );
+               $diff = $differenceEngine->getDiffBody();
+               $this->assertSame( "Line 1:\nLine 1:\n-xxx+yyy", $this->getPlainDiff( $diff ) );
+       }
+
+       public function testSetRevisions() {
+               $main1 = SlotRecord::newUnsaved( 'main',
+                       ContentHandler::makeContent( 'xxx', null, CONTENT_MODEL_TEXT ) );
+               $main2 = SlotRecord::newUnsaved( 'main',
+                       ContentHandler::makeContent( 'yyy', null, CONTENT_MODEL_TEXT ) );
+               $rev1 = $this->getRevisionRecord( $main1 );
+               $rev2 = $this->getRevisionRecord( $main2 );
+
+               $differenceEngine = new DifferenceEngine();
+               $differenceEngine->setRevisions( $rev1, $rev2 );
+               $this->assertSame( $rev1, $differenceEngine->getOldRevision() );
+               $this->assertSame( $rev2, $differenceEngine->getNewRevision() );
+               $this->assertSame( true, $differenceEngine->loadRevisionData() );
+               $this->assertSame( true, $differenceEngine->loadText() );
+
+               $differenceEngine->setRevisions( null, $rev2 );
+               $this->assertSame( null, $differenceEngine->getOldRevision() );
+       }
+
+       /**
+        * @dataProvider provideGetDiffBody
+        */
+       public function testGetDiffBody(
+               RevisionRecord $oldRevision = null, RevisionRecord $newRevision = null, $expectedDiff
+       ) {
+               // Set $wgExternalDiffEngine to something bogus to try to force use of
+               // the PHP engine rather than wikidiff2.
+               $this->setMwGlobals( [
+                       'wgExternalDiffEngine' => '/dev/null',
+               ] );
+
+               if ( $expectedDiff instanceof Exception ) {
+                       $this->setExpectedException( get_class( $expectedDiff ), $expectedDiff->getMessage() );
+               }
+               $differenceEngine = new DifferenceEngine();
+               $differenceEngine->setRevisions( $oldRevision, $newRevision );
+               if ( $expectedDiff instanceof Exception ) {
+                       return;
+               }
+
+               $diff = $differenceEngine->getDiffBody();
+               $this->assertSame( $expectedDiff, $this->getPlainDiff( $diff ) );
+       }
+
+       public function provideGetDiffBody() {
+               $main1 = SlotRecord::newUnsaved( 'main',
+                       ContentHandler::makeContent( 'xxx', null, CONTENT_MODEL_TEXT ) );
+               $main2 = SlotRecord::newUnsaved( 'main',
+                       ContentHandler::makeContent( 'yyy', null, CONTENT_MODEL_TEXT ) );
+               $slot1 = SlotRecord::newUnsaved( 'slot',
+                       ContentHandler::makeContent( 'aaa', null, CONTENT_MODEL_TEXT ) );
+               $slot2 = SlotRecord::newUnsaved( 'slot',
+                       ContentHandler::makeContent( 'bbb', null, CONTENT_MODEL_TEXT ) );
+
+               return [
+                       'revision vs. null' => [
+                               null,
+                               $this->getRevisionRecord( $main1, $slot1 ),
+                               '',
+                       ],
+                       'revision vs. itself' => [
+                               $this->getRevisionRecord( $main1, $slot1 ),
+                               $this->getRevisionRecord( $main1, $slot1 ),
+                               '',
+                       ],
+                       'different text in one slot' => [
+                               $this->getRevisionRecord( $main1, $slot1 ),
+                               $this->getRevisionRecord( $main1, $slot2 ),
+                               "slotLine 1:\nLine 1:\n-aaa+bbb",
+                       ],
+                       'different text in two slots' => [
+                               $this->getRevisionRecord( $main1, $slot1 ),
+                               $this->getRevisionRecord( $main2, $slot2 ),
+                               "Line 1:\nLine 1:\n-xxx+yyy\nslotLine 1:\nLine 1:\n-aaa+bbb",
+                       ],
+                       'new slot' => [
+                               $this->getRevisionRecord( $main1 ),
+                               $this->getRevisionRecord( $main1, $slot1 ),
+                               "slotLine 1:\nLine 1:\n- +aaa",
+                       ],
+               ];
+       }
+
+       public function testRecursion() {
+               // Set up a ContentHandler which will return a wrapped DifferenceEngine as
+               // SlotDiffRenderer, then pass it a content which uses the same ContentHandler.
+               // This tests the anti-recursion logic in DifferenceEngine::generateContentDiffBody.
+
+               $customDifferenceEngine = $this->getMockBuilder( DifferenceEngine::class )
+                       ->enableProxyingToOriginalMethods()
+                       ->getMock();
+               $customContentHandler = $this->getMockBuilder( ContentHandler::class )
+                       ->setConstructorArgs( [ 'foo', [] ] )
+                       ->setMethods( [ 'createDifferenceEngine' ] )
+                       ->getMockForAbstractClass();
+               $customContentHandler->expects( $this->any() )
+                       ->method( 'createDifferenceEngine' )
+                       ->willReturn( $customDifferenceEngine );
+               /** @var $customContentHandler ContentHandler */
+               $customContent = $this->getMockBuilder( Content::class )
+                       ->setMethods( [ 'getContentHandler' ] )
+                       ->getMockForAbstractClass();
+               $customContent->expects( $this->any() )
+                       ->method( 'getContentHandler' )
+                       ->willReturn( $customContentHandler );
+               /** @var $customContent Content */
+               $customContent2 = clone $customContent;
+
+               $slotDiffRenderer = $customContentHandler->getSlotDiffRenderer( RequestContext::getMain() );
+               $this->setExpectedException( Exception::class,
+                       ': could not maintain backwards compatibility. Please use a SlotDiffRenderer.' );
+               $slotDiffRenderer->getDiff( $customContent, $customContent2 );
+       }
+
+       /**
+        * Convert a HTML diff to a human-readable format and hopefully make the test less fragile.
+        * @param string diff
+        * @return string
+        */
+       private function getPlainDiff( $diff ) {
+               $replacements = [
+                       html_entity_decode( '&nbsp;' ) => ' ',
+                       html_entity_decode( '&minus;' ) => '-',
+               ];
+               return str_replace( array_keys( $replacements ), array_values( $replacements ),
+                       trim( strip_tags( $diff ), "\n" ) );
+       }
+
+       /**
+        * @param SlotRecord[] $slots
+        * @return MutableRevisionRecord
+        */
+       private function getRevisionRecord( ...$slots ) {
+               $title = Title::newFromText( 'Foo' );
+               $revision = new MutableRevisionRecord( $title );
+               foreach ( $slots as $slot ) {
+                       $revision->setSlot( $slot );
+               }
+               return $revision;
+       }
+
 }
diff --git a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php
new file mode 100644 (file)
index 0000000..ec45e29
--- /dev/null
@@ -0,0 +1,113 @@
+<?php
+
+/**
+ * @covers TextSlotDiffRenderer
+ */
+class TextSlotDiffRendererTest extends MediaWikiTestCase {
+
+       /**
+        * @dataProvider provideGetDiff
+        * @param Content|null $oldContent
+        * @param Content|null $newContent
+        * @param string|Exception $expectedResult
+        * @throws Exception
+        */
+       public function testGetDiff(
+               Content $oldContent = null, Content $newContent = null, $expectedResult
+       ) {
+               if ( $expectedResult instanceof Exception ) {
+                       $this->setExpectedException( get_class( $expectedResult ), $expectedResult->getMessage() );
+               }
+
+               $slotDiffRenderer = $this->getTextSlotDiffRenderer();
+               $diff = $slotDiffRenderer->getDiff( $oldContent, $newContent );
+               if ( $expectedResult instanceof Exception ) {
+                       return;
+               }
+               $plainDiff = $this->getPlainDiff( $diff );
+               $this->assertSame( $expectedResult, $plainDiff );
+       }
+
+       public function provideGetDiff() {
+               $this->mergeMwGlobalArrayValue( 'wgContentHandlers', [
+                       'testing' => DummyContentHandlerForTesting::class,
+                       'testing-nontext' => DummyNonTextContentHandler::class,
+               ] );
+
+               return [
+                       'same text' => [
+                               $this->makeContent( "aaa\nbbb\nccc" ),
+                               $this->makeContent( "aaa\nbbb\nccc" ),
+                               "",
+                       ],
+                       'different text' => [
+                               $this->makeContent( "aaa\nbbb\nccc" ),
+                               $this->makeContent( "aaa\nxxx\nccc" ),
+                               " aaa aaa\n-bbb+xxx\n ccc ccc",
+                       ],
+                       'no right content' => [
+                               $this->makeContent( "aaa\nbbb\nccc" ),
+                               null,
+                               "-aaa+ \n-bbb \n-ccc ",
+                       ],
+                       'no left content' => [
+                               null,
+                               $this->makeContent( "aaa\nbbb\nccc" ),
+                               "- +aaa\n +bbb\n +ccc",
+                       ],
+                       'no content' => [
+                               null,
+                               null,
+                               new InvalidArgumentException( '$oldContent and $newContent cannot both be null' ),
+                       ],
+                       'non-text left content' => [
+                               $this->makeContent( '', 'testing-nontext' ),
+                               $this->makeContent( "aaa\nbbb\nccc" ),
+                               new InvalidArgumentException( 'TextSlotDiffRenderer does not handle DummyNonTextContent' ),
+                       ],
+                       'non-text right content' => [
+                               $this->makeContent( "aaa\nbbb\nccc" ),
+                               $this->makeContent( '', 'testing-nontext' ),
+                               new InvalidArgumentException( 'TextSlotDiffRenderer does not handle DummyNonTextContent' ),
+                       ],
+               ];
+       }
+
+       // no separate test for getTextDiff() as getDiff() is just a thin wrapper around it
+
+       /**
+        * @return TextSlotDiffRenderer
+        */
+       private function getTextSlotDiffRenderer() {
+               $slotDiffRenderer = new TextSlotDiffRenderer();
+               $slotDiffRenderer->setStatsdDataFactory( new NullStatsdDataFactory() );
+               $slotDiffRenderer->setLanguage( Language::factory( 'en' ) );
+               $slotDiffRenderer->setWikiDiff2MovedParagraphDetectionCutoff( 0 );
+               $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_PHP );
+               return $slotDiffRenderer;
+       }
+
+       /**
+        * Convert a HTML diff to a human-readable format and hopefully make the test less fragile.
+        * @param string diff
+        * @return string
+        */
+       private function getPlainDiff( $diff ) {
+               $replacements = [
+                       html_entity_decode( '&nbsp;' ) => ' ',
+                       html_entity_decode( '&minus;' ) => '-',
+               ];
+               return str_replace( array_keys( $replacements ), array_values( $replacements ),
+                       trim( strip_tags( $diff ), "\n" ) );
+       }
+
+       /**
+        * @param string $str
+        * @param string $model
+        * @return null|TextContent
+        */
+       private function makeContent( $str, $model = CONTENT_MODEL_TEXT ) {
+               return ContentHandler::makeContent( $str, null, $model );
+       }
+
+}