Merge pull request #8664 from kenjis/feat-database-name-with-dot

feat: support database name with dots
codeigniter4 · Mar 28, 2024 · 33d7d2a · 33d7d2a
2 parents 0e6bd76 + 6a552e0
commit 33d7d2a
Show file tree

Hide file tree

Showing 13 changed files with 206 additions and 31 deletions.
diff --git a/phpstan-baseline.php b/phpstan-baseline.php
@@ -8981,11 +8981,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/system/Test/Mock/MockConnection.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Property CodeIgniter\\\\Test\\\\Mock\\\\MockConnection\\:\\:\\$returnValues has no type specified\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/system/Test/Mock/MockConnection.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Return type \\(array\\{code\\: int\\|string\\|null, message\\: string\\|null\\}\\) of method CodeIgniter\\\\Test\\\\Mock\\\\MockConnection\\:\\:error\\(\\) should be covariant with return type \\(array\\<string, int\\|string\\>\\) of method CodeIgniter\\\\Database\\\\ConnectionInterface\\<object\\|resource,object\\|resource\\>\\:\\:error\\(\\)$#',
 	'count' => 1,
@@ -12676,11 +12671,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/tests/system/Database/BaseConnectionTest.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Property class@anonymous/tests/system/Database/BaseConnectionTest\\.php\\:121\\:\\:\\$returnValues has no type specified\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/tests/system/Database/BaseConnectionTest.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Method CodeIgniter\\\\Database\\\\BaseQueryTest\\:\\:provideHighlightQueryKeywords\\(\\) return type has no value type specified in iterable type iterable\\.$#',
 	'count' => 1,

diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php
@@ -1188,6 +1188,24 @@ private function protectDotItem(string $item, string $alias, bool $protectIdenti
         return $item . $alias;
     }
 
