From 379f5e08b4e432c1848afb2a60405154e6e0c04a Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 4 Jun 2013 14:28:00 +0200 Subject: [PATCH 1/4] [DependencyInjection] Fix aliased access of shared services, fixes #8096 --- .../DependencyInjection/Container.php | 8 +++ .../DependencyInjection/Dumper/PhpDumper.php | 58 ++++++++++++++----- .../Tests/Fixtures/php/services9.php | 16 +++++ .../Tests/Fixtures/php/services9_compiled.php | 4 +- 4 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Container.php b/src/Symfony/Component/DependencyInjection/Container.php index 50d413b9d2..ef561e1ded 100644 --- a/src/Symfony/Component/DependencyInjection/Container.php +++ b/src/Symfony/Component/DependencyInjection/Container.php @@ -69,6 +69,7 @@ class Container implements IntrospectableContainerInterface protected $services; protected $methodMap; + protected $aliases; protected $scopes; protected $scopeChildren; protected $scopedServices; @@ -253,6 +254,12 @@ class Container implements IntrospectableContainerInterface { $id = strtolower($id); + // resolve aliases + if (isset($this->aliases[$id])) { + $id = $this->aliases[$id]; + } + + // re-use shared service instance if it exists if (array_key_exists($id, $this->services)) { return $this->services[$id]; } @@ -264,6 +271,7 @@ class Container implements IntrospectableContainerInterface if (isset($this->methodMap[$id])) { $method = $this->methodMap[$id]; } elseif (method_exists($this, $method = 'get'.strtr($id, array('_' => '', '.' => '_')).'Service')) { + // $method is set to the right value, proceed } else { if (self::EXCEPTION_ON_INVALID_REFERENCE === $invalidBehavior) { if (!$id) { diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index bfe6f6d220..0dd058e9ab 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -799,6 +799,9 @@ EOF; $code .= " \$this->scopeChildren = ".$this->dumpValue($this->container->getScopeChildren()).";\n"; } + $code .= $this->addMethodMap(); + $code .= $this->addAliases(); + $code .= <<container->getDefinitions(); - ksort($definitions); - foreach ($definitions as $id => $definition) { - $code .= ' '.var_export($id, true).' => '.var_export('get'.Container::camelize($id).'Service', true).",\n"; - } - $aliases = $this->container->getAliases(); - ksort($aliases); - foreach ($aliases as $alias => $id) { - $code .= ' '.var_export($alias, true).' => '.var_export('get'.Container::camelize($id).'Service', true).",\n"; - } - $code .= " );\n"; + $code .= $this->addMethodMap(); + $code .= $this->addAliases(); $code .= <<container->getDefinitions()) { + return ''; + } + + $code = " \$this->methodMap = array(\n"; + ksort($definitions); + foreach ($definitions as $id => $definition) { + $code .= ' '.var_export($id, true).' => '.var_export('get'.Container::camelize($id).'Service', true).",\n"; + } + + return $code . " );\n"; + } + + /** + * Adds the aliases property definition + * + * @return string + */ + private function addAliases() + { + if (!$aliases = $this->container->getAliases()) { + return ''; + } + + $code = " \$this->aliases = array(\n"; + ksort($aliases); + foreach ($aliases as $alias => $id) { + $code .= ' '.var_export($alias, true).' => '.var_export((string) $id, true).",\n"; + } + + return $code . " );\n"; + } + /** * Adds default parameters method. * diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php index 4aad5e6e9e..0840ee8843 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php @@ -24,6 +24,22 @@ class ProjectServiceContainer extends Container public function __construct() { parent::__construct(new ParameterBag($this->getDefaultParameters())); + $this->methodMap = array( + 'bar' => 'getBarService', + 'baz' => 'getBazService', + 'depends_on_request' => 'getDependsOnRequestService', + 'factory_service' => 'getFactoryServiceService', + 'foo' => 'getFooService', + 'foo.baz' => 'getFoo_BazService', + 'foo_bar' => 'getFooBarService', + 'foo_with_inline' => 'getFooWithInlineService', + 'inlined' => 'getInlinedService', + 'method_call1' => 'getMethodCall1Service', + 'request' => 'getRequestService', + ); + $this->aliases = array( + 'alias_for_foo' => 'foo', + ); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php index dde0543148..b81c4953e6 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php @@ -44,7 +44,9 @@ class ProjectServiceContainer extends Container 'foo_with_inline' => 'getFooWithInlineService', 'method_call1' => 'getMethodCall1Service', 'request' => 'getRequestService', - 'alias_for_foo' => 'getFooService', + ); + $this->aliases = array( + 'alias_for_foo' => 'foo', ); } From bb797ee75591f8461dbcc6fdc8a50b5d3aa8fe5a Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 4 Jun 2013 14:31:04 +0200 Subject: [PATCH 2/4] [DependencyInjection] Remove get*Alias*Service methods from compiled containers --- .../DependencyInjection/Dumper/PhpDumper.php | 43 +------------------ .../Tests/Fixtures/php/services9.php | 10 ----- .../Tests/Fixtures/php/services9_compiled.php | 10 ----- 3 files changed, 2 insertions(+), 61 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index 0dd058e9ab..c1c8da193f 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -594,39 +594,6 @@ EOF; return $code; } - /** - * Adds a service alias. - * - * @param string $alias - * @param string $id - * - * @return string - */ - private function addServiceAlias($alias, $id) - { - $name = Container::camelize($alias); - $type = 'Object'; - - if ($this->container->hasDefinition($id)) { - $class = $this->container->getDefinition($id)->getClass(); - $type = 0 === strpos($class, '%') ? 'Object' : $class; - } - - return <<getServiceCall($id)}; - } - -EOF; - } - /** * Adds multiple services * @@ -634,7 +601,7 @@ EOF; */ private function addServices() { - $publicServices = $privateServices = $aliasServices = $synchronizers = ''; + $publicServices = $privateServices = $synchronizers = ''; $definitions = $this->container->getDefinitions(); ksort($definitions); foreach ($definitions as $id => $definition) { @@ -647,13 +614,7 @@ EOF; $synchronizers .= $this->addServiceSynchronizer($id, $definition); } - $aliases = $this->container->getAliases(); - ksort($aliases); - foreach ($aliases as $alias => $id) { - $aliasServices .= $this->addServiceAlias($alias, $id); - } - - return $publicServices.$aliasServices.$synchronizers.$privateServices; + return $publicServices.$synchronizers.$privateServices; } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php index 0840ee8843..5929351e43 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php @@ -214,16 +214,6 @@ class ProjectServiceContainer extends Container throw new RuntimeException('You have requested a synthetic service ("request"). The DIC does not know how to construct this service.'); } - /** - * Gets the alias_for_foo service alias. - * - * @return FooClass An instance of the foo service - */ - protected function getAliasForFooService() - { - return $this->get('foo'); - } - /** * Updates the 'request' service. */ diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php index b81c4953e6..4170ad1f30 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php @@ -219,16 +219,6 @@ class ProjectServiceContainer extends Container throw new RuntimeException('You have requested a synthetic service ("request"). The DIC does not know how to construct this service.'); } - /** - * Gets the alias_for_foo service alias. - * - * @return FooClass An instance of the foo service - */ - protected function getAliasForFooService() - { - return $this->get('foo'); - } - /** * Updates the 'request' service. */ From d8c0ef705c68123ac5b79d60d70daccc3c68a57f Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 4 Jun 2013 15:12:01 +0200 Subject: [PATCH 3/4] [DependencyInjection] Rename ContainerBuilder::$aliases to avoid conflicting with the parent class --- .../DependencyInjection/ContainerBuilder.php | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 064aaaa6f9..733d016d66 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -57,7 +57,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface /** * @var Alias[] */ - private $aliases = array(); + private $aliasDefinitions = array(); /** * @var ResourceInterface[] @@ -404,7 +404,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface $this->obsoleteDefinitions[$id] = $this->definitions[$id]; } - unset($this->definitions[$id], $this->aliases[$id]); + unset($this->definitions[$id], $this->aliasDefinitions[$id]); parent::set($id, $service, $scope); @@ -438,7 +438,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface { $id = strtolower($id); - return isset($this->definitions[$id]) || isset($this->aliases[$id]) || parent::has($id); + return isset($this->definitions[$id]) || isset($this->aliasDefinitions[$id]) || parent::has($id); } /** @@ -475,8 +475,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface throw new LogicException(sprintf('The service "%s" has a circular reference to itself.', $id), 0, $e); } - if (!$this->hasDefinition($id) && isset($this->aliases[$id])) { - return $this->get($this->aliases[$id]); + if (!$this->hasDefinition($id) && isset($this->aliasDefinitions[$id])) { + return $this->get($this->aliasDefinitions[$id]); } try { @@ -640,7 +640,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ public function getServiceIds() { - return array_unique(array_merge(array_keys($this->getDefinitions()), array_keys($this->aliases), parent::getServiceIds())); + return array_unique(array_merge(array_keys($this->getDefinitions()), array_keys($this->aliasDefinitions), parent::getServiceIds())); } /** @@ -666,7 +666,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ public function setAliases(array $aliases) { - $this->aliases = array(); + $this->aliasDefinitions = array(); $this->addAliases($aliases); } @@ -697,7 +697,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface unset($this->definitions[$alias]); - $this->aliases[$alias] = $id; + $this->aliasDefinitions[$alias] = $id; } /** @@ -709,7 +709,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ public function removeAlias($alias) { - unset($this->aliases[strtolower($alias)]); + unset($this->aliasDefinitions[strtolower($alias)]); } /** @@ -723,7 +723,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ public function hasAlias($id) { - return isset($this->aliases[strtolower($id)]); + return isset($this->aliasDefinitions[strtolower($id)]); } /** @@ -735,7 +735,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ public function getAliases() { - return $this->aliases; + return $this->aliasDefinitions; } /** @@ -757,7 +757,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface throw new InvalidArgumentException(sprintf('The service alias "%s" does not exist.', $id)); } - return $this->aliases[$id]; + return $this->aliasDefinitions[$id]; } /** @@ -837,7 +837,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface $id = strtolower($id); - unset($this->aliases[$id]); + unset($this->aliasDefinitions[$id]); return $this->definitions[$id] = $definition; } From 81b122dd7bdd90b40fca6281956aa32671816619 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 5 Jun 2013 11:51:05 +0200 Subject: [PATCH 4/4] [DependencyInjection] Add support for aliases of aliases + regression test --- .../DependencyInjection/Dumper/PhpDumper.php | 6 +++++- .../DependencyInjection/Dumper/XmlDumper.php | 11 ++++++++--- .../DependencyInjection/Dumper/YamlDumper.php | 6 +++++- .../Tests/Dumper/PhpDumperTest.php | 14 ++++++++++++++ .../Tests/Fixtures/containers/container9.php | 1 + .../Tests/Fixtures/php/services9.php | 1 + .../Tests/Fixtures/php/services9_compiled.php | 1 + .../Tests/Fixtures/xml/services9.xml | 1 + .../Tests/Fixtures/yaml/services9.yml | 1 + 9 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index c1c8da193f..0c622747cc 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -855,7 +855,11 @@ EOF; $code = " \$this->aliases = array(\n"; ksort($aliases); foreach ($aliases as $alias => $id) { - $code .= ' '.var_export($alias, true).' => '.var_export((string) $id, true).",\n"; + $id = (string) $id; + while (isset($aliases[$id])) { + $id = (string) $aliases[$id]; + } + $code .= ' '.var_export($alias, true).' => '.var_export($id, true).",\n"; } return $code . " );\n"; diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php index a5ceb2c68d..a311af348e 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php @@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Parameter; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Exception\RuntimeException; /** @@ -182,10 +183,10 @@ class XmlDumper extends Dumper * Adds a service alias. * * @param string $alias - * @param string $id + * @param Alias $id * @param \DOMElement $parent */ - private function addServiceAlias($alias, $id, \DOMElement $parent) + private function addServiceAlias($alias, Alias $id, \DOMElement $parent) { $service = $this->document->createElement('service'); $service->setAttribute('id', $alias); @@ -213,7 +214,11 @@ class XmlDumper extends Dumper $this->addService($definition, $id, $services); } - foreach ($this->container->getAliases() as $alias => $id) { + $aliases = $this->container->getAliases(); + foreach ($aliases as $alias => $id) { + while (isset($aliases[(string) $id])) { + $id = $aliases[(string) $id]; + } $this->addServiceAlias($alias, $id, $services); } $parent->appendChild($services); diff --git a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php index 0059f0d0d7..807e656283 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php @@ -182,7 +182,11 @@ class YamlDumper extends Dumper $code .= $this->addService($id, $definition); } - foreach ($this->container->getAliases() as $alias => $id) { + $aliases = $this->container->getAliases(); + foreach ($aliases as $alias => $id) { + while (isset($aliases[(string) $id])) { + $id = $aliases[(string) $id]; + } $code .= $this->addServiceAlias($alias, $id); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index 72d587ff07..0ea42d473b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -120,6 +120,20 @@ class PhpDumperTest extends \PHPUnit_Framework_TestCase } } + public function testAliases() + { + $container = include self::$fixturesPath.'/containers/container9.php'; + $container->compile(); + $dumper = new PhpDumper($container); + eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Aliases'))); + + $container = new \Symfony_DI_PhpDumper_Test_Aliases(); + $container->set('foo', $foo = new \stdClass); + $this->assertSame($foo, $container->get('foo')); + $this->assertSame($foo, $container->get('alias_for_foo')); + $this->assertSame($foo, $container->get('alias_for_alias')); + } + public function testOverrideServiceWhenUsingADumpedContainer() { require_once self::$fixturesPath.'/php/services9.php'; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php index 6ba3ad349d..6abe5e2a91 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php @@ -43,6 +43,7 @@ $container->getParameterBag()->add(array( 'foo' => 'bar', )); $container->setAlias('alias_for_foo', 'foo'); +$container->setAlias('alias_for_alias', 'alias_for_foo'); $container-> register('method_call1', 'FooClass')-> setFile(realpath(__DIR__.'/../includes/foo.php'))-> diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php index 5929351e43..9361a0e87c 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php @@ -38,6 +38,7 @@ class ProjectServiceContainer extends Container 'request' => 'getRequestService', ); $this->aliases = array( + 'alias_for_alias' => 'foo', 'alias_for_foo' => 'foo', ); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php index 4170ad1f30..b60cbff8e8 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php @@ -46,6 +46,7 @@ class ProjectServiceContainer extends Container 'request' => 'getRequestService', ); $this->aliases = array( + 'alias_for_alias' => 'foo', 'alias_for_foo' => 'foo', ); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml index d04d492e8d..cb3a1f69de 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml @@ -74,5 +74,6 @@ + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml index 46c3edfca4..6a4c3d5d97 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml @@ -69,3 +69,4 @@ services: - [setRequest, ['@?request']] alias_for_foo: @foo + alias_for_alias: @foo