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

Wiring up Migrations #44

Open
rieschl opened this issue May 29, 2021 · 5 comments
Open

Wiring up Migrations #44

rieschl opened this issue May 29, 2021 · 5 comments

Comments

@rieschl
Copy link
Contributor

rieschl commented May 29, 2021

I just tried version 3 of this package which requires doctrine migrations v3 and it took me a while to figure out to correctly wire up the migrations CLI commands.

I added the factories as outlined in the docs (copy/paste from the full-config example) but to actually use the migration command I also had to add all migrations command to a $commands array in cli-config.php, which seems to be an (undocumented?) feature of the Doctrine CLI.

So, my question is... Did I do it wrong? If so, what is the correct way?
Should we add more documentation for this or is it obvious and I just didn't get it?
Also, should we add some possibilities to fetch the factory config and commands config (for CLI) from a helper? Searching for all possible Migrations commands is tedious.

When I was browsing the code I also noticed that the migrations stuff is neatly separated in a sub namespace, so maybe we could/should move the dependency from require to a suggestion for people who don't use migrations?

Any opinions? Thanks!

@Xerkus
Copy link

Xerkus commented Jun 23, 2021

Doctrine Migrations wants cli-config.php to return dependency factory, which conflicts with what doctrine ORM wants.

If you want to use vendor/bin/doctrine-migrations and don't care about doctrine orm cli then cli-config.php would look like this:

<?php

declare(strict_types=1);

use Doctrine\Migrations\DependencyFactory;

$container = require __DIR__ . '/container.php';
return $container->get(DependencyFactory::class);

With this approach command factories are not needed and not used.

@rieschl
Copy link
Contributor Author

rieschl commented Jun 23, 2021

Thanks, @Xerkus I also noticed that. But I do need the ORM stuff (mostly for clearing cache).
So the only thing I can think of is to check the content of $argv[0] if vendor/bin/doctrine or vendor/bin/doctrine-migrations was called and return the correct thing. But that's kind of ugly, isn't it?

@Xerkus
Copy link

Xerkus commented Jun 24, 2021

I totally agree.
Share the way you configured it, it would help others encountering this issue.

@Tigerman55
Copy link

@rieschl did you find a better way to do this? I am using $argv[0] as well since I couldn't find a better solution.

@rieschl
Copy link
Contributor Author

rieschl commented Jul 7, 2021

@rieschl did you find a better way to do this? I am using $argv[0] as well since I couldn't find a better solution.

I use the cli-config.php from the default Doctrine ORM ConsoleRunner and not the one from migrations and added all migrations commands manually. I used a factory for that.

First, I registered all factories as described here, then I created a wrapper for all the commands:

declare(strict_types=1);

namespace App\Factory;

use Doctrine\Migrations\Tools\Console\Command\CurrentCommand;
use Doctrine\Migrations\Tools\Console\Command\DiffCommand;
use Doctrine\Migrations\Tools\Console\Command\DumpSchemaCommand;
use Doctrine\Migrations\Tools\Console\Command\ExecuteCommand;
use Doctrine\Migrations\Tools\Console\Command\GenerateCommand;
use Doctrine\Migrations\Tools\Console\Command\LatestCommand;
use Doctrine\Migrations\Tools\Console\Command\ListCommand;
use Doctrine\Migrations\Tools\Console\Command\MigrateCommand;
use Doctrine\Migrations\Tools\Console\Command\RollupCommand;
use Doctrine\Migrations\Tools\Console\Command\StatusCommand;
use Doctrine\Migrations\Tools\Console\Command\SyncMetadataCommand;
use Doctrine\Migrations\Tools\Console\Command\UpToDateCommand;
use Doctrine\Migrations\Tools\Console\Command\VersionCommand;
use Psr\Container\ContainerInterface;

final class MigrationsCommandsFactory
{
    private ContainerInterface $container;

    public function __construct(ContainerInterface $container)
    {
        $this->container = $container;
    }

    /**
     * @return list<object>
     */
    public function create(): array
    {
        return [
            $this->container->get(CurrentCommand::class),
            $this->container->get(DiffCommand::class),
            $this->container->get(DumpSchemaCommand::class),
            $this->container->get(ExecuteCommand::class),
            $this->container->get(GenerateCommand::class),
            $this->container->get(LatestCommand::class),
            $this->container->get(ListCommand::class),
            $this->container->get(MigrateCommand::class),
            $this->container->get(RollupCommand::class),
            $this->container->get(SyncMetadataCommand::class),
            $this->container->get(StatusCommand::class),
            $this->container->get(UpToDateCommand::class),
            $this->container->get(VersionCommand::class),
        ];
    }
}

in the cli-config.php I then added:

$migrationCommands = $container->get(MigrationsCommandsFactory::class);
$commands = $migrationCommands->create();

The key point here is the $commands variable, because it is picked up by vendor/bin/doctrine.

All of that seems a bit verbose, considering both of them are doctrine packages. Having to register all commands manually is a bit flimsy (I had to check out the source of the migrations CLI to get all commands) and if a command is added, I have to add it manually in my cli-config.
That's why I initially opened this issue, because the way I was doing it felt wrong.
It would be better to be able to satisfy both tools, e.g. by providing the DependencyFactory in the HelperSet.

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

No branches or pull requests

3 participants