[RFC] Aliases for migrations

[rob006]

Migrations names can't be changed after release, because it will break any installation that already applied them. This means that:

• you can never fix typo in migration name,
• you can not migrate from non-namespaced migrations to namespaced migrations,
• if you already use namespaced migration, you can never change namespace of your project or change structure of directories that can affect migrations namespaces.

Basically, migrations can be really PITA for long-term projects, especially if you decide to use namespaced migrations.

I propose to add new static property to yii\console\Migration:

namespace yii\log\migrations;

use yii\db\Migration;

class M141106185632LogInit extends Migration
{
    public static $aliases = [
        'm141106_185632_log_init', // old non-namespaced migrations
        'yii\log\migrations\M141106185632LogInt', // typo
    ];

    // ...
}

On migration/up

1. Check if any of migration aliases are already present in migration history.
2. If it is present - add current migration and all non-applied aliases to migration history with current timestamp and some prefix for name (like -), without running up() method.
3. If it's not applied - run up(), add migration (without prefix for name) and all aliases (with prefix) to history with current timestamp.

On migrate/down

1. Pick last applied migration from history.
2. If name of this migration is prefixed - remove from history without running down(). Back to step 1. (removing prefixed migration from history should not count as revert).
3. If migration does not have alias prefix in history - run down(), remove migration from history, and all related aliases (so if we're reverting m141106_185632_log_init we should also remove yii\log\migrations\M141106185632LogInit and yii\log\migrations\M141106185632LogInt from history).

Tricky part

The obvious problem is finding real migration class for an alias (if we're reverting m141106_185632_log_init we need actually revert yii\log\migrations\M141106185632LogInit). In the worst case we could scan all migrations and use $aliases values to create alias => real-class map. But usually we could avoid this - in most cases we first load a real class, so we could start building map from already loaded migrations and do a full scan only if migration to revert can not be found.

Funding

• You can sponsor this specific effort via a Polar.sh pledge below
• We receive the pledge once the issue is completed & verified

[samdark]

Overall it complicates migrations significantly while the benefit isn't as significant.

Indeed, migration name could not be adjusted but that's fine. That's the nature of it.

[rob006]

In that case using namespaced migrations does not make any sense in most cases - you just can't do anything with it. Non-namespaced migrations can be moved to different directory, and after config adjustment their works fine. But you can't do the same for namespaced migration, since it affects namespace. This limits refactoring a lot.

AFAIK you have a plans to change current non-namespaced migrations shipped with Yii (like m141106_185632_log_init) to namespaced ones (like yii\log\migrations\M141106185632LogInit). This will introduce massive BC break which is nearly impossible to fix in app code.

[samdark]

Why would you refactor placements of migrations?

[rob006]

This is side effect of project refactoring. I keep migrations in modules, and sometimes I need to remove or rename module, so I need also move migrations. I can do this only because I don't use namespaces in migrations, but I still can't fix any typo in migration name.

[samdark]

I didn't think about this aspect of namespaced migrations... @klimov-paul what do you think?

[francislavoie]

I'm giving my +1 to this issue.

I use yii2 to build an on-prem solution for my customers, meaning we install our app on their servers, and they run the migrations. The current setup makes it hard to both support existing applications and new installs when there can be bugs in the migrations related to both. We experimented with writing our own plugin system so that we can enable/disable features for customers who don't need them or aren't paying for them yet, and that includes conditionally loading some migrations. That means the ordering isn't consistent across all our installs, because the plugins might be enabled from the start, where they'll just be naturally ordered by date. But, if the plugin is enabled after the fact, it'll have the migrations from the plugin ordered at the end. That sort of use-case doesn't seem well covered in the current migration system, so any features to help ease that would be greatly appreciated.

I'm using migrationNamespaces to implement the plugin system, but I'm starting to see the downsides of that and that it's quite rigid once it's written. I don't see an easy way to transition out of that if we decide to rename or move things, as Rob said. Aliasing seems like a decent solution for that.

For example, I do this at boot-time of the application if a plugin is enabled:

app()->setModule('pluginname', [
    'class' => '\app\modules\pluginname\Plugin',
]);

app()->controllerMap['migrate']['migrationNamespaces'][] = 'app\modules\pluginname\migrations';

One bug we just recently ran into last week was that some DB operations meant for one of our plugins was mistakenly put in a mainline migration. We can't just delete that migration now, because it's already been run in some places and the name is depended on because of the contents of the migrations table. I ended up having to cut the code from the mainline migration and paste it in one of our existing plugin migrations (the one with the date closest to the mistaken one). While that was a mistake on our part and it's our issue, I think there could use some improvements in the migrations system to help alleviate the pain of those kinds of situations.

[samdark]

You still can do a migration that updates migrations table replacing namespace part but yes, that's a bit hackish.

[klimov-paul]

I can not see a problem with moving namespaced migrations to another directory: in order to make it work all you need to setup a path alias equal to migration namespace to another folder:

Yii::setAlias('@app/migrations', '/path/to/project/new/migrations/path');

[rob006]

@klimov-paul You can't move single migration in this way. It is also more like a hack - your migrations namespace is out of sync with directory structure.

[klimov-paul]

And just how putting aliases into migration class is better?

This 'hack' as you call it - is a basics of class autoloading. Similar hacks are used to replace standard helpers (like Inflector) with custom ones.

[rob006]

Example of changing namespace in migrations: yiisoft/yii2-queue@9edb0a0#diff-035fdbf9bbbbf357e2eeeeee4f737f4f

[klimov-paul]

Modification of the migration history is just the same matter as modification of the GIT history.
Taking about changing particular migration name is the same as changing comment for the GIT commit.
While such changes are possible - they break any existing working copy of the project after being made.

Being published migrations can not adjusted any further as well as published GIT commits can not be modified (squash, hard reset, comment modifications etc.).

You need to keep your migration intact after they being published and can be picked up by other working copies. If you wish to modify them afterwards - you will have to deal with consequences on your own.

[klimov-paul]

Using namespace in migrations you can adjust the path alias to it and move them under different folder, so this feature of non-namespace migration is actually intact. It allows you to restructurize your code while keeping migration history intact.

[rob006]

I don't want to modify my migrations history, I want to modify code of my application (including migrations). I already proposed solution for this with keeping history intact (none of the existing entries in migration table are modified or removed). And your answer is:

You don't need it because migrations should never change.

But we already did it and we are going to do it again.

[klimov-paul]

I don't want to modify my migrations history, I want to modify code of my application (including migrations)

That is the same thing: any modfications of the migrations source is a BC break over any other project working copy. You can not add, for example, extra index to some migration.

I already proposed solution for this with keeping history intact (none of the existing entries in migration table are modified or removed)

Your solution requires loading ALL migration classes source codes at once per each migration operation (up, down, history, new and so on) - otherwise it can not function. This will cause a huge performance degradation especially for the long-live projects.

And your answer is: You don't need it because migrations should never change.

My answer is you can move migrations directory to another place you like https://github.com/yiisoft/yii2/issues/14481#issuecomment-316672352

My point is the solution is already exists.

You can also use 2 different namespaces: one for 'legacy' migrations and another - for the new ones (let is if you like to do so) and combine them under single config at single command without breaking anything.

I can not see absolutely no necessity to rename existing migration in the project, and I see absolutely no reason to introduce a huge twisted codebase in order to support it.

I understand you wish to keep your code clean, but keeping those $aliases does not look clean either.

But we already did it and we are going to do it again.

First of all we did nothing yet: #13141 is not merged, while actions of mister @zhuravljov are beyond the reach of @yiisoft. If you have a concern about his commit - you should contact him instead.

However, I should admit that future of code built-in migrations is questionable and at some point thier modifications or thier removal will be inevitable. That is why I always against introduction and usage of external migrations in the code. Still so far I am unable to convince the rest of the team on this matter, so it is for them to decide what to do here.

[rob006]

That is the same thing: any modfications of the migrations source is a BC break over any other project working copy.

No, it's not. It is BC break only when I change migration logic. Changing name of migration does not change it's logic - it will work exactly the same as before change. And sometimes you actually must edit old migrations, to make it compatible with new version of framework or PHP.

Your solution requires loading ALL migration classes source codes at once per each migration operation (up, down, history, new and so on) - otherwise it can not function.

You are wrong again. You never need to load all migrations on migrate/up. You may need this on migrate/down but only in some particular case (for example if you're reverting migrations step by step and revert alias without reverting real migration). And you probably will never need to load all migrations - you need load only non-applied.

But if this is still a problem for you, I'm sure that this could be optimized.

[klimov-paul]

You are wrong again. You never need to load all migrations on migrate/up

Is that so?
Consider following I rename migration m...Legacy to m...New and according to your solution I put m...Legacy into m...New::$aliases. Thus in my history there is m...Legacy but there is no m...New.
When I run migration/up all files under migration path will be picked up including m...New and only after it is loaded we can find out is is already present in the history after resolving $aliases.
In case you are moving several hundreds migrations from one namespace to another one as you want here - all those migrations will be loaded on migration/up before they can be recognized as already applied.

[rob006]

It works in the same way as installing app from scratch - if you have 1k migrations, first installation will load them all and run. But it will not load unchanged migrations.

[klimov-paul]

So you propose to save both migration legacy name (alias) and new name in the history doubling the entries in the history table?

[rob006]

Yes.

[klimov-paul]

In that case how history reverting (migrate/down) should work if same migration is mentioned several times there?

Besides It is better to create a separated command or console command behavior, which will just iterate migration history replacing old names with new ones.

[rob006]

In that case how history reverting (migrate/down) should work if same migration is mentioned several times there?

It is explained in first post - did you read it?

Besides It is better to create a separated command or console command behavior, which will just iterate migration history replacing old names with new ones.

And how it is supposed to be better? It looks event more complicated to implement and definitely less convenient for end user (he should always run two commands except one for database update?).

[klimov-paul]

And how it is supposed to be better? It looks event more complicated to implement and definitely less convenient for end user (he should always run two commands except one for database update?).

???

Here is an implementation:

<?php

namespace yii\filters;

use yii\base\ActionFilter;
use yii\console\controllers\MigrateController;
use yii\db\Connection;
use yii\db\Query;
use yii\di\Instance;

/**
 * ```php
 * return [
 *     'controllerMap' => [
 *         'migrate' => [
 *             'class' => 'yii\console\controllers\MigrateController',
 *             'as versionReplacer' => [
 *                 'class' => 'yii\filters\AdjustHistoryFilter',
 *                 'versionReplaces' => [
 *                     'm140506_102106_rbac_init' => 'yii\rbac\M140506102106RbacInit',
 *                 ],
 *             ],
 *             'migrationNamespaces' => [
 *                 'app\migrations',
 *                 'some\extension\migrations',
 *             ],
 *         ],
 *     ],
 * ];
 * ```
 */
class AdjustHistoryFilter extends ActionFilter
{
    /**
     * @var array versions to be replaced in format: `[legacyVersion => newVersion]`, for example:
     *
     * ```php
     * [
     *     'm140506_102106_rbac_init' => 'yii\rbac\M140506102106RbacInit',
     * ]
     * ```
     */
    public $versionReplaces = [];


    /**
     * @inheritdoc
     */
    public function beforeAction($action)
    {
        if ($action->id === 'create') {
            return true;
        }

        /* @var $migrateController MigrateController */
        $migrateController = $action->controller;
        $migrateController->migrationTable;

        $migrateController->db = Instance::ensure($migrateController->db, Connection::className());

        $legacyVersions = array_keys($this->versionReplaces);

        $existingLegacyVersions = (new Query())
            ->select(['version'])
            ->from($migrateController->migrationTable)
            ->andWhere(['in', 'version', $legacyVersions])
            ->column($migrateController->db);

        if (empty($existingLegacyVersions)) {
            return true;
        }

        foreach ($existingLegacyVersions as $legacyVersion) {
            $newVersion = $this->versionReplaces[$legacyVersion];
            $migrateController->db->createCommand()
                ->update($migrateController->migrationTable, ['version' => $newVersion], ['version' => $legacyVersion])
                ->execute();
        }

        return true;
    }
}

[klimov-paul]

It will adjust migration history automatically on any migrate command execution.

[rob006]

1. Your implementation does not support reverting.
2. It need to be manually configured by the end user - it is useless for extensions/modules authors.
3. It actually modifies migration history, and AFAIK you're against this (see https://github.com/yiisoft/yii2/issues/14481#issuecomment-316681665).

[sergeymakinen]

Personally I solved this problem using a @klimov-paul's way. But yes, it would be nice to refactor class names/namespaces without all these tricks. And I don't see a clean way beside an UUID usage in each migration. But it starts suffering as a project gets bigger.

[klimov-paul]

Your implementation does not support reverting.

Reverting of what?

It need to be manually configured by the end user - it is useless for extensions/modules authors.

I think it is much more reliable.
So you suppose that extension author should be first of all concern about your project built on his extension. So he has no right to having a clean code without legacy class names - this is only yours priveledge. If I a author of the extension and I wish to rename some migration class I should provide an alias to the old migration name inside it, while regulary such things are handled via UPGRADE notes.

And if I use some extension, which has some migration renamed I should enforce its author to provide these aliases even if he values his clean code. I suppose I should hunt him down and turture him until he agrees or something.

While picking up any external code you are doing it on your own risk. And external library breaks a BC, including migration renaming - it should provide UPGRADE note, which you should apply on your own.

It actually modifies migration history

That is exactly what this issue is about! Migration class name IS A PART OF THE HISTORY as it serves as version ID. Modification of the migration class name means necessity of modification of the history.
Renaming migration is the same thing as renaming GIT commit changing its original hash to some other pretty one. While technically it is possible - it breaks the history.

Migrations are not ordinally classes, which can be manipulated as you like - they are special are require special treatment.

[sergeymakinen]

Migration class name IS A PART OF THE HISTORY as it serves as version ID.

So a version ID should not depend on this in 2.1. Probably…

[klimov-paul]

So a version ID should not depend on this in 2.1

This is unlikely to be possible as program should be able to find migration source using its ID.
The only working solution based on such approach will be creating a manual map for migration locations to thier ID somewhere in migration config, like following:

return [
    'controllerMap' => [
        'migrate' => [
            'class' => 'yii\console\controllers\MigrateController',
            'versions' => [
                'abcdef12341' => 'app\migrations\M1',
                'abcdef12342' => 'app\migrations\M2',
                'abcdef12343' => 'yii\rbac\migrations\rbac_init',
                ...,
            ],
        ],
    ],
];

Then history table will contain only UIDs, but this require registering every migration in the versions map.

However in general versions map can be generated on the fly, while picking up new migrations, and then be adjusted at will. But still it looks like overcomplecating things.

[rob006]

Reverting of what?

Migrations. If I upgrade some module from 1.x to 2.x (which changes migrations names), apply migrations and find some regressions, I want to go back to 1.x. Normally I revert migrations, revert to 1.x version of module and I should have a state before whole changes. This is not the case with your solution.

So you suppose that extension author should be first of all concern about your project built on his extension. So he has no right to having a clean code without legacy class names - this is only yours priveledge. If I a author of the extension and I wish to rename some migration class I should provide an alias to the old migration name inside it, while regulary such things are handled via UPGRADE notes.

Thats because framework does not provide any tools for such situations.That does not mean that it always has to be in this way - if there is a way to improve this, why don't do this?

That is exactly what this issue is about! Migration class name IS A PART OF THE HISTORY as it serves as version ID.

Migrations history is in migration table. Migration class is not a migration history, it is only class with some code, and real migration history (in migration table) only referencing it. If we have mechanism for finding this code even after renaming migration class, which keeps all old entries in migration table intact, then there is no modification of existing history of migrations.

[rob006]

So you suppose that extension author should be first of all concern about your project built on his extension. So he has no right to having a clean code without legacy class names - this is only yours priveledge. If I a author of the extension and I wish to rename some migration class I should provide an alias to the old migration name inside it, while regulary such things are handled via UPGRADE notes.

Alternative if we keep this, there should be explicit note in documentation, that modules/extensions consumer should never load migrations directly from module/extension. So basically he should never do something like this:

yii migrate --migrationPath=@yii/rbac/migrations/

Source: http://www.yiiframework.com/doc-2.0/yii-rbac-dbmanager.html

He should create own migration that extends migrations provided with framework/extension/module and add new migrations manually on every composer update

[sergeymakinen]

@klimov-paul I do agree with you. And yes, the solution you proposed is what I'm thinking about: an id to class map & a migration map scanner/builder.
For existing migrations we can suppose their ids equal to their class names so if someone wants to refactor the migrations it would be required to set (for example) $id to match an existing history entry.
The only problems I see ATM are 1) a scanner/builder performance 2) if we really need all of this.

[francislavoie]

I just want to point out how Laravel does migrations, it's worth discussing the differences to see what good ideas they might have implemented to help improve migrations in Yii

https://laravel.com/docs/5.4/migrations
https://laravel.com/docs/5.4/packages#migrations

[sergeymakinen]

Laravel's Migrator builds a list of files in all specified directories and uses a file name as a migration name and a class name. Similar to Yii but a different implementation. No namespaces, no refactorings, no problems.

[sergeymakinen]

BTW Phinx works similar to the comment: it loads all migration classes and uses a getVersion(): float method to sort them. Versions are assumed to be microtome() and by default they are extracted from file names.
Looks good and should be simple enough to implement.
The standard migration schema even could be auto migrated without any BC break as far as I see. And existing migrations won't be affected as well as all new default migrations.

[samdark]

Yes, that could be a solution.

[schmunk42]

Just for reference: https://forum.yiiframework.com/t/alternative-approach-to-migrations/126274