Skip to content

Commit 2bc665a

Browse files
gr8mangr8man
committed
test(Database): fix state leaks and random-order test determinism for MySQLi, PostgreSQL, and OCI8
Fixes database tests to be deterministic under random execution order for MySQLi, PostgreSQL, and OCI8 drivers. Ref #9968. - Fix random-order test execution issues and state leakage under PostgreSQL, MySQL, and OCI8 - Isolate per-component database schemas for parallel random-test runs - Safely extract dbParams keys with null coalescing in Registrar - Drop team_members table in migration down method - Run Oracle components sequentially and exclude Oracle from the random test matrix (single shared schema via DSN causes ORA-00955 collisions) - Handle case-insensitive cached tableExists and catalog lookups in OCI8 - Fix OCI8 insertID() to preserve quoted table name case so Oracle USER_TAB_COLUMNS lookups match forge-created (lowercase) tables
1 parent 514b9f4 commit 2bc665a

20 files changed

Lines changed: 272 additions & 59 deletions

‎.github/scripts/random-tests-config.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Config
1818
Cookie
1919
# DataCaster
2020
# DataConverter
21-
# Database
21+
Database
2222
# Debug
2323
Email
2424
# Encryption

‎.github/workflows/test-random-execution.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ jobs:
7575
- Postgre
7676
- SQLSRV
7777
- SQLite3
78-
- Oracle
78+
# Oracle is excluded: OCI8 uses a single shared schema via DSN
79+
# and cannot be isolated with per-component databases, causing
80+
# ORA-00955 collisions when components run in parallel.
81+
# - Oracle
7982

8083
services:
8184
mysql:
@@ -177,7 +180,7 @@ jobs:
177180
uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2
178181
with:
179182
php-version: ${{ matrix.php-version }}
180-
extensions: gd, curl, iconv, json, mbstring, openssl, sodium
183+
extensions: gd, curl, iconv, json, mbstring, openssl, sodium, mysqli, oci8, pgsql, sqlsrv, sqlite3
181184
ini-values: opcache.enable_cli=0
182185
coverage: none
183186

