diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index e80b9520e5d..b01b23f4ddd 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -146,17 +146,10 @@ class LoadBalancer implements ILoadBalancer { } } - $this->localDomain = isset( $params['localDomain'] ) + $localDomain = isset( $params['localDomain'] ) ? DatabaseDomain::newFromId( $params['localDomain'] ) : DatabaseDomain::newUnspecified(); - // In case a caller assumes that the domain ID is simply -, which is almost - // always true, gracefully handle the case when they fail to account for escaping. - if ( $this->localDomain->getTablePrefix() != '' ) { - $this->localDomainIdAlias = - $this->localDomain->getDatabase() . '-' . $this->localDomain->getTablePrefix(); - } else { - $this->localDomainIdAlias = $this->localDomain->getDatabase(); - } + $this->setLocalDomain( $localDomain ); $this->mWaitTimeout = isset( $params['waitTimeout'] ) ? $params['waitTimeout'] : 10; @@ -821,7 +814,11 @@ class LoadBalancer implements ILoadBalancer { $server = $this->mServers[$i]; $server['serverIndex'] = $i; $server['autoCommitOnly'] = $autoCommit; - $conn = $this->reallyOpenConnection( $server, false ); + if ( $this->localDomain->getDatabase() !== null ) { + // Use the local domain table prefix if the local domain is specified + $server['tablePrefix'] = $this->localDomain->getTablePrefix(); + } + $conn = $this->reallyOpenConnection( $server, $this->localDomain->getDatabase() ); $host = $this->getServerName( $i ); if ( $conn->isOpen() ) { $this->connLogger->debug( "Connected to database $i at '$host'." ); @@ -899,8 +896,6 @@ class LoadBalancer implements ILoadBalancer { // Reuse a free connection from another domain $conn = reset( $this->mConns[$connFreeKey][$i] ); $oldDomain = key( $this->mConns[$connFreeKey][$i] ); - // The empty string as a DB name means "don't care". - // DatabaseMysqlBase::open() already handle this on connection. if ( strlen( $dbName ) && !$conn->selectDB( $dbName ) ) { $this->mLastError = "Error selecting database '$dbName' on server " . $conn->getServer() . " from client host {$this->host}"; @@ -909,7 +904,8 @@ class LoadBalancer implements ILoadBalancer { } else { $conn->tablePrefix( $prefix ); unset( $this->mConns[$connFreeKey][$i][$oldDomain] ); - $this->mConns[$connInUseKey][$i][$domain] = $conn; + // Note that if $domain is an empty string, getDomainID() might not match it + $this->mConns[$connInUseKey][$i][$conn->getDomainId()] = $conn; $this->connLogger->debug( __METHOD__ . ": reusing free connection from $oldDomain for $domain" ); } @@ -929,8 +925,9 @@ class LoadBalancer implements ILoadBalancer { $this->errorConnection = $conn; $conn = false; } else { - $conn->tablePrefix( $prefix ); - $this->mConns[$connInUseKey][$i][$domain] = $conn; + $conn->tablePrefix( $prefix ); // as specified + // Note that if $domain is an empty string, getDomainID() might not match it + $this->mConns[$connInUseKey][$i][$conn->getDomainID()] = $conn; $this->connLogger->debug( __METHOD__ . ": opened new connection for $i/$domain" ); } } @@ -960,22 +957,22 @@ class LoadBalancer implements ILoadBalancer { } /** - * Really opens a connection. Uncached. + * Open a new network connection to a server (uncached) + * * Returns a Database object whether or not the connection was successful. - * @access private * * @param array $server - * @param string|bool $dbNameOverride Use "" to not select any database + * @param string|null $dbNameOverride Use "" to not select any database * @return Database * @throws DBAccessError * @throws InvalidArgumentException */ - protected function reallyOpenConnection( array $server, $dbNameOverride = false ) { + protected function reallyOpenConnection( array $server, $dbNameOverride ) { if ( $this->disabled ) { throw new DBAccessError(); } - if ( $dbNameOverride !== false ) { + if ( $dbNameOverride !== null ) { $server['dbname'] = $dbNameOverride; } @@ -1691,17 +1688,35 @@ class LoadBalancer implements ILoadBalancer { "Foreign domain connections are still in use ($domains)." ); } - $this->localDomain = new DatabaseDomain( + $oldDomain = $this->localDomain->getId(); + $this->setLocalDomain( new DatabaseDomain( $this->localDomain->getDatabase(), null, $prefix - ); + ) ); - $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix ) { - $db->tablePrefix( $prefix ); + $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix, $oldDomain ) { + if ( !$db->getLBInfo( 'foreign' ) ) { + $db->tablePrefix( $prefix ); + } } ); } + /** + * @param DatabaseDomain $domain + */ + private function setLocalDomain( DatabaseDomain $domain ) { + $this->localDomain = $domain; + // In case a caller assumes that the domain ID is simply -, which is almost + // always true, gracefully handle the case when they fail to account for escaping. + if ( $this->localDomain->getTablePrefix() != '' ) { + $this->localDomainIdAlias = + $this->localDomain->getDatabase() . '-' . $this->localDomain->getTablePrefix(); + } else { + $this->localDomainIdAlias = $this->localDomain->getDatabase(); + } + } + /** * Make PHP ignore user aborts/disconnects until the returned * value leaves scope. This returns null and does nothing in CLI mode. diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php b/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php index 79d250f6a06..c73756330ae 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php @@ -72,7 +72,7 @@ class LoadBalancerSingle extends LoadBalancer { return new static( [ 'connection' => $db ] + $params ); } - protected function reallyOpenConnection( array $server, $dbNameOverride = false ) { + protected function reallyOpenConnection( array $server, $dbNameOverride ) { return $this->db; } } diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php index e33de20b2ee..81856702060 100644 --- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php +++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php @@ -44,7 +44,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase { ->getMock(); $lb->method( 'reallyOpenConnection' )->willReturnCallback( - function ( array $server, $dbNameOverride = false ) { + function ( array $server, $dbNameOverride ) { return $this->getDatabaseMock( $server ); } ); diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php index e46a0ddbf99..e52dac69087 100644 --- a/tests/phpunit/includes/db/LBFactoryTest.php +++ b/tests/phpunit/includes/db/LBFactoryTest.php @@ -27,6 +27,7 @@ use Wikimedia\Rdbms\LBFactorySimple; use Wikimedia\Rdbms\LBFactoryMulti; use Wikimedia\Rdbms\ChronologyProtector; use Wikimedia\Rdbms\MySQLMasterPos; +use Wikimedia\Rdbms\DatabaseDomain; /** * @group Database @@ -314,7 +315,8 @@ class LBFactoryTest extends MediaWikiTestCase { } private function newLBFactoryMulti( array $baseOverride = [], array $serverOverride = [] ) { - global $wgDBserver, $wgDBuser, $wgDBpassword, $wgDBname, $wgDBtype, $wgSQLiteDataDir; + global $wgDBserver, $wgDBuser, $wgDBpassword, $wgDBname, $wgDBprefix, $wgDBtype; + global $wgSQLiteDataDir; return new LBFactoryMulti( $baseOverride + [ 'sectionsByDB' => [], @@ -325,6 +327,7 @@ class LBFactoryTest extends MediaWikiTestCase { ], 'serverTemplate' => $serverOverride + [ 'dbname' => $wgDBname, + 'tablePrefix' => $wgDBprefix, 'user' => $wgDBuser, 'password' => $wgDBpassword, 'type' => $wgDBtype, @@ -335,7 +338,7 @@ class LBFactoryTest extends MediaWikiTestCase { 'test-db1' => $wgDBserver, ], 'loadMonitorClass' => 'LoadMonitorNull', - 'localDomain' => wfWikiID() + 'localDomain' => new DatabaseDomain( $wgDBname, null, $wgDBprefix ) ] ); } @@ -361,7 +364,7 @@ class LBFactoryTest extends MediaWikiTestCase { if ( $wgDBtype !== 'sqlite' ) { $db = $lb->getConnectionRef( DB_MASTER ); $this->assertEquals( - $wgDBname, + wfWikiID(), $db->getDomainID() ); unset( $db ); @@ -369,34 +372,45 @@ class LBFactoryTest extends MediaWikiTestCase { /** @var Database $db */ $db = $lb->getConnection( DB_MASTER, [], '' ); - $lb->reuseConnection( $db ); // don't care $this->assertEquals( - '', - $db->getDomainID() + $wgDBname, + $db->getDomainId(), + 'Main domain ID handle used; same DB name' + ); + $this->assertEquals( + $wgDBname, + $db->getDBname(), + 'Main domain ID handle used; same DB name' + ); + $this->assertEquals( + '', + $db->tablePrefix(), + 'Main domain ID handle used; prefix is empty though' ); - $this->assertEquals( $this->quoteTable( $db, 'page' ), $db->tableName( 'page' ), "Correct full table name" ); - $this->assertEquals( $this->quoteTable( $db, $wgDBname ) . '.' . $this->quoteTable( $db, 'page' ), $db->tableName( "$wgDBname.page" ), "Correct full table name" ); - $this->assertEquals( $this->quoteTable( $db, 'nice_db' ) . '.' . $this->quoteTable( $db, 'page' ), $db->tableName( 'nice_db.page' ), "Correct full table name" ); + $lb->reuseConnection( $db ); // don't care + + $db = $lb->getConnection( DB_MASTER ); // local domain connection $factory->setDomainPrefix( 'my_' ); + $this->assertEquals( - '', + "$wgDBname-my_", $db->getDomainID() ); $this->assertEquals( @@ -415,7 +429,7 @@ class LBFactoryTest extends MediaWikiTestCase { } public function testTrickyDomain() { - global $wgDBtype; + global $wgDBtype, $wgDBname; if ( $wgDBtype === 'sqlite' ) { $tmpDir = $this->getNewTempDirectory(); @@ -427,18 +441,17 @@ class LBFactoryTest extends MediaWikiTestCase { $dbPath = null; } - $dbname = 'unittest-domain'; + $dbname = 'unittest-domain'; // explodes if DB is selected $factory = $this->newLBFactoryMulti( - [ 'localDomain' => $dbname ], - [ 'dbname' => $dbname, 'dbFilePath' => $dbPath ] + [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ], + [ 'dbFilePath' => $dbPath ] ); $lb = $factory->getMainLB(); /** @var Database $db */ $db = $lb->getConnection( DB_MASTER, [], '' ); - $lb->reuseConnection( $db ); // don't care $this->assertEquals( - '', + $wgDBname, $db->getDomainID() ); @@ -460,7 +473,10 @@ class LBFactoryTest extends MediaWikiTestCase { "Correct full table name" ); + $lb->reuseConnection( $db ); // don't care + $factory->setDomainPrefix( 'my_' ); + $db = $lb->getConnection( DB_MASTER, [], "$wgDBname-my_" ); $this->assertEquals( $this->quoteTable( $db, 'my_page' ), @@ -472,7 +488,6 @@ class LBFactoryTest extends MediaWikiTestCase { $db->tableName( 'other_nice_db.page' ), "Correct full table name" ); - $this->assertEquals( $this->quoteTable( $db, 'garbage-db' ) . '.' . $this->quoteTable( $db, 'page' ), $db->tableName( 'garbage-db.page' ), @@ -494,6 +509,8 @@ class LBFactoryTest extends MediaWikiTestCase { \MediaWiki\restoreWarnings(); } + $lb->reuseConnection( $db ); // don't care + $factory->closeAll(); $factory->destroy(); } diff --git a/tests/phpunit/includes/jobqueue/JobQueueTest.php b/tests/phpunit/includes/jobqueue/JobQueueTest.php index 7b34b59b152..bd21dc8a3eb 100644 --- a/tests/phpunit/includes/jobqueue/JobQueueTest.php +++ b/tests/phpunit/includes/jobqueue/JobQueueTest.php @@ -1,5 +1,7 @@ 'JobQueueDB' ]; + $baseConfig = [ 'class' => 'JobQueueDBSingle' ]; } $baseConfig['type'] = 'null'; $baseConfig['wiki'] = wfWikiID(); @@ -381,3 +383,11 @@ class JobQueueTest extends MediaWikiTestCase { [ 'lives' => 0, 'usleep' => 0, 'removeDuplicates' => 1, 'i' => $i ] + $rootJob ); } } + +class JobQueueDBSingle extends JobQueueDB { + protected function getDB( $index ) { + $lb = MediaWikiServices::getInstance()->getDBLoadBalancer(); + // Override to not use CONN_TRX_AUTO so that we see the same temporary `job` table + return $lb->getConnection( $index, [], $this->wiki ); + } +}