About migrations and multiple class names

[joao-jlcm]

I'll try to simplify my problem with a common case:

1- On the version 1.0 of my app I create a migration "create_news_table".
2- On 1.1, I create a migration which removes the news table.
3- On 1.2, I try to create the migration "create_news_table" with a totally different table.

On the step 3, the class name of the migration is the same of the first migration (causing an error). If I rename the class name of the last migration, the first one will be created.

Any solution for this? Why do we have to use names for migrations? They should have been using lambdas intead of classes as they are not unique and don't have an identity.

I found another issue talking about this, but the comment of taylorotwell is ridiculous: #5459

You shouldn't really answer issues like this if you don't want to see Laravel being used only for static sites and/or landing pages like CodeIgniter was. It's a problem that affects all the users that's using Laravel on complex websites/systems with a short to medium sized versions story.

[franzliedke]

Just use different names? recreate_news_table?

[joao-jlcm]

No, this is still a bug. If I have to recreate it again, I'd have to use recreate_news_table_again, and then recreate_news_table_again_again. This is not what we're looking for.

As I said, migrations should not have an unique name, because this is a wrong concept. Also, the migrations filename has a timestamp on it, meaning it should NOT be unique, but the classes are (???).

[GrahamCampbell]

This is not a bug. It's just an inconvenience.

[nsa-yoda]

Can we have core just add the timestamp to the class name? Something like class RecreateNewsTable20140922142354 extends Migration {} would solve the issue.

Edit: I've modified MigrationCreator.php to reflect my idea above, though I doubt Taylor would accept it as it wouldn't jive with the LR philosophy: 99fb2c4

[joao-jlcm]

@jsanc623 It seems a good alternative with much less code, but I'll ask again, why do migrations uses classes rather than lambdas? They're very similar to routes. Just imagine if each route had one class...

[nsa-yoda]

@joaocagnoni Not quite sure on the design decision behind using classes - though I would imagine its easier to organize the migration. For example, in a project I worked on, this was one of the migrations:

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Migrations\Migration;

class Users extends Migration {

    public $table = "users";

    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up() {
        # Check if table exists, if not create it and call this function again
        if ( Schema::hasTable( $this->table ) ) {
            # Update table
            Schema::table( $this->table, function ( $table ) {
                $table->string( "first_name", 40 )->nullable();
                $table->string( "last_name", 50 )->nullable();
                $table->string( "username", 30 )->unique();
                $table->string( "password", 128 );
                $table->string( "uuid_private", 128 );
                $table->string( "uuid_public", 64 );
                $table->integer( "facebook_id" )->nullable();
                $table->string( "email", 156 )->unique();
                $table->integer( "birth_month" )->nullable();
                $table->integer( "birth_day" )->nullable();
                $table->integer( "birth_year" )->nullable();
                $table->string( "gender", 1 )->nullable();
            } );
        } else {
            # Create the base users table
            Schema::create( $this->table, function ( $table ) {
                $table->engine = 'InnoDB';
                $table->increments( "id" );
                $table->timestamps();
            } );

            # Call this function again now that the table exists
            $this->up();
        }
    }

    /**
     * Reverse the migrations.
     *
     * @return void
     */
    public function down() {
        Schema::drop( $this->table );
    }

}

Doing the above with lambdas would be a pain and would create very ugly code.

[aik099]

The timestamp is added to migration filename automatically. Why just we can't add same timestamp to migration class name as well?

[GrahamCampbell]

Maybe something to discus on the forums/irc.

[joao-jlcm]

@jsanc623 Why are you checking if a table exists? Are you changing the DB without migrations? It's a bad practice.

I think the code i'll look much cleaner with closures. There's no sense at creating a class using only 2 methods in 99% of the time. So here it is:

Migration::create(function() {
    Schema::create('news', function ($table) {
        $table->string("title");
        $table->string("category");
        $table->string("author");
    });
}, function() {
    Schema::drop('news');
});

Syntax looks like a "hover" on jQuery. Everybody understands a hover.

[nsa-yoda]

@joaocagnoni I didn't write the migration - I took over the project and this was in there

@aik099 This is exactly what I referenced in an earlier comment: 99fb2c4

[crynobone]

Why not just extends MigrationCreator and maintain it per app.

<?php namespace App\Database\Migrations;

class MigrationCreator extends \Illuminate\Database\Migrations\MigrationCreator 
{
     // customize.
}

Create your own MigrationServiceProvider:

<?php namespace App\Providers;

use App\Database\Migrations\MigrationCreator;

class MigrationServiceProvider extends \Illuminate\Database\MigrationServiceProvider
{
    /**
     * Register the migration creator.
     *
     * @return void
     */
    protected function registerCreator()
    {
        $this->app->bindShared('migration.creator', function($app)
        {
            return new MigrationCreator($app['files']);
        });
    }
}

This however will not work if you use any package that use migration, and I don't think most package developer would love the idea to change the migration structure.

p/s: Also maintaining a SaaS, and don't have a problem with current implementation.

[isometriq]

@crynobone Thanks for that, I've been able to override the MigrationCreator class.
For example, now the command artisan make:migration add_fieldname will create the file 2016_02_07_161200_add_fieldname.php and the class name will be AddFieldnameMigration_20160207161200

For reference, here's how the class looks like

<?php

namespace App\Database\Migrations;

class MigrationCreator extends \Illuminate\Database\Migrations\MigrationCreator
{
    protected function getClassName($name)
    {
        return parent::getClassName($name) . 'Migration_' . str_replace('_', '', $this->getDatePrefix());
    }
}

[isometriq]

Forgot a thing, for the migrate to work, I had to override the Migrator class, to apply the same migration class name pattern. Essentially it tries to use my custom class name, but if it fails, it will try with the default behavior of the parent class. That should allow the vendors/packages migrations to work. The override is also done in the MigrationServiceProvider class, as described by @crynobone

For reference, here's how the class looks like

<?php

namespace App\Database\Migrations;

use Illuminate\Support\Str;

class Migrator extends \Illuminate\Database\Migrations\Migrator
{
    /**
     * Resolve a migration instance from a file.
     *
     * @param  string  $file
     * @return object
     */
    public function resolve($file)
    {
        $fileParts = explode('_', $file);
        $class = Str::studly(implode(' ', array_slice($fileParts, 4))) . 'Migration_' . implode('', array_slice($fileParts, 0, 4));

        if (!class_exists($class)) {
            return parent::resolve($file);
        }

        return new $class;
    }
}

[asvae]

Had a fairly nasty deploy because of that "inconvenience".
On dev I did migrations one by one. I causes no class name clashes that way and I didn't notice one name is duplicate. Then, on production, bad things happened. Namely, failed migration for no reason. Troubles.

@isometriq That's sensible approach. Thanks for your solution.