From fa1720d084a29276162c0db3e4e9895718754ec3 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Wed, 8 Jul 2026 19:39:01 +0200 Subject: [PATCH] Make the database adapter available inside shouldExecute() The adapter was only set on the migration/seed by the Environment, right before running the migration body - after the shouldExecute() gate in the Manager. As a result shouldExecute() could not query the database (calling getAdapter() threw "Adapter not set."), so eligibility could only depend on local application state, not the state of the database. Set the environment's adapter on the migration and seed before the shouldExecute() check. getEnvironment()->getAdapter() is memoized, so this adds no extra connection, and the Environment still (re)sets and wraps the adapter before executing the body, leaving migrate/rollback behavior unchanged. --- .../writing-migrations/migration-methods.md | 23 +++++++++++++++++++ src/Migration/Manager.php | 7 ++++++ tests/TestCase/Migration/ManagerTest.php | 16 +++++++++++++ ...05100_should_execute_with_db_migration.php | 20 ++++++++++++++++ 4 files changed, 66 insertions(+) create mode 100644 tests/test_app/config/ShouldExecuteWithDb/20201207205100_should_execute_with_db_migration.php diff --git a/docs/en/guides/writing-migrations/migration-methods.md b/docs/en/guides/writing-migrations/migration-methods.md index 2ac07c99..b72cad02 100644 --- a/docs/en/guides/writing-migrations/migration-methods.md +++ b/docs/en/guides/writing-migrations/migration-methods.md @@ -101,6 +101,29 @@ migration. This can be used to prevent the migration from being executed at this time. It always returns `true` by default. You can override it in your custom `BaseMigration` implementation. +The database adapter is available inside `shouldExecute()`, so the decision can +be based on the current state of the database (for example the server version or +whether a table already exists): + +```php +hasTable('some_table'); + } + + public function change(): void + { + // ... + } +} +``` + ## Working With Tables The Table object enables you to manipulate database tables using PHP code. You diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 8b011c37..311085d0 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -509,6 +509,10 @@ public function executeMigration(MigrationInterface $migration, string $directio { $this->getIo()->out(''); + // Make the adapter available so shouldExecute() can inspect the database. + // The environment sets it again (and wraps it for down/change) before running the migration body. + $migration->setAdapter($this->getEnvironment()->getAdapter()); + // Skip the migration if it should not be executed if (!$migration->shouldExecute()) { $this->printMigrationStatus($migration, 'skipped'); @@ -540,6 +544,9 @@ public function executeMigration(MigrationInterface $migration, string $directio */ public function executeSeed(SeedInterface $seed, bool $force = false, bool $fake = false): void { + // Make the adapter available so shouldExecute() can inspect the database. + $seed->setAdapter($this->getEnvironment()->getAdapter()); + // Skip the seed if it should not be executed if (!$seed->shouldExecute()) { $this->getIo()->out(''); diff --git a/tests/TestCase/Migration/ManagerTest.php b/tests/TestCase/Migration/ManagerTest.php index 3d7ce3d4..8d88b575 100644 --- a/tests/TestCase/Migration/ManagerTest.php +++ b/tests/TestCase/Migration/ManagerTest.php @@ -2987,4 +2987,20 @@ public function testMigrationWillNotBeExecuted(): void $this->assertTrue($adapter->hasTable('info')); } + + public function testMigrationShouldExecuteCanQueryDatabase(): void + { + $configArray = $this->getConfigArray(); + $adapter = $this->manager->getEnvironment()->getAdapter(); + + // override the migrations directory to use a shouldExecute() that queries the database + $configArray['paths']['migrations'] = ROOT . '/config/ShouldExecuteWithDb/'; + $config = new Config($configArray); + + // shouldExecute() calls getAdapter()->hasTable(); before the fix this threw 'Adapter not set.' + $this->manager->setConfig($config); + $this->manager->migrate(20201207205100); + + $this->assertTrue($adapter->hasTable('info')); + } } diff --git a/tests/test_app/config/ShouldExecuteWithDb/20201207205100_should_execute_with_db_migration.php b/tests/test_app/config/ShouldExecuteWithDb/20201207205100_should_execute_with_db_migration.php new file mode 100644 index 00000000..0f158bda --- /dev/null +++ b/tests/test_app/config/ShouldExecuteWithDb/20201207205100_should_execute_with_db_migration.php @@ -0,0 +1,20 @@ +table('info')->create(); + } + + public function shouldExecute(): bool + { + // Query the database to decide eligibility. Before the fix the adapter + // was not set at this point and getAdapter() threw 'Adapter not set.'. + return !$this->getAdapter()->hasTable('info'); + } +}