Skip to content

Commit b7d34d5

Browse files
committed
Fix checkSync() to only consider migrations from current context
When using the unified `cake_migrations` table, `BakeMigrationDiffCommand::checkSync()` was comparing the last migrated version (from ALL contexts including plugins) against the last migration file in the current context (app or specific plugin). This caused false "not in sync" errors when a plugin had more recent migrations than the app being baked. The fix filters migrated versions to only consider those that have corresponding files in the current context before comparing. Fixes #1060
1 parent e485847 commit b7d34d5

File tree

2 files changed

+86
-5
lines changed

2 files changed

+86
-5
lines changed

src/Command/BakeMigrationDiffCommand.php

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -484,22 +484,56 @@ protected function getIndexes(): void
484484
/**
485485
* Checks that the migrations history is in sync with the migrations files
486486
*
487+
* Compares the last migrated version against the last migration file,
488+
* but only considers migrations that belong to the current context (app or plugin).
489+
* This avoids false positives when migrations from other plugins have been
490+
* run more recently than the last app migration.
491+
*
492+
* @see https://github.com/cakephp/migrations/issues/1060
487493
* @return bool Whether migrations history is sync or not
488494
*/
489495
protected function checkSync(): bool
490496
{
497+
// No files and no migrations - nothing to check
491498
if (!$this->migrationsFiles && !$this->migratedItems) {
492499
return true;
493500
}
494501

495-
if ($this->migratedItems) {
496-
$lastVersion = $this->migratedItems[0]['version'];
497-
$lastFile = end($this->migrationsFiles);
502+
// No files in this context - nothing to sync
503+
// This allows baking a snapshot for a new plugin even when
504+
// the unified migrations table has records from other contexts
505+
if (!$this->migrationsFiles) {
506+
return true;
507+
}
508+
509+
// Have files - need to verify they're synced
510+
// Extract version numbers from current context's migration files
511+
$fileVersions = [];
512+
foreach ($this->migrationsFiles as $file) {
513+
$filename = basename($file);
514+
if (preg_match('/^(\d+)_/', $filename, $matches)) {
515+
$fileVersions[] = $matches[1];
516+
}
517+
}
518+
519+
// Filter migrated versions to only those that have corresponding files in this context
520+
$contextMigratedVersions = [];
521+
foreach ($this->migratedItems as $item) {
522+
if (in_array((string)$item['version'], $fileVersions, true)) {
523+
$contextMigratedVersions[] = (string)$item['version'];
524+
}
525+
}
498526

499-
return $lastFile && str_contains($lastFile, (string)$lastVersion);
527+
if (empty($contextMigratedVersions)) {
528+
// No migrations from this context have been run yet
529+
return false;
500530
}
501531

502-
return false;
532+
// Get the most recent migrated version within this context
533+
$lastVersion = max($contextMigratedVersions);
534+
$lastFile = end($this->migrationsFiles);
535+
536+
return $lastFile && str_contains($lastFile, $lastVersion);
503537
}
504538

505539
/**

tests/TestCase/Command/BakeMigrationDiffCommandTest.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,53 @@ public function testHistoryNotInSync()
142142
$this->assertExitCode(BaseCommand::CODE_ERROR);
143143
}
144144

145+
/**
146+
* Tests that baking a diff succeeds when app migrations are in sync
147+
* even if a plugin has more recent migrations.
148+
*
149+
* This test specifically tests the unified table mode where all migrations
150+
* (app and plugins) are stored in a single cake_migrations table.
151+
*
152+
* @see https://github.com/cakephp/migrations/issues/1060
153+
* @return void
154+
*/
155+
public function testCheckSyncWithPluginMigrationsMoreRecent(): void
156+
{
157+
// This bug only affects unified table mode
158+
Configure::write('Migrations.legacyTables', false);
159+
160+
// Run app migrations first to get them in sync
161+
$this->exec('migrations migrate -c test --no-lock');
162+
$this->assertExitSuccess();
163+
164+
// Create a schema dump file (required for baking a diff)
165+
$this->exec('migrations dump -c test');
166+
$this->assertExitSuccess();
167+
168+
// Insert a more recent migration record for a plugin
169+
// This simulates having a plugin migration that was run after the last app migration
170+
$this->insertMigrationRecord('test', 99999999999999, 'PluginMigration', 'SomePlugin');
171+
172+
// Now try to bake a diff for the app - this should succeed
173+
// because all app migration files have been migrated
174+
$this->exec('bake migration_diff CheckSyncTest -c test');
175+
176+
// Should not contain the "not in sync" error
177+
$this->assertOutputNotContains('Your migrations history is not in sync');
178+
$this->assertExitSuccess();
179+
180+
// Clean up generated files
181+
$path = ROOT . DS . 'config' . DS . 'Migrations' . DS;
182+
$this->generatedFiles = array_merge(
183+
$this->generatedFiles,
184+
glob($path . '*_CheckSyncTest.php') ?: [],
185+
);
186+
$this->generatedFiles[] = $path . 'schema-dump-test.lock';
187+
188+
// Restore legacy table mode
189+
Configure::delete('Migrations.legacyTables');
190+
}
191+
145192
/**
146193
* Tests that baking a diff while history is empty and no migration files exists
147194
* will fall back to baking a snapshot

0 commit comments

Comments
 (0)