fix: migrations not using custom DB connection of migration runner (#…

…8221) * Add failing test * Add fix * Fix setting of group * Change priority order of migration's db group
codeigniter4 · Dec 13, 2023 · 2420424 · 2420424
1 parent 8f2c065
commit 2420424
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 25 deletions.
diff --git a/phpstan-baseline.php b/phpstan-baseline.php
@@ -1146,11 +1146,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/system/Database/Migration.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Property CodeIgniter\\\\Database\\\\Migration\\:\\:\\$DBGroup \\(string\\) on left side of \\?\\? is not nullable\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/system/Database/Migration.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#',
 	'count' => 8,

diff --git a/system/Database/Migration.php b/system/Database/Migration.php
@@ -21,7 +21,7 @@ abstract class Migration
     /**
      * The name of the database group to use.
      *
-     * @var string
+     * @var string|null
      */
     protected $DBGroup;
 
@@ -39,12 +39,15 @@ abstract class Migration
      */
     protected $forge;
 
-    /**
-     * Constructor.
-     */
     public function __construct(?Forge $forge = null)
     {
-        $this->forge = $forge ?? Database::forge($this->DBGroup ?? config(Database::class)->defaultGroup);
+        if (isset($this->DBGroup)) {
+            $this->forge = Database::forge($this->DBGroup);
+        } elseif ($forge !== null) {
+            $this->forge = $forge;
+        } else {
+            $this->forge = Database::forge(config(Database::class)->defaultGroup);
+        }
 
         $this->db = $this->forge->getConnection();
     }

diff --git a/system/Database/MigrationRunner.php b/system/Database/MigrationRunner.php
@@ -135,16 +135,12 @@ public function __construct(MigrationsConfig $config, $db = null)
         $this->enabled = $config->enabled ?? false;
         $this->table   = $config->table ?? 'migrations';
 
-        // Default name space is the app namespace
         $this->namespace = APP_NAMESPACE;
 
-        // get default database group
-        $config      = config(Database::class);
-        $this->group = $config->defaultGroup;
-        unset($config);
+        // Even if a DB connection is passed, since it is a test,
+        // it is assumed to use the default group name
+        $this->group = is_string($db) ? $db : config(Database::class)->defaultGroup;
 
-        // If no db connection passed in, use
-        // default database group.
         $this->db = db_connect($db);
     }
 
@@ -836,8 +832,9 @@ protected function migrate($direction, $migration): bool
             throw new RuntimeException($message);
         }
 
-        $instance = new $class();
-        $group    = $instance->getDBGroup() ?? config(Database::class)->defaultGroup;
+        /** @var Migration $instance */
+        $instance = new $class(Database::forge($this->db));
+        $group    = $instance->getDBGroup() ?? $this->group;
 
         if (ENVIRONMENT !== 'testing' && $group === 'tests' && $this->groupFilter !== 'tests') {
             // @codeCoverageIgnoreStart
@@ -853,8 +850,6 @@ protected function migrate($direction, $migration): bool
             return true;
         }
 
-        $this->setGroup($group);
-
         if (! is_callable([$instance, $direction])) {
             $message = sprintf(lang('Migrations.missingMethod'), $direction);
 

diff --git a/tests/system/Database/Migrations/MigrationRunnerTest.php b/tests/system/Database/Migrations/MigrationRunnerTest.php
@@ -12,7 +12,6 @@
 namespace CodeIgniter\Database\Migrations;
 
 use CodeIgniter\Database\BaseConnection;
-use CodeIgniter\Database\Config;
 use CodeIgniter\Database\MigrationRunner;
 use CodeIgniter\Events\Events;
 use CodeIgniter\Exceptions\ConfigException;
@@ -454,11 +453,34 @@ public function testGetBatchVersions(): void
         $this->assertSame('2018-01-24-102302', $runner->getBatchEnd(1));
     }
 
-    protected function resetTables(): void
+    public function testMigrationUsesSameConnectionAsMigrationRunner(): void
     {
-        $forge = Config::forge();
+        $config = ['database' => WRITEPATH . 'runner.sqlite', 'DBDriver' => 'SQLite3', 'DBDebug' => true];
 
-        foreach (db_connect()->listTables() as $table) {
+        $database = Database::connect($config, false);
+        $this->resetTables($database);
+
+        $runner = new MigrationRunner(config(Migrations::class), $database);
+        $runner->clearCliMessages();
+        $runner->clearHistory();
+        $runner->setNamespace('Tests\Support\MigrationTestMigrations');
+        $runner->latest();
+
+        $tables = $database->listTables();
+        $this->assertCount(2, $tables);
+        $this->assertSame('migrations', $tables[0]);
+        $this->assertSame('foo', $tables[1]);
+    }
+
+    protected function resetTables($db = null): void
+    {
+        $forge = Database::forge($db);
+
+        /** @var BaseConnection $conn */
+        $conn = $forge->getConnection();
+        $conn->resetDataCache();
+
+        foreach (db_connect($db)->listTables() as $table) {
             $table = str_replace('db_', '', $table);
             $forge->dropTable($table, true);
         }