From ca00e47f535129e067a078c96a90c6dfbae716b0 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 26 Jun 2019 18:33:18 -0700 Subject: [PATCH] revision: rename various $wikiId fields/parameters to $dbDomain These fields are passed to methods like LoadBalancer::getConnection() and are already expected to be DB domains. Update various comments as well. Fix a few minor IDEA warnings. Change-Id: I7cf76700690aec449872a80d30b5ba540d2bf315 --- includes/Revision/MutableRevisionRecord.php | 7 ++- includes/Revision/RevisionArchiveRecord.php | 7 ++- includes/Revision/RevisionRecord.php | 9 ++-- includes/Revision/RevisionRenderer.php | 14 +++--- includes/Revision/RevisionStore.php | 48 ++++++++++--------- .../Revision/RevisionStoreCacheRecord.php | 7 ++- includes/Revision/RevisionStoreFactory.php | 16 +++---- includes/Revision/RevisionStoreRecord.php | 7 ++- .../Revision/RevisionStoreFactoryTest.php | 6 +-- .../includes/parser/ParserOutputTest.php | 6 +-- 10 files changed, 62 insertions(+), 65 deletions(-) diff --git a/includes/Revision/MutableRevisionRecord.php b/includes/Revision/MutableRevisionRecord.php index f287c05db7..e9136cbb5d 100644 --- a/includes/Revision/MutableRevisionRecord.php +++ b/includes/Revision/MutableRevisionRecord.php @@ -70,15 +70,14 @@ class MutableRevisionRecord extends RevisionRecord { * in RevisionStore instead. * * @param Title $title The title of the page this Revision is associated with. - * @param bool|string $wikiId the wiki ID of the site this Revision belongs to, - * or false for the local site. + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one. * * @throws MWException */ - function __construct( Title $title, $wikiId = false ) { + function __construct( Title $title, $dbDomain = false ) { $slots = new MutableRevisionSlots(); - parent::__construct( $title, $slots, $wikiId ); + parent::__construct( $title, $slots, $dbDomain ); $this->mSlots = $slots; // redundant, but nice for static analysis } diff --git a/includes/Revision/RevisionArchiveRecord.php b/includes/Revision/RevisionArchiveRecord.php index 67dc9b26d9..6e8db7fa87 100644 --- a/includes/Revision/RevisionArchiveRecord.php +++ b/includes/Revision/RevisionArchiveRecord.php @@ -54,8 +54,7 @@ class RevisionArchiveRecord extends RevisionRecord { * @param object $row An archive table row. Use RevisionStore::getArchiveQueryInfo() to build * a query that yields the required fields. * @param RevisionSlots $slots The slots of this revision. - * @param bool|string $wikiId the wiki ID of the site this Revision belongs to, - * or false for the local site. + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one. */ function __construct( Title $title, @@ -63,9 +62,9 @@ class RevisionArchiveRecord extends RevisionRecord { CommentStoreComment $comment, $row, RevisionSlots $slots, - $wikiId = false + $dbDomain = false ) { - parent::__construct( $title, $slots, $wikiId ); + parent::__construct( $title, $slots, $dbDomain ); Assert::parameterType( 'object', $row, '$row' ); $timestamp = wfTimestamp( TS_MW, $row->ar_timestamp ); diff --git a/includes/Revision/RevisionRecord.php b/includes/Revision/RevisionRecord.php index 70a891cfed..a0b85d84eb 100644 --- a/includes/Revision/RevisionRecord.php +++ b/includes/Revision/RevisionRecord.php @@ -94,17 +94,16 @@ abstract class RevisionRecord { * * @param Title $title The title of the page this Revision is associated with. * @param RevisionSlots $slots The slots of this revision. - * @param bool|string $wikiId the wiki ID of the site this Revision belongs to, - * or false for the local site. + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one. * * @throws MWException */ - function __construct( Title $title, RevisionSlots $slots, $wikiId = false ) { - Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' ); + function __construct( Title $title, RevisionSlots $slots, $dbDomain = false ) { + Assert::parameterType( 'string|boolean', $dbDomain, '$dbDomain' ); $this->mTitle = $title; $this->mSlots = $slots; - $this->mWiki = $wikiId; + $this->mWiki = $dbDomain; // XXX: this is a sensible default, but we may not have a Title object here in the future. $this->mPageId = $title->getArticleID(); diff --git a/includes/Revision/RevisionRenderer.php b/includes/Revision/RevisionRenderer.php index a63e4f1e9a..80aee783a3 100644 --- a/includes/Revision/RevisionRenderer.php +++ b/includes/Revision/RevisionRenderer.php @@ -54,21 +54,21 @@ class RevisionRenderer { private $roleRegistery; /** @var string|bool */ - private $wikiId; + private $dbDomain; /** * @param ILoadBalancer $loadBalancer * @param SlotRoleRegistry $roleRegistry - * @param bool|string $wikiId + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one */ public function __construct( ILoadBalancer $loadBalancer, SlotRoleRegistry $roleRegistry, - $wikiId = false + $dbDomain = false ) { $this->loadBalancer = $loadBalancer; $this->roleRegistery = $roleRegistry; - $this->wikiId = $wikiId; + $this->dbDomain = $dbDomain; $this->saveParseLogger = new NullLogger(); } @@ -105,7 +105,7 @@ class RevisionRenderer { User $forUser = null, array $hints = [] ) { - if ( $rev->getWikiId() !== $this->wikiId ) { + if ( $rev->getWikiId() !== $this->dbDomain ) { throw new InvalidArgumentException( 'Mismatching wiki ID ' . $rev->getWikiId() ); } @@ -169,7 +169,7 @@ class RevisionRenderer { $flags = defined( 'MW_PHPUNIT_TEST' ) || $dbIndex === DB_REPLICA ? 0 : ILoadBalancer::CONN_TRX_AUTOCOMMIT; - $db = $this->loadBalancer->getConnectionRef( $dbIndex, [], $this->wikiId, $flags ); + $db = $this->loadBalancer->getConnectionRef( $dbIndex, [], $this->dbDomain, $flags ); return 1 + (int)$db->selectField( 'revision', @@ -216,7 +216,7 @@ class RevisionRenderer { $slotOutput[$role] = $out; // XXX: should the SlotRoleHandler be able to intervene here? - $combinedOutput->mergeInternalMetaDataFrom( $out, $role ); + $combinedOutput->mergeInternalMetaDataFrom( $out ); $combinedOutput->mergeTrackingMetaDataFrom( $out ); } diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php index 56867eb6ff..0bfb39326c 100644 --- a/includes/Revision/RevisionStore.php +++ b/includes/Revision/RevisionStore.php @@ -87,7 +87,7 @@ class RevisionStore /** * @var bool|string */ - private $wikiId; + private $dbDomain; /** * @var boolean @@ -142,7 +142,7 @@ class RevisionStore * @param ILoadBalancer $loadBalancer * @param SqlBlobStore $blobStore * @param WANObjectCache $cache A cache for caching revision rows. This can be the local - * wiki's default instance even if $wikiId refers to a different wiki, since + * wiki's default instance even if $dbDomain refers to a different wiki, since * makeGlobalKey() is used to constructed a key that allows cached revision rows from * the same database to be re-used between wikis. For example, enwiki and frwiki will * use the same cache keys for revision rows from the wikidatawiki database, regardless @@ -153,7 +153,7 @@ class RevisionStore * @param SlotRoleRegistry $slotRoleRegistry * @param int $mcrMigrationStage An appropriate combination of SCHEMA_COMPAT_XXX flags * @param ActorMigration $actorMigration - * @param bool|string $wikiId + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one * */ public function __construct( @@ -166,9 +166,9 @@ class RevisionStore SlotRoleRegistry $slotRoleRegistry, $mcrMigrationStage, ActorMigration $actorMigration, - $wikiId = false + $dbDomain = false ) { - Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' ); + Assert::parameterType( 'string|boolean', $dbDomain, '$dbDomain' ); Assert::parameterType( 'integer', $mcrMigrationStage, '$mcrMigrationStage' ); Assert::parameter( ( $mcrMigrationStage & SCHEMA_COMPAT_READ_BOTH ) !== SCHEMA_COMPAT_READ_BOTH, @@ -207,7 +207,7 @@ class RevisionStore $this->slotRoleRegistry = $slotRoleRegistry; $this->mcrMigrationStage = $mcrMigrationStage; $this->actorMigration = $actorMigration; - $this->wikiId = $wikiId; + $this->dbDomain = $dbDomain; $this->logger = new NullLogger(); } @@ -227,7 +227,7 @@ class RevisionStore * @throws RevisionAccessException */ private function assertCrossWikiContentLoadingIsSafe() { - if ( $this->wikiId !== false && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) { + if ( $this->dbDomain !== false && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) { throw new RevisionAccessException( "Cross-wiki content loading is not supported by the pre-MCR schema" ); @@ -285,7 +285,7 @@ class RevisionStore */ private function getDBConnection( $mode, $groups = [] ) { $lb = $this->getDBLoadBalancer(); - return $lb->getConnection( $mode, $groups, $this->wikiId ); + return $lb->getConnection( $mode, $groups, $this->dbDomain ); } /** @@ -313,7 +313,7 @@ class RevisionStore */ private function getDBConnectionRef( $mode ) { $lb = $this->getDBLoadBalancer(); - return $lb->getConnectionRef( $mode, [], $this->wikiId ); + return $lb->getConnectionRef( $mode, [], $this->dbDomain ); } /** @@ -341,7 +341,7 @@ class RevisionStore $queryFlags = self::READ_NORMAL; } - $canUseTitleNewFromId = ( $pageId !== null && $pageId > 0 && $this->wikiId === false ); + $canUseTitleNewFromId = ( $pageId !== null && $pageId > 0 && $this->dbDomain === false ); list( $dbMode, $dbOptions ) = DBAccessObjectUtils::getDBOptions( $queryFlags ); $titleFlags = ( $dbMode == DB_MASTER ? Title::GAID_FOR_UPDATE : 0 ); @@ -631,7 +631,7 @@ class RevisionStore $comment, (object)$revisionRow, new RevisionSlots( $newSlots ), - $this->wikiId + $this->dbDomain ); return $rev; @@ -813,9 +813,11 @@ class RevisionStore throw new MWException( 'Failed to get database lock for T202032' ); } $fname = __METHOD__; - $dbw->onTransactionResolution( function ( $trigger, $dbw ) use ( $fname ) { - $dbw->unlock( 'fix-for-T202032', $fname ); - } ); + $dbw->onTransactionResolution( + function ( $trigger, IDatabase $dbw ) use ( $fname ) { + $dbw->unlock( 'fix-for-T202032', $fname ); + } + ); $dbw->delete( 'revision', [ 'rev_id' => $revisionRow['rev_id'] ], __METHOD__ ); @@ -1782,7 +1784,7 @@ class RevisionStore $row->ar_user ?? null, $row->ar_user_text ?? null, $row->ar_actor ?? null, - $this->wikiId + $this->dbDomain ); } catch ( InvalidArgumentException $ex ) { wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() ); @@ -1795,7 +1797,7 @@ class RevisionStore $slots = $this->newRevisionSlots( $row->ar_rev_id, $row, null, $queryFlags, $title ); - return new RevisionArchiveRecord( $title, $user, $comment, $row, $slots, $this->wikiId ); + return new RevisionArchiveRecord( $title, $user, $comment, $row, $slots, $this->dbDomain ); } /** @@ -1863,7 +1865,7 @@ class RevisionStore $row->rev_user ?? null, $row->rev_user_text ?? null, $row->rev_actor ?? null, - $this->wikiId + $this->dbDomain ); } catch ( InvalidArgumentException $ex ) { wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() ); @@ -1886,11 +1888,11 @@ class RevisionStore [ 'rev_id' => intval( $revId ) ] ); }, - $title, $user, $comment, $row, $slots, $this->wikiId + $title, $user, $comment, $row, $slots, $this->dbDomain ); } else { $rev = new RevisionStoreRecord( - $title, $user, $comment, $row, $slots, $this->wikiId ); + $title, $user, $comment, $row, $slots, $this->dbDomain ); } return $rev; } @@ -1975,7 +1977,7 @@ class RevisionStore } } - $revision = new MutableRevisionRecord( $title, $this->wikiId ); + $revision = new MutableRevisionRecord( $title, $this->dbDomain ); $this->initializeMutableRevisionFromArray( $revision, $fields ); if ( isset( $fields['content'] ) && is_array( $fields['content'] ) ) { @@ -2006,7 +2008,7 @@ class RevisionStore // remote wiki with unsuppressed ids, due to issues described in T222212. if ( isset( $fields['user'] ) && ( $fields['user'] instanceof UserIdentity ) && - ( $this->wikiId === false || + ( $this->dbDomain === false || ( !$fields['user']->getId() && !$fields['user']->getActorId() ) ) ) { $user = $fields['user']; @@ -2016,7 +2018,7 @@ class RevisionStore $fields['user'] ?? null, $fields['user_text'] ?? null, $fields['actor'] ?? null, - $this->wikiId + $this->dbDomain ); } catch ( InvalidArgumentException $ex ) { $user = null; @@ -2247,7 +2249,7 @@ class RevisionStore * @throws MWException */ private function checkDatabaseWikiId( IDatabase $db ) { - $storeWiki = $this->wikiId; + $storeWiki = $this->dbDomain; $dbWiki = $db->getDomainID(); if ( $dbWiki === $storeWiki ) { diff --git a/includes/Revision/RevisionStoreCacheRecord.php b/includes/Revision/RevisionStoreCacheRecord.php index ef5f10e312..0420d34d89 100644 --- a/includes/Revision/RevisionStoreCacheRecord.php +++ b/includes/Revision/RevisionStoreCacheRecord.php @@ -53,8 +53,7 @@ class RevisionStoreCacheRecord extends RevisionStoreRecord { * @param object $row A row from the revision table. Use RevisionStore::getQueryInfo() to build * a query that yields the required fields. * @param RevisionSlots $slots The slots of this revision. - * @param bool|string $wikiId the wiki ID of the site this Revision belongs to, - * or false for the local site. + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one. */ function __construct( $callback, @@ -63,9 +62,9 @@ class RevisionStoreCacheRecord extends RevisionStoreRecord { CommentStoreComment $comment, $row, RevisionSlots $slots, - $wikiId = false + $dbDomain = false ) { - parent::__construct( $title, $user, $comment, $row, $slots, $wikiId ); + parent::__construct( $title, $user, $comment, $row, $slots, $dbDomain ); $this->mCallback = $callback; } diff --git a/includes/Revision/RevisionStoreFactory.php b/includes/Revision/RevisionStoreFactory.php index 6b3117fc78..0475557387 100644 --- a/includes/Revision/RevisionStoreFactory.php +++ b/includes/Revision/RevisionStoreFactory.php @@ -116,24 +116,24 @@ class RevisionStoreFactory { /** * @since 1.32 * - * @param bool|string $wikiId false for the current domain / wikid + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one * * @return RevisionStore for the given wikiId with all necessary services and a logger */ - public function getRevisionStore( $wikiId = false ) { - Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' ); + public function getRevisionStore( $dbDomain = false ) { + Assert::parameterType( 'string|boolean', $dbDomain, '$dbDomain' ); $store = new RevisionStore( - $this->dbLoadBalancerFactory->getMainLB( $wikiId ), - $this->blobStoreFactory->newSqlBlobStore( $wikiId ), + $this->dbLoadBalancerFactory->getMainLB( $dbDomain ), + $this->blobStoreFactory->newSqlBlobStore( $dbDomain ), $this->cache, // Pass local cache instance; Leave cache sharing to RevisionStore. $this->commentStore, - $this->nameTables->getContentModels( $wikiId ), - $this->nameTables->getSlotRoles( $wikiId ), + $this->nameTables->getContentModels( $dbDomain ), + $this->nameTables->getSlotRoles( $dbDomain ), $this->slotRoleRegistry, $this->mcrMigrationStage, $this->actorMigration, - $wikiId + $dbDomain ); $store->setLogger( $this->loggerProvider->getLogger( 'RevisionStore' ) ); diff --git a/includes/Revision/RevisionStoreRecord.php b/includes/Revision/RevisionStoreRecord.php index 955cc82de6..469e494a3d 100644 --- a/includes/Revision/RevisionStoreRecord.php +++ b/includes/Revision/RevisionStoreRecord.php @@ -51,8 +51,7 @@ class RevisionStoreRecord extends RevisionRecord { * @param object $row A row from the revision table. Use RevisionStore::getQueryInfo() to build * a query that yields the required fields. * @param RevisionSlots $slots The slots of this revision. - * @param bool|string $wikiId the wiki ID of the site this Revision belongs to, - * or false for the local site. + * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one. */ function __construct( Title $title, @@ -60,9 +59,9 @@ class RevisionStoreRecord extends RevisionRecord { CommentStoreComment $comment, $row, RevisionSlots $slots, - $wikiId = false + $dbDomain = false ) { - parent::__construct( $title, $slots, $wikiId ); + parent::__construct( $title, $slots, $dbDomain ); Assert::parameterType( 'object', $row, '$row' ); $this->mId = intval( $row->rev_id ); diff --git a/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php b/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php index 4f06ee24e3..84c815d487 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php +++ b/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php @@ -55,7 +55,7 @@ class RevisionStoreFactoryTest extends MediaWikiTestCase { * @covers \MediaWiki\Revision\RevisionStoreFactory::getRevisionStore */ public function testGetRevisionStore( - $wikiId, + $dbDomain, $mcrMigrationStage = MIGRATION_OLD, $contentHandlerUseDb = true ) { @@ -81,14 +81,14 @@ class RevisionStoreFactoryTest extends MediaWikiTestCase { $contentHandlerUseDb ); - $store = $factory->getRevisionStore( $wikiId ); + $store = $factory->getRevisionStore( $dbDomain ); $wrapper = TestingAccessWrapper::newFromObject( $store ); // ensure the correct object type is returned $this->assertInstanceOf( RevisionStore::class, $store ); // ensure the RevisionStore is for the given wikiId - $this->assertSame( $wikiId, $wrapper->wikiId ); + $this->assertSame( $dbDomain, $wrapper->dbDomain ); // ensure all other required services are correctly set $this->assertSame( $cache, $wrapper->cache ); diff --git a/tests/phpunit/includes/parser/ParserOutputTest.php b/tests/phpunit/includes/parser/ParserOutputTest.php index 812702b6ae..34ddb1f1fd 100644 --- a/tests/phpunit/includes/parser/ParserOutputTest.php +++ b/tests/phpunit/includes/parser/ParserOutputTest.php @@ -877,7 +877,7 @@ EOF $bClocks = $b->mParseStartTime; - $a->mergeInternalMetaDataFrom( $b->object, 'b' ); + $a->mergeInternalMetaDataFrom( $b->object ); $mergedClocks = $a->mParseStartTime; foreach ( $mergedClocks as $clock => $timestamp ) { @@ -890,7 +890,7 @@ EOF $a->resetParseStartTime(); $aClocks = $a->mParseStartTime; - $a->mergeInternalMetaDataFrom( $b->object, 'b' ); + $a->mergeInternalMetaDataFrom( $b->object ); $mergedClocks = $a->mParseStartTime; foreach ( $mergedClocks as $clock => $timestamp ) { @@ -902,7 +902,7 @@ EOF $a = new ParserOutput(); $a = TestingAccessWrapper::newFromObject( $a ); - $a->mergeInternalMetaDataFrom( $b->object, 'b' ); + $a->mergeInternalMetaDataFrom( $b->object ); $mergedClocks = $a->mParseStartTime; foreach ( $mergedClocks as $clock => $timestamp ) { -- 2.20.1