Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Concept] Configurable model table name #164

Closed
antonkomarev opened this issue May 1, 2020 · 4 comments
Closed

[Concept] Configurable model table name #164

antonkomarev opened this issue May 1, 2020 · 4 comments
Assignees
Milestone

Comments

@antonkomarev
Copy link
Member

antonkomarev commented May 1, 2020

Motivation

To have more control over the data storage.

Use cases

Cross-database queries

Define schema/database name to enable cross-database queries. Be careful with it, maybe it will be better to use lower-level API instead of higher-level facades.

Related issue: #163

Different table naming convention

Redefine database table names to follow another naming convention or due to naming conflicts.

Tenant aware table name prefixes

API which provides reaction system. Each tenant data may be separated by table prefixes. Personally I prefer to separate it by schemas/databases.

Proposal

Introduce models database table mappings mechanism.

Things to consider

Performance loss

Introducing one more configurable value will decrease performance. Unit tests are running slower on ~2.5% with new feature because of resoling table name from the Laravel Config. This possibly could be partially solved by using static mappings registry, but it will increase system complexity. This could be revised in future versions.

Here is how static mappings registry might look like:

final class ModelTables
{
    private static $mappings = [];

    public static function register(array $mappings): void
    {
        self::$mappings = $mappings;
    }

    public static function get(string $defaultTableName): string
    {
        return $mappings[$defaultTableName] ?? $defaultTableName;
    }
}

It could be registered it in LoveServiceProvider or in AppServiceProvider:

private function registerLoveModelTables(): void
{
    ModelTables::register(Config::get('love.storage.database.tables'));
}

Method getTable in abstract model will look like:

protected function getTable(): string
{
    return ModelTables::get($this->table); 
}
@antonkomarev
Copy link
Member Author

antonkomarev commented May 1, 2020

Solution №1

Add getTable method to abstract Cog\Laravel\Love\Support\Database\Eloquent\Model which will resolve table names from the package config file. Mapping keys are default table names.

Mapping in config file will look like:

'storage' => [
    'database' => [
        'connection' => env('DB_CONNECTION', 'mysql'),
        'tables' => [
            'love_reacters' => 'custom_reacters',
            'love_reactants' => 'custom_reactants',
            'love_reaction_types' => null,
            'love_reactions' => null,
            'love_reactant_reaction_counters' => null,
            'love_reactant_reaction_totals' => null,
        ],
    ],
],

Abstract model will have a method:

protected function getTable(): string
{
    return Config::get("love.storage.database.tables.{$this->table}")
        ?? $this->table;
}

Migrations will look like:

public function up(): void
{
    $this->schema->create((new Reaction())->getTable(), function (Blueprint $table) {
        $table->bigIncrements('id');
        $table->unsignedBigInteger('reactant_id');
        $table->unsignedBigInteger('reacter_id');
        $table->unsignedBigInteger('reaction_type_id');
        $table->decimal('rate', 4, 2);
        $table->timestamps();

        $table
            ->foreign('reactant_id')
            ->references('id')
            ->on((new Reactant())->getTable())
            ->onDelete('cascade');
        $table
            ->foreign('reacter_id')
            ->references('id')
            ->on((new Reacter())->getTable())
            ->onDelete('cascade');
        $table
            ->foreign('reaction_type_id')
            ->references('id')
            ->on((new ReactionType())->getTable())
            ->onDelete('cascade');
    });
}

Pros

  • Very minor chance of breaking changes between versions
  • Config looks intuitive when you see list of all tables and their mapping values
  • If you adding custom model implementation - there is no need to change anything in config

Cons

  • ?

@antonkomarev
Copy link
Member Author

antonkomarev commented May 1, 2020

Solution №2

Add getTable method to abstract Cog\Laravel\Love\Support\Database\Eloquent\Model which will resolve table names from the package config file. Mapping keys are model class names.

Mapping in config file will look like:

'storage' => [
    'database' => [
        'connection' => env('DB_CONNECTION', 'mysql'),
        'tables' => [
            Reacter::class => 'custom_reacters',
            Reactant::class => 'custom_reactants',
            ReactionType::class => null,
            Reaction::class => null,
            ReactionCounter::class => null,
            ReactionTotal::class => null,
        ],
    ],
],

Abstract model will have a method:

protected function getTable(): string
{
    return Config::get('love.storage.database.tables.' . static::class)
        ?? $this->table; 
}

Migrations will look like:

public function up(): void
{
    $this->schema->create((new Reaction())->getTable(), function (Blueprint $table) {
        $table->bigIncrements('id');
        $table->unsignedBigInteger('reactant_id');
        $table->unsignedBigInteger('reacter_id');
        $table->unsignedBigInteger('reaction_type_id');
        $table->decimal('rate', 4, 2);
        $table->timestamps();

        $table
            ->foreign('reactant_id')
            ->references('id')
            ->on((new Reactant())->getTable())
            ->onDelete('cascade');
        $table
            ->foreign('reacter_id')
            ->references('id')
            ->on((new Reacter())->getTable())
            ->onDelete('cascade');
        $table
            ->foreign('reaction_type_id')
            ->references('id')
            ->on((new ReactionType())->getTable())
            ->onDelete('cascade');
    });
}

Pros

  • Config looks intuitive when you see list of all models and their table names

Cons

  • One more place for breaking changes on models renaming or moving
  • If you adding custom model implementation - you need change classname in config as well

@antonkomarev
Copy link
Member Author

antonkomarev commented May 17, 2020

Solution №3

Add getTable method to abstract Cog\Laravel\Love\Support\Database\Eloquent\Model which will resolve table names from the static registry class. Mapping keys are model class names.

Static registry class will look like:

final class Love
{
    public static $reactionsTableName = 'love_reactions';

    public static function setReactionsTableName(string $tableName): void
    {
        static::$reactionsTableName = $tableName;
    }

    public static function getReactionsTableName(): string
    {
        return new static::$reactionsTableName;
    }
}

In AppServiceProvider we will need to define each table name like this:

Love::setReactionsTableName('custom_reactions');
Love::setReactantsTableName('custom_reactants');
Love::setReactersTableName('custom_reacters');

Abstract model will have a method:

protected function getTable(): string
{
    return Love::getReactionsTableName(); 
}

Migrations will look like:

public function up(): void
{
    $this->schema->create(Love::getReactionsTableName(), function (Blueprint $table) {
        $table->bigIncrements('id');
        $table->unsignedBigInteger('reactant_id');
        $table->unsignedBigInteger('reacter_id');
        $table->unsignedBigInteger('reaction_type_id');
        $table->decimal('rate', 4, 2);
        $table->timestamps();

        $table
            ->foreign('reactant_id')
            ->references('id')
            ->on(Love::getReactantsTableName())
            ->onDelete('cascade');
        $table
            ->foreign('reacter_id')
            ->references('id')
            ->on(Love::getReactersTableName())
            ->onDelete('cascade');
        $table
            ->foreign('reaction_type_id')
            ->references('id')
            ->on(Love::getReactionTypesTableName())
            ->onDelete('cascade');
    });
}

Pros

  • ?

Cons

  • Not obvious configuration place

@antonkomarev
Copy link
Member Author

antonkomarev commented May 22, 2020

Solution №1 was implemented and released in Laravel Love 8.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant