rdbms: use a direct "USE" query for doSelectDomain() for mysql

This should give better error messages on failure.

Bug: T212284
Change-Id: I55260c6e3db1770f01e3d6a6a363b917a57265be

diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 25d78da..77bb677 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -225,6 +225,39 @@ abstract class DatabaseMysqlBase extends Database {
                }
        }
 
+       protected function doSelectDomain( DatabaseDomain $domain ) {
+               if ( $domain->getSchema() !== null ) {
+                       throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
+               }
+
+               $database = $domain->getDatabase();
+               // A null database means "don't care" so leave it as is and update the table prefix
+               if ( $database === null ) {
+                       $this->currentDomain = new DatabaseDomain(
+                               $this->currentDomain->getDatabase(),
+                               null,
+                               $domain->getTablePrefix()
+                       );
+
+                       return true;
+               }
+
+               if ( $database !== $this->getDBname() ) {
+                       $sql = 'USE ' . $this->addIdentifierQuotes( $database );
+                       $ret = $this->doQuery( $sql );
+                       if ( $ret === false ) {
+                               $error = $this->lastError();
+                               $errno = $this->lastErrno();
+                               $this->reportQueryError( $error, $errno, $sql, __METHOD__ );
+                       }
+               }
+
+               // Update that domain fields on success (no exception thrown)
+               $this->currentDomain = $domain;
+
+               return true;
+       }
+
        /**
         * Open a connection to a MySQL server
         *

diff --git a/includes/libs/rdbms/database/DatabaseMysqli.php b/includes/libs/rdbms/database/DatabaseMysqli.php
index 15eeccf..1a5cdab 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqli.php
+++ b/includes/libs/rdbms/database/DatabaseMysqli.php
@@ -178,25 +178,6 @@ class DatabaseMysqli extends DatabaseMysqlBase {
                return $conn->affected_rows;
        }
 
-       function doSelectDomain( DatabaseDomain $domain ) {
-               if ( $domain->getSchema() !== null ) {
-                       throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
-               }
-
-               $database = $domain->getDatabase();
-               if ( $database !== $this->getDBname() ) {
-                       $conn = $this->getBindingHandle();
-                       if ( !$conn->select_db( $database ) ) {
-                               throw new DBExpectedError( $this, "Could not select database '$database'." );
-                       }
-               }
-
-               // Update that domain fields on success (no exception thrown)
-               $this->currentDomain = $domain;
-
-               return true;
-       }
-
        /**
         * @param mysqli_result $res
         * @return bool

diff --git a/includes/libs/rdbms/exception/DBQueryError.php b/includes/libs/rdbms/exception/DBQueryError.php
index b1353b7..9b664fc 100644 (file)
--- a/includes/libs/rdbms/exception/DBQueryError.php
+++ b/includes/libs/rdbms/exception/DBQueryError.php
@@ -45,7 +45,7 @@ class DBQueryError extends DBExpectedError {
        public function __construct( IDatabase $db, $error, $errno, $sql, $fname, $message = null ) {
                if ( $message === null ) {
                        if ( $db instanceof Database && $db->wasConnectionError( $errno ) ) {
-                               $message = "A connection error occurred. \n" .
+                               $message = "A connection error occurred during a query. \n" .
                                         "Query: $sql\n" .
                                         "Function: $fname\n" .
                                         "Error: $errno $error\n";

diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php
index 3d1bf59..2559b67 100644 (file)
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -438,6 +438,13 @@ class LBFactoryTest extends MediaWikiTestCase {
                ] );
        }
 
+       /**
+        * @covers \Wikimedia\Rdbms\LoadBalancer::getConnection
+        * @covers \Wikimedia\Rdbms\DatabaseMysqlBase::doSelectDomain
+        * @covers \Wikimedia\Rdbms\DatabaseMysqlBase::selectDB
+        * @covers \Wikimedia\Rdbms\DatabaseMssql::selectDB
+        * @covers DatabaseOracle::selectDB
+        */
        public function testNiceDomains() {
                global $wgDBname;
 
@@ -518,6 +525,13 @@ class LBFactoryTest extends MediaWikiTestCase {
                $factory->destroy();
        }
 
+       /**
+        * @covers \Wikimedia\Rdbms\LoadBalancer::getConnection
+        * @covers \Wikimedia\Rdbms\DatabaseMysqlBase::doSelectDomain
+        * @covers \Wikimedia\Rdbms\DatabaseMysqlBase::selectDB
+        * @covers \Wikimedia\Rdbms\DatabaseMssql::selectDB
+        * @covers DatabaseOracle::selectDB
+        */
        public function testTrickyDomain() {
                global $wgDBname;
 
@@ -530,7 +544,7 @@ class LBFactoryTest extends MediaWikiTestCase {
                $factory = $this->newLBFactoryMulti(
                        [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ],
                        [
-                               'dbName' => 'do_not_select_me' // explodes if DB is selected
+                               'dbname' => 'do_not_select_me' // explodes if DB is selected
                        ]
                );
                $lb = $factory->getMainLB();
@@ -584,46 +598,94 @@ class LBFactoryTest extends MediaWikiTestCase {
                $factory->destroy();
        }
 
+       /**
+        * @covers \Wikimedia\Rdbms\LoadBalancer::getConnection
+        * @covers \Wikimedia\Rdbms\DatabaseMysqlBase::doSelectDomain
+        * @covers \Wikimedia\Rdbms\DatabaseMysqlBase::selectDB
+        * @covers \Wikimedia\Rdbms\DatabaseMssql::selectDB
+        * @covers DatabaseOracle::selectDB
+        */
        public function testInvalidSelectDB() {
-               // FIXME: fails under sqlite
-               $this->markTestSkippedIfDbType( 'sqlite' );
+               if ( wfGetDB( DB_MASTER )->databasesAreIndependent() ) {
+                       $this->markTestSkipped( "Not applicable per databasesAreIndependent()" );
+               }
+
                $dbname = 'unittest-domain'; // explodes if DB is selected
                $factory = $this->newLBFactoryMulti(
                        [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ],
                        [
-                               'dbName' => 'do_not_select_me' // explodes if DB is selected
+                               'dbname' => 'do_not_select_me' // explodes if DB is selected
                        ]
                );
                $lb = $factory->getMainLB();
                /** @var IDatabase $db */
                $db = $lb->getConnection( DB_MASTER, [], '' );
 
-               if ( $db->getType() === 'sqlite' ) {
+               \Wikimedia\suppressWarnings();
+               try {
                        $this->assertFalse( $db->selectDB( 'garbage-db' ) );
-               } elseif ( $db->databasesAreIndependent() ) {
-                       try {
-                               $e = null;
-                               $db->selectDB( 'garbage-db' );
-                       } catch ( \Wikimedia\Rdbms\DBConnectionError $e ) {
-                               // expected
-                       }
-                       $this->assertInstanceOf( \Wikimedia\Rdbms\DBConnectionError::class, $e );
-                       $this->assertFalse( $db->isOpen() );
-               } else {
-                       \Wikimedia\suppressWarnings();
-                       try {
-                               $this->assertFalse( $db->selectDB( 'garbage-db' ) );
-                               $this->fail( "No error thrown." );
-                       } catch ( \Wikimedia\Rdbms\DBExpectedError $e ) {
-                               $this->assertEquals(
-                                       "Could not select database 'garbage-db'.",
-                                       $e->getMessage()
-                               );
-                       }
-                       \Wikimedia\restoreWarnings();
+                       $this->fail( "No error thrown." );
+               } catch ( \Wikimedia\Rdbms\DBQueryError $e ) {
+                       $this->assertRegExp( '/[\'"]garbage-db[\'"]/', $e->getMessage() );
+               }
+               \Wikimedia\restoreWarnings();
+       }
+
+       /**
+        * @covers \Wikimedia\Rdbms\DatabaseSqlite::selectDB
+        * @covers \Wikimedia\Rdbms\DatabasePostgres::selectDB
+        * @expectedException \Wikimedia\Rdbms\DBConnectionError
+        */
+       public function testInvalidSelectDBIndependant() {
+               $dbname = 'unittest-domain'; // explodes if DB is selected
+               $factory = $this->newLBFactoryMulti(
+                       [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ],
+                       [
+                               'dbname' => 'do_not_select_me' // explodes if DB is selected
+                       ]
+               );
+               $lb = $factory->getMainLB();
+
+               if ( !wfGetDB( DB_MASTER )->databasesAreIndependent() ) {
+                       $this->markTestSkipped( "Not applicable per databasesAreIndependent()" );
+               }
+
+               /** @var IDatabase $db */
+               $lb->getConnection( DB_MASTER, [], '' );
+       }
+
+       /**
+        * @covers \Wikimedia\Rdbms\DatabaseSqlite::selectDB
+        * @covers \Wikimedia\Rdbms\DatabasePostgres::selectDB
+        * @expectedException \Wikimedia\Rdbms\DBConnectionError
+        */
+       public function testInvalidSelectDBIndependant2() {
+               $dbname = 'unittest-domain'; // explodes if DB is selected
+               $factory = $this->newLBFactoryMulti(
+                       [ 'localDomain' => ( new DatabaseDomain( $dbname, null, '' ) )->getId() ],
+                       [
+                               'dbname' => 'do_not_select_me' // explodes if DB is selected
+                       ]
+               );
+               $lb = $factory->getMainLB();
+
+               if ( !wfGetDB( DB_MASTER )->databasesAreIndependent() ) {
+                       $this->markTestSkipped( "Not applicable per databasesAreIndependent()" );
                }
+
+               $db = $lb->getConnection( DB_MASTER );
+               \Wikimedia\suppressWarnings();
+               $db->selectDB( 'garbage-db' );
+               \Wikimedia\restoreWarnings();
        }
 
+       /**
+        * @covers \Wikimedia\Rdbms\LoadBalancer::getConnection
+        * @covers \Wikimedia\Rdbms\LoadBalancer::redefineLocalDomain
+        * @covers \Wikimedia\Rdbms\DatabaseMysqlBase::selectDB
+        * @covers \Wikimedia\Rdbms\DatabaseMssql::selectDB
+        * @covers DatabaseOracle::selectDB
+        */
        public function testRedefineLocalDomain() {
                global $wgDBname;