migrations icon indicating copy to clipboard operation
migrations copied to clipboard

Add a way to correctly migrate custom DBAL ENUM types on value change

Open Pixelshaped opened this issue 4 years ago • 0 comments

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

Precision: I'm not talking about columnDefinition ENUMs but about custom DBAL ENUM types.

Currently, custom ENUM types defined like the docs recommend it work nicely when you add/remove a column. But adding/removing a value to the ENUM doesn't result in a migration.

Break down

Let's say I have a CountryEnumType class extending the EnumType class:

<?php

namespace App\DBAL;

class CountryEnumType extends EnumType
{
    const FRANCE = "fr";

    protected string $name = 'countryenum';
}
<?php
namespace MyProject\DBAL;

use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Platforms\AbstractPlatform;

abstract class EnumType extends Type
{
    protected $name;
    protected $values = array();

    public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
    {
        $values = array_map(function($val) { return "'".$val."'"; }, $this->values);

        return "ENUM(".implode(", ", $values).")";
    }

    public function convertToPHPValue($value, AbstractPlatform $platform)
    {
        return $value;
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        if (!in_array($value, $this->values)) {
            throw new \InvalidArgumentException("Invalid '".$this->name."' value.");
        }
        return $value;
    }

    public function getName()
    {
        return $this->name;
    }

    public function requiresSQLCommentHint(AbstractPlatform $platform)
    {
        return true;
    }
}

The first time my country column is defined, the migration is correctly produced. country ENUM(\'fr\') NOT NULL COMMENT \'(DC2Type:countryenum)\'

Let's say we now want to add another value to that enum:

<?php

namespace App\DBAL;

class CountryEnumType extends EnumType
{
    const FRANCE = "fr";
    const ENGLAND = "en";
    
    protected string $name = 'countryenum';

    // Added for the sake of the example below, on my project we use reflection to get constants
    public static function getConstants(): array {
        return [self::FRANCE, self::ENGLAND];
    }
}

The migration bundle doesn't pick up the migration.

Part of a solution?

After some research, I found half a solution in a Symfony context, which I modified a bit:

<?php
declare(strict_types=1);

namespace App\EventListener;

use App\DBAL\EnumType;
use Doctrine\DBAL\Schema\Column;
use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs;

class DoctrinePostGenerateSchemaListener
{
    public function postGenerateSchema(GenerateSchemaEventArgs $eventArgs): void
    {
        $columns = [];

        foreach ($eventArgs->getSchema()->getTables() as $table) {
            foreach ($table->getColumns() as $column) {
                if ($column->getType() instanceof EnumType) {
                    $columns[] = $column;
                }
            }
        }

        /** @var Column $column */
        foreach ($columns as $column) {
            /** @var EnumType $enumType */
            $enumType = $column->getType();
            $column->setComment(trim(sprintf('%s (%s)', $column->getComment(), md5(implode(',', $enumType->getConstants())))));
        }
    }
}

This correctly generates an up migration because the column's comment effectively changes. But the down migration is still bad:

<?php

declare(strict_types=1);

namespace DoctrineMigrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
 * Auto-generated Migration: Please modify to your needs!
 */
final class Version20210106140859 extends AbstractMigration
{
    public function getDescription() : string
    {
        return '';
    }

    public function up(Schema $schema) : void
    {
        // this up() migration is auto-generated, please modify it to your needs
        $this->addSql('ALTER TABLE address CHANGE country country ENUM(\'fr\', \'en\') NOT NULL COMMENT \'(814cabde0c7625e7e308af63d2a43dd5)(DC2Type:countryenum)\'');
    }

    public function down(Schema $schema) : void
    {
        // this down() migration is auto-generated, please modify it to your needs
        $this->addSql('ALTER TABLE address CHANGE country country ENUM(\'fr\', \'en\') CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_unicode_ci` COMMENT \'(82a9e4d26595c87ab6e442391d8c5bba)(DC2Type:countryenum)\'');
    }
}

I'm not sure I can progress further as the comment modification is really the only thing that's picked up by the migration system (see changedProperties below): debug

I marked this issue as Feature Request because I think it would be a nice addition to migrate those types, but if it's something that can already be done, please redirect me to the proper resources.

Pixelshaped avatar Jan 06 '21 14:01 Pixelshaped