kaliop-uk / ezmigrationbundle

This bundle makes it easy to handle eZPlatform / eZPublish5 content upgrades/migrations
GNU General Public License v2.0
53 stars 81 forks source link

improve eZ Migration Bundle compatibility with Doctrine Migrations Bundle #193

Closed SalvatorePollaci closed 2 years ago

SalvatorePollaci commented 5 years ago

It seems that eZ Migration Bundle is not compatible with Doctrine Migrations Bundle.

Scenario 1 - Doctrine Standard Config

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                driver: '%database_driver%'
                host: '%database_host%'
                port: '%database_port%'
                dbname: '%database_name%'
                user: '%database_user%'
                password: '%database_password%'
                charset: UTF8
                schema_filter: ~^(?!ez)~

When I run:

bin/console doctrine:migrations:diff 

the generated migration file tries to DROP and CREATE the 'kaliop_migrations' table.

Scenario 2 - Filter out kaliop_migrations in schema_filter doctrine config option

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                driver: '%database_driver%'
                host: '%database_host%'
                port: '%database_port%'
                dbname: '%database_name%'
                user: '%database_user%'
                password: '%database_password%'
                charset: UTF8
                schema_filter: ~^(?!ez|kaliop_migrations)~

Whatever ez migration bundle command I run, for example:

bin/console kaliop:migration:status

I get the following error message:

In ConnectionHandler.php line 366:

  An exception occurred while executing 'CREATE TABLE kaliop_migrations (migration VARCHAR(255) NOT NULL, path VARCHAR(4000) NOT NULL, md5 VARCHAR(32) NOT NULL, execution_date INT DEFAULT NULL, status INT NOT NULL, execution_error VARCHAR(4000) DEFAULT NULL, PRIMARY KE
  Y(migration)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB':

  SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'kaliop_migrations' already exists

In AbstractMySQLDriver.php line 58:

  An exception occurred while executing 'CREATE TABLE kaliop_migrations (migration VARCHAR(255) NOT NULL, path VARCHAR(4000) NOT NULL, md5 VARCHAR(32) NOT NULL, execution_date INT DEFAULT NULL, status INT NOT NULL, execution_error VARCHAR(4000) DEFAULT NULL, PRIMARY KE
  Y(migration)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB':

  SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'kaliop_migrations' already exists

In PDOConnection.php line 62:

  SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'kaliop_migrations' already exists

In PDOConnection.php line 60:

  SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'kaliop_migrations' already exists

kaliop:migration:status [--path [PATH]] [--summary] [--todo] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env ENV] [--no-debug] [--siteaccess [SITEACCESS]] [--] <command>
gggeek commented 5 years ago

I think that the error messages that you get in the 2nd case is due to the fact that the Mig Bundle uses \Doctrine\DBAL\Schema\AbstractSchemaManager::listTables to determine if its tables are present in the db (if not, it tries to create them on the fly). Apparently, by adding the kalipomigrations* tables to the schema_filter, they disappear from the SchemaManager. Otoh, if you don't, Doctrine messes up those tables. I will have to investigate this a bit - am a bit rusty here...

gggeek commented 5 years ago

Ok, at first sight the quick fix is to use the filter-expression parameter on the command-line when running doctrine:migrations:diff. Would you be able to test?

gggeek commented 5 years ago

ps: asked the same question on the Doctrine DBAL tracker: https://github.com/doctrine/dbal/issues/3460

SalvatorePollaci commented 5 years ago

@gggeek I just tried the filter-expression workaround and I can confirm it is working:

bin/console doctrine:migrations:diff --filter-expression='~^(?!ez|kaliop_migrations)~' --env=prod

This workaround is more than sufficient for my current needs but it would be great to have a bug fix like the one proposed by Ocramius.

Thanks

gggeek commented 5 years ago

Good to know. I will start by adding this info in the readme. As for the suggested fix: the problem I see with it is that I can not let this bundle overtake by default a service that some other bundle or app code might have customized. What I could do is f.e. add a class that implements the modified service and leave it commented out in services.yml, with appropriate instructions on how to enable it. That is better than nothing but, by experience, not something that most developers would easily discover on their own...

SalvatorePollaci commented 5 years ago

What I could do is f.e. add a class that implements the modified service and leave it commented out in services.yml, with appropriate instructions on how to enable it. That is better than nothing but, by experience, not something that most developers would easily discover on their own...

Yes I agree with you that it's not the best of solutions. But if such instructions are given in the main readme file(e.g. troubleshooting section) I think that would be more than sufficient, or at least for me it would be.

gggeek commented 5 years ago

I added one paragraph about this is the main Readme, see: https://github.com/kaliop-uk/ezmigrationbundle/commit/0b35debe3a70c643579cb981a852706c47592943.

I will downgrade this ticket from bug to enhancement and keep it open, in case one day there is time to implement the custom doctrine loader...

SalvatorePollaci commented 5 years ago

@gggeek Seems ok to me