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

Input values setted by console.command event are cleared #19441

Closed
lainosantos opened this issue Jul 26, 2016 · 11 comments
Closed

Input values setted by console.command event are cleared #19441

lainosantos opened this issue Jul 26, 2016 · 11 comments

Comments

@lainosantos
Copy link

The input values setted by console.command event are cleared by Symfony\Component\Console\Command\Command::bin() called by Symfony\Component\Console\Command\Command::run()

@xabbuh
Copy link
Member

xabbuh commented Aug 3, 2016

Please provide some more details about the issue you are experiencing (what did you expect, what happened instead) and how to reproduce it.

@lainosantos
Copy link
Author

It occurs when:

  1. Create a console.command listener;
  2. In the listener onCommand method, get the input object (InputArgv) and set an valid option for intended run command;
  3. In cli execute the intended command without set that option;
  4. In execute method of intended command get that option value (setted by listener) ;
  5. The option value will be empty, because before call execute method, run method clear the options calling bin method;

@Renrhaf
Copy link

Renrhaf commented Aug 12, 2016

I'm not sure if it's realted to the original issue. But at the moment it's not easy to add application wide options to the commands. I have a use case where I need to add some global options to all commands, splitted in various listeners, and I'm having a hard time.

I think the command needs one more event :

  1. An event dispatched before the "mergeApplicationDefinition" and "bind" methods are called, for example an event "ConsoleEvents::DEFINITION". This event is here to allow us to modify/add/remove some of the command defined options/arguments.
  2. The actual "ConsoleEvents::COMMAND" is dispatched and allow us to set/work with the real input values.
  3. Exception and terminate events as it's already done.

What do you think of this ?

ATM I've found a workaroung like this :

<?php

namespace AppBundle\EventListener;

use Symfony\Component\Console\ConsoleEvents;
use Symfony\Component\Console\Event\ConsoleCommandEvent;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

class CommandEventListener implements EventSubscriberInterface
{
    /**
     * Returns an array of events this subscriber wants to listen to.
     *
     * @return array
     */
    public static function getSubscribedEvents()
    {
        return [
            ConsoleEvents::COMMAND => [
                // Adding options there. Must be called early.
                ['onCommandRunEarly', 2051],
                // Reading option values here. Must be called after.
                ['onCommandRunLately', 2050],
            ],
        ];
    }

    /**
     * On early command run event.
     *
     * Add an option "--without-indexation".
     *
     * @param ConsoleCommandEvent $event
     */
    public function onCommandRunEarly(ConsoleCommandEvent $event)
    {
        $definition = $event->getCommand()->getDefinition();

        // Add the --without-indexation option.
        $definition->addOption(new InputOption('without-indexation', 'wi', InputOption::VALUE_OPTIONAL, 'Disable indexation temporary.'));

        // Refresh definition of the command for later use.
        $event->getCommand()->setDefinition($definition);
        $event->getCommand()->mergeApplicationDefinition(false);
    }

    /**
     * On lately command run event.
     *
     * Reads the option "--without-indexation" value to disable indexation while migrating data.
     *
     * @param ConsoleCommandEvent $event
     */
    public function onCommandRunLately(ConsoleCommandEvent $event)
    {
        // Add the --without-indexation option.
        $definition = $event->getCommand()->getDefinition();
        $input = $event->getInput();
        $input->bind($definition);

        // Disable indexing if asked.
        if ($definition->hasOption('without-indexation') && $input->hasOption('without-indexation')) {
            // DO HERE YOUR PROCESSING.
        }
    }
}
<?php

namespace AppBundle\EventListener;

