Add unified cake_migrations table support with BC autodetect#965
Add unified cake_migrations table support with BC autodetect#965
Conversation
This change introduces a consolidated migration tracking approach for v5.0: **New Features:** - `cake_migrations` table: Single table with `plugin` column to track all migrations - `UnifiedMigrationsTableStorage`: New storage class for unified table operations - `migrations upgrade` command: Migrates data from legacy phinxlog tables **Backward Compatibility:** - Autodetect mode (default): If any `phinxlog` or `*_phinxlog` table exists, legacy mode is used automatically - no breaking changes on upgrade - Fresh installations automatically use the new `cake_migrations` table **Configuration:** - `Migrations.legacyTables = null` (default): Autodetect - `Migrations.legacyTables = false`: Force unified table - `Migrations.legacyTables = true`: Force legacy phinxlog tables **Upgrade workflow:** 1. Upgrade to v5.0 (existing apps continue working with phinxlog) 2. Run `bin/cake migrations upgrade` to migrate data 3. Set `Migrations.legacyTables = false` in config 4. Application now uses unified `cake_migrations` table Refs #822 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
| */ | ||
| protected function isUsingUnifiedTable(): bool | ||
| { | ||
| $config = Configure::read('Migrations.legacyTables'); |
There was a problem hiding this comment.
This should be added to the documentation and the upgrade guide, as we'll need a guide so folks know how to upgrade and rollback between schema if necessary.
- Simplify NULL handling using 'plugin IS' => $this->plugin pattern - Remove cache complexity from UtilTrait - Replace empty() with explicit === [] check - Simplify upgradeTable() to no-op for new table format - Use in_array for phinxlog detection (simpler than loop) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Uses the new hasTable() method from CakePHP 5.3 as suggested in the PR feedback. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add LEGACY_TABLES CI matrix option to test both modes - Use schemaDialect()->hasTable() for phinxlog detection - Add documentation for unified table upgrade process - Add basic test for UnifiedMigrationsTableStorage - Update bootstrap to read LEGACY_TABLES from environment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
When `Migrations.legacyTables` is set to `false`, the upgrade command is not needed since the user has already opted into the unified table. Only show it when in autodetect mode (null) or legacy mode (true). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
I tested it on dereuromark/cakephp-sandbox#99 A rerun with also cleans out the old empty ones. Added LEGACY_TABLES=false matrix option for MySQL PHP 8.3 |
The unified table feature works, but tests have extensive hardcoded phinxlog assumptions (99 failures). Removing CI matrix for now. Partial fixes to BakeMigrationDiffCommandTest to show the pattern: - Add UtilTrait to get correct schema table name - Update table cleanup to include both table types - Update queries to use dynamic table name TODO: Follow-up PR needed to update all tests for both table modes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add helper methods to TestCase for migration table operations: - getMigrationsTableName() - gets correct table name based on mode - clearMigrationRecords() - clears records with plugin filtering - getMigrationRecordCount() - counts records with plugin filtering - insertMigrationRecord() - inserts with correct structure - isUsingUnifiedTable() - checks current mode - Update command tests to use new helper methods: - MigrateCommandTest - RollbackCommandTest - StatusCommandTest - MarkMigratedTest - DumpCommandTest - BakeMigrationDiffCommandTest - Add cleanup for both phinxlog and cake_migrations tables - Re-add LEGACY_TABLES=false CI matrix option 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Check Migrations.legacyTables config to use the correct table name. When set to false, use the new 'cake_migrations' table instead of 'phinxlog'. See: cakephp/migrations#965 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update Util::tableName() to return cake_migrations when unified table mode is enabled - Update Manager::cleanupMissingMigrations() to use correct table name and filter by plugin - Skip cake_migrations table in bake snapshot/diff commands (like phinxlog tables) - Update TableFinder to skip cake_migrations table - Update Migrator to not drop cake_migrations during table cleanup - Update tests to use helper methods for migration record operations - Add mode-aware assertions in adapter tests - Skip some Migrator tests in unified mode that test legacy-specific behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Mark Story <mark@mark-story.com>
Co-authored-by: Mark Story <mark@mark-story.com>
Co-authored-by: Mark Story <mark@mark-story.com>
Co-authored-by: Mark Story <mark@mark-story.com>
Co-authored-by: Mark Story <mark@mark-story.com>
Co-authored-by: Mark Story <mark@mark-story.com>
Co-authored-by: Mark Story <mark@mark-story.com>
I thought the conditional logic in Manager, and unwrapping decorators pointed to a missing abstraction method. Since we're in a major anyways, I could extend the interface and have better layering as well.
Table creation we broken for everything except mysql. Now it isn't.
Tests fail without this directory being around.
|
@dereuromark Do you know what is up with the tests? They don't fail for me locally. |
|
This one lock file is super annoying to me too I will take another quick look |
The checkSync() method compares the last migration file version against the last migrated version. After the test deletes the migration record from the phinxlog table, the migration file still exists, causing checkSync() to fail because it sees an unmigrated file. The fix deletes the migration file after running migrate and before baking the diff, so checkSync() passes correctly.
|
claude fixed it: Summary Root Cause: The runDiffBakingTest() method was failing because the checkSync() method in BakeMigrationDiffCommand checks if the last migration file version matches the last migrated version. The test was:
After step 3, the migration FILE still existed with version 20160415220805, but the last migrated version was now 20160414193900 (the previous migration). The checkSync() method saw this discrepancy and failed with "Your migrations history is not in sync with your migrations files." Fix: Added unlink($destination) after deleting the migration record from the database. This removes the migration file so that checkSync() sees the last file as version 20160414193900, which matches the last migrated version. |
|
Thanks for figuring out what was going on there. I wasn't able to reproduce locally.
I don't see the |
Summary
This PR introduces a consolidated migration tracking approach for v5.0, addressing #822.
Note: this is just one possible approach that combines BC with a clear forwards-approach.
@markstory I am still curious about your approach and where it could be a better fit.
New Features
cake_migrationstable: Single table withplugincolumn to track all migrations (app + plugins)UnifiedMigrationsTableStorage: New storage class for unified table operationsmigrations upgradecommand: Migrates data from legacy phinxlog tables to the new unified tableBackward Compatibility (Zero Breaking Changes)
phinxlogor*_phinxlogtable exists, legacy mode is used automaticallycake_migrationstableConfiguration
Upgrade Workflow (for existing apps wanting to migrate)
bin/cake migrations upgrade --dry-runto previewbin/cake migrations upgradeto migrate data'Migrations' => ['legacyTables' => false]in configcake_migrationstableNew Table Schema
Test Plan
legacyTables = truein bootstrapTODO