[MCR] Render multi-slot diffs
authorGergő Tisza <tgr.huwiki@gmail.com>
Wed, 11 Jul 2018 09:24:07 +0000 (11:24 +0200)
committerGergő Tisza <tgr.huwiki@gmail.com>
Mon, 20 Aug 2018 13:39:12 +0000 (15:39 +0200)
Move logic for rendering a diff between two content objects out of
DifferenceEngine, into a new SlotDiffRenderer class. Make
DifferenceEngine use multiple SlotDiffRenderers, one per slot.

This separates the class tree for changing high-level diff properties
such as the header or the revision selection method (same as before:
subclass DifferenceEngine and override ContentHandler::getDiffEngineClass
or implement GetDifferenceEngine) and the one for changing the actual
diff rendering for a given content type (subclass SlotDiffRenderer and
override ContentHandler::getSlotDiffRenderer or implement
GetSlotDiffRenderer). To keep B/C, when SlotDiffRenderer is not overridden
for a given content type but DifferenceEngine is, that DifferenceEngine
will be used instead.
The weak point of the scheme is overriding the DifferenceEngine methods
passing control to the SlotDiffRenderers (the ones calling
getDifferenceEngines), without calling the parent. These are:
showDiffStyle, getDiffBody, getDiffBodyCacheKeyParams. Extensions doing
that will probably break in unpredictable ways (most likely, only
showing the main slot diff). Nothing in gerrit does it, at least.

A new GetSlotDiffRenderer hook is added to modify rendering for content
models not owned by the extension, much like how GetDifferenceEngine
works.

Also deprecates public access to mNewRev/mOldRev and creates public
getters instead. DifferenceEngine never supported external changes to
those properties, this just acknowledges it.

Bug: T194731
Change-Id: I2f8a9dbebd2290b7feafb20e2bb2a2693e18ba11
Depends-On: I04e885a33bfce5bccc807b9bcfe1eaa577a9fd47
Depends-On: I203d8895bf436b7fee53fe4718dede8a3b1768bc

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 158fbe2..55d2a95 100644 (file)
@@ -68,6 +68,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 ===
 * …
@@ -340,12 +344,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 );
+       }
+
+}