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

Have to specifiy file/database connection while I already have #780

Closed
MichaelBelgium opened this issue Jan 16, 2019 · 16 comments
Closed

Have to specifiy file/database connection while I already have #780

MichaelBelgium opened this issue Jan 16, 2019 · 16 comments
Milestone

Comments

@MichaelBelgium
Copy link

MichaelBelgium commented Jan 16, 2019

Bug Report

Q A
BC Break Yes
Version 2.0.0

Summary

Not sure if i'm missing something but after upgrading to 2.0.0 and executing - for example the status command - the output is

You have to specify a --db-configuration file or pass a Database Connection as a dependency to the Migrations.

I followed the upgrade log but any changes about the Helperset or configuration aren't mentioned. So I'm not sure what's going on.

This is my helperset

$helperset = new HelperSet([
    'dialog' => new QuestionHelper(),
    'db' => new ConnectionHelper($entityManager->getConnection()),
    'em' => new EntityManagerHelper($entityManager)
]);

which then gets set to the console application of symfony.

Current behavior

Executing a migration command results in an error about the helperset of symfony? While it did not before.

How to reproduce

All I did was:

  • Update doctrine/migrations to 2.0.0
  • Execute migrations:status
@jwage
Copy link
Member

jwage commented Jan 16, 2019

How are you then setting the $helperset?

@MichaelBelgium
Copy link
Author

@jwage $consoleApp->setHelperSet($helperset); where $consoleApp is an instance of symfony Application

Then after, the commands get added to $consoleApp which then (normally?) use that helperset.

@jwage
Copy link
Member

jwage commented Jan 16, 2019

Ok I will try to reproduce it.

@stof
Copy link
Member

stof commented Jan 16, 2019

The expected name of the helper is connection, not db:

new ConnectionHelperLoader($helperSet, 'connection'),

@stof
Copy link
Member

stof commented Jan 16, 2019

and that was already the case in 1.x

@jwage
Copy link
Member

jwage commented Jan 16, 2019

Interesting, I am not sure how it worked before then.

@jwage
Copy link
Member

jwage commented Jan 16, 2019

I don't see a string db anywhere in the 1.8 branch that would have correlated with this. I was thinking maybe it also supported db before too and I accidentally removed it.

@MichaelBelgium
Copy link
Author

MichaelBelgium commented Jan 16, 2019

Interesting, I am not sure how it worked before then.

Yeah, i noticed the same what @stof said but I was so confused, so hence this issue, I based it all on the docs. I downgraded back to 1.x before and checked for the same helper name but on previous versions it's also connection

I've tried to debug and follow the flow but i can't find anything yet. I'll try tomorrow with the name connection

@jwage
Copy link
Member

jwage commented Jan 17, 2019

I am suspecting that maybe previously if connection did not exist and em did, it would get the connection with $em->getConnection() somewhere. I can't find any evidence of that though.

Is it possible to share a more complete example of your setup? or preferably a test in the 2.0 branch? I suspect there might be something else going on that I can't see from your example.

@MichaelBelgium
Copy link
Author

So our use case is that we have a file console in our project where we create a symfony console application and add all commands which need a database connection (like all of them).

We put them in an array and loop through them. We do that because we need to overwrite their code (with setCode) and add command options to them because we have multiple databases. We did not find a way to mass add options to all commands so thats what we're doing.

And depending on those options we create the right entitymanager/database connection, following by the helperset and then set the helperset to the console application. Afterwards we add the command to the console application and run it.

$consoleApp = new Application();

$consoleApp->getDefinition()->addOptions([
    new InputOption('app', null, InputOption::VALUE_OPTIONAL, 'The app.', 'default'),
]);

$consoleApp->getDefinition()->addOption(
    new InputOption('env', null, InputOption::VALUE_OPTIONAL, 'The environment to operate in.', $app->environment())
);

$consoleApp->getDefinition()->addOption(
    new InputOption('config', null, InputOption::VALUE_OPTIONAL, 'The application environment.', $app->client())
);

$databaseAwareCommands = [
    //...
    \Doctrine\Migrations\Tools\Console\Command\DiffCommand::class,
    \Doctrine\Migrations\Tools\Console\Command\ExecuteCommand::class,
    \Doctrine\Migrations\Tools\Console\Command\GenerateCommand::class,
    \Doctrine\Migrations\Tools\Console\Command\LatestCommand::class,
    \Doctrine\Migrations\Tools\Console\Command\MigrateCommand::class,
    \Doctrine\Migrations\Tools\Console\Command\StatusCommand::class,
    \Doctrine\Migrations\Tools\Console\Command\UpToDateCommand::class,
    \Doctrine\Migrations\Tools\Console\Command\VersionCommand::class,

    //...
];