+    /**
+     * Escape the SQL Identifier
+     *
+     * This function escapes single identifier.
+     *
+     * @param non-empty-string $item
+     */
+    public function escapeIdentifier(string $item): string
+    {
+        return $this->escapeChar
+            . str_replace(
+                $this->escapeChar,
+                $this->escapeChar . $this->escapeChar,
+                $item
+            )
+            . $this->escapeChar;
+    }
+
     /**
      * Escape the SQL Identifiers
      *

diff --git a/system/Database/Forge.php b/system/Database/Forge.php
@@ -229,7 +229,14 @@ public function createDatabase(string $dbName, bool $ifNotExists = false): bool
         }
 
         try {
-            if (! $this->db->query(sprintf($ifNotExists ? $this->createDatabaseIfStr : $this->createDatabaseStr, $dbName, $this->db->charset, $this->db->DBCollat))) {
+            if (! $this->db->query(
+                sprintf(
+                    $ifNotExists ? $this->createDatabaseIfStr : $this->createDatabaseStr,
+                    $this->db->escapeIdentifier($dbName),
+                    $this->db->charset,
+                    $this->db->DBCollat
+                )
+            )) {
                 // @codeCoverageIgnoreStart
                 if ($this->db->DBDebug) {
                     throw new DatabaseException('Unable to create the specified database.');
@@ -286,7 +293,9 @@ public function dropDatabase(string $dbName): bool
             return false;
         }
 
-        if (! $this->db->query(sprintf($this->dropDatabaseStr, $dbName))) {
+        if (! $this->db->query(
+            sprintf($this->dropDatabaseStr, $this->db->escapeIdentifier($dbName))
+        )) {
             if ($this->db->DBDebug) {
                 throw new DatabaseException('Unable to drop the specified database.');
             }
@@ -295,7 +304,11 @@ public function dropDatabase(string $dbName): bool
         }
 
         if (! empty($this->db->dataCache['db_names'])) {
-            $key = array_search(strtolower($dbName), array_map('strtolower', $this->db->dataCache['db_names']), true);
+            $key = array_search(
+                strtolower($dbName),
+                array_map('strtolower', $this->db->dataCache['db_names']),
+                true
+            );
             if ($key !== false) {
                 unset($this->db->dataCache['db_names'][$key]);
             }

diff --git a/system/Database/MySQLi/Connection.php b/system/Database/MySQLi/Connection.php
@@ -389,7 +389,7 @@ public function escapeLikeStringDirect($str)
      */
     protected function _listTables(bool $prefixLimit = false, ?string $tableName = null): string
     {
-        $sql = 'SHOW TABLES FROM ' . $this->escapeIdentifiers($this->database);
+        $sql = 'SHOW TABLES FROM ' . $this->escapeIdentifier($this->database);
 
         if ($tableName !== null) {
             return $sql . ' LIKE ' . $this->escape($tableName);

diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php
@@ -14,7 +14,9 @@
 namespace CodeIgniter\Database\SQLSRV;
 
 use CodeIgniter\Database\BaseConnection;
+use CodeIgniter\Database\Exceptions\DatabaseException;
 use CodeIgniter\Database\Forge as BaseForge;
+use Throwable;
 
 /**
  * Forge for SQLSRV
@@ -42,7 +44,7 @@ class Forge extends BaseForge
      *
      * @var string
      */
-    protected $createDatabaseIfStr = "DECLARE @DBName VARCHAR(255) = '%s'\nDECLARE @SQL VARCHAR(max) = 'IF DB_ID( ''' + @DBName + ''' ) IS NULL CREATE DATABASE ' + @DBName\nEXEC( @SQL )";
+    protected $createDatabaseIfStr = "DECLARE @DBName VARCHAR(255) = '%s'\nDECLARE @SQL VARCHAR(max) = 'IF DB_ID( ''' + @DBName + ''' ) IS NULL CREATE DATABASE %s'\nEXEC( @SQL )";
 
     /**
      * CREATE DATABASE IF statement
@@ -119,6 +121,53 @@ public function __construct(BaseConnection $db)
         $this->dropIndexStr      = 'DROP INDEX %s ON ' . $this->db->escapeIdentifiers($this->db->schema) . '.%s';
     }
 
+    /**
+     * Create database
+     *
+     * @param bool $ifNotExists Whether to add IF NOT EXISTS condition
+     *
+     * @throws DatabaseException
+     */
+    public function createDatabase(string $dbName, bool $ifNotExists = false): bool
+    {
+        if ($ifNotExists) {
+            $sql = sprintf(
+                $this->createDatabaseIfStr,
+                $dbName,
+                $this->db->escapeIdentifier($dbName)
+            );
+        } else {
+            $sql = sprintf(
+                $this->createDatabaseStr,
+                $this->db->escapeIdentifier($dbName)
+            );
+        }
+
+        try {
+            if (! $this->db->query($sql)) {
+                // @codeCoverageIgnoreStart
+                if ($this->db->DBDebug) {
+                    throw new DatabaseException('Unable to create the specified database.');
+                }
+
+                return false;
+                // @codeCoverageIgnoreEnd
+            }
+
+            if (isset($this->db->dataCache['db_names'])) {
+                $this->db->dataCache['db_names'][] = $dbName;
+            }
+
+            return true;
+        } catch (Throwable $e) {
+            if ($this->db->DBDebug) {
+                throw new DatabaseException('Unable to create the specified database.', 0, $e);
+            }
+
+            return false; // @codeCoverageIgnore
+        }
+    }
+
     /**
      * CREATE TABLE attributes
      */

diff --git a/system/Test/Mock/MockConnection.php b/system/Test/Mock/MockConnection.php
@@ -23,6 +23,9 @@
  */
 class MockConnection extends BaseConnection
 {
+    /**
+     * @var array{connect?: mixed, execute?: bool|object}
+     */
     protected $returnValues = [];
 
     /**
@@ -84,7 +87,7 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s
         $query->setDuration($startTime);
 
         // resultID is not false, so it must be successful
-        if ($query->isWriteType()) {
+        if ($query->isWriteType($sql)) {
             return true;
         }
 

diff --git a/tests/system/Database/BaseConnectionTest.php b/tests/system/Database/BaseConnectionTest.php
@@ -283,4 +283,54 @@ public static function provideProtectIdentifiers(): iterable
             ],
         ];
     }
+
+    /**
+     * These tests are intended to confirm the current behavior.
+     *
+     * @dataProvider provideEscapeIdentifiers
+     */
+    public function testEscapeIdentifiers(string $item, string $expected): void
+    {
+        $db = new MockConnection($this->options);
+
+        $return = $db->escapeIdentifiers($item);
+
+        $this->assertSame($expected, $return);
+    }
+
+    /**
+     * @return array<string, list<string>>
+     */
+    public static function provideEscapeIdentifiers(): iterable
+    {
+        yield from [
+            // $item, $expected
+            'simple'    => ['test', '"test"'],
+            'with dots' => ['com.sitedb.web', '"com"."sitedb"."web"'],
+        ];
+    }
+
+    /**
+     * @dataProvider provideEscapeIdentifier
+     */
+    public function testEscapeIdentifier(string $item, string $expected): void
+    {
+        $db = new MockConnection($this->options);
+
+        $return = $db->escapeIdentifier($item);
+
+        $this->assertSame($expected, $return);
+    }
+
+    /**
+     * @return array<string, list<string>>
+     */
+    public static function provideEscapeIdentifier(): iterable
+    {
+        yield from [
+            // $item, $expected
+            'simple'    => ['test', '"test"'],
+            'with dots' => ['com.sitedb.web', '"com.sitedb.web"'],
+        ];
+    }
 }
diff --git a/tests/system/Database/Live/ForgeTest.php b/tests/system/Database/Live/ForgeTest.php
@@ -54,40 +54,89 @@ public function testCreateDatabase(): void
         if ($this->db->DBDriver === 'OCI8') {
             $this->markTestSkipped('OCI8 does not support create database.');
         }
+
         $databaseCreated = $this->forge->createDatabase('test_forge_database');
 
         $this->assertTrue($databaseCreated);
     }
 
+    public function testCreateDatabaseWithDots(): void
+    {
+        if ($this->db->DBDriver === 'OCI8') {
+            $this->markTestSkipped('OCI8 does not support create database.');
+        }
+
+        $dbName = 'test_com.sitedb.web';
+
+        $databaseCreated = $this->forge->createDatabase($dbName);
+
+        $this->assertTrue($databaseCreated);
+
+        // Checks if tableExists() works.
+        $config             = config(Database::class)->{$this->DBGroup};
+        $config['database'] = $dbName;
+        $db                 = db_connect($config);
+        $result             = $db->tableExists('not_exist');
+
+        $this->assertFalse($result);
+
+        $db->close();
+        if ($this->db->DBDriver !== 'SQLite3') {
+            $this->forge->dropDatabase($dbName);
+        }
+    }
+
     public function testCreateDatabaseIfNotExists(): void
     {
         if ($this->db->DBDriver === 'OCI8') {
             $this->markTestSkipped('OCI8 does not support create database.');
         }
+
         $dbName = 'test_forge_database_exist';
 
         $databaseCreateIfNotExists = $this->forge->createDatabase($dbName, true);
+
+        $this->assertTrue($databaseCreateIfNotExists);
+
         if ($this->db->DBDriver !== 'SQLite3') {
             $this->forge->dropDatabase($dbName);
         }
-
-        $this->assertTrue($databaseCreateIfNotExists);
     }
 
     public function testCreateDatabaseIfNotExistsWithDb(): void
     {
         if ($this->db->DBDriver === 'OCI8') {
             $this->markTestSkipped('OCI8 does not support create database.');
         }
+
         $dbName = 'test_forge_database_exist';
 
         $this->forge->createDatabase($dbName);
         $databaseExists = $this->forge->createDatabase($dbName, true);
+
+        $this->assertTrue($databaseExists);
+
         if ($this->db->DBDriver !== 'SQLite3') {
             $this->forge->dropDatabase($dbName);
         }
+    }
+
+    public function testCreateDatabaseIfNotExistsWithDbWithDots(): void
+    {
+        if ($this->db->DBDriver === 'OCI8') {
+            $this->markTestSkipped('OCI8 does not support create database.');
+        }
+
+        $dbName = 'test_forge.database.exist';
+
+        $this->forge->createDatabase($dbName);
+        $databaseExists = $this->forge->createDatabase($dbName, true);
 
         $this->assertTrue($databaseExists);
+
+        if ($this->db->DBDriver !== 'SQLite3') {
+            $this->forge->dropDatabase($dbName);
+        }
     }
 
     public function testDropDatabase(): void
@@ -106,17 +155,14 @@ public function testDropDatabase(): void
 
     public function testCreateDatabaseExceptionNoCreateStatement(): void
     {
-        $this->setPrivateProperty($this->forge, 'createDatabaseStr', false);
+        if ($this->db->DBDriver !== 'OCI8') {
+            $this->markTestSkipped($this->db->DBDriver . ' does support drop database.');
+        }
 
-        if ($this->db->DBDriver === 'SQLite3') {
-            $databaseCreated = $this->forge->createDatabase('test_forge_database');
-            $this->assertTrue($databaseCreated);
-        } else {
-            $this->expectException(DatabaseException::class);
-            $this->expectExceptionMessage('This feature is not available for the database you are using.');
+        $this->expectException(DatabaseException::class);
+        $this->expectExceptionMessage('This feature is not available for the database you are using.');
 
-            $this->forge->createDatabase('test_forge_database');
-        }
+        $this->forge->createDatabase('test_forge_database');
     }
 
     public function testDropDatabaseExceptionNoDropStatement(): void

diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst
@@ -72,6 +72,8 @@ Forge
 Others
 ------
 
+- Added support for database names containing dots (``.``).
+
 Model
 =====
 

diff --git a/user_guide_src/source/database/configuration.rst b/user_guide_src/source/database/configuration.rst
@@ -149,7 +149,8 @@ Explanation of Values
 **password**     The password used to connect to the database. (``SQLite3`` does not use this.)
 **database**     The name of the database you want to connect to.
 
-                 .. note:: CodeIgniter doesn't support dots (``.``) in the database, table, and column names.
+                 .. note:: CodeIgniter doesn't support dots (``.``) in the table and column names.
+                    Since v4.5.0, database names with dots are supported.
 **DBDriver**     The database driver name. The case must match the driver name.
                  You can set a fully qualified classname to use your custom driver.
                  Supported drivers: ``MySQLi``, ``Postgre``, ``SQLite3``, ``SQLSRV``, and ``OCI8``.

diff --git a/user_guide_src/source/database/examples.rst b/user_guide_src/source/database/examples.rst
@@ -6,7 +6,8 @@ The following page contains example code showing how the database class
 is used. For complete details please read the individual pages
 describing each function.
 
-.. note:: CodeIgniter doesn't support dots (``.``) in the database, table, and column names.
+.. note:: CodeIgniter doesn't support dots (``.``) in the table and column names.
+    Since v4.5.0, database names with dots are supported.
 
 .. contents::
     :local:

diff --git a/user_guide_src/source/database/queries.rst b/user_guide_src/source/database/queries.rst
@@ -10,7 +10,8 @@ Queries
 Query Basics
 ************
 
-.. note:: CodeIgniter doesn't support dots (``.``) in the database, table, and column names.
+.. note:: CodeIgniter doesn't support dots (``.``) in the table and column names.
+    Since v4.5.0, database names with dots are supported.
 
 Regular Queries
 ===============