From a5c7fd0db2d962834127ec2362d0dfe8ef6852d5 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 4 Jul 2019 14:24:34 -0700 Subject: [PATCH] Move callers away from Title::GAID_FOR_UPDATE These callers just need to load some data from DB_MASTER. Subsequent code needing that latest title data should also use the required flags, rather than relying on flakey global cache state. Change-Id: I53248ea4b5bf1cd953f956c41b8244831ec5ef04 --- includes/MovePage.php | 3 ++- includes/Revision.php | 2 +- includes/Revision/RevisionStore.php | 2 +- includes/api/ApiSetNotificationTimestamp.php | 2 +- includes/cache/LinkCache.php | 1 + includes/deferred/LinksUpdate.php | 2 +- includes/jobqueue/jobs/RefreshLinksJob.php | 4 ++-- includes/page/WikiPage.php | 2 +- includes/user/User.php | 2 +- maintenance/cleanupTitles.php | 2 +- tests/phpunit/includes/TitleTest.php | 6 +++--- tests/phpunit/includes/api/ApiDeleteTest.php | 2 +- tests/phpunit/includes/api/ApiRevisionDeleteTest.php | 3 +-- 13 files changed, 17 insertions(+), 16 deletions(-) diff --git a/includes/MovePage.php b/includes/MovePage.php index 564c8f4d6c..634e7affe2 100644 --- a/includes/MovePage.php +++ b/includes/MovePage.php @@ -473,6 +473,7 @@ class MovePage { $mp = new MovePage( $oldSubpage, $newSubpage ); $method = $checkPermissions ? 'moveIfAllowed' : 'move'; + /** @var Status $status */ $status = $mp->$method( $user, $reason, $createRedirect, $changeTags ); if ( $status->isOK() ) { $status->setResult( true, $newSubpage->getPrefixedText() ); @@ -508,7 +509,7 @@ class MovePage { Hooks::run( 'TitleMoveStarting', [ $this->oldTitle, $this->newTitle, $user ] ); - $pageid = $this->oldTitle->getArticleID( Title::GAID_FOR_UPDATE ); + $pageid = $this->oldTitle->getArticleID( Title::READ_LATEST ); $protected = $this->oldTitle->isProtected(); // Do the actual move; if this fails, it will throw an MWException(!) diff --git a/includes/Revision.php b/includes/Revision.php index 292d6ba574..828f647552 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -1005,7 +1005,7 @@ class Revision implements IDBAccessObject { $comment = CommentStoreComment::newUnsavedComment( $summary, null ); - $title = Title::newFromID( $pageId, Title::GAID_FOR_UPDATE ); + $title = Title::newFromID( $pageId, Title::READ_LATEST ); if ( $title === null ) { return null; } diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php index 73f622a3db..735a2124d4 100644 --- a/includes/Revision/RevisionStore.php +++ b/includes/Revision/RevisionStore.php @@ -326,10 +326,10 @@ class RevisionStore $canUseTitleNewFromId = ( $pageId !== null && $pageId > 0 && $this->dbDomain === false ); list( $dbMode, $dbOptions ) = DBAccessObjectUtils::getDBOptions( $queryFlags ); - $titleFlags = ( $dbMode == DB_MASTER ? Title::GAID_FOR_UPDATE : 0 ); // Loading by ID is best, but Title::newFromID does not support that for foreign IDs. if ( $canUseTitleNewFromId ) { + $titleFlags = ( $dbMode == DB_MASTER ? Title::READ_LATEST : 0 ); // TODO: better foreign title handling (introduce TitleFactory) $title = Title::newFromID( $pageId, $titleFlags ); if ( $title ) { diff --git a/includes/api/ApiSetNotificationTimestamp.php b/includes/api/ApiSetNotificationTimestamp.php index d2bbe7bb88..7e9f56d09c 100644 --- a/includes/api/ApiSetNotificationTimestamp.php +++ b/includes/api/ApiSetNotificationTimestamp.php @@ -93,7 +93,7 @@ class ApiSetNotificationTimestamp extends ApiBase { $titles = $pageSet->getGoodTitles(); $title = reset( $titles ); if ( $title ) { - $revid = $title->getNextRevisionID( $params['newerthanrevid'], Title::GAID_FOR_UPDATE ); + $revid = $title->getNextRevisionID( $params['newerthanrevid'], Title::READ_LATEST ); if ( $revid ) { $timestamp = $dbw->timestamp( MediaWikiServices::getInstance()->getRevisionStore()->getTimestampFromId( $title, $revid ) diff --git a/includes/cache/LinkCache.php b/includes/cache/LinkCache.php index 80181179e4..b2f24525a5 100644 --- a/includes/cache/LinkCache.php +++ b/includes/cache/LinkCache.php @@ -89,6 +89,7 @@ class LinkCache { * * @param bool|null $update * @return bool + * @deprecated Since 1.34 */ public function forUpdate( $update = null ) { return wfSetVar( $this->mForUpdate, $update ); diff --git a/includes/deferred/LinksUpdate.php b/includes/deferred/LinksUpdate.php index 74e236fd4d..8345ee6575 100644 --- a/includes/deferred/LinksUpdate.php +++ b/includes/deferred/LinksUpdate.php @@ -125,7 +125,7 @@ class LinksUpdate extends DataUpdate { if ( !$this->mId ) { // NOTE: subclasses may initialize mId before calling this constructor! - $this->mId = $title->getArticleID( Title::GAID_FOR_UPDATE ); + $this->mId = $title->getArticleID( Title::READ_LATEST ); } if ( !$this->mId ) { diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php b/includes/jobqueue/jobs/RefreshLinksJob.php index b4046a61bc..33b05b8571 100644 --- a/includes/jobqueue/jobs/RefreshLinksJob.php +++ b/includes/jobqueue/jobs/RefreshLinksJob.php @@ -276,8 +276,8 @@ class RefreshLinksJob extends Job { $title = $page->getTitle(); // Get the latest ID since acquirePageLock() in runForTitle() flushed the transaction. // This is used to detect edits/moves after loadPageData() but before the scope lock. - // The works around the chicken/egg problem of determining the scope lock key name. - $latest = $title->getLatestRevID( Title::GAID_FOR_UPDATE ); + // The works around the chicken/egg problem of determining the scope lock key name + $latest = $title->getLatestRevID( Title::READ_LATEST ); $triggeringRevisionId = $this->params['triggeringRevisionId'] ?? null; if ( $triggeringRevisionId && $triggeringRevisionId !== $latest ) { diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index c167f3a4cf..bad75da4fb 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -726,7 +726,7 @@ class WikiPage implements Page, IDBAccessObject { // Try using the replica DB first, then try the master $rev = $this->mTitle->getFirstRevision(); if ( !$rev ) { - $rev = $this->mTitle->getFirstRevision( Title::GAID_FOR_UPDATE ); + $rev = $this->mTitle->getFirstRevision( Title::READ_LATEST ); } return $rev; } diff --git a/includes/user/User.php b/includes/user/User.php index 666f2b6979..a1637c98cd 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -3717,7 +3717,7 @@ class User implements IDBAccessObject, UserIdentity { // If there is a new, unseen, revision, use its timestamp $nextid = $oldid - ? $title->getNextRevisionID( $oldid, Title::GAID_FOR_UPDATE ) + ? $title->getNextRevisionID( $oldid, Title::READ_LATEST ) : null; if ( $nextid ) { $this->setNewtalk( true, Revision::newFromId( $nextid ) ); diff --git a/maintenance/cleanupTitles.php b/maintenance/cleanupTitles.php index cad612266b..5602e39646 100644 --- a/maintenance/cleanupTitles.php +++ b/maintenance/cleanupTitles.php @@ -133,7 +133,7 @@ class TitleCleanup extends TableCleanup { * @param Title $title */ protected function moveInconsistentPage( $row, Title $title ) { - if ( $title->exists( Title::GAID_FOR_UPDATE ) + if ( $title->exists( Title::READ_LATEST ) || $title->getInterwiki() || !$title->canExist() ) { diff --git a/tests/phpunit/includes/TitleTest.php b/tests/phpunit/includes/TitleTest.php index 18faeea3fc..82ccb9ae32 100644 --- a/tests/phpunit/includes/TitleTest.php +++ b/tests/phpunit/includes/TitleTest.php @@ -768,12 +768,12 @@ class TitleTest extends MediaWikiTestCase { $this->assertEquals( false, $title->exists(), - 'exists() should rely on link cache unless GAID_FOR_UPDATE is used' + 'exists() should rely on link cache unless READ_LATEST is used' ); $this->assertEquals( true, - $title->exists( Title::GAID_FOR_UPDATE ), - 'exists() should re-query database when GAID_FOR_UPDATE is used' + $title->exists( Title::READ_LATEST ), + 'exists() should re-query database when READ_LATEST is used' ); } diff --git a/tests/phpunit/includes/api/ApiDeleteTest.php b/tests/phpunit/includes/api/ApiDeleteTest.php index c68954c077..d2bbc1f837 100644 --- a/tests/phpunit/includes/api/ApiDeleteTest.php +++ b/tests/phpunit/includes/api/ApiDeleteTest.php @@ -67,7 +67,7 @@ class ApiDeleteTest extends ApiTestCase { $jobs->loadParamsAndArgs( null, [ 'quiet' => true ], null ); $jobs->execute(); - $this->assertFalse( Title::newFromText( $name )->exists( Title::GAID_FOR_UPDATE ) ); + $this->assertFalse( Title::newFromText( $name )->exists( Title::READ_LATEST ) ); } public function testDeleteNonexistent() { diff --git a/tests/phpunit/includes/api/ApiRevisionDeleteTest.php b/tests/phpunit/includes/api/ApiRevisionDeleteTest.php index a4ca8a10a1..5dcea65fa7 100644 --- a/tests/phpunit/includes/api/ApiRevisionDeleteTest.php +++ b/tests/phpunit/includes/api/ApiRevisionDeleteTest.php @@ -22,8 +22,7 @@ class ApiRevisionDeleteTest extends ApiTestCase { // Make a few edits for us to play with for ( $i = 1; $i <= 5; $i++ ) { self::editPage( self::$page, MWCryptRand::generateHex( 10 ), 'summary' ); - $this->revs[] = Title::newFromText( self::$page ) - ->getLatestRevID( Title::GAID_FOR_UPDATE ); + $this->revs[] = Title::newFromText( self::$page )->getLatestRevID( Title::READ_LATEST ); } } -- 2.20.1