diff --git a/src/Command/BakeMigrationDiffCommand.php b/src/Command/BakeMigrationDiffCommand.php index d0490daf..920af937 100644 --- a/src/Command/BakeMigrationDiffCommand.php +++ b/src/Command/BakeMigrationDiffCommand.php @@ -33,6 +33,7 @@ use Cake\Event\Event; use Cake\Event\EventManager; use Error; +use Migrations\Db\Adapter\UnifiedMigrationsTableStorage; use Migrations\Migration\ManagerFactory; use Migrations\Util\TableFinder; use Migrations\Util\UtilTrait; @@ -169,13 +170,18 @@ protected function setup(Arguments $args): void $migratedItems = []; if ($tableExists) { - $query = $connection->selectQuery(); - /** @var array $migratedItems */ - $migratedItems = $query + $query = $connection->selectQuery() ->select(['version']) ->from($this->phinxTable) - ->orderBy(['version DESC']) - ->execute()->fetchAll('assoc'); + ->orderBy(['version DESC']); + + // In unified table mode, filter by the current plugin context + if ($this->phinxTable === UnifiedMigrationsTableStorage::TABLE_NAME) { + $query->where(['plugin IS' => $this->plugin]); + } + + /** @var array $migratedItems */ + $migratedItems = $query->execute()->fetchAll('assoc'); } $this->migratedItems = $migratedItems; @@ -484,22 +490,56 @@ protected function getIndexes(): void /** * Checks that the migrations history is in sync with the migrations files * + * Compares the last migrated version against the last migration file, + * but only considers migrations that belong to the current context (app or plugin). + * This avoids false positives when migrations from other plugins have been + * run more recently than the last app migration. + * + * @see https://github.com/cakephp/migrations/issues/1060 * @return bool Whether migrations history is sync or not */ protected function checkSync(): bool { + // No files and no migrations - nothing to check if (!$this->migrationsFiles && !$this->migratedItems) { return true; } - if ($this->migratedItems) { - $lastVersion = $this->migratedItems[0]['version']; - $lastFile = end($this->migrationsFiles); + // No files in this context - nothing to sync + // This allows baking a snapshot for a new plugin even when + // the unified migrations table has records from other contexts + if (!$this->migrationsFiles) { + return true; + } - return $lastFile && str_contains($lastFile, (string)$lastVersion); + // Have files - need to verify they're synced + // Extract version numbers from current context's migration files + $fileVersions = []; + foreach ($this->migrationsFiles as $file) { + $filename = basename($file); + if (preg_match('/^(\d+)_/', $filename, $matches)) { + $fileVersions[] = $matches[1]; + } + } + + // Filter migrated versions to only those that have corresponding files in this context + $contextMigratedVersions = []; + foreach ($this->migratedItems as $item) { + if (in_array((string)$item['version'], $fileVersions, true)) { + $contextMigratedVersions[] = (string)$item['version']; + } } - return false; + if (empty($contextMigratedVersions)) { + // No migrations from this context have been run yet + return false; + } + + // Get the most recent migrated version within this context + $lastVersion = max($contextMigratedVersions); + $lastFile = end($this->migrationsFiles); + + return $lastFile && str_contains($lastFile, $lastVersion); } /** diff --git a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php index 2dc4d388..963571e1 100644 --- a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php +++ b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php @@ -142,6 +142,53 @@ public function testHistoryNotInSync() $this->assertExitCode(BaseCommand::CODE_ERROR); } + /** + * Tests that baking a diff succeeds when app migrations are in sync + * even if a plugin has more recent migrations. + * + * This test specifically tests the unified table mode where all migrations + * (app and plugins) are stored in a single cake_migrations table. + * + * @see https://github.com/cakephp/migrations/issues/1060 + * @return void + */ + public function testCheckSyncWithPluginMigrationsMoreRecent(): void + { + // This bug only affects unified table mode + Configure::write('Migrations.legacyTables', false); + + // Run app migrations first to get them in sync + $this->exec('migrations migrate -c test --no-lock'); + $this->assertExitSuccess(); + + // Create a schema dump file (required for baking a diff) + $this->exec('migrations dump -c test'); + $this->assertExitSuccess(); + + // Insert a more recent migration record for a plugin + // This simulates having a plugin migration that was run after the last app migration + $this->insertMigrationRecord('test', 99999999999999, 'PluginMigration', 'SomePlugin'); + + // Now try to bake a diff for the app - this should succeed + // because all app migration files have been migrated + $this->exec('bake migration_diff CheckSyncTest -c test'); + + // Should not contain the "not in sync" error + $this->assertOutputNotContains('Your migrations history is not in sync'); + $this->assertExitSuccess(); + + // Clean up generated files + $path = ROOT . DS . 'config' . DS . 'Migrations' . DS; + $this->generatedFiles = array_merge( + $this->generatedFiles, + glob($path . '*_CheckSyncTest.php') ?: [], + ); + $this->generatedFiles[] = $path . 'schema-dump-test.lock'; + + // Restore legacy table mode + Configure::delete('Migrations.legacyTables'); + } + /** * Tests that baking a diff while history is empty and no migration files exists * will fall back to baking a snapshot