Make LoadBalancer domain handling more robust

* Cleanup setDomainPrefix() methods to handle existing LBs/DBs.
* This also makes it set just the prefix as the prefix rather
  than as the whole domain.
* Fix regression from 789a54a5f1 where "read" mode of
  tablePrefix()/dbSchema() still set the field.
* Make getConnectionRef() always have the domain set.
* Add a check in openConnection() for explicit local
  connections, treating $domain as false and avoiding
  openForeignConnection().

Bug: T145840
Change-Id: Idf392bd9992a215c4fa3bddf275562f3916596aa

diff --git a/includes/db/CloneDatabase.php b/includes/db/CloneDatabase.php
index ee82bdf..2af742e 100644 (file)
--- a/includes/db/CloneDatabase.php
+++ b/includes/db/CloneDatabase.php
@@ -132,12 +132,6 @@ class CloneDatabase {
 
                $lbFactory = wfGetLBFactory();
                $lbFactory->setDomainPrefix( $prefix );
-               $lbFactory->forEachLB( function( LoadBalancer $lb ) use ( $prefix ) {
-                       $lb->setDomainPrefix( $prefix );
-                       $lb->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix ) {
-                               $db->tablePrefix( $prefix );
-                       } );
-               } );
                $wgDBprefix = $prefix;
        }
 }

diff --git a/includes/db/Database.php b/includes/db/Database.php
index e908824..3d9b41a 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -93,9 +93,9 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
        protected $mTrxEndCallbacksSuppressed = false;
 
        /** @var string */
-       protected $mTablePrefix;
+       protected $mTablePrefix = '';
        /** @var string */
-       protected $mSchema;
+       protected $mSchema = '';
        /** @var integer */
        protected $mFlags;
        /** @var array */
@@ -367,7 +367,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        $p['variables'] = isset( $p['variables'] ) ? $p['variables'] : [];
                        $p['tablePrefix'] = isset( $p['tablePrefix'] ) ? $p['tablePrefix'] : '';
                        if ( !isset( $p['schema'] ) ) {
-                               $p['schema'] = isset( $defaultSchemas[$dbType] ) ? $defaultSchemas[$dbType] : null;
+                               $p['schema'] = isset( $defaultSchemas[$dbType] ) ? $defaultSchemas[$dbType] : '';
                        }
                        $p['foreign'] = isset( $p['foreign'] ) ? $p['foreign'] : false;
 
@@ -466,14 +466,18 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
 
        public function tablePrefix( $prefix = null ) {
                $old = $this->mTablePrefix;
-               $this->mTablePrefix = $prefix;
+               if ( $prefix !== null ) {
+                       $this->mTablePrefix = $prefix;
+               }
 
                return $old;
        }
 
        public function dbSchema( $schema = null ) {
                $old = $this->mSchema;
-               $this->mSchema = $schema;
+               if ( $schema !== null ) {
+                       $this->mSchema = $schema;
+               }
 
                return $old;
        }
@@ -699,7 +703,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
        }
 
        public function getWikiID() {
-               if ( $this->mTablePrefix ) {
+               if ( $this->mTablePrefix != '' ) {
                        return "{$this->mDBname}-{$this->mTablePrefix}";
                } else {
                        return $this->mDBname;
@@ -1884,7 +1888,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        # In dbs that support it, $database may actually be the schema
                        # but that doesn't affect any of the functionality here
                        $prefix = '';
-                       $schema = null;
+                       $schema = '';
                } else {
                        list( $table ) = $dbDetails;
                        if ( isset( $this->tableAliases[$table] ) ) {

diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php
index b6f3317..e011ff0 100644 (file)
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -633,15 +633,18 @@ abstract class LBFactory {
        }
 
        /**
-        * Define a new local domain (for testing)
+        * Set a new table prefix for the existing local domain ID for testing
         *
-        * Caller should make sure no local connection are open to the old local domain
-        *
-        * @param string $domain
+        * @param string $prefix
         * @since 1.28
         */
-       public function setDomainPrefix( $domain ) {
-               $this->localDomain = $domain;
+       public function setDomainPrefix( $prefix ) {
+               list( $dbName, ) = explode( '-', $this->localDomain, 2 );
+               $this->localDomain = "{$dbName}-{$prefix}";
+
+               $this->forEachLB( function( LoadBalancer $lb ) use ( $prefix ) {
+                       $lb->setDomainPrefix( $prefix );
+               } );
        }
 
        /**

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index db69de1..57c905f 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -515,7 +515,7 @@ class LoadBalancer implements ILoadBalancer {
                }
 
                if ( $domain === $this->localDomain ) {
-                       $domain = false;
+                       $domain = false; // local connection requested
                }
 
                $groups = ( $groups === false || $groups === [] )
@@ -627,6 +627,8 @@ class LoadBalancer implements ILoadBalancer {
         * @since 1.22
         */
        public function getConnectionRef( $db, $groups = [], $domain = false ) {
+               $domain = ( $domain !== false ) ? $domain : $this->localDomain;
+
                return new DBConnRef( $this, $this->getConnection( $db, $groups, $domain ) );
        }
 
@@ -650,6 +652,10 @@ class LoadBalancer implements ILoadBalancer {
        }
 
        public function openConnection( $i, $domain = false ) {
+               if ( $domain === $this->localDomain ) {
+                       $domain = false; // local connection requested
+               }
+
                if ( $domain !== false ) {
                        $conn = $this->openForeignConnection( $i, $domain );
                } elseif ( isset( $this->mConns['local'][$i][0] ) ) {
@@ -1607,7 +1613,10 @@ class LoadBalancer implements ILoadBalancer {
         */
        public function setDomainPrefix( $prefix ) {
                list( $dbName, ) = explode( '-', $this->localDomain, 2 );
-
                $this->localDomain = "{$dbName}-{$prefix}";
+
+               $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix ) {
+                       $db->tablePrefix( $prefix );
+               } );
        }
 }

diff --git a/tests/phpunit/includes/db/DatabaseTest.php b/tests/phpunit/includes/db/DatabaseTest.php
index d4be6e4..134caf4 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseTest.php
+++ b/tests/phpunit/includes/db/DatabaseTest.php
@@ -427,4 +427,26 @@ class DatabaseTest extends MediaWikiTestCase {
                $this->assertEquals( $origSsl, $db->getFlag( DBO_SSL ) );
                $this->assertEquals( $origTrx, $db->getFlag( DBO_TRX ) );
        }
+
+       /**
+        * @covers DatabaseBase::tablePrefix()
+        * @covers DatabaseBase::dbSchema()
+        */
+       public function testMutators() {
+               $old = $this->db->tablePrefix();
+               $this->assertType( 'string', $old, 'Prefix is string' );
+               $this->assertEquals( $old, $this->db->tablePrefix(), "Prefix unchanged" );
+               $this->assertEquals( $old, $this->db->tablePrefix( 'xxx' ) );
+               $this->assertEquals( 'xxx', $this->db->tablePrefix(), "Prefix set" );
+               $this->db->tablePrefix( $old );
+               $this->assertNotEquals( 'xxx', $this->db->tablePrefix() );
+
+               $old = $this->db->dbSchema();
+               $this->assertType( 'string', $old, 'Schema is string' );
+               $this->assertEquals( $old, $this->db->dbSchema(), "Schema unchanged" );
+               $this->assertEquals( $old, $this->db->dbSchema( 'xxx' ) );
+               $this->assertEquals( 'xxx', $this->db->dbSchema(), "Schema set" );
+               $this->db->dbSchema( $old );
+               $this->assertNotEquals( 'xxx', $this->db->dbSchema() );
+       }
 }