use Symfony\Component\Console\ConsoleEvents;
use Symfony\Component\Console\Event\ConsoleCommandEvent;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class OtherEventListener implements EventSubscriberInterface
{
    /**
     * Returns an array of events this subscriber wants to listen to.
     *
     * @return array
     */
    public static function getSubscribedEvents()
    {
        return [
            ConsoleEvents::COMMAND => [
                // Adding options there. Must be called early.
                ['onCommandRunEarly', 2051],
                // Reading option values here. Must be called after.
                ['onCommandRunLately', 2050],
            ],
        ];
    }

    /**
     * On early command run event.
     *
     * Add an option "--subdomain" which allow to chose on which tenant database to act.
     * Add an option "--tenant" which allow to chose on which tenant database to act.
     *
     * @param ConsoleCommandEvent $event
     */
    public function onCommandRunEarly(ConsoleCommandEvent $event)
    {
        // Adding a global option to be able to specify the subdomain.
        $definition = $event->getCommand()->getDefinition();

        $definition->addOption(new InputOption('subdomain', 'sub', InputOption::VALUE_OPTIONAL, 'The subdomain on which throwing the command.'));
        $definition->addOption(new InputOption('tenant', 't', InputOption::VALUE_OPTIONAL, 'The tenant id on which throwing the command.'));

        // Refresh definition of the command for later use.
        $event->getCommand()->setDefinition($definition);
        $event->getCommand()->mergeApplicationDefinition(false);
    }

    /**
     * On lately command run event.
     *
     * Analyze subdomain and tenant options and select the proper tenant database.
     *
     * @param ConsoleCommandEvent $event
     *
     * @throws \Exception
     */
    public function onCommandRunLately(ConsoleCommandEvent $event)
    {
        $command = $event->getCommand();
        $definition = $command->getDefinition();
        $input = $event->getInput();
        $input->bind($definition);

       // DO HERE YOUR PROCESSING
       if ($definition->hasOption('tenant') && $tenant = $input->getOption('tenant')) {
            // DO HERE YOUR PROCESSING.
        }
    }
}

Moreover, the "parseShortOptionSet" method seems buggy and does throw an error for a custom alias of mine "wi" (so I'm forced to defined two aliases, "w|wi"):

private function parseShortOptionSet($name)
{
        $len = strlen($name);
        for ($i = 0; $i < $len; ++$i) {
            if (!$this->definition->hasShortcut($name[$i])) { /* Why $name[$i]; ? */
                throw new RuntimeException(sprintf('The "-%s" option does not exist.', $name[$i]));
            }
            ...
        }
}

@jzawadzki
Copy link
Contributor

I was able to confirm that issue.

Currently updating input arguments values on command event doesn't work (even tho we are able to access method $event->getInput()->setArgument($name,$value)) inside the listener.
As @lainosantos said those values are updated again with input from the command line just before command run.
Current behaviour:

  1. In https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Application.php#L835 arguments values are bind to Input

  2. In https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Application.php#L841 event (ConsoleEvents::COMMAND is dispatched

  3. In https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Command/Command.php#L220 values are bind again with values from Input clearing any changes to Input made in the event.

Please see gist reproducing the issue: https://gist.github.com/jzawadzki/62bfc3fec5afe7a93163deb08ddc1e29

@xabbuh xabbuh removed the Unconfirmed label Dec 3, 2016
@Bavragor
Copy link

Confirmed it also 👍

@JonasHaouzi
Copy link

Confirmed here too!

@chalasr
Copy link
Member

chalasr commented Mar 2, 2017

Status: reviewed

See #21841

fabpot added a commit that referenced this issue Mar 5, 2017
…mmand event (chalasr)

This PR was merged into the 2.8 branch.

Discussion
----------

[Console] Do not squash input changes made from console.command event

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19441
| License       | MIT
| Doc PR        | n/a

Setting arguments/options from the `console.command` event is expected to work since #15938

Commits
-------

c8d364b [Console] Do not squash input changes made from console.command event
@fabpot fabpot closed this as completed Mar 5, 2017
@chalasr
Copy link
Member

chalasr commented Mar 23, 2017

This should be re-opened as the patch has been reverted.
See #22131 for a different (BC) approach.

@fabpot fabpot reopened this Mar 23, 2017
@ddegasperi
Copy link

Hi,

I have just had the same problem and used configurators, to add options/arguments for a subset of commands.

The functionality of configurators are described in the following url:
https://symfony.com/doc/current/service_container/configurators.html

To apply options/arguments only for a subset of commands, I've declared a Interface, tagged all commands implementing those interface and set the configurator for all tagged services.

I know it has nothing todo with the patch, but is a alternative approach to achieve the expected behavior.

Best regards,
Daniel

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@chalasr
Copy link
Member

chalasr commented Dec 20, 2020

I have been trying to solve this through different ways, but didn't manage to find a sensible fix.
I'm going to close as won't fix.
Note that we could reword the ConsoleCommandEvent docblock to be more precise about what it allows and what it does not, PR welcome in that direction.

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