[Feature] register the bundle as a Doctrine Migrations provider

[dkarlovi]

https://www.goetas.com/blog/multi-namespace-migrations-with-doctrinemigrations-30/

Basically, when you do doctrine:migrations:status, you can see bundles with their config auto-configured, then the migration config path is not required.

[emodric]

I'll take a look at it!

Thanks @dkarlovi 👍 !

[dkarlovi]

IIUC, you don't really need to do anything special expect having your bundle load a YAML config file (or via config extension / compiler pass, boils down to the same thing) which defines the path and namespace (same as in Twig).

Example:
https://github.com/pimcore/pimcore/blob/10.x/bundles/CoreBundle/Resources/config/pimcore/doctrine_migrations.yaml#L3-L4

[emodric]

Hm... Our migrations are not part of any bundle, so I doubt notation similar to @PimcoreCoreBundle/Migrations will work. Is there any alternative, e.g. a path relative to project root?

[dkarlovi]

You could try

doctrine_migrations:
    migrations_paths:
        'Netgen\Core\Migrations': '../../layouts-core/migrations' # assumes you're in vendor/netgen together

maybe? Not sure if it will work, but it might.

[emodric]

Will do.

And I need to check if this will allow us to keep using our custom migrations table, since doctrine:migrations:status shows its own, the default one.

I'd like to keep it separate, to have more control over upgrade process of Netgen Layouts (without fears that some other unknown migration will interfere), even if it means using the path to config file.

[dkarlovi]

I think the migration table should be stable since it's managed by Doctrine, but I see your point. 👍

[weaverryan]

Hi!

I was also hoping we could resolve this issue. However, I have a different solution. I don't know if you'll agree Edi... since you said that you'd like to keep the custom migrations table... but I very much prefer the below proposal :).

There are 2 things that I'd like to see improved:

1. Needing to run the long php bin/console doctrine:migrations:migrate --configuration=vendor/netgen/layouts-core/migrations/doctrine.yaml during install is just unfortunate. Not a killer, but unfortunate.

2. More importantly, while I keep working on my app and adding some custom entities, when I run doctrine:migrations:diff, it generates a migration that tries to drop of all of the netgen layouts tables... since it sees those as "extra tables".

The way we solve this in Symfony's core is by hooking into the "schema system" of Doctrine. Basically, when Doctrine is building it's schema (e.g. because you're running doctrine:migrations:diff or event doctrine:schema:update), it will call your listener and you can add your custom schema. This is super nice because doctrine:schema:updte --force will now also include your (i.e. netgen layouts) tables and doctrine:migration:diff will also generate statements to add your tables. It's as if you have entities in your project mapping to these tables: everything is very "normal". Also, if netgen changed the schema from one version to the next, the user would only need to run doctrine:migrations:diff to get a migration for the updataee.

Here is an example from Symfony: this is to add a table to the database if you're using Doctrine for caching: https://github.com/symfony/symfony/blob/6.2/src/Symfony/Bridge/Doctrine/SchemaListener/DoctrineDbalCacheAdapterSchemaSubscriber.php - the actual code that creates the schema is here - https://github.com/symfony/symfony/blob/6.2/src/Symfony/Component/Cache/Adapter/DoctrineDbalAdapter.php#L104-L116 - and the service wiring is in DoctrineBundle: https://github.com/doctrine/DoctrineBundle/blob/5523a816c2725ad836c8c2c03243fc904c3969c2/Resources/config/orm.xml#L137-L140

Cheers!

[emodric]

Hi guys,

I finally played a little bit with this, so here are my findings:

Adding a custom namespace to Doctrine Migrations config as @dkarlovi suggested works fine, however:

When using doctrine:migrations:diff to generate a new migration file, it is now created it Netgen Layouts vendor dir, because we now prepended Netgen Layouts migration config to the app one, and Doctrine Migrations presumably just finds the first folder and uses that to create the migration file and we don't want that.

I had a thought to create a wrapper around doctrine:migrations:migrate command which would be called via e.g. nglayouts:migrate, which would remove the need to manually provide the path to the migration config file, but then, doctrine:migrations:status wouldn't work, but then again it didn't work for Netgen Layouts up to this point, so this is not a problem I guess.

As for doctrine:migrations:diff generating DROP TABLE DDL for Netgen Layouts tables, it looks to me that this cannot be solved with Doctrine Schema Listeners. I mean, how would the process know that Layouts tables were created by Migrations when we use pure SQL in our migration files and then exclude them when generating the diff?

The example you provided @weaverryan uses the Schema API to add the table, but Netgen Layouts mainly uses SQL, so not really compatible with schema listeners I believe.

In any case, I'm really not sure what would be the best approach here.

[weaverryan]

The example you provided @weaverryan uses the Schema API to add the table, but Netgen Layouts mainly uses SQL, so not really compatible with schema listeners I believe.

Hmm, yea, one practical problem I can now see is that your migrations not only add tables, but also insert data, which isn't really compatible with schema listeners. The only option I can think of to avoid the problem where userland doctrine:migrations:diff tries to DROP the layouts tables would be to leverage DoctrineBundle's schema:filter: https://symfony.com/bundles/DoctrineMigrationsBundle/current/index.html#manual-tables

I'm not sure this is something layouts should prepend automatically (because if the user configured this manually later, it would replace the prepended value), but it could at least be documented with the correct regex to get it working.

[emodric]

I'm not sure this is something layouts should prepend automatically (because if the user configured this manually later, it would replace the prepended value), but it could at least be documented with the correct regex to get it working.

Yep, that could definitely work 👍

[emodric]

@weaverryan Does this look okay to you?

[weaverryan]

I like it!