From c4e284f1130cdc2a4811120594bdb8013d17eaaa Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 25 Mar 2019 07:51:45 -0700 Subject: [PATCH] rdbms: codify DatabaseDomain table "_" prefix convention Alos simplify isCompatible() slightly and make the string case in convertToString() explicit. Change-Id: Ifb61bb5fb012491520525bbebfbde2269fa55b52 --- includes/DefaultSettings.php | 3 +- .../libs/rdbms/database/DatabaseDomain.php | 12 +++-- tests/phpunit/includes/WikiMapTest.php | 22 ++++---- .../includes/db/DatabaseSqliteTest.php | 4 +- tests/phpunit/includes/db/LBFactoryTest.php | 6 +-- .../rdbms/database/DatabaseDomainTest.php | 50 +++++++++---------- .../libs/rdbms/database/DatabaseTest.php | 14 +++--- 7 files changed, 57 insertions(+), 54 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 7a645a6bc8..d173d355ef 100644 --- 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 7559b06c3b..8d8285426a 100644 --- 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 e87e434f94..df05671dd2 100644 --- 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 e61bd05a67..2ea737fb94 100644 --- 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 2559b67db5..02c25e7b0d 100644 --- 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 b36fe11f85..e188ba8f69 100644 --- 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 bd1c1126c9..f81e9bb42b 100644 --- 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() ); -- 2.20.1