foreach ($databaseAwareCommands as $command) {
    /** @var \Symfony\Component\Console\Command\Command $c */
    $c = new $command();
    $c->setCode(function (InputInterface $input, OutputInterface $output) use ($consoleApp, $app, $command) {
        //...
        $entityManager = AthenaManager::get(); //returns new EntityManager based on the options

        $helperset = new HelperSet([
            'dialog' => new QuestionHelper(),
            'db' => new ConnectionHelper($entityManager->getConnection()),
            'em' => new EntityManagerHelper($entityManager)
        ]);

        $consoleApp->setHelperSet($helperset);

        $com = $consoleApp->add(new $command());

        try {
            $com->run($input, $output);
        } catch (Exception $exception) {
            $io = new SymfonyStyle($input, $output);
            $io->error($exception->getMessage());
        }
    });

    $consoleApp->add($c);
}

$consoleApp->run();

The annoying thing is also I'm not able to debug the anonymous function of setcode, nor do a var_dump or an echo (using phpstorm).
To execute a command we do php console migrations:status for example, which then leads to that error, but on version 1.8.x it does not and works normally.

@MichaelBelgium
Copy link
Author

MichaelBelgium commented Jan 17, 2019

I've tracked the debugger and concluded in version 1.8 it gets the connection via ConnectionHelperLoader#chosen(). The helperset has has both 'db' and 'connection' keys, trying to find where it gets the 'connection' from now

EDIT: so in the constructor of HelperSet, the keys you pass in the helper array to new HelperSet are not the names of the helpers but are aliases. The names are defined in each Helper class, so it doesn't really matter which key you provide?! The helper class in 1.8 for the connection is ConnectionHelper from Doctrine\DBAL\Tools\Console\Helper

So ok i think i got it @jwage :
In 2.0.0 it does not execute the set code in my use case, in 1.8 it does. In AbstractCommand the initialize function got added, In 1.8 that wasn't there before. In the Initialize function getMigrationConfiguration gets called and in my case, the helpers aren't set yet because I set them in the setCode so in ConnectionHelperLoader#chosen, line 42, it returns false and in the end it returns null, returning no helper with name 'connection'. And then it throws an excepting resulting that error.

So the point is I probably need to find a different approach to be able to support multiple databases, or issue #503

@stof
Copy link
Member

stof commented Jan 17, 2019

or you could add the helper as connection instead of db, as that's the expected name...

@MichaelBelgium
Copy link
Author

MichaelBelgium commented Jan 17, 2019

or you could add the helper as connection instead of db, as that's the expected name...

That doesn't work because it will still call initialize first (where an exception occurs then from AbstractCommand) and then the setCode where I create the helperset, and like i said:

the keys you pass in the helper array to new HelperSet are not the names of the helpers but are aliases. The names are defined in each Helper class, so it doesn't really matter

@stof
Copy link
Member

stof commented Jan 17, 2019

hmm indeed. The callable passed to setCode is equivalent to the execute method of the Command class.

Your logic instantiating a command and replacing its execution code will indeed never work as soon as the command relies on other extension points of the Symfony Console component. That's not an expected usage. I would say you are on your own here.

@MichaelBelgium
Copy link
Author

I would say you are on your own here.

Well, yeah I've fixed it by using symfony events for now. The same way someone else handled this: #503 (comment)

//....
$consoleApp->addCommands(array_map(function($cmd) { return new $cmd(); }, $databaseAwareCommands));

$eventDispatcher = new EventDispatcher();
$eventDispatcher->addListener(ConsoleEvents::COMMAND,
    function (ConsoleCommandEvent $consoleCommandEvent) use ($app) {
        //...

        $entityManager = AthenaManager::get();

        $helperset = new HelperSet([
            'dialog' => new QuestionHelper(),
            'db' => new ConnectionHelper($entityManager->getConnection()),
            'em' => new EntityManagerHelper($entityManager)
        ]);

        $consoleCommandEvent->getCommand()->setHelperSet($helperset);

        $output->writeln("Executing in environment '{$input->getOption('env')}' with config '{$input->getOption('config')}' in app '{$input->getOption('app')}'");
    }
);

$consoleApp->setDispatcher($eventDispatcher);

$consoleApp->run();

I guess this issue can be closed? I'll let u guys decide

@goetas goetas added this to the 3.0.0 milestone Mar 3, 2020
@goetas
Copy link
Member

goetas commented Mar 6, 2020

In 3.x that is possilbe using the Doctrine\Migrations\Configuration\Connection\ExistingConnection class. See https://github.com/doctrine/migrations/blob/master/docs/en/reference/custom-configuration.rst for an example

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

4 participants