Symfony2: Console Commands as Services - Why?

Posted on by Matthias Noback

As you may have read on the Symfony blog: as of Symfony 2.4 you can register console commands using the service tag console.command.

What is the big deal, you would say. Well, let's discover it now. This is how the documentation tells you to write your console commands:

namespace Matthias\ConsoleBundle\Command;

use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class MyCommand extends ContainerAwareCommand
{
    protected function configure()
    {
        $this->setName('my:action');
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        // do something
    }
}

How to make sure your command will be noticed?

Until Symfony 2.4 there were two requirements for your command to be picked up by the console application itself:

  1. Your command class should extend Symfony\Component\Console\Command\Command

  2. Your command class should be placed inside the Command directory (which means also in the Command namespace) of your bundle, in a file with the same name as the class.

Following these rules, it would indeed be possible to register all the available commands, which in reality goes like this:

First, in app/console an instance of Symfony\Bundle\FrameworkBundle\Console\Application is created. This object is aware of the service container, your application's kernel (e.g. AppKernel) and the bundles that are registered inside the kernel. At the end of the file, Application::run() is called:

$kernel = new AppKernel($env, $debug);
$application = new Application($kernel);
$application->run($input);

Then inside Application::run() all registered bundles are asked to register their commands:

class Application extends BaseApplication
{
    public function doRun(InputInterface $input, OutputInterface $output)
    {
        ...

        $this->registerCommands();

        ...
    }

    protected function registerCommands()
    {
        foreach ($this->kernel->getBundles() as $bundle) {
            if ($bundle instanceof Bundle) {
                $bundle->registerCommands($this);
            }
        }
    }
}

Then each bundle can register any command on its own, but most bundles extend the abstract Bundle class which contains the default implementation for registering commands:

// by the way, an abstract class should have "Abstract" as a prefix, right?

abstract class Bundle extends ContainerAware implements BundleInterface
{
    public function registerCommands(Application $application)
    {
        if (!is_dir($dir = $this->getPath().'/Command')) {
            return;
        }

        $finder = new Finder();
        $finder->files()->name('*Command.php')->in($dir);

        $prefix = $this->getNamespace().'\\Command';
        foreach ($finder as $file) {
            $ns = $prefix;
            if ($relativePath = $file->getRelativePath()) {
                $ns .= '\\'.strtr($relativePath, '/', '\\');
            }
            $r = new \ReflectionClass($ns.'\\'.$file->getBasename('.php'));
            if ($r->isSubclassOf('Symfony\\Component\\Console\\Command\\Command') && !$r->isAbstract() && !$r->getConstructor()->getNumberOfRequiredParameters()) {
                $application->add($r->newInstance());
            }
        }
    }
}

So this is where the magic happens! My clean code eyes don't like this at all, but nevertheless: what happens inside this method?

  1. The Finder (yes, the Finder!) is used to find files ending with "Command.php" in the directory "Command".

  2. It tries to load the class inside this file, assuming that its name will correspond to the file name (it should, considering PSR-0).

  3. It checks using reflection if the class is an instance of the Command class I mentioned earlier.

  4. It makes sure that the class is not an abstract class.

  5. It also makes sure that its constructor has no required arguments.

  6. If this is not the case, it can be instantiated, so: it creates an instance of the class.

Bad bundle!

I really dislike all of this, because:

  1. All this code is executed each time you run any console command, which is obviously overkill.

  2. All commands from the entire application are being instantiated every time.

  3. This code violates the open closed principle in that there may be situations where a command is accidentally registered, or not registered when it was intended to be. These corner cases will result in more conditions being added (what about private constructors for instance?).

  4. In fact, the whole command class is being robbed of its creation logic. The only way such an object may be created is using newInstance(), which is the same as new ClassName().

  5. I am supposed to put my commands in the Command directory, but what if one of my commands is not Symfony-specific (like the Doctrine commands) and I want to import it from some PHP library package?

I think it is possible to refactor the registerCommands() method so that number 1 is no problem anymore. I think 2 is a design problem, which is not easy to solve, since you can only know the name of a command and its arguments when you have an instance of it and you can call its getName() method. So commands can never be truly lazy-loading.

