From 8a88d51536cd2180c5d50eaf1bc7ffb5b4f882e9 Mon Sep 17 00:00:00 2001 From: DannyS712 Date: Fri, 24 Apr 2020 00:55:09 +0000 Subject: [PATCH] WikiPage::updateRevisionOn - replace uses, hard deprecate Revision use See codesearch - only deployed call outside of core is in flaggedrevs, and already passes a RevisionRecord: https://codesearch.wmflabs.org/deployed/?q=-%3EupdateRevisionOn%5C(&i=nope&files=&repos= Also fixed a use of Revision::newFromId in orphans.php Bug: T249561 Bug: T249021 Change-Id: I5933a278de8645b7005c11026c87ae27c0373770 --- includes/MovePage.php | 11 ++++++++--- includes/Storage/PageUpdater.php | 8 ++++---- includes/filerepo/file/LocalFile.php | 3 +-- includes/page/WikiPage.php | 10 ++++++++-- includes/specials/helpers/ImportReporter.php | 8 ++++---- maintenance/attachLatest.php | 7 +++---- maintenance/orphans.php | 3 ++- .../includes/Revision/RevisionStoreDbTestBase.php | 2 +- tests/phpunit/includes/page/WikiPageDbTest.php | 4 ++++ 9 files changed, 35 insertions(+), 21 deletions(-) diff --git a/includes/MovePage.php b/includes/MovePage.php index 02a65b0309f..f90e441122d 100644 --- a/includes/MovePage.php +++ b/includes/MovePage.php @@ -862,9 +862,10 @@ class MovePage { $this->oldTitle->resetArticleID( 0 ); // 0 == non existing $newpage->loadPageData( WikiPage::READ_LOCKING ); // T48397 - $nullRevisionObj = new Revision( $nullRevision ); - $newpage->updateRevisionOn( $dbw, $nullRevisionObj ); + $newpage->updateRevisionOn( $dbw, $nullRevision ); + // TODO cleanup hooks and use of $nullRevisionObj + $nullRevisionObj = new Revision( $nullRevision ); $fakeTags = []; Hooks::run( 'NewRevisionFromEditComplete', [ $newpage, $nullRevisionObj, $nullRevision->getParentId(), $user, &$fakeTags ] ); @@ -889,7 +890,11 @@ class MovePage { 'comment' => $comment, 'content' => $redirectContent ] ); $redirectRevId = $redirectRevision->insertOn( $dbw ); - $redirectArticle->updateRevisionOn( $dbw, $redirectRevision, 0 ); + $redirectArticle->updateRevisionOn( + $dbw, + $redirectRevision->getRevisionRecord(), + 0 + ); $fakeTags = []; Hooks::run( 'NewRevisionFromEditComplete', diff --git a/includes/Storage/PageUpdater.php b/includes/Storage/PageUpdater.php index dd9a29d3798..ebe528c9bad 100644 --- a/includes/Storage/PageUpdater.php +++ b/includes/Storage/PageUpdater.php @@ -1025,16 +1025,16 @@ class PageUpdater { // Save revision content and meta-data $newRevisionRecord = $this->revisionStore->insertRevisionOn( $newRevisionRecord, $dbw ); - $newLegacyRevision = new Revision( $newRevisionRecord ); // Update page_latest and friends to reflect the new revision // TODO: move to storage service $wasRedirect = $this->derivedDataUpdater->wasRedirect(); - if ( !$wikiPage->updateRevisionOn( $dbw, $newLegacyRevision, null, $wasRedirect ) ) { + if ( !$wikiPage->updateRevisionOn( $dbw, $newRevisionRecord, null, $wasRedirect ) ) { throw new PageUpdateException( "Failed to update page row to use new revision." ); } // TODO: replace legacy hook! + $newLegacyRevision = new Revision( $newRevisionRecord ); $tags = $this->computeEffectiveTags( $flags ); Hooks::run( 'NewRevisionFromEditComplete', @@ -1160,15 +1160,15 @@ class PageUpdater { // Save the revision text... $newRevisionRecord = $this->revisionStore->insertRevisionOn( $newRevisionRecord, $dbw ); - $newLegacyRevision = new Revision( $newRevisionRecord ); // Update the page record with revision data // TODO: move to storage service - if ( !$wikiPage->updateRevisionOn( $dbw, $newLegacyRevision, 0 ) ) { + if ( !$wikiPage->updateRevisionOn( $dbw, $newRevisionRecord, 0 ) ) { throw new PageUpdateException( "Failed to update page row to use new revision." ); } // TODO: replace legacy hook! + $newLegacyRevision = new Revision( $newRevisionRecord ); $tags = $this->computeEffectiveTags( $flags ); Hooks::run( 'NewRevisionFromEditComplete', diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index dba455cd844..1e8922c0569 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -1646,7 +1646,6 @@ class LocalFile extends File { if ( $nullRevRecord ) { $inserted = $revStore->insertRevisionOn( $nullRevRecord, $dbw ); - // TODO WikiPage::updateRevisionOn should accept a RevisionRecord // TODO replace hook $nullRevision = new Revision( $inserted ); Hooks::run( @@ -1659,7 +1658,7 @@ class LocalFile extends File { &$tags ] ); - $wikiPage->updateRevisionOn( $dbw, $nullRevision ); + $wikiPage->updateRevisionOn( $dbw, $inserted ); // Associate null revision id $logEntry->setAssociatedRevId( $inserted->getId() ); } diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 523d20477ef..44cf9866598 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -1379,7 +1379,7 @@ class WikiPage implements Page, IDBAccessObject { * * @param IDatabase $dbw * @param Revision|RevisionRecord $revision For ID number, and text used to set - * length and redirect status fields + * length and redirect status fields. Passing a Revision is deprecated since 1.35 * @param int|null $lastRevision If given, will not overwrite the page field * when different from the currently set value. * Giving 0 indicates the new page flag should be set on. @@ -1402,6 +1402,7 @@ class WikiPage implements Page, IDBAccessObject { } if ( $revision instanceof Revision ) { + wfDeprecated( __METHOD__ . ' with a Revision object', '1.35' ); $revision = $revision->getRevisionRecord(); } @@ -1525,7 +1526,12 @@ class WikiPage implements Page, IDBAccessObject { $lastRevIsRedirect = null; } - $ret = $this->updateRevisionOn( $dbw, $revision, $prev, $lastRevIsRedirect ); + $ret = $this->updateRevisionOn( + $dbw, + $revision->getRevisionRecord(), + $prev, + $lastRevIsRedirect + ); return $ret; } diff --git a/includes/specials/helpers/ImportReporter.php b/includes/specials/helpers/ImportReporter.php index 35522639f88..e547a2d2209 100644 --- a/includes/specials/helpers/ImportReporter.php +++ b/includes/specials/helpers/ImportReporter.php @@ -148,13 +148,13 @@ class ImportReporter extends ContextSource { $inserted = $revStore->insertRevisionOn( $nullRevRecord, $dbw ); $nullRevId = $inserted->getId(); $page = WikiPage::factory( $title ); + // Update page record - // TODO WikiPage::updateRevisionOn should accept RevisionRecord - $nullRevision = new Revision( $inserted ); - $page->updateRevisionOn( $dbw, $nullRevision ); - $fakeTags = []; + $page->updateRevisionOn( $dbw, $inserted ); // TODO replace hook + $fakeTags = []; + $nullRevision = new Revision( $inserted ); Hooks::run( 'NewRevisionFromEditComplete', [ $page, $nullRevision, $latest, $this->getUser(), &$fakeTags ] diff --git a/maintenance/attachLatest.php b/maintenance/attachLatest.php index db0c421ee11..cd038eaa4e3 100644 --- a/maintenance/attachLatest.php +++ b/maintenance/attachLatest.php @@ -81,14 +81,13 @@ class AttachLatest extends Maintenance { "$dbDomain $pageId [[$name]] latest time $latestTime, can't find revision id\n" ); continue; - } else { - $revision = new Revision( $revRecord ); } - $id = $revision->getId(); + + $id = $revRecord->getId(); $this->output( "$dbDomain $pageId [[$name]] latest time $latestTime, rev id $id\n" ); if ( $this->hasOption( 'fix' ) ) { $page = WikiPage::factory( $title ); - $page->updateRevisionOn( $dbw, $revision ); + $page->updateRevisionOn( $dbw, $revRecord ); $lbFactory->waitForReplication(); } $n++; diff --git a/maintenance/orphans.php b/maintenance/orphans.php index e13468ea8bf..ca419f5e6f8 100644 --- a/maintenance/orphans.php +++ b/maintenance/orphans.php @@ -157,6 +157,7 @@ class Orphans extends Maintenance { FROM $page LEFT JOIN $revision ON page_latest=rev_id " ); $found = 0; + $revLookup = MediaWikiServices::getInstance()->getRevisionLookup(); foreach ( $result as $row ) { $result2 = $dbw->query( " SELECT MAX(rev_timestamp) as max_timestamp @@ -185,7 +186,7 @@ class Orphans extends Maintenance { 'rev_page' => $row->page_id, 'rev_timestamp' => $row2->max_timestamp ] ); $this->output( "... updating to revision $maxId\n" ); - $maxRev = Revision::newFromId( $maxId ); + $maxRev = $revLookup->getRevisionById( $maxId ); $title = Title::makeTitle( $row->page_namespace, $row->page_title ); $article = WikiPage::factory( $title ); $article->updateRevisionOn( $dbw, $maxRev ); diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php index c76d7168984..c4b52265e14 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php @@ -616,7 +616,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { $dbw = wfGetDB( DB_MASTER ); $baseRev = $store->insertRevisionOn( $baseRev, $dbw ); - $page->updateRevisionOn( $dbw, new Revision( $baseRev ), $page->getLatest() ); + $page->updateRevisionOn( $dbw, $baseRev, $page->getLatest() ); $record = $store->newNullRevision( wfGetDB( DB_MASTER ), diff --git a/tests/phpunit/includes/page/WikiPageDbTest.php b/tests/phpunit/includes/page/WikiPageDbTest.php index 65e68291b10..2604db1b50a 100644 --- a/tests/phpunit/includes/page/WikiPageDbTest.php +++ b/tests/phpunit/includes/page/WikiPageDbTest.php @@ -1896,6 +1896,8 @@ more stuff */ public function testUpdateRevisionOn_existingPage() { $this->hideDeprecated( 'WikiPage::getRevision' ); + $this->hideDeprecated( 'WikiPage::updateRevisionOn with a Revision object' ); + $user = $this->getTestSysop()->getUser(); $page = $this->createPage( __METHOD__, 'StartText' ); @@ -1926,6 +1928,8 @@ more stuff * @covers WikiPage::updateRevisionOn */ public function testUpdateRevisionOn_NonExistingPage() { + $this->hideDeprecated( 'WikiPage::updateRevisionOn with a Revision object' ); + $user = $this->getTestSysop()->getUser(); $page = $this->createPage( __METHOD__, 'StartText' ); $page->doDeleteArticleReal( 'reason', $user );