diff --git a/RELEASE-NOTES-1.35 b/RELEASE-NOTES-1.35 index 634f74adf1c..ea49b27a487 100644 --- a/RELEASE-NOTES-1.35 +++ b/RELEASE-NOTES-1.35 @@ -100,6 +100,7 @@ because of Phabricator reports. getFallbacksFor, getFallbacksIncludingSiteLanguage. Use the corresponding new methods on the LanguageFallback class: getFirst, getAll, and getAllIncludingSiteLanguage. +* Title::countRevisionsBetween has been deprecated and moved into RevisionStore. === Other changes in 1.35 === * … diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php index 491292ae075..d0d17f9e466 100644 --- a/includes/Revision/RevisionStore.php +++ b/includes/Revision/RevisionStore.php @@ -3258,8 +3258,50 @@ class RevisionStore ); } - // TODO: move relevant methods from Title here, e.g. getFirstRevision, isBigDeletion, etc. + /** + * Get the number of revisions between the given revisions. + * Used for diffs and other things that really need it. + * + * @since 1.35 + * + * @param RevisionRecord $old Old revision or rev ID (first before range). + * @param RevisionRecord $new New revision or rev ID (first after range). + * @param int|null $max Limit of Revisions to count, will be incremented to detect truncations. + * @throws InvalidArgumentException in case either revision is unsaved or + * the revisions do not belong to the same page. + * @return int Number of revisions between these revisions. + */ + public function countRevisionsBetween( RevisionRecord $old, RevisionRecord $new, $max = null ) { + if ( $old->getId() === null || $new->getId() === null ) { + throw new InvalidArgumentException( 'Unsaved revision passed' ); + } + if ( $old->getPageId() !== $new->getPageId() ) { + throw new InvalidArgumentException( + "Revisions {$old->getId()} and {$new->getId()} do not belong to the same page" + ); + } + + $dbr = $this->getDBConnectionRef( DB_REPLICA ); + $oldTs = $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ); + $newTs = $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) ); + $conds = [ + 'rev_page' => $old->getPageId(), + "(rev_timestamp = {$oldTs} AND rev_id > {$old->getId()}) OR rev_timestamp > {$oldTs}", + "(rev_timestamp = {$newTs} AND rev_id < {$new->getId()}) OR rev_timestamp < {$newTs}", + ]; + if ( $max !== null ) { + return $dbr->selectRowCount( 'revision', '1', + $conds, + __METHOD__, + [ 'LIMIT' => $max + 1 ] // extra to detect truncation + ); + } else { + return (int)$dbr->selectField( 'revision', 'count(*)', $conds, __METHOD__ ); + } + } + + // TODO: move relevant methods from Title here, e.g. getFirstRevision, isBigDeletion, etc. } /** diff --git a/includes/Title.php b/includes/Title.php index baa57b64e69..b29684de402 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -4021,6 +4021,8 @@ class Title implements LinkTarget, IDBAccessObject { * Get the number of revisions between the given revision. * Used for diffs and other things that really need it. * + * @deprecated since 1.35 Use RevisionStore::countRevisionsBetween instead. + * * @param int|Revision $old Old revision or rev ID (first before range) * @param int|Revision $new New revision or rev ID (first after range) * @param int|null $max Limit of Revisions to count, will be incremented to detect truncations @@ -4036,21 +4038,9 @@ class Title implements LinkTarget, IDBAccessObject { if ( !$old || !$new ) { return 0; // nothing to compare } - $dbr = wfGetDB( DB_REPLICA ); - $conds = [ - 'rev_page' => $this->getArticleID(), - 'rev_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ), - 'rev_timestamp < ' . $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) ) - ]; - if ( $max !== null ) { - return $dbr->selectRowCount( 'revision', '1', - $conds, - __METHOD__, - [ 'LIMIT' => $max + 1 ] // extra to detect truncation - ); - } else { - return (int)$dbr->selectField( 'revision', 'count(*)', $conds, __METHOD__ ); - } + return MediaWikiServices::getInstance() + ->getRevisionStore() + ->countRevisionsBetween( $old->getRevisionRecord(), $new->getRevisionRecord(), $max ); } /** diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index a39ca82d384..493665a5f93 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -1463,7 +1463,12 @@ class DifferenceEngine extends ContextSource { // Sanity: don't show the notice if too many rows must be scanned // @todo show some special message for that case - $nEdits = $this->mNewPage->countRevisionsBetween( $oldRev, $newRev, 1000 ); + $nEdits = MediaWikiServices::getInstance()->getRevisionStore() + ->countRevisionsBetween( + $oldRev->getRevisionRecord(), + $newRev->getRevisionRecord(), + 1000 + ); if ( $nEdits > 0 && $nEdits <= 1000 ) { $limit = 100; // use diff-multi-manyusers if too many users $users = $this->mNewPage->getAuthorsBetween( $oldRev, $newRev, $limit ); diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php index 653884f2169..eea532100c0 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php @@ -2177,4 +2177,52 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { $this->assertTrue( $status->hasMessage( 'internalerror' ) ); } + /** + * @covers \MediaWiki\Revision\RevisionStore::countRevisionsBetween + */ + public function testCountRevisionsBetween() { + $NUM = 5; + $MAX = 1; + $page = $this->getTestPage(); + $revisions = []; + for ( $revNum = 0; $revNum < $NUM; $revNum++ ) { + $editStatus = $this->editPage( $page->getTitle()->getPrefixedDBkey(), 'Revision ' . $revNum ); + $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision ' . $revNum ); + $revisions[] = $editStatus->getValue()['revision']->getRevisionRecord(); + } + + $revisionStore = MediaWikiServices::getInstance()->getRevisionStore(); + $this->assertEquals( $NUM - 2, // The count is non-inclusive on both ends. + $revisionStore->countRevisionsBetween( $revisions[0], $revisions[$NUM - 1] ) ); + $this->assertEquals( $MAX + 1, // Returns $max + 1 to detect truncation. + $revisionStore->countRevisionsBetween( $revisions[0], $revisions[$NUM - 1], $MAX ) ); + } + + /** + * @covers \MediaWiki\Revision\RevisionStore::countRevisionsBetween + */ + public function testCountRevisionsBetween_differentPages() { + $page1 = $this->getTestPage(); + $page2 = $this->getTestPage( 'Other_Page' ); + $editStatus = $this->editPage( $page1->getTitle()->getPrefixedDBkey(), 'Revision 1' ); + $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision 1' ); + $rev1 = $editStatus->getValue()['revision']->getRevisionRecord(); + $editStatus = $this->editPage( $page2->getTitle()->getPrefixedDBkey(), 'Revision 1' ); + $this->assertTrue( $editStatus->isGood(), 'Sanity: must create revision 1' ); + $rev2 = $editStatus->getValue()['revision']->getRevisionRecord(); + + $this->expectException( InvalidArgumentException::class ); + MediaWikiServices::getInstance()->getRevisionStore()->countRevisionsBetween( $rev1, $rev2 ); + } + + /** + * @covers \MediaWiki\Revision\RevisionStore::countRevisionsBetween + */ + public function testCountRevisionsBetween_unsavedRevision() { + $rev1 = new MutableRevisionRecord( $this->getTestPageTitle() ); + $rev2 = new MutableRevisionRecord( $this->getTestPageTitle() ); + + $this->expectException( InvalidArgumentException::class ); + MediaWikiServices::getInstance()->getRevisionStore()->countRevisionsBetween( $rev1, $rev2 ); + } }