Number 3 is not such a big problem anymore, now that you have your own way to register commands (i.e. using the console.command service tag, since this means that you already have to make sure that a service does not point to an abstract class, or has constructor arguments which may not be supplied. This also solves number 4, since all creation logic is entirely in your own hands again. Number 5 is also no problem anymore: you can just create a new service for the command that exists outside of your bundle.

As a matter of fact, I have been registering my commands manually since some time now, which skips all the not-so-nice code in the regular Bundle class:


namespace Matthias; use Symfony\Component\HttpKernel\Bundle\Bundle; use Matthias\ConsoleBundle\Command\MyCommand; class ConsoleBundle extends Bundle { public function registerCommands(Application $application) { // ah, nice and clean! $application->add(new MyCommand()); } }

Coupling...

All seems well! But it isn't... As you may have seen in the sample command class above: it extends ContainerAwareCommand. This means that inside the execute() method you can take any service you like from the service container:

class MyCommand extends ContainerAwareCommand
{
    protected function configure()
    {
        $this->setName('my:action');
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $someService = $this->getContainer()->get('some_service');

        // $someService is expected to be an instance SomeService

        $someService->doSomething();
    }
}

As you can imagine: this couples your command horribly to the Symfony2 service container. But there is nothing to a console command that requires a service container really. You may do everything inline, you may even use another service container/locator from another vendor. The only thing you need in this particular command is an instance of SomeService.

Making your console command a service using the new service tag does not make any difference when it comes to this kind of coupling. It just skips all the ugly digging around in directories and files (or in fact, it just adds another way to this, it will still take a look in the Command directory!).

... and decoupling

What can you do, to make your commands cleaner, less coupled to the framework? This!

use Symfony\Component\Console\Command\Command

class MyCommand extends Command
{
    private $someService;

    public function __construct(SomeService $someService)
    {
        $this->someService = $someService;

        parent::__construct();
    }

    protected function configure()
    {
        $this
            ->setName('my:action');
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $this->someService->doSomething();
    }
}

We know now that $this->someService is an instance of SomeService. We don't need a service container anymore, since dependencies are injected nicely via constructor arguments. We only need to register the command like this:

<service id="my_command" class="Matthias\ConsoleBundle\MyCommand">
  <argument type="service" id="some_service" />
  <tag name="console.command" />
</service>
PHP Symfony2 console dependency injection service container coupling
Comments
This website uses MailComments: you can send your comments to this post by email. Read more about MailComments, including suggestions for writing your comments (in HTML or Markdown).
Tomáš Votruba

Good news in Symfony 3.4, magic to register commands was removed!
https://github.com/symfony/...

Matthias Noback

That's cool, thanks for getting back to this old post ;)

Tomáš Votruba

I'm very grateful for your post, because it took lot of my frustration off. I was the only one around who though this "magic" is very bad practise, so I was happy to see I'm not the only one.

I'm happy to spread good news to others then :)

Matthias Noback

Cool :)

Asim

Hi Matthias, Do the the console commands follow security.yml? I mean, usually we have to create service for console command and controller as service if we need to execute some tasks. If we define any security for controller route, will it also required for command? I didn't see that command is following security....

Matthias Noback

Security is a web request only thing - it doesn't work for commands.

cordoval

Matthias I have a question, when one runs php app/console for instance, i believe that even though we have done lazy="true" in our services that the app console would need to instantiate every command to find the right one and then execute it. So it sounds to me like the impact of loading all those services is going to happen anyways no? In Gush i closed this issue https://github.com/gushphp/... but I suspect that because also i am not using a container that the impact is moderated by knowing that my commands use more or less the same amount of services anyway. Can you have some insight into this?

Matthias Noback

All commands will be constructed, even when you call just one of them. If you choose to inject the required services via its constructor instead of using the service container, you should make those injected services "lazy" to prevent them from being initialized while they are not going to be used.

Sebastiaan Hilbers

Never mind! I have found it out. I can fetch the DiC within the RegisterCommands class using $application->getKernel()->getContainer()

Sebastiaan Hilbers

Some of my commands are refactored to be registered in the DiC, they now however dont show up in the console application anymore.

How do you deal with commands as a service that require dependencies and that needs to be coupled to the console application?

Christian Riesen

Tagging is much more flexible and could be used here to make it even better. Look ant the monolog.logger tag where you can give it the name of the channel you would like to use.

Why not have the console.command tag give the commands name? That way you could have the same basic class but thanks to different service definitions (and names in tags) you could save a lot of code. The getName would simply be something that the command supports in order to return the currently used command name.

Instantly, all commands are based on services, and all its attached goodness. Symfony 3.0 feature?

Matthias Noback

Hi Christian, that's a very good suggestion. It seems strange that a command should have to provide its own name. The list of commands would be better off as a map of names to commands.

Christian Riesen

That is exactly what the service container tag method would be, you map the name of the command in your services definition to the class. I currently work on a project that has a lot importers running and really the code is identical to all, but the really important bits are in a couple of injectable classes. Now I have to take the injected classes to construct names out of things I implement myself in the classes. This makes it impossible to search for the command name.
I wonder if both things could be combined, the current way with the tagged way of commands to allow the console component to exist as is, while under the full framework you have this nice added benefit.

Matthias Noback

That could very well be, though I'm not sure if such a change would be trivial. Indeed something for 3.0!

Nicolai Fröhlich

Nice article - as always. Keep up the good work Matthias!

Made me smile ... i came up with a pretty similar implementation of auto-registering commands using the finder about 2 weeks ago for a terminal-dashboard project :-)

Matthias Noback

Thank you, Nicolai!

mickael andrieu

A problem, a solution :)

Thanks !

Matthias Noback

Thank you too ;)

Luciano Mammino

Great article as usual, Matthias!

Matthias Noback

Thank you, Luciano!

Nice ! Didn't know anything about this feature. Thanks

Kris Wallsmith

Thanks for the article! The problem I see with using service-injection instead of container-injection is that every command is eagerly instantiated every time you run the console. That means you're going to be spinning up a ton of services, just to get a list of available commands.

Matthias Noback

Thanks Kris! That's right, it could be a problem.

As of Symfony 2.3 it is possible to mark services as lazy, which means the service would only be fully instantiated upon the first call to any of its public methods. In the context of your remark this would be great, since you can then inject services without a high cost. So I think every service that is injected into a command as a constructor argument should be lazy:

[xml]
<service id="my_command" class="Matthias\ConsoleBundle\MyCommand">
<argument type="service" id="some_service" />
<tag name="console.command" />
</service>

<service id="some_service" class="..." lazy="true" />
[/xml]

By the way, if you inject a service from a bundle that is not yours and you still want to make it lazy-loading, you may want to use my LazyServicesBundle.

Greg

Hello.

Can you make a PR on the doc to make clear command that are registered in the DIC should not extends ContainerAware but just Command ?

Thanks.

Matthias Noback

That's a good idea, I will do that.

Greg

Hello.

I was not able to find your PR. Did you open it ?

Matthias Noback

I didn't do it (yet). If you would want to do it, go ahead!