Skip to content

Commit d57c816

Browse files
kenjiskenjis
authored
Merge pull request #6869 from paulbalandan/refactor-metadata-test
Improve testing for `MetadataTest`
2 parents cef02c8 + 925406d commit d57c816

File tree

2 files changed

+85
-65
lines changed

2 files changed

+85
-65
lines changed

‎system/Database/BaseConnection.php

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,20 +1334,21 @@ protected function getDriverFunctionPrefix(): string
13341334
/**
13351335
* Returns an array of table names
13361336
*
1337-
* @return array|bool
1337+
* @return array|false
13381338
*
13391339
* @throws DatabaseException
13401340
*/
13411341
public function listTables(bool $constrainByPrefix = false)
13421342
{
1343-
// Is there a cached result?
13441343
if (isset($this->dataCache['table_names']) && $this->dataCache['table_names']) {
1345-
return $constrainByPrefix ?
1346-
preg_grep("/^{$this->DBPrefix}/", $this->dataCache['table_names'])
1344+
return $constrainByPrefix
1345+
? preg_grep("/^{$this->DBPrefix}/", $this->dataCache['table_names'])
13471346
: $this->dataCache['table_names'];
13481347
}
13491348

1350-
if (false === ($sql = $this->_listTables($constrainByPrefix))) {
1349+
$sql = $this->_listTables($constrainByPrefix);
1350+
1351+
if ($sql === false) {
13511352
if ($this->DBDebug) {
13521353
throw new DatabaseException('This feature is not available for the database you are using.');
13531354
}
@@ -1360,24 +1361,9 @@ public function listTables(bool $constrainByPrefix = false)
13601361
$query = $this->query($sql);
13611362

13621363
foreach ($query->getResultArray() as $row) {
1363-
// Do we know from which column to get the table name?
1364-
if (! isset($key)) {
1365-
if (isset($row['table_name'])) {
1366-
$key = 'table_name';
1367-
} elseif (isset($row['TABLE_NAME'])) {
1368-
$key = 'TABLE_NAME';
1369-
} else {
1370-
/* We have no other choice but to just get the first element's key.
1371-
* Due to array_shift() accepting its argument by reference, if
1372-
* E_STRICT is on, this would trigger a warning. So we'll have to
1373-
* assign it first.
1374-
*/
1375-
$key = array_keys($row);
1376-
$key = array_shift($key);
1377-
}
1378-
}
1364+
$table = $row['table_name'] ?? $row['TABLE_NAME'] ?? $row[array_key_first($row)];
13791365

1380-
$this->dataCache['table_names'][] = $row[$key];
1366+
$this->dataCache['table_names'][] = $table;
13811367
}
13821368

13831369
return $this->dataCache['table_names'];

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

Lines changed: 77 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace CodeIgniter\Database\Live;
1313

14-
use CodeIgniter\Database\Forge;
1514
use CodeIgniter\Test\CIUnitTestCase;
1615
use CodeIgniter\Test\DatabaseTestTrait;
1716
use Config\Database;
@@ -26,22 +25,22 @@ final class MetadataTest extends CIUnitTestCase
2625
{
2726
use DatabaseTestTrait;
2827

29-
private ?Forge $forge = null;
30-
protected $refresh = true;
31-
protected $seed = CITestSeeder::class;
32-
3328
/**
34-
* Array of expected tables.
29+
* The seed file used for all tests within this test case.
30+
*
31+
* @var string
3532
*/
36-
private array $expectedTables;
33+
protected $seed = CITestSeeder::class;
34+
35+
private array $expectedTables = [];
3736

3837
protected function setUp(): void
3938
{
4039
parent::setUp();
4140

42-
// Prepare the array of expected tables once
43-
$prefix = $this->db->getPrefix();
44-
$this->expectedTables = [
41+
$prefix = $this->db->getPrefix();
42+
43+
$tables = [
4544
$prefix . 'migrations',
4645
$prefix . 'user',
4746
$prefix . 'job',
@@ -55,55 +54,90 @@ protected function setUp(): void
5554
];
5655

5756
if (in_array($this->db->DBDriver, ['MySQLi', 'Postgre'], true)) {
58-
$this->expectedTables[] = $prefix . 'ci_sessions';
57+
$tables[] = $prefix . 'ci_sessions';
5958
}
59+
60+
sort($tables);
61+
$this->expectedTables = $tables;
6062
}
6163

62-
public function testListTables()
64+
private function createExtraneousTable(): void
6365
{
64-
$result = $this->db->listTables(true);
66+
$oldPrefix = $this->db->getPrefix();
67+
$this->db->setPrefix('tmp_');
68+
69+
Database::forge($this->DBGroup)
70+
->addField([
71+
'name' => ['type' => 'varchar', 'constraint' => 31],
72+
'created_at' => ['type' => 'datetime', 'null' => true],
73+
])
74+
->createTable('widgets');
6575

66-
$this->assertSame($this->expectedTables, array_values($result));
76+
$this->db->setPrefix($oldPrefix);
6777
}
6878

69-
public function testListTablesConstrainPrefix()
79+
private function dropExtraneousTable(): void
7080
{
71-
$result = $this->db->listTables(true);
81+
$oldPrefix = $this->db->getPrefix();
82+
$this->db->setPrefix('tmp_');
83+
84+
Database::forge($this->DBGroup)->dropTable('widgets');
7285

73-
$this->assertSame($this->expectedTables, array_values($result));
86+
$this->db->setPrefix($oldPrefix);
7487
}
7588

76-
public function testConstrainPrefixIgnoresOtherTables()
89+
public function testListTablesUnconstrainedByPrefixReturnsAllTables()
7790
{
78-
$this->forge = Database::forge($this->DBGroup);
91+
try {
92+
$this->createExtraneousTable();
7993

80-
// Stash the prefix and change it
81-
$DBPrefix = $this->db->getPrefix();
82-
$this->db->setPrefix('tmp_');
94+
$tables = $this->db->listTables();
95+
$this->assertIsArray($tables);
96+
$this->assertNotSame([], $tables);
8397

84-
// Create a table with the new prefix
85-
$fields = [
86-
'name' => [
87-
'type' => 'varchar',
88-
'constraint' => 31,
89-
],
90-
'created_at' => [
91-
'type' => 'datetime',
92-
'null' => true,
93-
],
94-
];
95-
$this->forge->addField($fields);
96-
$this->forge->createTable('widgets');
98+
$expectedTables = $this->expectedTables;
99+
$expectedTables[] = 'tmp_widgets';
97100

98-
// Restore the prefix and get the tables with the original prefix
99-
$this->db->setPrefix($DBPrefix);
100-
$result = $this->db->listTables(true);
101+
sort($tables);
102+
$this->assertSame($expectedTables, array_values($tables));
103+
} finally {
104+
$this->dropExtraneousTable();
105+
}
106+
}
101107

102-
$this->assertSame($this->expectedTables, array_values($result));
108+
public function testListTablesConstrainedByPrefixReturnsOnlyTablesWithMatchingPrefix()
109+
{
110+
try {
111+
$this->createExtraneousTable();
103112

104-
// Clean up temporary table
105-
$this->db->setPrefix('tmp_');
106-
$this->forge->dropTable('widgets');
107-
$this->db->setPrefix($DBPrefix);
113+
$tables = $this->db->listTables(true);
114+
$this->assertIsArray($tables);
115+
$this->assertNotSame([], $tables);
116+
117+
sort($tables);
118+
$this->assertSame($this->expectedTables, array_values($tables));
119+
} finally {
120+
$this->dropExtraneousTable();
121+
}
122+
}
123+
124+
public function testListTablesConstrainedByExtraneousPrefixReturnsOnlyTheExtraneousTable()
125+
{
126+
try {
127+
$this->createExtraneousTable();
128+
129+
$oldPrefix = $this->db->getPrefix();
130+
$this->db->setPrefix('tmp_');
131+
132+
$tables = $this->db->listTables(true);
133+
$this->assertIsArray($tables);
134+
$this->assertNotSame([], $tables);
135+
136+
sort($tables);
137+
$this->assertSame(['tmp_widgets'], array_values($tables));
138+
} finally {
139+
$this->db->setPrefix($oldPrefix);
140+
$this->dropExtraneousTable();
141+
}
108142
}
109143
}

0 commit comments

Comments
 (0)