From d31580eeb0e9a88f46439301e27db273b7c63a7e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gerg=C5=91=20Tisza?= Date: Wed, 11 Jul 2018 11:24:07 +0200 Subject: [PATCH] [MCR] Render multi-slot diffs 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 --- RELEASE-NOTES-1.32 | 20 +- autoload.php | 3 + docs/hooks.txt | 6 + includes/content/ContentHandler.php | 60 ++ includes/diff/DifferenceEngine.php | 591 ++++++++++++------ .../diff/DifferenceEngineSlotDiffRenderer.php | 79 +++ includes/diff/SlotDiffRenderer.php | 65 ++ includes/diff/TextSlotDiffRenderer.php | 287 +++++++++ resources/src/mediawiki.diff.styles/diff.css | 4 + tests/common/TestsAutoLoader.php | 1 + .../includes/content/ContentHandlerTest.php | 126 ++++ .../includes/diff/CustomDifferenceEngine.php | 23 + .../DifferenceEngineSlotDiffRendererTest.php | 44 ++ .../includes/diff/DifferenceEngineTest.php | 206 ++++++ .../diff/TextSlotDiffRendererTest.php | 113 ++++ 15 files changed, 1431 insertions(+), 197 deletions(-) create mode 100644 includes/diff/DifferenceEngineSlotDiffRenderer.php create mode 100644 includes/diff/SlotDiffRenderer.php create mode 100644 includes/diff/TextSlotDiffRenderer.php create mode 100644 tests/phpunit/includes/diff/CustomDifferenceEngine.php create mode 100644 tests/phpunit/includes/diff/DifferenceEngineSlotDiffRendererTest.php create mode 100644 tests/phpunit/includes/diff/TextSlotDiffRendererTest.php diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index 158fbe2648..55d2a9589b 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -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. diff --git a/autoload.php b/autoload.php index a372dd19f0..8679827dcb 100644 --- a/autoload.php +++ b/autoload.php @@ -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', diff --git a/docs/hooks.txt b/docs/hooks.txt index 9a634ad35d..9cb22d23ed 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -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 diff --git a/includes/content/ContentHandler.php b/includes/content/ContentHandler.php index 0a451b2fa3..b3286a9a29 100644 --- a/includes/content/ContentHandler.php +++ b/includes/content/ContentHandler.php @@ -1,5 +1,6 @@ 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. * diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index 5138655763..2ceda216d0 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -20,12 +20,30 @@ * @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 = ' [' . @@ -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=&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 index 0000000000..0c632b97ec --- /dev/null +++ b/includes/diff/DifferenceEngineSlotDiffRenderer.php @@ -0,0 +1,79 @@ +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 index 0000000000..f20b4169db --- /dev/null +++ b/includes/diff/SlotDiffRenderer.php @@ -0,0 +1,65 @@ +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 ); + } + +} diff --git a/resources/src/mediawiki.diff.styles/diff.css b/resources/src/mediawiki.diff.styles/diff.css index c46922257f..2053843e06 100644 --- a/resources/src/mediawiki.diff.styles/diff.css +++ b/resources/src/mediawiki.diff.styles/diff.css @@ -126,6 +126,10 @@ td.diff-marker { unicode-bidi: embed; } +.mw-diff-slot-header { + text-align: center; +} + /*! * Wikidiff2 rendering for moved paragraphs */ diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index 0cb042a553..9626312a30 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -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 diff --git a/tests/phpunit/includes/content/ContentHandlerTest.php b/tests/phpunit/includes/content/ContentHandlerTest.php index 323a63d788..43edf60fea 100644 --- a/tests/phpunit/includes/content/ContentHandlerTest.php +++ b/tests/phpunit/includes/content/ContentHandlerTest.php @@ -1,5 +1,6 @@ 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 index 0000000000..c760d02978 --- /dev/null +++ b/tests/phpunit/includes/diff/CustomDifferenceEngine.php @@ -0,0 +1,23 @@ +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 index 0000000000..fe129b751a --- /dev/null +++ b/tests/phpunit/includes/diff/DifferenceEngineSlotDiffRendererTest.php @@ -0,0 +1,44 @@ +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 ); + } + +} diff --git a/tests/phpunit/includes/diff/DifferenceEngineTest.php b/tests/phpunit/includes/diff/DifferenceEngineTest.php index 57aeb20026..40f68077c9 100644 --- a/tests/phpunit/includes/diff/DifferenceEngineTest.php +++ b/tests/phpunit/includes/diff/DifferenceEngineTest.php @@ -1,5 +1,8 @@ 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( ' ' ) => ' ', + html_entity_decode( '−' ) => '-', + ]; + 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 index 0000000000..ec45e29d67 --- /dev/null +++ b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php @@ -0,0 +1,113 @@ +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( ' ' ) => ' ', + html_entity_decode( '−' ) => '-', + ]; + 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 ); + } + +} -- 2.20.1