From c6f2db387c627336a1c7be81f84d86f38ee0de13 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Mon, 14 Sep 2026 11:31:38 +0300 Subject: [PATCH 1/2] Load migration classes only when they are executed Checking migration status (status command, PendingMigrationsMiddleware, the test suite Migrator) previously required every migration class to be loaded and instantiated. Migration versions and names are now derived from the file names, and a migration class is only loaded when the migration is about to be executed, rolled back or have its breakpoint changed. Adds Manager::getMigrationVersions() and Manager::getMigration(). getMigrations() still loads all migrations for backwards compatibility. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EeBHdY3PAMKrNXQnogCUsQ --- .../PendingMigrationsMiddleware.php | 10 +- src/Migration/Manager.php | 358 ++++++++++++------ tests/TestCase/Migration/ManagerTest.php | 53 +++ 3 files changed, 291 insertions(+), 130 deletions(-) diff --git a/src/Middleware/PendingMigrationsMiddleware.php b/src/Middleware/PendingMigrationsMiddleware.php index 8c2ba542..da9ca621 100644 --- a/src/Middleware/PendingMigrationsMiddleware.php +++ b/src/Middleware/PendingMigrationsMiddleware.php @@ -111,9 +111,8 @@ protected function checkAppMigrations(): bool $manager = $this->getManager($this->_config); - $migrations = $manager->getMigrations(); - foreach ($migrations as $migration) { - if (!$manager->isMigrated($migration->getVersion())) { + foreach ($manager->getMigrationVersions() as $version) { + if (!$manager->isMigrated($version)) { return false; } } @@ -146,9 +145,8 @@ protected function checkPluginMigrations(string $plugin): bool $config['environment']['migration_table'] = $table; $manager = $this->getManager($config); - $migrations = $manager->getMigrations(); - foreach ($migrations as $migration) { - if (!$manager->isMigrated($migration->getVersion())) { + foreach ($manager->getMigrationVersions() as $version) { + if (!$manager->isMigrated($version)) { return false; } } diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 65ef382f..923a2d74 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -39,6 +39,20 @@ class Manager */ protected ?array $migrations = null; + /** + * Migration file paths indexed by version and sorted in ascending order. + * + * @var array|null + */ + protected ?array $migrationPaths = null; + + /** + * Migrations that have been loaded from their files, indexed by version. + * + * @var array + */ + protected array $loadedMigrations = []; + /** * @var \Migrations\SeedInterface[]|null */ @@ -67,27 +81,24 @@ public function printStatus(?string $format = null): array { $migrations = []; $isJson = $format === 'json'; - $defaultMigrations = $this->getMigrations(); - if ($defaultMigrations) { + $defaultVersions = $this->getMigrationVersions(); + if ($defaultVersions) { $env = $this->getEnvironment(); $versions = $env->getVersionLog(); - foreach ($defaultMigrations as $migration) { - if (array_key_exists($migration->getVersion(), $versions)) { + foreach ($defaultVersions as $version) { + if (array_key_exists($version, $versions)) { $status = 'up'; - unset($versions[$migration->getVersion()]); + unset($versions[$version]); } else { $status = 'down'; } - $version = $migration->getVersion(); - $migrationParams = [ + $migrations[$version] = [ 'status' => $status, - 'id' => $migration->getVersion(), - 'name' => $migration->getName(), + 'id' => $version, + 'name' => $this->getMigrationName($version), ]; - - $migrations[$version] = $migrationParams; } foreach ($versions as $missing) { @@ -123,8 +134,7 @@ public function printStatus(?string $format = null): array */ public function migrateToDateTime(DateTime $dateTime, bool $fake = false): void { - /** @var array $versions */ - $versions = array_keys($this->getMigrations()); + $versions = $this->getMigrationVersions(); $dateString = $dateTime->format('Ymdhis'); $versionToMigrate = null; foreach ($versions as $version) { @@ -344,8 +354,7 @@ protected function getMigrationClassName(string $path): string */ public function getVersionsToMark(Arguments $args): array { - $migrations = $this->getMigrations(); - $versions = array_keys($migrations); + $versions = $this->getMigrationVersions(); $versionArg = null; if ($args->hasArgument('version')) { @@ -434,19 +443,19 @@ public function markVersionsAsMigrated(string $path, array $versions): array */ public function migrate(?int $version = null, bool $fake = false, ?int $count = null): void { - $migrations = $this->getMigrations(); + $migrationVersions = $this->getMigrationVersions(); $env = $this->getEnvironment(); $versions = $env->getVersions(); $current = $env->getCurrentVersion(); - if (!$versions && !$migrations) { + if (!$versions && !$migrationVersions) { return; } if ($version === null) { - $candidates = [...$versions, ...array_keys($migrations)]; + $candidates = [...$versions, ...$migrationVersions]; $version = $candidates ? max($candidates) : 0; - } elseif ($version !== 0 && !isset($migrations[$version])) { + } elseif ($version !== 0 && !in_array($version, $migrationVersions, true)) { $this->getIo()->out(sprintf( 'warning %s is not a valid version', $version, @@ -460,27 +469,25 @@ public function migrate(?int $version = null, bool $fake = false, ?int $count = if ($direction === MigrationInterface::DOWN) { // run downs first - krsort($migrations); - foreach ($migrations as $migration) { - if ($migration->getVersion() <= $version) { + foreach (array_reverse($migrationVersions) as $migrationVersion) { + if ($migrationVersion <= $version) { break; } - if (in_array($migration->getVersion(), $versions)) { - $this->executeMigration($migration, MigrationInterface::DOWN, $fake); + if (in_array($migrationVersion, $versions)) { + $this->executeMigration($this->getMigration($migrationVersion), MigrationInterface::DOWN, $fake); } } } - ksort($migrations); $done = 0; - foreach ($migrations as $migration) { - if ($migration->getVersion() > $version || ($count && $done >= $count)) { + foreach ($migrationVersions as $migrationVersion) { + if ($migrationVersion > $version || ($count && $done >= $count)) { break; } - if (!in_array($migration->getVersion(), $versions)) { - $this->executeMigration($migration, MigrationInterface::UP, $fake); + if (!in_array($migrationVersion, $versions)) { + $this->executeMigration($this->getMigration($migrationVersion), MigrationInterface::UP, $fake); $done++; } } @@ -690,14 +697,14 @@ public function rollbackByCount(int $count, bool $force = false, bool $fake = fa */ public function rollback(int|string|null $target = null, bool $force = false, bool $targetMustMatchVersion = true, bool $fake = false): void { - // note that the migrations are indexed by name (aka creation time) in ascending order - $migrations = $this->getMigrations(); + // note that the migration versions (aka creation time) are sorted in ascending order + $migrationVersions = array_flip($this->getMigrationVersions()); // note that the version log are also indexed by name with the proper ascending order according to the version order $executedVersions = $this->getEnvironment()->getVersionLog(); - // get a list of migrations sorted in the opposite way of the executed versions - $sortedMigrations = []; + // get a list of migration versions sorted in the opposite way of the executed versions + $sortedVersions = []; $io = $this->getIo(); foreach ($executedVersions as $versionCreationTime => &$executedVersion) { @@ -709,8 +716,8 @@ public function rollback(int|string|null $target = null, bool $force = false, bo $executedVersion['start_time'] = $dateTime->format('YmdHis'); } - if (isset($migrations[$versionCreationTime])) { - array_unshift($sortedMigrations, $migrations[$versionCreationTime]); + if (isset($migrationVersions[$versionCreationTime])) { + array_unshift($sortedVersions, $versionCreationTime); } else { // this means the version is missing so we unset it so that we don't consider it when rolling back // migrations (or choosing the last up version as target) @@ -753,7 +760,7 @@ public function rollback(int|string|null $target = null, bool $force = false, bo } // If the target must match a version, check the target version exists - if ($targetMustMatchVersion && $target !== 0 && !isset($migrations[$target])) { + if ($targetMustMatchVersion && $target !== 0 && !isset($migrationVersions[$target])) { $io->out(sprintf('Target version (%s) not found', $target)); return; @@ -762,13 +769,13 @@ public function rollback(int|string|null $target = null, bool $force = false, bo // Rollback all versions until we find the wanted rollback target $rollbacked = false; - foreach ($sortedMigrations as $migration) { - if ($targetMustMatchVersion && $migration->getVersion() == $target) { + foreach ($sortedVersions as $migrationVersion) { + if ($targetMustMatchVersion && $migrationVersion == $target) { break; } - if (in_array($migration->getVersion(), $executedVersionCreationTimes)) { - $executedArray = $executedVersions[$migration->getVersion()]; + if (in_array($migrationVersion, $executedVersionCreationTimes)) { + $executedArray = $executedVersions[$migrationVersion]; if (!$targetMustMatchVersion && ($this->getConfig()->isVersionOrderCreationTime() && $executedArray['version'] <= $target || !$this->getConfig()->isVersionOrderCreationTime() && $executedArray['start_time'] <= $target)) { break; @@ -778,7 +785,7 @@ public function rollback(int|string|null $target = null, bool $force = false, bo $io->out('Breakpoint reached. Further rollbacks inhibited.'); break; } - $this->executeMigration($migration, MigrationInterface::DOWN, $fake); + $this->executeMigration($this->getMigration((int)$migrationVersion), MigrationInterface::DOWN, $fake); $rollbacked = true; } } @@ -908,105 +915,204 @@ public function setMigrations(array $migrations) * Gets an array of the database migrations, indexed by migration name (aka creation time) and sorted in ascending * order * + * This loads every migration class. Prefer getMigrationVersions() when the migration instances are not needed. + * * @throws \InvalidArgumentException * @return \Migrations\MigrationInterface[] */ public function getMigrations(): array { - if ($this->migrations === null) { - $phpFiles = $this->getMigrationFiles(); + if ($this->migrations !== null) { + return $this->migrations; + } - $io = $this->getIo(); - $io->verbose('Migration file'); - $io->verbose( - array_map( - function (string $phpFile): string { - return sprintf(' %s', $phpFile); - }, - $phpFiles, - ), - ); + $migrations = []; + foreach ($this->getMigrationVersions() as $version) { + $migrations[$version] = $this->getMigration($version); + } - // filter the files to only get the ones that match our naming scheme - $fileNames = []; - /** @var \Migrations\MigrationInterface[] $versions */ - $versions = []; + return $migrations; + } - $io = $this->getIo(); - foreach ($phpFiles as $filePath) { - if (Util::isValidMigrationFileName(basename($filePath))) { - $io->verbose(sprintf('Valid migration file %s.', $filePath)); + /** + * Gets the versions of the database migrations sorted in ascending order, without loading the migration classes. + * + * @throws \InvalidArgumentException + * @return list + */ + public function getMigrationVersions(): array + { + if ($this->migrations !== null) { + $versions = array_keys($this->migrations); + sort($versions); - $version = Util::getVersionFromFileName(basename($filePath)); + return $versions; + } - if (isset($versions[$version])) { - throw new InvalidArgumentException(sprintf('Duplicate migration - "%s" has the same version as "%s"', $filePath, $versions[$version]->getVersion())); - } + return array_keys($this->getMigrationPaths()); + } - // convert the filename to a class name - $class = Util::mapFileNameToClassName(basename($filePath)); + /** + * Gets a single database migration, loading its class if it has not been loaded yet. + * + * @param int $version Version of the migration + * @throws \InvalidArgumentException + * @return \Migrations\MigrationInterface + */ + public function getMigration(int $version): MigrationInterface + { + if ($this->migrations !== null) { + if (!isset($this->migrations[$version])) { + throw new InvalidArgumentException(sprintf('Migration `%d` was not found', $version)); + } - if (isset($fileNames[$class])) { - throw new InvalidArgumentException(sprintf( - 'Migration "%s" has the same name as "%s"', - basename($filePath), - $fileNames[$class], - )); - } + return $this->migrations[$version]; + } - $fileNames[$class] = basename($filePath); + if (!isset($this->loadedMigrations[$version])) { + $paths = $this->getMigrationPaths(); + if (!isset($paths[$version])) { + throw new InvalidArgumentException(sprintf('Migration `%d` was not found', $version)); + } - $io->verbose(sprintf('Loading class %s from %s.', $class, $filePath)); + $this->loadedMigrations[$version] = $this->loadMigration($version, $paths[$version]); + } - $this->checkMigrationClass($filePath); + return $this->loadedMigrations[$version]; + } - $orig_display_errors_setting = ini_get('display_errors'); - ini_set('display_errors', 'On'); + /** + * Gets the name of a database migration without loading its class. + * + * @param int $version Version of the migration + * @return string + */ + protected function getMigrationName(int $version): string + { + if ($this->migrations !== null) { + return $this->getMigration($version)->getName(); + } - // For anonymous classes, we need to use require instead of require_once - // to get the returned instance - $migrationInstance = null; - if (!class_exists($class)) { - $migrationInstance = require $filePath; - } else { - require_once $filePath; - } + return Util::mapFileNameToClassName(basename($this->getMigrationPaths()[$version])); + } - ini_set('display_errors', $orig_display_errors_setting); + /** + * Gets the paths of the migration files indexed by version and sorted in ascending order + * + * @throws \InvalidArgumentException + * @return array + */ + protected function getMigrationPaths(): array + { + if ($this->migrationPaths !== null) { + return $this->migrationPaths; + } - // Check if the file returns an anonymous class instance - if ($migrationInstance instanceof MigrationInterface) { - $io->verbose(sprintf('Using anonymous class from %s.', $filePath)); - $migration = $migrationInstance; - $migration->setVersion($version); - } elseif (class_exists($class)) { - // Fall back to traditional class-based migration - $io->verbose(sprintf('Constructing %s.', $class)); - $migration = new $class($version); - } else { - throw new InvalidArgumentException(sprintf( - 'Could not find class `%s` in file `%s` and file did not return a migration instance', - $class, - $filePath, - )); - } + $phpFiles = $this->getMigrationFiles(); - /** @var \Migrations\MigrationInterface $migration */ - $config = $this->getConfig(); - $migration->setConfig($config); - $migration->setIo($io); + $io = $this->getIo(); + $io->verbose('Migration file'); + $io->verbose( + array_map( + function (string $phpFile): string { + return sprintf(' %s', $phpFile); + }, + $phpFiles, + ), + ); - $versions[$version] = $migration; - } else { - $io->verbose(sprintf('Invalid migration file %s.', $filePath)); - } + // filter the files to only get the ones that match our naming scheme + $fileNames = []; + $paths = []; + + foreach ($phpFiles as $filePath) { + if (!Util::isValidMigrationFileName(basename($filePath))) { + $io->verbose(sprintf('Invalid migration file %s.', $filePath)); + continue; } - ksort($versions); - $this->setMigrations($versions); + $io->verbose(sprintf('Valid migration file %s.', $filePath)); + + $version = Util::getVersionFromFileName(basename($filePath)); + + if (isset($paths[$version])) { + throw new InvalidArgumentException(sprintf('Duplicate migration - "%s" has the same version as "%s"', $filePath, $version)); + } + + // convert the filename to a class name + $class = Util::mapFileNameToClassName(basename($filePath)); + + if (isset($fileNames[$class])) { + throw new InvalidArgumentException(sprintf( + 'Migration "%s" has the same name as "%s"', + basename($filePath), + $fileNames[$class], + )); + } + + $fileNames[$class] = basename($filePath); + $paths[$version] = $filePath; } - return (array)$this->migrations; + ksort($paths); + $this->migrationPaths = $paths; + + return $paths; + } + + /** + * Loads a migration class from its file and creates the migration instance. + * + * @param int $version Version of the migration + * @param string $filePath Path to the migration file + * @throws \InvalidArgumentException + * @return \Migrations\MigrationInterface + */ + protected function loadMigration(int $version, string $filePath): MigrationInterface + { + $io = $this->getIo(); + $class = Util::mapFileNameToClassName(basename($filePath)); + + $io->verbose(sprintf('Loading class %s from %s.', $class, $filePath)); + + $this->checkMigrationClass($filePath); + + $orig_display_errors_setting = ini_get('display_errors'); + ini_set('display_errors', 'On'); + + // For anonymous classes, we need to use require instead of require_once + // to get the returned instance + $migrationInstance = null; + if (!class_exists($class)) { + $migrationInstance = require $filePath; + } else { + require_once $filePath; + } + + ini_set('display_errors', $orig_display_errors_setting); + + // Check if the file returns an anonymous class instance + if ($migrationInstance instanceof MigrationInterface) { + $io->verbose(sprintf('Using anonymous class from %s.', $filePath)); + $migration = $migrationInstance; + $migration->setVersion($version); + } elseif (class_exists($class)) { + // Fall back to traditional class-based migration + $io->verbose(sprintf('Constructing %s.', $class)); + $migration = new $class($version); + } else { + throw new InvalidArgumentException(sprintf( + 'Could not find class `%s` in file `%s` and file did not return a migration instance', + $class, + $filePath, + )); + } + + /** @var \Migrations\MigrationInterface $migration */ + $migration->setConfig($this->getConfig()); + $migration->setIo($io); + + return $migration; } /** @@ -1277,11 +1383,11 @@ public function toggleBreakpoint(?int $version): void */ protected function markBreakpoint(?int $version, int $mark): void { - $migrations = $this->getMigrations(); + $migrationVersions = array_flip($this->getMigrationVersions()); $env = $this->getEnvironment(); $versions = $env->getVersionLog(); - if (!$versions || !$migrations) { + if (!$versions || !$migrationVersions) { return; } @@ -1291,7 +1397,7 @@ protected function markBreakpoint(?int $version, int $mark): void } $io = $this->getIo(); - if ($version !== 0 && (!isset($versions[$version]) || !isset($migrations[$version]))) { + if ($version !== 0 && (!isset($versions[$version]) || !isset($migrationVersions[$version]))) { $io->out(sprintf( 'warning %s is not a valid version', $version, @@ -1300,18 +1406,20 @@ protected function markBreakpoint(?int $version, int $mark): void return; } + $migration = $this->getMigration((int)$version); + switch ($mark) { case self::BREAKPOINT_TOGGLE: - $env->getAdapter()->toggleBreakpoint($migrations[$version]); + $env->getAdapter()->toggleBreakpoint($migration); break; case self::BREAKPOINT_SET: if ((int)$versions[$version]['breakpoint'] === 0) { - $env->getAdapter()->setBreakpoint($migrations[$version]); + $env->getAdapter()->setBreakpoint($migration); } break; case self::BREAKPOINT_UNSET: if ((int)$versions[$version]['breakpoint'] === 1) { - $env->getAdapter()->unsetBreakpoint($migrations[$version]); + $env->getAdapter()->unsetBreakpoint($migration); } break; } @@ -1321,7 +1429,7 @@ protected function markBreakpoint(?int $version, int $mark): void $io->out( ' Breakpoint ' . ($versions[$version]['breakpoint'] ? 'set' : 'cleared') . ' for ' . $version . '' . - ' ' . $migrations[$version]->getName() . '', + ' ' . $migration->getName() . '', ); } @@ -1368,6 +1476,8 @@ public function unsetBreakpoint(?int $version): void public function resetMigrations(): void { $this->migrations = null; + $this->migrationPaths = null; + $this->loadedMigrations = []; } /** @@ -1404,7 +1514,7 @@ public function getSchemaTableName(): string */ public function cleanupMissingMigrations(): int { - $defaultMigrations = $this->getMigrations(); + $migrationVersions = array_flip($this->getMigrationVersions()); $env = $this->getEnvironment(); $versions = $env->getVersionLog(); $adapter = $env->getAdapter(); @@ -1412,7 +1522,7 @@ public function cleanupMissingMigrations(): int // Find missing migrations (those in migration table but not in filesystem) $missingVersions = []; foreach (array_keys($versions) as $versionId) { - if (!isset($defaultMigrations[$versionId])) { + if (!isset($migrationVersions[$versionId])) { $missingVersions[] = $versionId; } } diff --git a/tests/TestCase/Migration/ManagerTest.php b/tests/TestCase/Migration/ManagerTest.php index 8d88b575..bb42b832 100644 --- a/tests/TestCase/Migration/ManagerTest.php +++ b/tests/TestCase/Migration/ManagerTest.php @@ -665,6 +665,59 @@ public function testGetMigrationsWithAnonymousClass(): void $this->assertEquals(20241208150000, $migration->getVersion()); } + public function testGetMigrationVersionsDoesNotLoadMigrations(): void + { + // Loading this migration throws, so the versions must come from the file names only. + $config = new Config(['paths' => ['migrations' => ROOT . '/config/LegacyAbstractMigration']]); + $manager = new Manager($config, $this->io); + + $this->assertSame([20260327000000], $manager->getMigrationVersions()); + } + + public function testPrintStatusDoesNotLoadMigrations(): void + { + $config = new Config(['paths' => ['migrations' => ROOT . '/config/LegacyAbstractMigration']]); + $manager = new Manager($config, $this->io); + + $envStub = $this->getMockBuilder(Environment::class) + ->setConstructorArgs(['mockenv', []]) + ->getMock(); + $envStub->expects($this->once()) + ->method('getVersionLog') + ->willReturn([]); + $manager->setEnvironment($envStub); + + $expected = [ + [ + 'status' => 'down', + 'id' => 20260327000000, + 'name' => 'LegacyAbstractMigration', + ], + ]; + $this->assertEquals($expected, $manager->printStatus()); + } + + public function testMigrateDoesNotLoadExecutedMigrations(): void + { + $config = new Config(['paths' => ['migrations' => ROOT . '/config/LegacyAbstractMigration']]); + $manager = new Manager($config, $this->io); + + $envStub = $this->getMockBuilder(Environment::class) + ->setConstructorArgs(['mockenv', []]) + ->getMock(); + $envStub->expects($this->any()) + ->method('getVersions') + ->willReturn([20260327000000]); + $envStub->expects($this->any()) + ->method('getCurrentVersion') + ->willReturn(20260327000000); + $envStub->expects($this->never()) + ->method('executeMigration'); + $manager->setEnvironment($envStub); + + $manager->migrate(); + } + public function testGettingAValidEnvironment(): void { $this->assertInstanceOf( From 410b1d94090eb52ed89cbe733b0b8df2137fc813 Mon Sep 17 00:00:00 2001 From: Nicos Panayides Date: Wed, 16 Sep 2026 08:59:05 +0300 Subject: [PATCH 2/2] Add an explicit way to validate migration files Migration classes are now only loaded when they are executed, so a file that cannot be loaded is no longer reported by status checks until the migration runs. Adds Manager::validateMigrations(), which loads every migration class and returns the error messages indexed by version, and exposes it as `bin/cake migrations status --validate`. The command exits with 1 and prints the offending versions when a migration cannot be loaded, so CI can validate every migration while status checks and the middleware stay fast. The option can be combined with --all to cover the app and every loaded plugin. Co-Authored-By: Claude Opus 5 --- .../running-and-managing-migrations.md | 21 +++++++ src/Command/StatusCommand.php | 60 +++++++++++++++++++ src/Migration/Manager.php | 25 ++++++++ tests/TestCase/Command/CompletionTest.php | 3 +- tests/TestCase/Command/StatusCommandTest.php | 32 ++++++++++ tests/TestCase/Migration/ManagerTest.php | 17 ++++++ 6 files changed, 157 insertions(+), 1 deletion(-) diff --git a/docs/en/getting-started/running-and-managing-migrations.md b/docs/en/getting-started/running-and-managing-migrations.md index 7c51a4f9..c26ca7cb 100644 --- a/docs/en/getting-started/running-and-managing-migrations.md +++ b/docs/en/getting-started/running-and-managing-migrations.md @@ -90,6 +90,27 @@ in CI. e.g. `{"app": [...], "PluginName": [...]}`. `--all` cannot be combined with `--plugin` or `--cleanup`. +### Validating Migration Files + +Migration classes are only loaded when the migration they contain is executed. +A migration file that cannot be loaded, for example one still extending the +removed `Migrations\AbstractMigration` class, will therefore not fail `status` +or the `PendingMigrationsMiddleware`. The `--validate` option loads every +migration class and reports the ones that cannot be loaded: + +```bash +bin/cake migrations status --validate +``` + +When any migration cannot be loaded, the offending versions are printed to +stderr and the command exits with `1`, which makes it a useful CI check. +Otherwise the regular status output follows. The option can be combined with +`--all` to validate the app and every loaded plugin in one call. + +The same check is available programmatically through +`Manager::validateMigrations()`, which returns the error messages indexed by +migration version. + ### Cleaning Up Missing Migrations Sometimes migration files may be deleted from the filesystem but still exist in diff --git a/src/Command/StatusCommand.php b/src/Command/StatusCommand.php index 4161d64f..87a587fb 100644 --- a/src/Command/StatusCommand.php +++ b/src/Command/StatusCommand.php @@ -20,6 +20,7 @@ use Cake\Core\Plugin; use Migrations\Config\ConfigInterface; use Migrations\Db\Adapter\UnifiedMigrationsTableStorage; +use Migrations\Migration\Manager; use Migrations\Migration\ManagerFactory; /** @@ -78,6 +79,8 @@ protected function buildOptionParser(ConsoleOptionParser $parser): ConsoleOption 'Add -v to also print the per-section migration tables.', 'migrations status --cleanup', 'Remove *MISSING* migrations from the migration tracking table', + 'migrations status --validate', + 'Load every migration class and fail if any of them cannot be loaded.', ])->addOption('plugin', [ 'short' => 'p', 'help' => 'The plugin to run migrations for', @@ -104,6 +107,11 @@ protected function buildOptionParser(ConsoleOptionParser $parser): ConsoleOption 'help' => 'Remove MISSING migrations from the migration tracking table', 'boolean' => true, 'default' => false, + ])->addOption('validate', [ + 'help' => 'Load every migration class and fail if any of them cannot be loaded. ' + . 'Migration classes are otherwise only loaded when they are executed.', + 'boolean' => true, + 'default' => false, ]); return $parser; @@ -146,6 +154,20 @@ public function execute(Arguments $args, ConsoleIo $io): ?int ]); $manager = $factory->createManager($io); + if ($args->getOption('validate')) { + /** @var string|null $plugin */ + $plugin = $args->getOption('plugin'); + if (!$this->validateMigrations($manager, $io, $plugin ?? 'app')) { + return Command::CODE_ERROR; + } + if ($format !== 'json') { + $io->out(sprintf( + 'All %d migrations can be loaded.', + count($manager->getMigrationVersions()), + )); + } + } + if ($clean) { $removed = $manager->cleanupMissingMigrations(); if ($removed === 0) { @@ -200,6 +222,8 @@ protected function executeAll(Arguments $args, ConsoleIo $io, ?string $format): } $verbose = (bool)$args->getOption('verbose'); + $validate = (bool)$args->getOption('validate'); + $validationFailed = false; $jsonResults = []; $summary = []; $exitCode = Command::CODE_SUCCESS; @@ -212,6 +236,11 @@ protected function executeAll(Arguments $args, ConsoleIo $io, ?string $format): 'dry-run' => $args->getOption('dry-run'), ]); $manager = $factory->createManager($io); + + if ($validate && !$this->validateMigrations($manager, $io, $label)) { + $validationFailed = true; + } + $migrations = $manager->printStatus($format); $sectionExit = $this->statusExitCode($migrations); @@ -241,6 +270,10 @@ protected function executeAll(Arguments $args, ConsoleIo $io, ?string $format): $this->display($migrations, $io, $manager->getSchemaTableName()); } + if ($validationFailed) { + return Command::CODE_ERROR; + } + if ($format === 'json') { $flags = 0; if ($verbose) { @@ -256,6 +289,33 @@ protected function executeAll(Arguments $args, ConsoleIo $io, ?string $format): return $exitCode; } + /** + * Load every migration class and print the ones that could not be loaded. + * + * @param \Migrations\Migration\Manager $manager The manager to load migrations with. + * @param \Cake\Console\ConsoleIo $io The console io. + * @param string $label The section the migrations belong to. + * @return bool True when every migration class could be loaded. + */ + protected function validateMigrations(Manager $manager, ConsoleIo $io, string $label): bool + { + $errors = $manager->validateMigrations(); + if (!$errors) { + return true; + } + + $io->err(sprintf( + '%s: %d migration(s) could not be loaded:', + $label === 'app' ? 'APP' : $label, + count($errors), + )); + foreach ($errors as $version => $message) { + $io->err(sprintf(' - %d: %s', $version, $message)); + } + + return false; + } + /** * Count actionable items (down + missing) in a section's migrations array. * diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 923a2d74..fb8cead7 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -19,6 +19,7 @@ use Migrations\Util\Util; use Psr\Container\ContainerInterface; use RuntimeException; +use Throwable; class Manager { @@ -981,6 +982,30 @@ public function getMigration(int $version): MigrationInterface return $this->loadedMigrations[$version]; } + /** + * Loads every migration class and collects the errors that prevent them from being loaded. + * + * Migration classes are loaded when the migration they contain is executed, so a broken + * migration file is only reported when that migration runs. This loads all of them upfront + * so that the migration files can be validated explicitly, for example in CI. + * + * @throws \InvalidArgumentException When two migrations share a version or a name + * @return array Error messages indexed by migration version. + */ + public function validateMigrations(): array + { + $errors = []; + foreach ($this->getMigrationVersions() as $version) { + try { + $this->getMigration($version); + } catch (Throwable $e) { + $errors[$version] = $e->getMessage(); + } + } + + return $errors; + } + /** * Gets the name of a database migration without loading its class. * diff --git a/tests/TestCase/Command/CompletionTest.php b/tests/TestCase/Command/CompletionTest.php index d61686dc..729a8b68 100644 --- a/tests/TestCase/Command/CompletionTest.php +++ b/tests/TestCase/Command/CompletionTest.php @@ -134,7 +134,8 @@ public function testMigrationsOptionsStatus(): void $this->exec('completion options migrations.migrations status'); $this->assertCount(1, $this->_out->messages()); $output = $this->_out->messages()[0]; - $expected = '--all --cleanup --connection -c --format -f --help -h --plugin -p --quiet -q --source -s --verbose -v'; + $expected = '--all --cleanup --connection -c --format -f --help -h --plugin -p --quiet -q --source -s'; + $expected .= ' --validate --verbose -v'; $outputExplode = explode(' ', trim($output)); sort($outputExplode); $expectedExplode = explode(' ', $expected); diff --git a/tests/TestCase/Command/StatusCommandTest.php b/tests/TestCase/Command/StatusCommandTest.php index 929b0707..8dafa101 100644 --- a/tests/TestCase/Command/StatusCommandTest.php +++ b/tests/TestCase/Command/StatusCommandTest.php @@ -182,4 +182,36 @@ public function testAllRejectsCleanupOption(): void $this->assertExitError(); $this->assertErrorContains('cannot be combined with --cleanup'); } + + public function testValidateHelp(): void + { + $this->exec('migrations status --help'); + $this->assertExitSuccess(); + $this->assertOutputContains('--validate'); + $this->assertOutputContains('Load every migration class'); + } + + public function testValidateWithValidMigrations(): void + { + $this->exec('migrations status -c test --validate'); + $this->assertExitSuccess(); + $this->assertOutputContains('migrations can be loaded'); + // The status table is still printed. + $this->assertOutputContains('Migration ID'); + } + + public function testValidateWithMigrationThatCannotBeLoaded(): void + { + $this->exec('migrations status -c test -s LegacyAbstractMigration --validate'); + $this->assertExitError(); + $this->assertErrorContains('could not be loaded'); + $this->assertErrorContains('20260327000000'); + $this->assertOutputNotContains('Migration ID'); + } + + public function testAllValidatesEverySection(): void + { + $this->exec('migrations status -c test --all --validate'); + $this->assertExitCode(StatusCommand::CODE_STATUS_DOWN); + } } diff --git a/tests/TestCase/Migration/ManagerTest.php b/tests/TestCase/Migration/ManagerTest.php index bb42b832..afe07af9 100644 --- a/tests/TestCase/Migration/ManagerTest.php +++ b/tests/TestCase/Migration/ManagerTest.php @@ -718,6 +718,23 @@ public function testMigrateDoesNotLoadExecutedMigrations(): void $manager->migrate(); } + public function testValidateMigrationsReportsMigrationsThatCannotBeLoaded(): void + { + $config = new Config(['paths' => ['migrations' => ROOT . '/config/LegacyAbstractMigration']]); + $manager = new Manager($config, $this->io); + + $errors = $manager->validateMigrations(); + + $this->assertCount(1, $errors); + $this->assertArrayHasKey(20260327000000, $errors); + $this->assertStringContainsString('uses the legacy', $errors[20260327000000]); + } + + public function testValidateMigrationsWithValidMigrations(): void + { + $this->assertSame([], $this->manager->validateMigrations()); + } + public function testGettingAValidEnvironment(): void { $this->assertInstanceOf(