Skip to content

Commit 73d4ac8

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 73d4ac8

File tree

2 files changed

+79
-3
lines changed

2 files changed

+79
-3
lines changed

src/Command/BakeMigrationDiffCommand.php

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,12 @@ 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
@@ -492,11 +498,34 @@ protected function checkSync(): bool
492498
return true;
493499
}
494500

495-
if ($this->migratedItems) {
496-
$lastVersion = $this->migratedItems[0]['version'];
501+
if ($this->migratedItems && $this->migrationsFiles) {
502+
// Extract version numbers from current context's migration files
503+
$fileVersions = [];
504+
foreach ($this->migrationsFiles as $file) {
505+
$filename = basename($file);
506+
if (preg_match('/^(\d+)_/', $filename, $matches)) {
507+
$fileVersions[] = $matches[1];
508+
}
509+
}
510+
511+
// Filter migrated versions to only those that have corresponding files in this context
512+
$contextMigratedVersions = [];
513+
foreach ($this->migratedItems as $item) {
514+
if (in_array((string)$item['version'], $fileVersions, true)) {
515+
$contextMigratedVersions[] = (string)$item['version'];
516+
}
517+
}
518+
519+
if (empty($contextMigratedVersions)) {
520+
// No migrations from this context have been run yet
521+
return false;
522+
}
523+
524+
// Get the most recent migrated version within this context
525+
$lastVersion = max($contextMigratedVersions);
497526
$lastFile = end($this->migrationsFiles);
498527

499-
return $lastFile && str_contains($lastFile, (string)$lastVersion);
528+
return $lastFile && str_contains($lastFile, $lastVersion);
500529
}
501530

502531
return false;

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)