rdbms: add another null db/schema sanity check to DatabaseDomain

Change-Id: Id75a47a3dee97b43c586faf06208dffcc30c9e6e

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index ae4b71a..18961bd 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -617,6 +617,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function dbSchema( $schema = null ) {
+               if ( strlen( $schema ) && $this->getDBname() === null ) {
+                       throw new DBUnexpectedError( $this, "Cannot set schema to '$schema'; no database set." );
+               }
+
                $old = $this->currentDomain->getSchema();
                if ( $schema !== null ) {
                        $this->currentDomain = new DatabaseDomain(

diff --git a/includes/libs/rdbms/database/domain/DatabaseDomain.php b/includes/libs/rdbms/database/domain/DatabaseDomain.php
index ca57938..5dd4b49 100644 (file)
--- a/includes/libs/rdbms/database/domain/DatabaseDomain.php
+++ b/includes/libs/rdbms/database/domain/DatabaseDomain.php
@@ -48,6 +48,8 @@ class DatabaseDomain {
                $this->database = $database;
                if ( $schema !== null && ( !is_string( $schema ) || $schema === '' ) ) {
                        throw new InvalidArgumentException( 'Schema must be null or a non-empty string.' );
+               } elseif ( $database === null && $schema !== null ) {
+                       throw new InvalidArgumentException( 'Schema must be null if database is null.' );
                }
                $this->schema = $schema;
                if ( !is_string( $prefix ) ) {
@@ -120,8 +122,8 @@ class DatabaseDomain {
         * Check whether the domain $other meets the specifications of this domain
         *
         * If this instance has a null database specifier, then $other can have any database
-        * specified, including the null, and likewise if the schema specifier is null. This
-        * is not transitive like equals() since a domain that explicitly wants a certain
+        * specifier, including null. This is likewise true if the schema specifier is null.
+        * This is not transitive like equals() since a domain that explicitly wants a certain
         * database or schema cannot be satisfied by one of another (nor null). If the prefix
         * is empty and the DB and schema are both null, then the entire domain is considered
         * unspecified, and any prefix of $other is considered compatible.

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php
index e188ba8..b1d4fad 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php
@@ -149,8 +149,6 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
                                [ '', null, null, '', true ],
                        'dontcaredb+dontcaredbschema+prefix' =>
                                [ 'mywiki-mediawiki-prefix_', null, null, 'prefix_', false ],
-                       'dontcaredb+schema+prefix' =>
-                               [ 'mywiki-schema-prefix_', null, 'schema', 'prefix_', false ],
                        'db+dontcareschema+prefix' =>
                                [ 'mywiki-schema-prefix_', 'mywiki', null, 'prefix_', false ],
                        'postgres-db-jobqueue' =>
@@ -181,8 +179,6 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
                                [ 'mywiki-schema-prefix_', 'thatwiki', 'schema', 'prefix_' ],
                        'dontcaredb+dontcaredbschema+prefix' =>
                                [ 'thatwiki-mediawiki-otherprefix_', null, null, 'prefix_' ],
-                       'dontcaredb+schema+prefix' =>
-                               [ 'mywiki-otherschema-prefix_', null, 'schema', 'prefix_' ],
                        'db+dontcareschema+prefix' =>
                                [ 'notmywiki-schema-prefix_', 'mywiki', null, 'prefix_' ],
                ];
@@ -202,6 +198,20 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
                $this->assertFalse( $fromId->isCompatible( $compareIdObj ), 'fromId equals string' );
        }
 
+       /**
+        * @expectedException InvalidArgumentException
+        */
+       public function testSchemaWithNoDB1() {
+               new DatabaseDomain( null, 'schema', '' );
+       }
+
+       /**
+        * @expectedException InvalidArgumentException
+        */
+       public function testSchemaWithNoDB2() {
+               DatabaseDomain::newFromId( '-schema-prefix' );
+       }
+
        /**
         * @covers Wikimedia\Rdbms\DatabaseDomain::isUnspecified
         */

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
index f81e9bb..8b24791 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
@@ -13,6 +13,8 @@ use Wikimedia\Rdbms\DatabaseMssql;
 use Wikimedia\Rdbms\DBUnexpectedError;
 
 class DatabaseTest extends PHPUnit\Framework\TestCase {
+       /** @var DatabaseTestHelper */
+       private $db;
 
        use MediaWikiCoversValidator;
 
@@ -629,6 +631,10 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
         * @covers Wikimedia\Rdbms\Database::dbSchema
         */
        public function testSchemaAndPrefixMutators() {
+               $ud = DatabaseDomain::newUnspecified();
+
+               $this->assertEquals( $ud->getId(), $this->db->getDomainID() );
+
                $old = $this->db->tablePrefix();
                $oldDomain = $this->db->getDomainId();
                $this->assertInternalType( 'string', $old, 'Prefix is string' );
@@ -643,11 +649,27 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $oldDomain = $this->db->getDomainId();
                $this->assertInternalType( 'string', $old, 'Schema is string' );
                $this->assertSame( $old, $this->db->dbSchema(), "Schema unchanged" );
+
+               $this->db->selectDB( 'y' );
                $this->assertSame( $old, $this->db->dbSchema( 'xxx' ) );
                $this->assertSame( 'xxx', $this->db->dbSchema(), "Schema set" );
                $this->db->dbSchema( $old );
                $this->assertNotEquals( 'xxx', $this->db->dbSchema() );
-               $this->assertSame( $oldDomain, $this->db->getDomainId() );
+               $this->assertSame( "y", $this->db->getDomainId() );
+       }
+
+       /**
+        * @covers Wikimedia\Rdbms\Database::tablePrefix
+        * @covers Wikimedia\Rdbms\Database::dbSchema
+        * @expectedException DBUnexpectedError
+        */
+       public function testSchemaWithNoDB() {
+               $ud = DatabaseDomain::newUnspecified();
+
+               $this->assertEquals( $ud->getId(), $this->db->getDomainID() );
+               $this->assertSame( '', $this->db->dbSchema() );
+
+               $this->db->dbSchema( 'xxx' );
        }
 
        /**