Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support database name with dots #8664

Merged
merged 12 commits into from
Mar 28, 2024
10 changes: 0 additions & 10 deletions phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions system/Database/BaseConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
19 changes: 16 additions & 3 deletions system/Database/Forge.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand Down Expand Up @@ -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.');
}
Expand All @@ -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]);
}
Expand Down
2 changes: 1 addition & 1 deletion system/Database/MySQLi/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
51 changes: 50 additions & 1 deletion system/Database/SQLSRV/Forge.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
*/
Expand Down
5 changes: 4 additions & 1 deletion system/Test/Mock/MockConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
*/
class MockConnection extends BaseConnection
{
/**
* @var array{connect?: mixed, execute?: bool|object}
*/
protected $returnValues = [];

/**
Expand Down Expand Up @@ -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;
}

Expand Down
50 changes: 50 additions & 0 deletions tests/system/Database/BaseConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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"'],
];
}
}
68 changes: 57 additions & 11 deletions tests/system/Database/Live/ForgeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.5.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ Forge
Others
------

- Added support for database names containing dots (``.``).

Model
=====

Expand Down
2 changes: 1 addition & 1 deletion user_guide_src/source/database/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ 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.
**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``.
Expand Down
2 changes: 1 addition & 1 deletion user_guide_src/source/database/examples.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ 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.

.. contents::
:local:
Expand Down
2 changes: 1 addition & 1 deletion user_guide_src/source/database/queries.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ 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.

Regular Queries
===============
Expand Down
Loading
Loading