From 51602a436c723fe8ccf353d5fe5940aec857935f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gerg=C5=91=20Tisza?= Date: Tue, 28 Aug 2018 17:34:25 +0200 Subject: [PATCH] [MCR] Move getSecondaryDataUpdates to the page level Replaces Content::getSecondaryDataUpdates with WikiPage::getSecondaryDataUpdates so that aggregation of data updates from multiple page slots can be handled without the caller having to care about it. Also adds a WikiPage::updateParserCache method for convenience. This is a temporary measure until DerivedPageDataUpdater (or its replacement) can be exposed directly, at which point the WikiPage methods will be deprecated. Also fixes a parameter handling bug in DerivedPageDataUpdater. Bug: T194043 Change-Id: Idbe7d582b49fcb7c90aea813773b7610ad44b1a8 --- includes/Storage/DerivedPageDataUpdater.php | 166 ++++++++++++++---- includes/Storage/PageUpdater.php | 4 + includes/api/ApiPurge.php | 60 +++---- includes/jobqueue/jobs/RefreshLinksJob.php | 51 ++---- includes/page/WikiPage.php | 103 ++++++++++- maintenance/refreshLinks.php | 19 +- .../Storage/DerivedPageDataUpdaterTest.php | 4 +- .../includes/page/WikiPageDbTestBase.php | 28 ++- 8 files changed, 308 insertions(+), 127 deletions(-) diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php index 2df1670c88..a7dfb4b707 100644 --- a/includes/Storage/DerivedPageDataUpdater.php +++ b/includes/Storage/DerivedPageDataUpdater.php @@ -51,6 +51,7 @@ use SiteStatsUpdate; use Title; use User; use Wikimedia\Assert\Assert; +use Wikimedia\Rdbms\LBFactory; use WikiPage; /** @@ -121,6 +122,11 @@ class DerivedPageDataUpdater implements IDBAccessObject { */ private $messageCache; + /** + * @var LBFactory + */ + private $loadbalancerFactory; + /** * @var string see $wgArticleCountMethod */ @@ -132,15 +138,22 @@ class DerivedPageDataUpdater implements IDBAccessObject { private $rcWatchCategoryMembership = false; /** - * See $options on prepareUpdate. + * Stores (most of) the $options parameter of prepareUpdate(). + * @see prepareUpdate() */ private $options = [ 'changed' => true, 'created' => false, 'moved' => false, 'restored' => false, + 'oldrevision' => null, 'oldcountable' => null, 'oldredirect' => null, + 'triggeringUser' => null, + // causeAction/causeAgent default to 'unknown' but that's handled where it's read, + // to make the life of prepareUpdate() callers easier. + 'causeAction' => null, + 'causeAgent' => null, ]; /** @@ -235,6 +248,7 @@ class DerivedPageDataUpdater implements IDBAccessObject { * @param JobQueueGroup $jobQueueGroup * @param MessageCache $messageCache * @param Language $contLang + * @param LBFactory $loadbalancerFactory */ public function __construct( WikiPage $wikiPage, @@ -243,7 +257,8 @@ class DerivedPageDataUpdater implements IDBAccessObject { ParserCache $parserCache, JobQueueGroup $jobQueueGroup, MessageCache $messageCache, - Language $contLang + Language $contLang, + LBFactory $loadbalancerFactory ) { $this->wikiPage = $wikiPage; @@ -253,6 +268,9 @@ class DerivedPageDataUpdater implements IDBAccessObject { $this->jobQueueGroup = $jobQueueGroup; $this->messageCache = $messageCache; $this->contLang = $contLang; + // XXX only needed for waiting for slaves to catch up; there should be a narrower + // interface for that. + $this->loadbalancerFactory = $loadbalancerFactory; } /** @@ -868,6 +886,14 @@ class DerivedPageDataUpdater implements IDBAccessObject { } } + private function assertHasRevision( $method ) { + if ( !$this->revision->getId() ) { + throw new LogicException( + 'Must call prepareUpdate() before calling ' . $method + ); + } + } + /** * Whether the edit creates the page. * @@ -998,7 +1024,8 @@ class DerivedPageDataUpdater implements IDBAccessObject { * - moved: bool, whether the page was moved (default false) * - restored: bool, whether the page was undeleted (default false) * - oldrevision: Revision object for the pre-update revision (default null) - * - triggeringuser: The user triggering the update (UserIdentity, default null) + * - triggeringUser: The user triggering the update (UserIdentity, defaults to the + * user who created the revision) * - oldredirect: bool, null, or string 'no-change' (default null): * - bool: whether the page was counted as a redirect before that * revision, only used in changed is true and created is false @@ -1010,6 +1037,10 @@ class DerivedPageDataUpdater implements IDBAccessObject { * is true, do update the article count * - 'no-change': don't update the article count, ever * When set to null, pageState['oldCountable'] will be used instead if available. + * - causeAction: an arbitrary string identifying the reason for the update. + * See DataUpdate::getCauseAction(). (default 'unknown') + * - causeAgent: name of the user who caused the update. See DataUpdate::getCauseAgent(). + * (string, default 'unknown') */ public function prepareUpdate( RevisionRecord $revision, array $options = [] ) { Assert::parameter( @@ -1020,9 +1051,9 @@ class DerivedPageDataUpdater implements IDBAccessObject { 'must be a RevisionRecord (or Revision)' ); Assert::parameter( - !isset( $options['triggeringuser'] ) - || $options['triggeringuser'] instanceof UserIdentity, - '$options["triggeringuser"]', + !isset( $options['triggeringUser'] ) + || $options['triggeringUser'] instanceof UserIdentity, + '$options["triggeringUser"]', 'must be a UserIdentity' ); @@ -1260,40 +1291,16 @@ class DerivedPageDataUpdater implements IDBAccessObject { $wikiPage = $this->getWikiPage(); // TODO: use only for legacy hooks! - // NOTE: this may trigger the first parsing of the new content after an edit (when not - // using pre-generated stashed output). - // XXX: we may want to use the PoolCounter here. This would perhaps allow the initial parse - // to be perform post-send. The client could already follow a HTTP redirect to the - // page view, but would then have to wait for a response until rendering is complete. - $output = $this->getCanonicalParserOutput(); - - // Save it to the parser cache. - // Make sure the cache time matches page_touched to avoid double parsing. - $this->parserCache->save( - $output, $wikiPage, $this->getCanonicalParserOptions(), - $this->revision->getTimestamp(), $this->revision->getId() - ); - $legacyUser = User::newFromIdentity( $this->user ); $legacyRevision = new Revision( $this->revision ); - // Update the links tables and other secondary data - $recursive = $this->options['changed']; // T52785 - $updates = $this->getSecondaryDataUpdates( $recursive ); + $this->doParserCacheUpdate(); - $triggeringUser = $this->options['triggeringuser'] ?? $this->user; - if ( !$triggeringUser instanceof User ) { - $triggeringUser = User::newFromIdentity( $triggeringUser ); - } - foreach ( $updates as $update ) { - // TODO: make an $option field for the cause - $update->setCause( 'edit-page', $triggeringUser->getName() ); - if ( $update instanceof LinksUpdate ) { - $update->setRevision( $legacyRevision ); - $update->setTriggeringUser( $triggeringUser ); - } - DeferredUpdates::addUpdate( $update ); - } + $this->doSecondaryDataUpdates( [ + // T52785 do not update any other pages on a null edit + 'recursive' => $this->options['changed'], + 'defer' => DeferredUpdates::POSTSEND, + ] ); // TODO: MCR: check if *any* changed slot supports categories! if ( $this->rcWatchCategoryMembership @@ -1421,4 +1428,91 @@ class DerivedPageDataUpdater implements IDBAccessObject { $this->doTransition( 'done' ); } + /** + * Do secondary data updates (such as updating link tables). + * + * MCR note: this method is temporarily exposed via WikiPage::doSecondaryDataUpdates. + * + * @param array $options + * - recursive: make the update recursive, i.e. also update pages which transclude the + * current page or otherwise depend on it (default: false) + * - defer: one of the DeferredUpdates constants, or false to run immediately after waiting + * for replication of the changes from the SecondaryDataUpdates hooks (default: false) + * - transactionTicket: a transaction ticket from LBFactory::getEmptyTransactionTicket(), + * only when defer is false (default: null) + * @since 1.32 + */ + public function doSecondaryDataUpdates( array $options = [] ) { + $this->assertHasRevision( __METHOD__ ); + $options += [ + 'recursive' => false, + 'defer' => false, + 'transactionTicket' => null, + ]; + $deferValues = [ false, DeferredUpdates::PRESEND, DeferredUpdates::POSTSEND ]; + if ( !in_array( $options['defer'], $deferValues, true ) ) { + throw new InvalidArgumentException( 'invalid value for defer: ' . $options['defer'] ); + } + Assert::parameterType( 'integer|null', $options['transactionTicket'], + '$options[\'transactionTicket\']' ); + + $updates = $this->getSecondaryDataUpdates( $options['recursive'] ); + + $triggeringUser = $this->options['triggeringUser'] ?? $this->user; + if ( !$triggeringUser instanceof User ) { + $triggeringUser = User::newFromIdentity( $triggeringUser ); + } + $causeAction = $this->options['causeAction'] ?? 'unknown'; + $causeAgent = $this->options['causeAgent'] ?? 'unknown'; + $legacyRevision = new Revision( $this->revision ); + + if ( $options['defer'] === false && $options['transactionTicket'] !== null ) { + // For legacy hook handlers doing updates via LinksUpdateConstructed, make sure + // any pending writes they made get flushed before the doUpdate() calls below. + // This avoids snapshot-clearing errors in LinksUpdate::acquirePageLock(). + $this->loadbalancerFactory->commitAndWaitForReplication( + __METHOD__, $options['transactionTicket'] + ); + } + + foreach ( $updates as $update ) { + $update->setCause( $causeAction, $causeAgent ); + if ( $update instanceof LinksUpdate ) { + $update->setRevision( $legacyRevision ); + $update->setTriggeringUser( $triggeringUser ); + } + if ( $options['defer'] === false ) { + if ( $options['transactionTicket'] !== null ) { + $update->setTransactionTicket( $options['transactionTicket'] ); + } + $update->doUpdate(); + } else { + DeferredUpdates::addUpdate( $update, $options['defer'] ); + } + } + } + + public function doParserCacheUpdate() { + $this->assertHasRevision( __METHOD__ ); + + $wikiPage = $this->getWikiPage(); // TODO: ParserCache should accept a RevisionRecord instead + + // NOTE: this may trigger the first parsing of the new content after an edit (when not + // using pre-generated stashed output). + // XXX: we may want to use the PoolCounter here. This would perhaps allow the initial parse + // to be performed post-send. The client could already follow a HTTP redirect to the + // page view, but would then have to wait for a response until rendering is complete. + $output = $this->getCanonicalParserOutput(); + + // Save it to the parser cache. Use the revision timestamp in the case of a + // freshly saved edit, as that matches page_touched and a mismatch would trigger an + // unnecessary reparse. + $timestamp = $this->options['changed'] ? $this->revision->getTimestamp() + : $output->getTimestamp(); + $this->parserCache->save( + $output, $wikiPage, $this->getCanonicalParserOptions(), + $timestamp, $this->revision->getId() + ); + } + } diff --git a/includes/Storage/PageUpdater.php b/includes/Storage/PageUpdater.php index 9d2f20981e..ec1f6e2e0d 100644 --- a/includes/Storage/PageUpdater.php +++ b/includes/Storage/PageUpdater.php @@ -1187,6 +1187,10 @@ class PageUpdater { $wikiPage, $newRevisionRecord, $user, $summary, $flags, $status, $hints ) { + // set debug data + $hints['causeAction'] = 'edit-page'; + $hints['causeAgent'] = $user->getName(); + $newLegacyRevision = new Revision( $newRevisionRecord ); $mainContent = $newRevisionRecord->getContent( 'main', RevisionRecord::RAW ); diff --git a/includes/api/ApiPurge.php b/includes/api/ApiPurge.php index bb0be68484..bb1f3d3bf9 100644 --- a/includes/api/ApiPurge.php +++ b/includes/api/ApiPurge.php @@ -1,5 +1,4 @@ pingLimiter( 'linkpurge' ) ) { - $popts = $page->makeParserOptions( 'canonical' ); - - # Parse content; note that HTML generation is only needed if we want to cache the result. - $content = $page->getContent( Revision::RAW ); - if ( $content ) { - $enableParserCache = $this->getConfig()->get( 'EnableParserCache' ); - $p_result = $content->getParserOutput( - $title, - $page->getLatest(), - $popts, - $enableParserCache + # Logging to better see expensive usage patterns + if ( $forceRecursiveLinkUpdate ) { + LoggerFactory::getInstance( 'RecursiveLinkPurge' )->info( + "Recursive link purge enqueued for {title}", + [ + 'user' => $this->getUser()->getName(), + 'title' => $title->getPrefixedText() + ] ); - - # Logging to better see expensive usage patterns - if ( $forceRecursiveLinkUpdate ) { - LoggerFactory::getInstance( 'RecursiveLinkPurge' )->info( - "Recursive link purge enqueued for {title}", - [ - 'user' => $this->getUser()->getName(), - 'title' => $title->getPrefixedText() - ] - ); - } - - # Update the links tables - $updates = $content->getSecondaryDataUpdates( - $title, null, $forceRecursiveLinkUpdate, $p_result ); - foreach ( $updates as $update ) { - $update->setCause( 'api-purge', $this->getUser()->getName() ); - DeferredUpdates::addUpdate( $update, DeferredUpdates::PRESEND ); - } - - $r['linkupdate'] = true; - - if ( $enableParserCache ) { - $pcache = MediaWikiServices::getInstance()->getParserCache(); - $pcache->save( $p_result, $page, $popts ); - } } + + $page->updateParserCache( [ + 'causeAction' => 'api-purge', + 'causeAgent' => $this->getUser()->getName(), + ] ); + $page->doSecondaryDataUpdates( [ + 'recursive' => $forceRecursiveLinkUpdate, + 'causeAction' => 'api-purge', + 'causeAgent' => $this->getUser()->getName(), + 'defer' => DeferredUpdates::PRESEND, + ] ); + $r['linkupdate'] = true; } else { $this->addWarning( 'apierror-ratelimited' ); $forceLinkUpdate = false; diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php b/includes/jobqueue/jobs/RefreshLinksJob.php index 5c92874e79..0b6d30776b 100644 --- a/includes/jobqueue/jobs/RefreshLinksJob.php +++ b/includes/jobqueue/jobs/RefreshLinksJob.php @@ -247,43 +247,24 @@ class RefreshLinksJob extends Job { $stats->increment( 'refreshlinks.parser_uncached' ); } - $updates = $content->getSecondaryDataUpdates( - $title, - null, - !empty( $this->params['useRecursiveLinksUpdate'] ), - $parserOutput - ); - - // For legacy hook handlers doing updates via LinksUpdateConstructed, make sure - // any pending writes they made get flushed before the doUpdate() calls below. - // This avoids snapshot-clearing errors in LinksUpdate::acquirePageLock(). - $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket ); - - foreach ( $updates as $update ) { - // Carry over cause in case so the update can do extra logging - $update->setCause( $this->params['causeAction'], $this->params['causeAgent'] ); - // FIXME: This code probably shouldn't be here? - // Needed by things like Echo notifications which need - // to know which user caused the links update - if ( $update instanceof LinksUpdate ) { - $update->setRevision( $revision ); - if ( !empty( $this->params['triggeringUser'] ) ) { - $userInfo = $this->params['triggeringUser']; - if ( $userInfo['userId'] ) { - $user = User::newFromId( $userInfo['userId'] ); - } else { - // Anonymous, use the username - $user = User::newFromName( $userInfo['userName'], false ); - } - $update->setTriggeringUser( $user ); - } + $options = [ + 'recursive' => !empty( $this->params['useRecursiveLinksUpdate'] ), + // Carry over cause so the update can do extra logging + 'causeAction' => $this->params['causeAction'], + 'causeAgent' => $this->params['causeAgent'], + 'defer' => false, + 'transactionTicket' => $ticket, + ]; + if ( !empty( $this->params['triggeringUser'] ) ) { + $userInfo = $this->params['triggeringUser']; + if ( $userInfo['userId'] ) { + $options['triggeringUser'] = User::newFromId( $userInfo['userId'] ); + } else { + // Anonymous, use the username + $options['triggeringUser'] = User::newFromName( $userInfo['userName'], false ); } } - - foreach ( $updates as $update ) { - $update->setTransactionTicket( $ticket ); - $update->doUpdate(); - } + $page->doSecondaryDataUpdates( $options ); InfoAction::invalidateCache( $title ); diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index b609d7bd79..bf21a56708 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -769,6 +769,18 @@ class WikiPage implements Page, IDBAccessObject { return null; } + /** + * Get the latest revision + * @return RevisionRecord|null + */ + public function getRevisionRecord() { + $this->loadLastEdit(); + if ( $this->mLastRevision ) { + return $this->mLastRevision->getRevisionRecord(); + } + return null; + } + /** * Get the content of the current revision. No side-effects... * @@ -1164,6 +1176,8 @@ class WikiPage implements Page, IDBAccessObject { * The parser cache will be used if possible. Cache misses that result * in parser runs are debounced with PoolCounter. * + * XXX merge this with updateParserCache()? + * * @since 1.19 * @param ParserOptions $parserOptions ParserOptions to use for the parse operation * @param null|int $oldid Revision ID to get the text from, passing null or 0 will @@ -1644,7 +1658,7 @@ class WikiPage implements Page, IDBAccessObject { JobQueueGroup::singleton(), MessageCache::singleton(), MediaWikiServices::getInstance()->getContentLanguage(), - LoggerFactory::getInstance( 'SaveParse' ) + MediaWikiServices::getInstance()->getDBLoadBalancerFactory() ); $derivedDataUpdater->setRcWatchCategoryMembership( $wgRCWatchCategoryMembership ); @@ -1958,7 +1972,13 @@ class WikiPage implements Page, IDBAccessObject { $updater->prepareContent( $user, $slots, $useCache ); if ( $revision ) { - $updater->prepareUpdate( $revision ); + $updater->prepareUpdate( + $revision, + [ + 'causeAction' => 'prepare-edit', + 'causeAgent' => $user->getName(), + ] + ); } } @@ -1987,8 +2007,17 @@ class WikiPage implements Page, IDBAccessObject { * - null: if created is false, don't update the article count; if created * is true, do update the article count * - 'no-change': don't update the article count, ever + * - causeAction: an arbitrary string identifying the reason for the update. + * See DataUpdate::getCauseAction(). (default 'edit-page') + * - causeAgent: name of the user who caused the update. See DataUpdate::getCauseAgent(). + * (string, defaults to the passed user) */ public function doEditUpdates( Revision $revision, User $user, array $options = [] ) { + $options += [ + 'causeAction' => 'edit-page', + 'causeAgent' => $user->getName(), + ]; + $revision = $revision->getRevisionRecord(); $updater = $this->getDerivedDataUpdater( $user, $revision ); @@ -1998,6 +2027,76 @@ class WikiPage implements Page, IDBAccessObject { $updater->doUpdates(); } + /** + * Update the parser cache. + * + * @note This is a temporary workaround until there is a proper data updater class. + * It will become deprecated soon. + * + * @param array $options + * - causeAction: an arbitrary string identifying the reason for the update. + * See DataUpdate::getCauseAction(). (default 'edit-page') + * - causeAgent: name of the user who caused the update (string, defaults to the + * user who created the revision) + * @since 1.32 + */ + public function updateParserCache( array $options = [] ) { + $revision = $this->getRevisionRecord(); + if ( !$revision || !$revision->getId() ) { + LoggerFactory::getInstance( 'wikipage' )->info( + __METHOD__ . 'called with ' . ( $revision ? 'unsaved' : 'no' ) . ' revision' + ); + return; + } + $user = User::newFromIdentity( $revision->getUser( RevisionRecord::RAW ) ); + + $updater = $this->getDerivedDataUpdater( $user, $revision ); + $updater->prepareUpdate( $revision, $options ); + $updater->doParserCacheUpdate(); + } + + /** + * Do secondary data updates (such as updating link tables). + * Secondary data updates are only a small part of the updates needed after saving + * a new revision; normally PageUpdater::doUpdates should be used instead (which includes + * secondary data updates). This method is provided for partial purges. + * + * @note This is a temporary workaround until there is a proper data updater class. + * It will become deprecated soon. + * + * @param array $options + * - recursive (bool, default true): whether to do a recursive update (update pages that + * depend on this page, e.g. transclude it). This will set the $recursive parameter of + * Content::getSecondaryDataUpdates. Typically this should be true unless the update + * was something that did not really change the page, such as a null edit. + * - triggeringUser: The user triggering the update (UserIdentity, defaults to the + * user who created the revision) + * - causeAction: an arbitrary string identifying the reason for the update. + * See DataUpdate::getCauseAction(). (default 'unknown') + * - causeAgent: name of the user who caused the update (string, default 'unknown') + * - defer: one of the DeferredUpdates constants, or false to run immediately (default: false). + * Note that even when this is set to false, some updates might still get deferred (as + * some update might directly add child updates to DeferredUpdates). + * - transactionTicket: a transaction ticket from LBFactory::getEmptyTransactionTicket(), + * only when defer is false (default: null) + * @since 1.32 + */ + public function doSecondaryDataUpdates( array $options = [] ) { + $options['recursive'] = $options['recursive'] ?? true; + $revision = $this->getRevisionRecord(); + if ( !$revision || !$revision->getId() ) { + LoggerFactory::getInstance( 'wikipage' )->info( + __METHOD__ . 'called with ' . ( $revision ? 'unsaved' : 'no' ) . ' revision' + ); + return; + } + $user = User::newFromIdentity( $revision->getUser( RevisionRecord::RAW ) ); + + $updater = $this->getDerivedDataUpdater( $user, $revision ); + $updater->prepareUpdate( $revision, $options ); + $updater->doSecondaryDataUpdates( $options ); + } + /** * Update the article's restriction field, and leave a log entry. * This works for protection both existing and non-existing pages. diff --git a/maintenance/refreshLinks.php b/maintenance/refreshLinks.php index 9ca4e986c8..8306243c08 100644 --- a/maintenance/refreshLinks.php +++ b/maintenance/refreshLinks.php @@ -268,17 +268,14 @@ class RefreshLinks extends Maintenance { return; } - $content = $page->getContent( Revision::RAW ); - if ( $content === null ) { - return; - } - - $updates = $content->getSecondaryDataUpdates( - $page->getTitle(), /* $old = */ null, /* $recursive = */ false ); - foreach ( $updates as $update ) { - DeferredUpdates::addUpdate( $update ); - DeferredUpdates::doUpdates(); - } + // Defer updates to post-send but then immediately execute deferred updates; + // this is the simplest way to run all updates immediately (including updates + // scheduled by other updates). + $page->doSecondaryDataUpdates( [ + 'defer' => DeferredUpdates::POSTSEND, + 'recursive' => false, + ] ); + DeferredUpdates::doUpdates(); } /** diff --git a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php index 189b79b880..19fa1047a4 100644 --- a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php +++ b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php @@ -22,7 +22,7 @@ use WikitextContent; /** * @group Database * - * @covers MediaWiki\Storage\DerivedPageDataUpdater + * @covers \MediaWiki\Storage\DerivedPageDataUpdater */ class DerivedPageDataUpdaterTest extends MediaWikiTestCase { @@ -736,6 +736,8 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase { /** * @covers \MediaWiki\Storage\DerivedPageDataUpdater::doUpdates() + * @covers \MediaWiki\Storage\DerivedPageDataUpdater::doSecondaryDataUpdates() + * @covers \MediaWiki\Storage\DerivedPageDataUpdater::doParserCacheUpdate() */ public function testDoUpdates() { $page = $this->getPage( __METHOD__ ); diff --git a/tests/phpunit/includes/page/WikiPageDbTestBase.php b/tests/phpunit/includes/page/WikiPageDbTestBase.php index 5973d9e444..dc805dfeec 100644 --- a/tests/phpunit/includes/page/WikiPageDbTestBase.php +++ b/tests/phpunit/includes/page/WikiPageDbTestBase.php @@ -1,5 +1,6 @@ prepareContentForEdit( $content, null, $user, null, false ); - $this->assertEquals( $edit, $sameEdit, 'equivalent PreparedEdit' ); + $this->assertPreparedEditEquals( $edit, $sameEdit, 'equivalent PreparedEdit' ); $this->assertSame( $edit->pstContent, $sameEdit->pstContent, 're-use output' ); $this->assertSame( $edit->output, $sameEdit->output, 're-use output' ); // Not re-using the same PreparedEdit if not possible $rev = $page->getRevision(); $edit2 = $page->prepareContentForEdit( $content2, null, $user, null, false ); - $this->assertNotEquals( $edit, $edit2 ); + $this->assertPreparedEditNotEquals( $edit, $edit2 ); $this->assertContains( 'At vero eos', $edit2->pstContent->serialize(), "content" ); // Check pre-safe transform @@ -179,7 +180,7 @@ abstract class WikiPageDbTestBase extends MediaWikiLangTestCase { $this->assertNotContains( '~~~~', $edit2->pstContent->serialize() ); $edit3 = $page->prepareContentForEdit( $content2, null, $sysop, null, false ); - $this->assertNotEquals( $edit2, $edit3 ); + $this->assertPreparedEditNotEquals( $edit2, $edit3 ); // TODO: test with passing revision, then same without revision. } @@ -2396,4 +2397,25 @@ more stuff $this->assertNotSame( $updater5, $updater6 ); } + protected function assertPreparedEditEquals( + PreparedEdit $edit, PreparedEdit $edit2, $message = '' + ) { + // suppress differences caused by a clock tick between generating the two PreparedEdits + if ( abs( $edit->timestamp - $edit2->timestamp ) < 3 ) { + $edit2 = clone $edit2; + $edit2->timestamp = $edit->timestamp; + } + $this->assertEquals( $edit, $edit2, $message ); + } + + protected function assertPreparedEditNotEquals( + PreparedEdit $edit, PreparedEdit $edit2, $message = '' + ) { + if ( abs( $edit->timestamp - $edit2->timestamp ) < 3 ) { + $edit2 = clone $edit2; + $edit2->timestamp = $edit->timestamp; + } + $this->assertNotEquals( $edit, $edit2, $message ); + } + } -- 2.20.1