‎system/Database/BaseConnection.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1720,7 +1720,13 @@ public function listTables(bool $constrainByPrefix = false)
17201720
public function tableExists(string $tableName, bool $cached = true): bool
17211721
{
17221722
if ($cached) {
1723-
return in_array($this->protectIdentifiers($tableName, true, false, false), $this->listTables(), true);
1723+
$tableName = $this->protectIdentifiers($tableName, true, false, false);
1724+
1725+
return in_array(
1726+
strtolower($tableName),
1727+
array_map(strtolower(...), $this->listTables()),
1728+
true,
1729+
);
17241730
}
17251731

17261732
if (false === ($sql = $this->_listTables(false, $tableName))) {

‎system/Database/Forge.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,8 +572,16 @@ public function createTable(string $table, bool $ifNotExists = false, array $att
572572
$sql = $this->_createTable($table, false, $attributes);
573573

574574
if (($result = $this->db->query($sql)) !== false) {
575-
if (isset($this->db->dataCache['table_names']) && ! in_array($table, $this->db->dataCache['table_names'], true)) {
576-
$this->db->dataCache['table_names'][] = $table;
575+
if (isset($this->db->dataCache['table_names'])) {
576+
$exists = in_array(
577+
strtolower($table),
578+
array_map(strtolower(...), $this->db->dataCache['table_names']),
579+
true,
580+
);
581+
582+
if (! $exists) {
583+
$this->db->dataCache['table_names'][] = $table;
584+
}
577585
}
578586

579587
// Most databases don't support creating indexes from within the CREATE TABLE statement

‎system/Database/OCI8/Connection.php‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
use ErrorException;
2121
use stdClass;
2222

23+
defined('OCI_COMMIT_ON_SUCCESS') || define('OCI_COMMIT_ON_SUCCESS', 32);
24+
2325
/**
2426
* Connection for OCI8
2527
*
@@ -155,6 +157,22 @@ public function connect(bool $persistent = false)
155157
: $func($this->username, $this->password, $this->DSN, $this->charset);
156158
}
157159

160+
public function initialize()
161+
{
162+
parent::initialize();
163+
164+
if ($this->connID) {
165+
$this->simpleQuery("ALTER SESSION SET NLS_DATE_FORMAT='YYYY-MM-DD HH24:MI:SS'");
166+
$this->simpleQuery("ALTER SESSION SET NLS_TIMESTAMP_FORMAT='YYYY-MM-DD HH24:MI:SS'");
167+
$this->simpleQuery("ALTER SESSION SET NLS_TIMESTAMP_TZ_FORMAT='YYYY-MM-DD HH24:MI:SS'");
168+
}
169+
}
170+
171+
/**
172+
* Close the database connection.
173+
*
174+
* @return void
175+
*/
158176
protected function _close()
159177
{
160178
if (is_resource($this->cursorId)) {
@@ -264,7 +282,7 @@ public function parseInsertTableName(string $sql): string
264282
return '';
265283
}
266284

267-
preg_match('/(?is)\b(?:into)\s+("?\w+"?)/', $commentStrippedSql, $match);
285+
preg_match('/(?is)\b(?:into)\s+(?:\"?\w+\"?\.)?(\"?\w+\"?)/', $commentStrippedSql, $match);
268286
$tableName = $match[1] ?? '';
269287

270288
return str_starts_with($tableName, '"') ? trim($tableName, '"') : strtoupper($tableName);
@@ -404,7 +422,7 @@ protected function _indexData(string $table): array
404422
$retVal[$row->INDEX_NAME] = new stdClass();
405423
$retVal[$row->INDEX_NAME]->name = $row->INDEX_NAME;
406424
$retVal[$row->INDEX_NAME]->fields = [$row->COLUMN_NAME];
407-
$retVal[$row->INDEX_NAME]->type = $constraintTypes[$row->CONSTRAINT_TYPE] ?? 'INDEX';
425+
$retVal[$row->INDEX_NAME]->type = $constraintTypes[$row->CONSTRAINT_TYPE ?? ''] ?? 'INDEX';
408426
}
409427

410428
return $retVal;
@@ -620,7 +638,7 @@ public function insertID(): int
620638
}
621639

622640
$primaryColumnName = $this->protectIdentifiers($index->fields[0], false, false);
623-
$primaryColumnType = $columnTypeList[$primaryColumnName];
641+
$primaryColumnType = $columnTypeList[$primaryColumnName] ?? $columnTypeList[strtoupper($primaryColumnName)] ?? null;
624642

625643
if ($primaryColumnType !== 'NUMBER') {
626644
$primaryColumnName = '';

‎system/Database/Postgre/Connection.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use PgSql\Result as PgSqlResult;
2323
use stdClass;
2424
use Stringable;
25+
use Throwable;
2526

2627
/**
2728
* Connection for Postgre
@@ -159,15 +160,30 @@ private function convertDSN()
159160

160161
protected function _close()
161162
{
162-
pg_close($this->connID);
163+
if ($this->connID !== false) {
164+
try {
165+
pg_close($this->connID);
166+
} catch (Throwable) {
167+
} finally {
168+
$this->connID = false;
169+
}
170+
}
163171
}
164172

165173
/**
166174
* Ping the database connection.
167175
*/
168176
protected function _ping(): bool
169177
{
170-
return pg_ping($this->connID);
178+
if ($this->connID === false) {
179+
return false;
180+
}
181+
182+
try {
183+
return pg_ping($this->connID);
184+
} catch (Throwable) {
185+
return false;
186+
}
171187
}
172188

173189
/**

‎tests/_support/Config/Registrar.php‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313

1414
namespace Tests\Support\Config;
1515

16+
use mysqli;
17+
use PDO;
18+
use Throwable;
19+
1620
/**
1721
* Class Registrar
1822
*
@@ -137,7 +141,68 @@ public static function Database(): array
137141
// so that we can test against multiple databases.
138142
$group = env('DB', 'SQLite3');
139143

140-
$config['tests'] = self::$dbConfig[$group] ?? [];
144+
if ($group === 'Oracle') {
145+
$group = 'OCI8';
146+
}
147+
148+
$dbParams = self::$dbConfig[$group] ?? [];
149+
150+
if (! empty($dbParams) && ! in_array($group, ['SQLite3', 'OCI8'], true)) {
151+
$componentName = '';
152+
153+
foreach ($_SERVER['argv'] ?? [] as $arg) {
154+
if (str_contains($arg, 'tests/system/')) {
155+
$parts = explode('tests/system/', $arg);
156+
if (isset($parts[1])) {
157+
$componentName = explode('/', $parts[1])[0];
158+
break;
159+
}
160+
}
161+
}
162+
163+
if ($componentName !== '') {
164+
$dbParams['database'] = 'test_' . strtolower($componentName);
165+
166+
try {
167+
if ($group === 'MySQLi') {
168+
$conn = new mysqli(
169+
$dbParams['hostname'],
170+
$dbParams['username'],
171+
$dbParams['password'],
172+
'',
173+
(int) $dbParams['port'],
174+
);
175+
if (! $conn->connect_error) {
176+
$conn->query('CREATE DATABASE IF NOT EXISTS ' . $conn->real_escape_string($dbParams['database']));
177+
$conn->close();
178+
}
179+
} elseif ($group === 'Postgre') {
180+
$dsn = 'pgsql:host=' . $dbParams['hostname'] . ';port=' . $dbParams['port'] . ';user=' . $dbParams['username'] . ';password=' . $dbParams['password'];
181+
$pdo = new PDO($dsn);
182+
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
183+
$stmt = $pdo->prepare('SELECT 1 FROM pg_database WHERE datname = ?');
184+
$stmt->execute([$dbParams['database']]);
185+
if (! $stmt->fetchColumn()) {
186+
$dbName = str_replace('"', '""', $dbParams['database']);
187+
$pdo->exec('CREATE DATABASE "' . $dbName . '"');
188+
}
189+
} elseif ($group === 'SQLSRV') {
190+
$dsn = 'sqlsrv:Server=' . $dbParams['hostname'] . ',' . $dbParams['port'] . ';Encrypt=False;TrustServerCertificate=True';
191+
$pdo = new PDO($dsn, $dbParams['username'], $dbParams['password']);
192+
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
193+
$stmt = $pdo->prepare('SELECT 1 FROM sys.databases WHERE name = ?');
194+
$stmt->execute([$dbParams['database']]);
195+
if (! $stmt->fetchColumn()) {
196+
$pdo->exec('CREATE DATABASE [' . str_replace(']', ']]', $dbParams['database']) . '] COLLATE Latin1_General_100_CS_AS_SC_UTF8');
197+
}
198+
}
199+
} catch (Throwable) {
200+
// Ignore any error and let the connection fail naturally
201+
}
202+
}
203+
}
204+
205+
$config['tests'] = $dbParams;
141206

142207
return $config;
143208
}

‎tests/_support/Database/Migrations/20160428212500_Create_test_tables.php‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace Tests\Support\Database\Migrations;
1515

1616
use CodeIgniter\Database\Migration;
17+
use Throwable;
1718

1819
class Migration_Create_test_tables extends Migration
1920
{
@@ -183,6 +184,7 @@ public function down(): void
183184
$this->forge->dropTable('user', true);
184185
$this->forge->dropTable('job', true);
185186
$this->forge->dropTable('misc', true);
187+
$this->forge->dropTable('team_members', true);
186188
$this->forge->dropTable('type_test', true);
187189
$this->forge->dropTable('empty', true);
188190
$this->forge->dropTable('secondary', true);
@@ -196,9 +198,25 @@ public function down(): void
196198
}
197199

198200
if ($this->db->DBDriver === 'OCI8') {
199-
$this->db->query('DROP PROCEDURE one');
200-
$this->db->query('DROP PROCEDURE plus');
201-
$this->db->query('DROP PACKAGE BODY calculator');
201+
try {
202+
$this->db->query('DROP PROCEDURE one');
203+
} catch (Throwable) {
204+
}
205+
206+
try {
207+
$this->db->query('DROP PROCEDURE plus');
208+
} catch (Throwable) {
209+
}
210+
211+
try {
212+
$this->db->query('DROP PACKAGE BODY calculator');
213+
} catch (Throwable) {
214+
}
215+
216+
try {
217+
$this->db->query('DROP PACKAGE calculator');
218+
} catch (Throwable) {
219+
}
202220
}
203221
}
204222
}

‎tests/system/Database/BaseConnectionTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,4 +504,18 @@ protected function getDriverFunctionPrefix(): string
504504

505505
$this->assertTrue($db->callFunction('contains', 'CodeIgniter', 'Ignite'));
506506
}
507+
508+
public function testTableExistsIsCaseInsensitiveForCachedTables(): void
509+
{
510+
$db = new class ($this->options) extends MockConnection {
511+
public function listTables(bool $constrainByPrefix = false): array
512+
{
513+
return ['test_USER', 'test_JOB'];
514+
}
515+
};
516+
517+
$this->assertTrue($db->tableExists('user', true));
518+
$this->assertTrue($db->tableExists('USER', true));
519+
$this->assertTrue($db->tableExists('test_user', true));
520+
}
507521
}

‎tests/system/Database/Live/ConnectTest.php‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,28 @@ protected function setUp(): void
5757
$this->group2['DBDriver'] = 'Postgre';
5858
}
5959

60+
protected function tearDown(): void
61+
{
62+
parent::tearDown();
63+
$this->setPrivateProperty(Database::class, 'instances', []);
64+
}
65+
6066
public function testConnectWithMultipleCustomGroups(): void
6167
{
68+
$this->group1['DBPrefix'] = uniqid('g1_', true);
69+
$this->group2['DBPrefix'] = uniqid('g2_', true);
70+
6271
// We should have our test database connection already.
63-
$instances = $this->getPrivateProperty(Database::class, 'instances');
64-
$this->assertCount(1, $instances);
72+
$instances = $this->getPrivateProperty(Database::class, 'instances');
73+
$initialCount = count($instances);
6574

6675
$db1 = Database::connect($this->group1);
6776
$db2 = Database::connect($this->group2);
6877

6978
$this->assertNotSame($db1, $db2);
7079

7180
$instances = $this->getPrivateProperty(Database::class, 'instances');
72-
$this->assertCount(3, $instances);
81+
$this->assertCount($initialCount + 2, $instances);
7382
}
7483

7584
public function testConnectReturnsProvidedConnection(): void

0 commit comments

Comments
 (0)