cakephp / migrations

CakePHP database migrations plugin
Other
137 stars 115 forks source link

Allow behavior of getMigrations() method in CakeManager to be modified or overridden #740

Open zejji opened 3 weeks ago

zejji commented 3 weeks ago

Description

We maintain a large CakePHP 4.4 application with many hundreds of thousands of lines of code. In recent years, we have increasingly moved towards a trunk-based development model, i.e.:

Overall, trunk-based development works well for us. However, the one problem we have with trunk-based development when using CakePHP relates to migrations.

Specifically, we want to be able to only run certain migrations if a related feature flag is enabled.

We have tried using feature flags within the up/down/change methods themselves, but this does not work, because the migration itself is still marked as migrated once bin/cake migrations migrate has been run. This leads to the migration not being run again when the feature flag is ultimately enabled.

Furthermore, it's not possible for us to override the Phinx Manager class which contains the getMigrations method, or the CakeManager class which overrides it, as these classes are instantiated via the new keyword. In other words, dependency injection is not used within the CakePHP framework code - it is tightly coupled to the concrete Manager and CakeManager classes.

Please could you consider providing a mechanism by which the behavior of the getMigrations() method in CakeManager can be modified/overridden/extended.

CakePHP Version

4.4

markstory commented 2 weeks ago

Having a custom migration manager that skips migrations without running them could cause issues elsewhere in the plugin as there are some assumptions around migrations being applied in the order they are defined/added.

Making the manager created through DI could be an option with the new built-in backend implementation that will be available for cake5 applications, but there aren't any plans to backport that to 4.x compatible migrations releases.

Specifically, we want to be able to only run certain migrations if a related feature flag is enabled.

Why? additive schema changes can be done without feature flags, and destructive schema changes are generally not compatible with feature flag based development as feature flags can be turned on an off outside of deploys and thus you would need schema that is compatible with both sides of each feature flag.

zejji commented 2 weeks ago

@markstory - Thanks for getting back to me.

In answer to your question why we would only want to run certain migrations if a feature flag is enabled - this is because some of our migrations add 'system data' as opposed to making schema changes. By 'system data' I mean data that is required in all environments in order for the application to run.

Where data is added by a migration, such changes are generally not backwards-compatible and will affect the operation of the system, which is why we want to be able to defer the addition of the data until the associated feature flag is enabled. An example of such 'system data' might be default permissions associated with a new feature. We have many hundreds of such migrations in our system over the years.

When we write such migrations, we can easily design them in such a way that it does not matter if the feature flag is enabled much later than the original time of creating the migration. Furthermore, we have automated test pipelines that can run the migrations with both the feature enabled and disabled, ensuring that both options work correctly.

Update:

Just to clarify - for our purposes it would be sufficient to have the ability to either: (i) make migrations that require the feature flag invisible to the migrations commands; or (ii) skip such migrations without making them invisible.

markstory commented 2 weeks ago

In answer to your question why we would only want to run certain migrations if a feature flag is enabled - this is because some of our migrations add 'system data' as opposed to making schema changes. By 'system data' I mean data that is required in all environments in order for the application to run.

Interesting, I've not worked on a system that operated this way. Having feature flagged code manipulate existing application configuration sounds challenging.

make migrations that require the feature flag invisible to the migrations commands

I can think of two possible solutions with existing behavior that might cover your scenario:

  1. Migrations have a shouldExecute hook method that you could use to 'skip' migrations that don't have feature flags enabled. If a migration returns false from shouldExecute it takes no action and prints out a message noting that the migration was skipped.
  2. You could use multiple 'sources'. Within config/Migrations you can create additional directories of migrations for your feature flags. For example you could have config/Migrations/BlueSidebar/20240828123045_add_stuff_for_blue_bar.php. Once the migration is ready to be run it can be moved to config/Migrations to be run as part of the mainline migration history. Would that give you enough control to make migrations that require feature flags invisible? You would only run migrations for the feature flags that you have migrations enabled.
zejji commented 2 weeks ago

@markstory - The shouldExecute hook in particular sounds as though it would be very useful!

This would give the flexibility to handle feature flags as well as other scenarios requiring conditional migrations.

For example, our codebase is also deployed to multiple sites, some of which require different 'system data' migrations. Again, these could be handled by a shouldExecute hook. This would be neater than having to apply an environment check in both the up and down methods.

markstory commented 1 week ago

Cool. Let me know how it goes using shouldExecute(). It is existing behavior, and should work today.

zejji commented 1 week ago

@markstory - Many thanks.

Do you happen to know when shouldExecute was added? We're currently on CakePHP 4.3 (upgrading to 4.4 imminently).

markstory commented 1 week ago

shouldExecute was added to phinx in cakephp/phinx@da9474e9a63937a59f2c8c0f0b3d8ce065d381b6 which was first part of 0.13.0