rdbms: codify DatabaseDomain table "_" prefix convention

Alos simplify isCompatible() slightly and make the string
case in convertToString() explicit.

Change-Id: Ifb61bb5fb012491520525bbebfbde2269fa55b52

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 7a645a6..d173d35 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -1935,7 +1935,8 @@ $wgSearchType = null;
 $wgSearchTypeAlternatives = null;
 
 /**
- * Table name prefix; this should be alphanumeric and not contain spaces nor hyphens
+ * Table name prefix.
+ * This should be alphanumeric, contain neither spaces nor hyphens, and end in "_"
  */
 $wgDBprefix = '';
 

diff --git a/includes/libs/rdbms/database/DatabaseDomain.php b/includes/libs/rdbms/database/DatabaseDomain.php
index 7559b06..8d82854 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseDomain.php
+++ b/includes/libs/rdbms/database/DatabaseDomain.php
@@ -43,15 +43,17 @@ class DatabaseDomain {
         */
        public function __construct( $database, $schema, $prefix ) {
                if ( $database !== null && ( !is_string( $database ) || !strlen( $database ) ) ) {
-                       throw new InvalidArgumentException( "Database must be null or a non-empty string." );
+                       throw new InvalidArgumentException( 'Database must be null or a non-empty string.' );
                }
                $this->database = $database;
                if ( $schema !== null && ( !is_string( $schema ) || !strlen( $schema ) ) ) {
-                       throw new InvalidArgumentException( "Schema must be null or a non-empty string." );
+                       throw new InvalidArgumentException( 'Schema must be null or a non-empty string.' );
                }
                $this->schema = $schema;
                if ( !is_string( $prefix ) ) {
-                       throw new InvalidArgumentException( "Prefix must be a string." );
+                       throw new InvalidArgumentException( 'Prefix must be a string.' );
+               } elseif ( $prefix !== '' && substr( $prefix, -1, 1 ) !== '_' ) {
+                       throw new InvalidArgumentException( 'A non-empty prefix must end with "_".' );
                }
                $this->prefix = $prefix;
        }
@@ -133,7 +135,7 @@ class DatabaseDomain {
                        return true; // even the prefix doesn't matter
                }
 
-               $other = ( $other instanceof self ) ? $other : self::newFromId( $other );
+               $other = self::newFromId( $other );
 
                return (
                        ( $this->database === $other->database || $this->database === null ) &&
@@ -188,7 +190,7 @@ class DatabaseDomain {
         * @return string
         */
        private function convertToString() {
-               $parts = [ $this->database ];
+               $parts = [ (string)$this->database ];
                if ( $this->schema !== null ) {
                        $parts[] = $this->schema;
                }

diff --git a/tests/phpunit/includes/WikiMapTest.php b/tests/phpunit/includes/WikiMapTest.php
index e87e434..df05671 100644 (file)
--- a/tests/phpunit/includes/WikiMapTest.php
+++ b/tests/phpunit/includes/WikiMapTest.php
@@ -237,13 +237,13 @@ class WikiMapTest extends MediaWikiLangTestCase {
 
        public function provideGetWikiIdFromDomain() {
                return [
-                       [ 'db-prefix', 'db-prefix' ],
+                       [ 'db-prefix_', 'db-prefix_' ],
                        [ wfWikiID(), wfWikiID() ],
-                       [ new DatabaseDomain( 'db-dash', null, 'prefix' ), 'db-dash-prefix' ],
+                       [ new DatabaseDomain( 'db-dash', null, 'prefix_' ), 'db-dash-prefix_' ],
                        [ wfWikiID(), wfWikiID() ],
-                       [ new DatabaseDomain( 'db-dash', null, 'prefix' ), 'db-dash-prefix' ],
-                       [ new DatabaseDomain( 'db', 'mediawiki', 'prefix' ), 'db-prefix' ], // schema ignored
-                       [ new DatabaseDomain( 'db', 'custom', 'prefix' ), 'db-custom-prefix' ],
+                       [ new DatabaseDomain( 'db-dash', null, 'prefix_' ), 'db-dash-prefix_' ],
+                       [ new DatabaseDomain( 'db', 'mediawiki', 'prefix_' ), 'db-prefix_' ], // schema ignored
+                       [ new DatabaseDomain( 'db', 'custom', 'prefix_' ), 'db-custom-prefix_' ],
                ];
        }
 
@@ -279,15 +279,15 @@ class WikiMapTest extends MediaWikiLangTestCase {
                        [ 'db', 'db', null, '' ],
                        [ 'db-schema-','db', 'schema', '' ],
                        [ 'db','db', 'mediawiki', '' ], // common b/c case
-                       [ 'db-prefix', 'db', null, 'prefix' ],
-                       [ 'db-schema-prefix', 'db', 'schema', 'prefix' ],
-                       [ 'db-prefix', 'db', 'mediawiki', 'prefix' ], // common b/c case
+                       [ 'db-prefix_', 'db', null, 'prefix_' ],
+                       [ 'db-schema-prefix_', 'db', 'schema', 'prefix_' ],
+                       [ 'db-prefix_', 'db', 'mediawiki', 'prefix_' ], // common b/c case
                        // Bad hyphen cases (best effort support)
                        [ 'db-stuff', 'db-stuff', null, '' ],
-                       [ 'db-stuff-prefix', 'db-stuff', null, 'prefix' ],
+                       [ 'db-stuff-prefix_', 'db-stuff', null, 'prefix_' ],
                        [ 'db-stuff-schema-', 'db-stuff', 'schema', '' ],
-                       [ 'db-stuff-schema-prefix', 'db-stuff', 'schema', 'prefix' ],
-                       [ 'db-stuff-prefix', 'db-stuff', 'mediawiki', 'prefix' ] // common b/c case
+                       [ 'db-stuff-schema-prefix_', 'db-stuff', 'schema', 'prefix_' ],
+                       [ 'db-stuff-prefix_', 'db-stuff', 'mediawiki', 'prefix_' ] // common b/c case
                ];
        }
 

diff --git a/tests/phpunit/includes/db/DatabaseSqliteTest.php b/tests/phpunit/includes/db/DatabaseSqliteTest.php
index e61bd05..2ea737f 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseSqliteTest.php
+++ b/tests/phpunit/includes/db/DatabaseSqliteTest.php
@@ -164,9 +164,9 @@ class DatabaseSqliteTest extends MediaWikiTestCase {
                $db = DatabaseSqlite::newStandaloneInstance( ':memory:' );
                $this->assertEquals( 'foo', $db->tableName( 'foo' ) );
                $this->assertEquals( 'sqlite_master', $db->tableName( 'sqlite_master' ) );
-               $db->tablePrefix( 'foo' );
+               $db->tablePrefix( 'foo_' );
                $this->assertEquals( 'sqlite_master', $db->tableName( 'sqlite_master' ) );
-               $this->assertEquals( 'foobar', $db->tableName( 'bar' ) );
+               $this->assertEquals( 'foo_bar', $db->tableName( 'bar' ) );
        }
 
        /**

diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php
index 2559b67..02c25e7 100644 (file)
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -707,10 +707,10 @@ class LBFactoryTest extends MediaWikiTestCase {
                );
                unset( $conn1 );
 
-               $factory->redefineLocalDomain( 'somedb-prefix' );
-               $this->assertEquals( 'somedb-prefix', $factory->getLocalDomainID() );
+               $factory->redefineLocalDomain( 'somedb-prefix_' );
+               $this->assertEquals( 'somedb-prefix_', $factory->getLocalDomainID() );
 
-               $domain = new DatabaseDomain( $wgDBname, null, 'pref' );
+               $domain = new DatabaseDomain( $wgDBname, null, 'pref_' );
                $factory->redefineLocalDomain( $domain );
 
                $n = 0;

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php
index b36fe11..e188ba8 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseDomainTest.php
@@ -13,7 +13,7 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
        public static function provideConstruct() {
                return [
                        'All strings' =>
-                               [ 'foo', 'bar', 'baz', 'foo-bar-baz' ],
+                               [ 'foo', 'bar', 'baz_', 'foo-bar-baz_' ],
                        'Nothing' =>
                                [ null, null, '', '' ],
                        'Invalid $database' =>
@@ -23,9 +23,9 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
                        'Invalid $prefix' =>
                                [ 'foo', 'bar', 0, '', true ],
                        'Dash' =>
-                               [ 'foo-bar', 'baz', 'baa', 'foo?hbar-baz-baa' ],
+                               [ 'foo-bar', 'baz', 'baa_', 'foo?hbar-baz-baa_' ],
                        'Question mark' =>
-                               [ 'foo?bar', 'baz', 'baa', 'foo??bar-baz-baa' ],
+                               [ 'foo?bar', 'baz', 'baa_', 'foo??bar-baz-baa_' ],
                ];
        }
 
@@ -53,17 +53,17 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
                        'Basic' =>
                                [ 'foo', 'foo', null, '' ],
                        'db+prefix' =>
-                               [ 'foo-bar', 'foo', null, 'bar' ],
+                               [ 'foo-bar_', 'foo', null, 'bar_' ],
                        'db+schema+prefix' =>
-                               [ 'foo-bar-baz', 'foo', 'bar', 'baz' ],
+                               [ 'foo-bar-baz_', 'foo', 'bar', 'baz_' ],
                        '?h -> -' =>
-                               [ 'foo?hbar-baz-baa', 'foo-bar', 'baz', 'baa' ],
+                               [ 'foo?hbar-baz-baa_', 'foo-bar', 'baz', 'baa_' ],
                        '?? -> ?' =>
-                               [ 'foo??bar-baz-baa', 'foo?bar', 'baz', 'baa' ],
+                               [ 'foo??bar-baz-baa_', 'foo?bar', 'baz', 'baa_' ],
                        '? is left alone' =>
-                               [ 'foo?bar-baz-baa', 'foo?bar', 'baz', 'baa' ],
+                               [ 'foo?bar-baz-baa_', 'foo?bar', 'baz', 'baa_' ],
                        'too many parts' =>
-                               [ 'foo-bar-baz-baa', '', '', '', true ],
+                               [ 'foo-bar-baz-baa_', '', '', '', true ],
                        'from instance' =>
                                [ DatabaseDomain::newUnspecified(), null, null, '' ],
                ];
@@ -90,13 +90,13 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
                        'Basic' =>
                                [ 'foo', 'foo', null, '' ],
                        'db+prefix' =>
-                               [ 'foo-bar', 'foo', null, 'bar' ],
+                               [ 'foo-bar_', 'foo', null, 'bar_' ],
                        'db+schema+prefix' =>
-                               [ 'foo-bar-baz', 'foo', 'bar', 'baz' ],
+                               [ 'foo-bar-baz_', 'foo', 'bar', 'baz_' ],
                        '?h -> -' =>
-                               [ 'foo?hbar-baz-baa', 'foo-bar', 'baz', 'baa' ],
+                               [ 'foo?hbar-baz-baa_', 'foo-bar', 'baz', 'baa_' ],
                        '?? -> ?' =>
-                               [ 'foo??bar-baz-baa', 'foo?bar', 'baz', 'baa' ],
+                               [ 'foo??bar-baz-baa_', 'foo?bar', 'baz', 'baa_' ],
                        'Nothing' =>
                                [ '', null, null, '' ],
                ];
@@ -136,23 +136,23 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
                        'Basic' =>
                                [ 'foo', 'foo', null, '', true ],
                        'db+prefix' =>
-                               [ 'foo-bar', 'foo', null, 'bar', true ],
+                               [ 'foo-bar_', 'foo', null, 'bar_', true ],
                        'db+schema+prefix' =>
-                               [ 'foo-bar-baz', 'foo', 'bar', 'baz', true ],
+                               [ 'foo-bar-baz_', 'foo', 'bar', 'baz_', true ],
                        'db+dontcare_schema+prefix' =>
-                               [ 'foo-bar-baz', 'foo', null, 'baz', false ],
+                               [ 'foo-bar-baz_', 'foo', null, 'baz_', false ],
                        '?h -> -' =>
-                               [ 'foo?hbar-baz-baa', 'foo-bar', 'baz', 'baa', true ],
+                               [ 'foo?hbar-baz-baa_', 'foo-bar', 'baz', 'baa_', true ],
                        '?? -> ?' =>
-                               [ 'foo??bar-baz-baa', 'foo?bar', 'baz', 'baa', true ],
+                               [ 'foo??bar-baz-baa_', 'foo?bar', 'baz', 'baa_', true ],
                        'Nothing' =>
                                [ '', null, null, '', true ],
                        'dontcaredb+dontcaredbschema+prefix' =>
-                               [ 'mywiki-mediawiki-prefix', null, null, 'prefix', false ],
+                               [ 'mywiki-mediawiki-prefix_', null, null, 'prefix_', false ],
                        'dontcaredb+schema+prefix' =>
-                               [ 'mywiki-schema-prefix', null, 'schema', 'prefix', false ],
+                               [ 'mywiki-schema-prefix_', null, 'schema', 'prefix_', false ],
                        'db+dontcareschema+prefix' =>
-                               [ 'mywiki-schema-prefix', 'mywiki', null, 'prefix', false ],
+                               [ 'mywiki-schema-prefix_', 'mywiki', null, 'prefix_', false ],
                        'postgres-db-jobqueue' =>
                                [ 'postgres-mediawiki-', 'postgres', null, '', false ]
                ];
@@ -178,13 +178,13 @@ class DatabaseDomainTest extends PHPUnit\Framework\TestCase {
        public static function provideIsCompatible2() {
                return [
                        'db+schema+prefix' =>
-                               [ 'mywiki-schema-prefix', 'thatwiki', 'schema', 'prefix' ],
+                               [ 'mywiki-schema-prefix_', 'thatwiki', 'schema', 'prefix_' ],
                        'dontcaredb+dontcaredbschema+prefix' =>
-                               [ 'thatwiki-mediawiki-otherprefix', null, null, 'prefix' ],
+                               [ 'thatwiki-mediawiki-otherprefix_', null, null, 'prefix_' ],
                        'dontcaredb+schema+prefix' =>
-                               [ 'mywiki-otherschema-prefix', null, 'schema', 'prefix' ],
+                               [ 'mywiki-otherschema-prefix_', null, 'schema', 'prefix_' ],
                        'db+dontcareschema+prefix' =>
-                               [ 'notmywiki-schema-prefix', 'mywiki', null, 'prefix' ],
+                               [ 'notmywiki-schema-prefix_', 'mywiki', null, 'prefix_' ],
                ];
        }
 

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
index bd1c112..f81e9bb 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
@@ -633,10 +633,10 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $oldDomain = $this->db->getDomainId();
                $this->assertInternalType( 'string', $old, 'Prefix is string' );
                $this->assertSame( $old, $this->db->tablePrefix(), "Prefix unchanged" );
-               $this->assertSame( $old, $this->db->tablePrefix( 'xxx' ) );
-               $this->assertSame( 'xxx', $this->db->tablePrefix(), "Prefix set" );
+               $this->assertSame( $old, $this->db->tablePrefix( 'xxx_' ) );
+               $this->assertSame( 'xxx_', $this->db->tablePrefix(), "Prefix set" );
                $this->db->tablePrefix( $old );
-               $this->assertNotEquals( 'xxx', $this->db->tablePrefix() );
+               $this->assertNotEquals( 'xxx_', $this->db->tablePrefix() );
                $this->assertSame( $oldDomain, $this->db->getDomainId() );
 
                $old = $this->db->dbSchema();
@@ -659,10 +659,10 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $oldSchema = $this->db->dbSchema();
                $oldPrefix = $this->db->tablePrefix();
 
-               $this->db->selectDomain( 'testselectdb-xxx' );
+               $this->db->selectDomain( 'testselectdb-xxx_' );
                $this->assertSame( 'testselectdb', $this->db->getDBname() );
                $this->assertSame( '', $this->db->dbSchema() );
-               $this->assertSame( 'xxx', $this->db->tablePrefix() );
+               $this->assertSame( 'xxx_', $this->db->tablePrefix() );
 
                $this->db->selectDomain( $oldDomain );
                $this->assertSame( $oldDatabase, $this->db->getDBname() );
@@ -670,10 +670,10 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $this->assertSame( $oldPrefix, $this->db->tablePrefix() );
                $this->assertSame( $oldDomain, $this->db->getDomainId() );
 
-               $this->db->selectDomain( 'testselectdb-schema-xxx' );
+               $this->db->selectDomain( 'testselectdb-schema-xxx_' );
                $this->assertSame( 'testselectdb', $this->db->getDBname() );
                $this->assertSame( 'schema', $this->db->dbSchema() );
-               $this->assertSame( 'xxx', $this->db->tablePrefix() );
+               $this->assertSame( 'xxx_', $this->db->tablePrefix() );
 
                $this->db->selectDomain( $oldDomain );
                $this->assertSame( $oldDatabase, $this->db->getDBname() );