Skip to content

Loading…

[FrameworkBundle] Added events for CLI commands #6124

Closed
wants to merge 5 commits into from
@bamarni

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes

Fixes the following tickets: #1929, #3889

I've allowed myself to continue the effort from @flevour in #3889 as it was only missing unit tests and a small tweak from my pov, I've kept the authorship in the commit.

@stof stof commented on an outdated diff
...Bundle/FrameworkBundle/Console/Event/ConsoleEvent.php
@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Console\Event;
@stof Symfony member
stof added a note

I would move the event object outside the Console namespace

@bamarni
bamarni added a note

You mean Symfony\Bundle\FrameworkBundle\Event\ConsoleEvent?

@stof Symfony member
stof added a note

exactly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Tobion Tobion commented on an outdated diff
...ny/Bundle/FrameworkBundle/Console/AbstractCommand.php
((41 lines not shown))
+ /**
+ * Stores the EventDispatcher dispatching console events.
+ *
+ * @param EventDispatcherInterface $dispatcher
+ */
+ public function setDispatcher(EventDispatcherInterface $dispatcher)
+ {
+ $this->dispatcher = $dispatcher;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function run(InputInterface $input, OutputInterface $output)
+ {
+ if ($this->dispatcher) {
@Tobion Symfony member
Tobion added a note

imo this also needs to consider isSilent().

@bamarni
bamarni added a note

fair enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lolautruche lolautruche commented on an outdated diff
...ny/Bundle/FrameworkBundle/Console/AbstractCommand.php
((41 lines not shown))
+ /**
+ * Stores the EventDispatcher dispatching console events.
+ *
+ * @param EventDispatcherInterface $dispatcher
+ */
+ public function setDispatcher(EventDispatcherInterface $dispatcher)
+ {
+ $this->dispatcher = $dispatcher;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function run(InputInterface $input, OutputInterface $output)
+ {
+ $doDispatch = $this->dispatcher && !$this->isSilent();

Better do an isset($this->dispatcher) ?

@rande
rande added a note

Why the isSilent here ?

@bamarni
bamarni added a note

@rande : I think it's better to check it here too, relying only on whether or not the dispatcher is there is not entirely true as it is just an optimization of the current application, avoiding the service to be build when it is not needed, checking here too makes the command more robust.

@lolautruche : I don't think it is a big deal, and I often see this without isset() in symfony code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pborreli pborreli commented on an outdated diff
...ny/Bundle/FrameworkBundle/Console/AbstractCommand.php
((17 lines not shown))
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+
+/**
+ * Abstract command providing event dispatching capability.
+ *
+ * @author Bilal Amarni <bilal.amarni@gmail.com>
+ */
+abstract class AbstractCommand extends Command
+{
+ private $dispatcher;
+
+ /**
+ * Wether or not events should be dispatched for the command.

Whether

@bamarni
bamarni added a note

thx, fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nomack84 nomack84 commented on an outdated diff
...ny/Bundle/FrameworkBundle/Console/AbstractCommand.php
((19 lines not shown))
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+
+/**
+ * Abstract command providing event dispatching capability.
+ *
+ * @author Bilal Amarni <bilal.amarni@gmail.com>
+ */
+abstract class AbstractCommand extends Command
+{
+ private $dispatcher;
+
+ /**
+ * Whether or not events should be dispatched for the command.
+ *
+ * @return boolean

Ii should say Boolean.

@bamarni
bamarni added a note

I can see both in the symfony codebase so I guess there is no rule for that yet (or that it doesn't really matter).

it's Boolean

@bamarni
bamarni added a note

I've just found an issue about it (symfony/symfony#4655), it will be discussed for the 2.2 release apparently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@DenisGorbachev

I second this PR, very much needed.

@bamarni
@stof
Symfony member

I see another use case: setting the base url in the routing RequestContext so that generated urls are right.

and for Swiftmailer, it would send emails only if your command send an email. The MemorySpool will never send an email spooled in a different proces.

I don't like the idea of introducing a separate class for this. It would mean that every developer writing a command would have to decide if the command will dispatch events or no, making it confusing for people using the event ("Why don't I see the event for this command ?")

@bamarni
@lsmith77
@bamarni

I've just took time to look at it and I've been able to implement @stof suggestion without so much pain, so I'm fine with all commands dispatching events.

@stof stof commented on an outdated diff
.../Bundle/FrameworkBundle/Command/CacheClearCommand.php
((6 lines not shown))
{
+ protected $dispatcher;
@stof Symfony member
stof added a note

why the dispatcher here ?

@bamarni
bamarni added a note

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@flevour flevour [FrameworkBundle] Added events for CLI commands
This adds an init and terminate event for commands. They are
dispatched from ContainerAwareCommand.

The cache:clear command can't implement this (cf. #3889 on Github).
e429365
@bamarni

from my pov this PR is now finished.

@ghost

@bamarni - I think you need to rebase this to current master as it cannot be automatically merged as it stands.

@bamarni bamarni Merge branch 'master' into standard-events-cli
Conflicts:
	src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
9270e1c
@bamarni

argh... it should be fixed now, thx

@bamarni

Is it too late for adding this to 2.2? As a reminder this in not only a new feature, but it'd provide a way to fix a bug, with the standard edition, by default, all mails sent from commands are lost (cf. symfony/symfony-standard#249).

cc @fabpot

@lsmith77

ping

@r1pp3rj4ck

I think it would be nice to have it in 2.2 too.

@rande

+1000

@marcospassos

IMO the dialog helper should be passed through the event. I've an use case of a multitenant application that I need to ask the host before some commands are executed (like doctrine:*), in order to set the RequestContext.

@XWB
XWB commented

+1 I was looking for such an implementation, it would be nice to have it in 2.2

@fabpot ping

@startupz

I need it too

@fabpot ping

@bamarni

I've added the helperSet as per @marcospassos suggestion and also made the tests more robusts by checking both events are getting dispatched properly.

@XWB

@fabpot ping

@bamarni

there is no need to ping I guess :) 2.2 is in rc now, so it's obviously too late, and this is not the best moment for the core team to review 2.3 features.

@XWB

Fair enough :)

@lsmith77

i guess now its time to ping again :)

@XWB
XWB commented

@fabpot ping ;)

@fabpot
Symfony member

closing in favor of #7466

@fabpot fabpot closed this
@fabpot fabpot added a commit that referenced this pull request
@fabpot fabpot merged branch fabpot/console-dispatcher (PR #7466)
This PR was merged into the master branch.

Discussion
----------

Console dispatcher

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #3889, #6124
| License       | MIT
| Doc PR        | symfony/symfony-docs#2352

refs #1884, #1929

This is an alternative implementation for adding events to console applications.

This implementation has the following features:

* Available for anyone using the Console component and it is not tied to
  FrameworkBundle (this is important as one thing we are trying to solve is
  email sending from a command, and frameworks like Silex using the Console
  component needs a solution too);

* Non-intrusive as the current code has not been changed (except for renaming
  an internal variable that was wrongly named -- so that's not strictly needed
  for this PR)

* The new DispatchableApplication class also works without a dispatcher,
  falling back to the regular behavior. That makes easy to create applications
  that can benefit from a dispatcher when available, but can still work
  otherwise.

* Besides the *before* and *after* events, there is also an *exception* event
  that is dispatched whenever an exception is thrown.

* Each event is quite powerful and can manipulate the input, the output, but
  also the command to be executed.

Commits
-------

4f9a55a refactored the implementation of how a console application can handle events
4edf29d added helperSet to console event objects
f224102 Added events for CLI commands
c1bd3b5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 9, 2012
  1. @flevour @bamarni

    [FrameworkBundle] Added events for CLI commands

    flevour committed with bamarni
    This adds an init and terminate event for commands. They are
    dispatched from ContainerAwareCommand.
    
    The cache:clear command can't implement this (cf. #3889 on Github).
Commits on Dec 29, 2012
  1. @bamarni

    Merge branch 'master' into standard-events-cli

    bamarni committed
    Conflicts:
    	src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Commits on Feb 6, 2013
  1. @bamarni
  2. @bamarni
  3. @bamarni
View
1 src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -21,6 +21,7 @@ CHANGELOG
You can customize it for your functional tests or for generating urls with
the right base url when your are in the cli context.
* Added support for default templates per render tag
+ * added an init and terminate event dispatched by CLI commands
2.1.0
-----
View
17 src/Symfony/Bundle/FrameworkBundle/Command/CacheClearCommand.php
@@ -11,6 +11,7 @@
namespace Symfony\Bundle\FrameworkBundle\Command;
+use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
@@ -23,10 +24,12 @@
* @author Francis Besset <francis.besset@gmail.com>
* @author Fabien Potencier <fabien@symfony.com>
*/
-class CacheClearCommand extends ContainerAwareCommand
+class CacheClearCommand extends Command
{
protected $name;
+ private $container;
+
/**
* @see Command
*/
@@ -174,4 +177,16 @@ protected function getContainerClass()
return new $class($parent->getEnvironment(), $parent->isDebug());
}
+
+ /**
+ * @return ContainerInterface
+ */
+ protected function getContainer()
+ {
+ if (null === $this->container) {
+ $this->container = $this->getApplication()->getKernel()->getContainer();
+ }
+
+ return $this->container;
+ }
}
View
26 src/Symfony/Bundle/FrameworkBundle/Command/ContainerAwareCommand.php
@@ -11,7 +11,12 @@
namespace Symfony\Bundle\FrameworkBundle\Command;
+use Symfony\Bundle\FrameworkBundle\Console\ConsoleEvents;
+use Symfony\Bundle\FrameworkBundle\Event\ConsoleEvent;
+use Symfony\Bundle\FrameworkBundle\Event\ConsoleTerminateEvent;
use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
@@ -28,6 +33,27 @@
private $container;
/**
+ * {@inheritdoc}
+ */
+ public function run(InputInterface $input, OutputInterface $output)
+ {
+ $dispatcher = $this->getContainer()->get('event_dispatcher');
+ $helperSet = $this->getHelperSet();
+
+ $initEvent = new ConsoleEvent($input, $output);
+ $initEvent->setHelperSet($helperSet);
+ $dispatcher->dispatch(ConsoleEvents::INIT, $initEvent);
+
+ $exitCode = parent::run($input, $output);
+
+ $terminateEvent = new ConsoleTerminateEvent($input, $output, $exitCode);
+ $terminateEvent->setHelperSet($helperSet);
+ $dispatcher->dispatch(ConsoleEvents::TERMINATE, $terminateEvent);
+
+ return $exitCode;
+ }
+
+ /**
* @return ContainerInterface
*/
protected function getContainer()
View
43 src/Symfony/Bundle/FrameworkBundle/Console/ConsoleEvents.php
@@ -0,0 +1,43 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Console;
+
+/**
+ * Contains all events thrown during Console commands execution
+ *
+ * @author Francesco Levorato <git@flevour.net>
+ */
+final class ConsoleEvents
+{
+ /**
+ * The INIT event allows you to attach listeners before any command is
+ * executed by the console. It also allows you to modify the input and output
+ * before they are handled to the command.
+ *
+ * The event listener method receives a \Symfony\Bundle\FrameworkBundle\Event\ConsoleEvent
+ * instance.
+ *
+ * @var string
+ */
+ const INIT = 'console.init';
+
+ /**
+ * The TERMINATE event allows you to attach listeners after a command is
+ * executed by the console.
+ *
+ * The event listener method receives a \Symfony\Bundle\FrameworkBundle\Event\ConsoleTerminateEvent
+ * instance.
+ *
+ * @var string
+ */
+ const TERMINATE = 'console.terminate';
+}
View
77 src/Symfony/Bundle/FrameworkBundle/Event/ConsoleEvent.php
@@ -0,0 +1,77 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Event;
+
+use Symfony\Component\Console\Helper\HelperSet;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\EventDispatcher\Event;
+
+/**
+ * Allows to inspect input and output of a command.
+ *
+ * @author Francesco Levorato <git@flevour.net>
+ */
+class ConsoleEvent extends Event
+{
+ private $input;
+
+ private $output;
+
+ private $helperSet;
+
+ public function __construct(InputInterface $input, OutputInterface $output)
+ {
+ $this->input = $input;
+ $this->output = $output;
+ }
+
+ /**
+ * Sets the helper set.
+ *
+ * @param HelperSet $helperSet A HelperSet instance
+ */
+ public function setHelperSet(HelperSet $helperSet)
+ {
+ $this->helperSet = $helperSet;
+ }
+
+ /**
+ * Gets the helper set.
+ *
+ * @return HelperSet A HelperSet instance
+ */
+ public function getHelperSet()
+ {
+ return $this->helperSet;
+ }
+
+ /**
+ * Returns the input object
+ *
+ * @return InputInterface
+ */
+ public function getInput()
+ {
+ return $this->input;
+ }
+
+ /**
+ * Returns the output object
+ *
+ * @return OutputInterface
+ */
+ public function getOutput()
+ {
+ return $this->output;
+ }
+}
View
46 src/Symfony/Bundle/FrameworkBundle/Event/ConsoleTerminateEvent.php
@@ -0,0 +1,46 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Event;
+
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+/**
+ * Allows to receive the exit code of a command after its execution.
+ *
+ * @author Francesco Levorato <git@flevour.net>
+ */
+class ConsoleTerminateEvent extends ConsoleEvent
+{
+ /**
+ * The exit code of the command.
+ *
+ * @var integer
+ */
+ private $exitCode;
+
+ public function __construct(InputInterface $input, OutputInterface $output, $exitCode)
+ {
+ parent::__construct($input, $output);
+ $this->exitCode = $exitCode;
+ }
+
+ /**
+ * Returns the exit code.
+ *
+ * @return integer
+ */
+ public function getExitCode()
+ {
+ return $this->exitCode;
+ }
+}
View
54 src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php
@@ -12,7 +12,9 @@
namespace Symfony\Bundle\FrameworkBundle\Tests\Console;
use Symfony\Bundle\FrameworkBundle\Tests\TestCase;
+use Symfony\Bundle\FrameworkBundle\Tests\Console\Fixtures\FooCommand;
use Symfony\Bundle\FrameworkBundle\Console\Application;
+use Symfony\Bundle\FrameworkBundle\Console\ConsoleEvents;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\NullOutput;
@@ -22,7 +24,7 @@ public function testBundleInterfaceImplementation()
{
$bundle = $this->getMock("Symfony\Component\HttpKernel\Bundle\BundleInterface");
- $kernel = $this->getKernel(array($bundle));
+ $kernel = $this->getKernel(array($bundle), $this->never());
$application = new Application($kernel);
$application->doRun(new ArrayInput(array('list')), new NullOutput());
@@ -33,13 +35,23 @@ public function testBundleCommandsAreRegistered()
$bundle = $this->getMock("Symfony\Component\HttpKernel\Bundle\Bundle");
$bundle->expects($this->once())->method('registerCommands');
- $kernel = $this->getKernel(array($bundle));
+ $kernel = $this->getKernel(array($bundle), $this->never());
$application = new Application($kernel);
$application->doRun(new ArrayInput(array('list')), new NullOutput());
}
- private function getKernel(array $bundles)
+ public function testCommandDispatchEvents()
+ {
+ $kernel = $this->getKernel(array(), $this->once());
+
+ $application = new Application($kernel);
+ $application->add(new FooCommand('foo'));
+
+ $application->doRun(new ArrayInput(array('foo')), new NullOutput());
+ }
+
+ private function getKernel(array $bundles, $dispatcherExpected = null)
{
$kernel = $this->getMock("Symfony\Component\HttpKernel\KernelInterface");
$kernel
@@ -47,6 +59,42 @@ private function getKernel(array $bundles)
->method('getBundles')
->will($this->returnValue($bundles))
;
+
+ $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
+
+ $dispatcherExpected = $dispatcherExpected ?: $this->any();
+ if ($this->never() == $dispatcherExpected) {
+ $container
+ ->expects($dispatcherExpected)
+ ->method('get');
+ } else {
+ $eventDispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
+ $eventDispatcher
+ ->expects($this->at(0))
+ ->method('dispatch')
+ ->with(
+ $this->equalTo(ConsoleEvents::INIT),
+ $this->isInstanceOf('Symfony\Bundle\FrameworkBundle\Event\ConsoleEvent')
+ );
+ $eventDispatcher
+ ->expects($this->at(1))
+ ->method('dispatch')
+ ->with(
+ $this->equalTo(ConsoleEvents::TERMINATE),
+ $this->isInstanceOf('Symfony\Bundle\FrameworkBundle\Event\ConsoleTerminateEvent')
+ );
+ $container
+ ->expects($dispatcherExpected)
+ ->method('get')
+ ->with($this->equalTo('event_dispatcher'))
+ ->will($this->returnValue($eventDispatcher));
+ }
+
+ $kernel
+ ->expects($this->any())
+ ->method('getContainer')
+ ->will($this->returnValue($container))
+ ;
return $kernel;
}
View
24 src/Symfony/Bundle/FrameworkBundle/Tests/Console/Fixtures/FooCommand.php
@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\Console\Fixtures;
+
+use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class FooCommand extends ContainerAwareCommand
+{
+ protected function execute(InputInterface $input, OutputInterface $output)
+ {
+ return 0;
+ }
+}
Something went wrong with that request. Please try again.