Skip to content

Commit

Permalink
minor #25593 [Console] Simplify parameters in DI (Tobion)
Browse files Browse the repository at this point in the history
This PR was merged into the 4.0 branch.

Discussion
----------

[Console] Simplify parameters in DI

| Q             | A
| ------------- | ---
| Branch?       | 4.0
| Bug fix?      | no
| New feature?  |no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Currently the container gets filled with alot of ugly params like

```
'console.command.ids' => array(
                'console.command.symfony_bundle_frameworkbundle_command_aboutcommand' => 'console.command.about',
                'console.command.symfony_bundle_frameworkbundle_command_assetsinstallcommand' => 'console.command.assets_install',
                'console.command.symfony_bundle_frameworkbundle_command_cacheclearcommand' => 'console.command.cache_clear',
...
            ),
'console.lazy_command.ids' => array(
                'console.command.about' => true,
                'console.command.assets_install' => true,
                'console.command.cache_clear' => true,
...
```

We can get rid of these in 4.0 with a little refactoring.
- SF 4.0 does not include the auto-registration of commands anymore which was the reason why the `console.command.ids` used the class name as index to prevent commands already defined as service to not triggger auto-registration. -> The param does not need the index lookup anymore in 4.0
- What I now also changed is that this param only contains the command IDs of services that are NOT lazy loaded. This way, we don't need `console.lazy_command.ids` at all. This is a simplification of #24073 and still ensures framework bundle console application is compatible with console component 3.x and 4.x

Commits
-------

ae47805 [Console] Simplify parameters in DI
  • Loading branch information
Robin Chalas committed Dec 25, 2017
2 parents 1a6cdfe + ae47805 commit 8535fec
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 24 deletions.
Expand Up @@ -41,14 +41,11 @@ public function process(ContainerBuilder $container)
$lazyCommandMap = array();
$lazyCommandRefs = array();
$serviceIds = array();
$lazyServiceIds = array();

foreach ($commandServices as $id => $tags) {
$definition = $container->getDefinition($id);
$class = $container->getParameterBag()->resolveValue($definition->getClass());

$commandId = 'console.command.'.strtolower(str_replace('\\', '_', $class));

if (isset($tags[0]['command'])) {
$commandName = $tags[0]['command'];
} else {
Expand All @@ -62,20 +59,16 @@ public function process(ContainerBuilder $container)
}

if (null === $commandName) {
if (isset($serviceIds[$commandId]) || $container->hasAlias($commandId)) {
$commandId = $commandId.'_'.$id;
}
if (!$definition->isPublic() || $definition->isPrivate()) {
$commandId = 'console.command.public_alias.'.$id;
$container->setAlias($commandId, $id)->setPublic(true);
$id = $commandId;
}
$serviceIds[$commandId] = $id;
$serviceIds[] = $id;

continue;
}

$serviceIds[$commandId] = $id;
$lazyServiceIds[$id] = true;
unset($tags[0]);
$lazyCommandMap[$commandName] = $id;
$lazyCommandRefs[$id] = new TypedReference($id, $class);
Expand All @@ -101,6 +94,5 @@ public function process(ContainerBuilder $container)
->setArguments(array(ServiceLocatorTagPass::register($container, $lazyCommandRefs), $lazyCommandMap));

$container->setParameter('console.command.ids', $serviceIds);
$container->setParameter('console.lazy_command.ids', $lazyServiceIds);
}
}
Expand Up @@ -31,28 +31,27 @@ public function testProcess($public)
$container->addCompilerPass(new AddConsoleCommandPass());
$container->setParameter('my-command.class', 'Symfony\Component\Console\Tests\DependencyInjection\MyCommand');

$id = 'my-command';
$definition = new Definition('%my-command.class%');
$definition->setPublic($public);
$definition->addTag('console.command');
$container->setDefinition('my-command', $definition);
$container->setDefinition($id, $definition);

$container->compile();

$alias = 'console.command.symfony_component_console_tests_dependencyinjection_mycommand';
$alias = 'console.command.public_alias.my-command';

if ($public) {
$this->assertFalse($container->hasAlias($alias));
$id = 'my-command';
} else {
$id = $alias;
// The alias is replaced by a Definition by the ReplaceAliasByActualDefinitionPass
// in case the original service is private
$this->assertFalse($container->hasDefinition('my-command'));
$this->assertFalse($container->hasDefinition($id));
$this->assertTrue($container->hasDefinition($alias));
}

$this->assertTrue($container->hasParameter('console.command.ids'));
$this->assertSame(array($alias => $id), $container->getParameter('console.command.ids'));
$this->assertSame(array($public ? $id : $alias), $container->getParameter('console.command.ids'));
}

public function testProcessRegistersLazyCommands()
Expand All @@ -73,8 +72,7 @@ public function testProcessRegistersLazyCommands()
$this->assertSame(ContainerCommandLoader::class, $commandLoader->getClass());
$this->assertSame(array('my:command' => 'my-command', 'my:alias' => 'my-command'), $commandLoader->getArgument(1));
$this->assertEquals(array(array('my-command' => new ServiceClosureArgument(new TypedReference('my-command', MyCommand::class)))), $commandLocator->getArguments());
$this->assertSame(array('console.command.symfony_component_console_tests_dependencyinjection_mycommand' => 'my-command'), $container->getParameter('console.command.ids'));
$this->assertSame(array('my-command' => true), $container->getParameter('console.lazy_command.ids'));
$this->assertSame(array(), $container->getParameter('console.command.ids'));
$this->assertSame(array(array('setName', array('my:command')), array('setAliases', array(array('my:alias')))), $command->getMethodCalls());
}

Expand All @@ -96,8 +94,7 @@ public function testProcessFallsBackToDefaultName()
$this->assertSame(ContainerCommandLoader::class, $commandLoader->getClass());
$this->assertSame(array('default' => 'with-default-name'), $commandLoader->getArgument(1));
$this->assertEquals(array(array('with-default-name' => new ServiceClosureArgument(new TypedReference('with-default-name', NamedCommand::class)))), $commandLocator->getArguments());
$this->assertSame(array('console.command.symfony_component_console_tests_dependencyinjection_namedcommand' => 'with-default-name'), $container->getParameter('console.command.ids'));
$this->assertSame(array('with-default-name' => true), $container->getParameter('console.lazy_command.ids'));
$this->assertSame(array(), $container->getParameter('console.command.ids'));

$container = new ContainerBuilder();
$container
Expand Down Expand Up @@ -170,10 +167,9 @@ public function testProcessPrivateServicesWithSameCommand()

(new AddConsoleCommandPass())->process($container);

$alias1 = 'console.command.symfony_component_console_tests_dependencyinjection_mycommand';
$alias2 = $alias1.'_my-command2';
$this->assertTrue($container->hasAlias($alias1));
$this->assertTrue($container->hasAlias($alias2));
$aliasPrefix = 'console.command.public_alias.';
$this->assertTrue($container->hasAlias($aliasPrefix.'my-command1'));
$this->assertTrue($container->hasAlias($aliasPrefix.'my-command2'));
}
}

Expand Down

0 comments on commit 8535fec

Please sign in to comment.