FeastFramework / framework

FEAST Framework
https://docs.feast-framework.com
Apache License 2.0
70 stars 3 forks source link

Improve migrations #5

Closed rafaelbreno closed 3 years ago

rafaelbreno commented 3 years ago

Now you have something like:

<?php

declare(strict_types=1);

namespace Migrations;

use Feast\Database\Migration;
use Feast\Database\Table\TableFactory;

class migration2_jobs extends Migration
{

    protected const NAME = 'Jobs';

    public function up(): void
    {
        /** @todo Create up query */
        $table = TableFactory::getTable('jobs');
        $table->varChar('job_id')
            ->varChar('job_name')
            ->text('job_context')
            ->timestamp('created_at')
            ->dateTime('ran_at',null,true)
            ->varChar('status', default: 'pending')
            ->tinyInt('tries')
            ->tinyInt('max_tries')
            ->varChar('queue_name');
        $table->create();
        $this->connection->rawQuery('ALTER TABLE jobs add PRIMARY KEY (job_id)');
        parent::up();
    }

    public function down(): void
    {
        /** @todo Create down query */
        TableFactory::getTable('jobs')->drop();
        parent::down();
    }
}

But I think we could modify each column separately, like:

<?php

declare(strict_types=1);

namespace Migrations;

use Feast\Database\Migration;
use Feast\Database\Table\TableFactory;

class migration2_jobs extends Migration
{

    protected const NAME = 'Jobs';

    public function up(): void
    {
        /** @todo Create up query */
        $table = TableFactory::getTable('jobs');
        $table->primary()->varChar('job_id');
        $table->varChar('job_name');
        $table->text('job_context');
        $table->timestamp('created_at');
        $table->dateTime('ran_at',null,true);
        $table->varChar('status', default: 'pending')
        $table->tinyInt('tries')
        $table->tinyInt('max_tries')
        $table->varChar('queue_name');

        $table->create();
        parent::up();
    }

    public function down(): void
    {
        /** @todo Create down query */
        TableFactory::getTable('jobs')->drop();
        parent::down();
    }
}

If there's anything that you think that could be done different, let me know. I could start developing just the base for it.

jpresutti commented 3 years ago

This code would already work to a point. There IS a primary method but it currently forces auto increment. Might be worth starting there.

ArrMeeR commented 3 years ago

I looked into the code and I noticed that there is a method autoIncrement in Feast\Database\Table, which creates a new column and sets it as primary key, but as @jpresutti mentioned, it forces auto increment. I think that adding the ability to create the primary key separately is good, however, I'm not sure if the solution suggested by @rafaelbreno will work. That's because every method for creating columns of Feast\Database\Table returns current instance of a class and I think that it would require a lot of work to add a possibility to "chain" primary method to a column method.

Instead, I suggest to create primary method in Table class just like any other method there. It could look something like this: $table->primary('id'); primary method would accept one argument - the column name. Of course, this method would need to check if the provided column exists and if primary key hasn't been set before.

What do you think about it?

jpresutti commented 3 years ago

I am 100% in favor of this idea. I would say that the current autoIncrement method also needs to be modified to call this new primary method as well automatically.

jpresutti commented 3 years ago

@rafaelbreno or @ArrMeeR do either of you want to try your hand at this? If not, I'll probably get to it this weekend.

ArrMeeR commented 3 years ago

@jpresutti I will willingly try to implement it tomorrow :)