Skip to content

Commit

Permalink
merged branch Seldaek/fix-container-opt (PR #8193)
Browse files Browse the repository at this point in the history 
This PR was merged into the 2.3 branch.

Discussion
----------

[DI] Fixes access of aliases shared services

Fixes #8096 and I noticed that the aliases methods weren't really needed anymore so I removed them. I think it's fine since they were protected, but did it in a separate commit in case you just want the bugfix.

Note that while the DI tests pass, I didn't run this patch as part of a real app. I'd appreciate if someone has the time to verify it still works given it's slightly critical code.

Commits
-------

81b122d [DependencyInjection] Add support for aliases of aliases + regression test
d8c0ef7 [DependencyInjection] Rename ContainerBuilder::$aliases to avoid conflicting with the parent class
bb797ee [DependencyInjection] Remove get*Alias*Service methods from compiled containers
379f5e0 [DependencyInjection] Fix aliased access of shared services, fixes #8096
  • Loading branch information
@fabpot
fabpot committed Jun 5, 2013
2 parents b219e0a + 81b122d commit df0a02d
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 91 deletions.
8 changes: 8 additions & 0 deletions src/Symfony/Component/DependencyInjection/Container.php
View file
Expand Up @@ -69,6 +69,7 @@ class Container implements IntrospectableContainerInterface

protected $services;
protected $methodMap;
protected $aliases;
protected $scopes;
protected $scopeChildren;
protected $scopedServices;
Expand Down Expand Up @@ -253,6 +254,12 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
{
$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];
}
Expand All @@ -264,6 +271,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
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) {
Expand Down
26 changes: 13 additions & 13 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
View file
Expand Up @@ -57,7 +57,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
/**
* @var Alias[]
*/
private $aliases = array();
private $aliasDefinitions = array();

/**
* @var ResourceInterface[]
Expand Down Expand Up @@ -404,7 +404,7 @@ public function set($id, $service, $scope = self::SCOPE_CONTAINER)
$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);

Expand Down Expand Up @@ -438,7 +438,7 @@ public function has($id)
{
$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);
}

/**
Expand Down Expand Up @@ -475,8 +475,8 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
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 {
Expand Down Expand Up @@ -640,7 +640,7 @@ public function compile()
*/
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()));
}

/**
Expand All @@ -666,7 +666,7 @@ public function addAliases(array $aliases)
*/
public function setAliases(array $aliases)
{
$this->aliases = array();
$this->aliasDefinitions = array();
$this->addAliases($aliases);
}

Expand Down Expand Up @@ -697,7 +697,7 @@ public function setAlias($alias, $id)

unset($this->definitions[$alias]);

$this->aliases[$alias] = $id;
$this->aliasDefinitions[$alias] = $id;
}

/**
Expand All @@ -709,7 +709,7 @@ public function setAlias($alias, $id)
*/
public function removeAlias($alias)
{
unset($this->aliases[strtolower($alias)]);
unset($this->aliasDefinitions[strtolower($alias)]);
}

/**
Expand All @@ -723,7 +723,7 @@ public function removeAlias($alias)
*/
public function hasAlias($id)
{
return isset($this->aliases[strtolower($id)]);
return isset($this->aliasDefinitions[strtolower($id)]);
}

/**
Expand All @@ -735,7 +735,7 @@ public function hasAlias($id)
*/
public function getAliases()
{
return $this->aliases;
return $this->aliasDefinitions;
}

/**
Expand All @@ -757,7 +757,7 @@ public function getAlias($id)
throw new InvalidArgumentException(sprintf('The service alias "%s" does not exist.', $id));
}

return $this->aliases[$id];
return $this->aliasDefinitions[$id];
}

/**
Expand Down Expand Up @@ -837,7 +837,7 @@ public function setDefinition($id, Definition $definition)

$id = strtolower($id);

unset($this->aliases[$id]);
unset($this->aliasDefinitions[$id]);

return $this->definitions[$id] = $definition;
}
Expand Down
103 changes: 50 additions & 53 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
View file
Expand Up @@ -594,47 +594,14 @@ private function addService($id, $definition)
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 <<<EOF
/**
* Gets the $alias service alias.
*
* @return $type An instance of the $id service
*/
protected function get{$name}Service()
{
return {$this->getServiceCall($id)};
}
EOF;
}

/**
* Adds multiple services
*
* @return string
*/
private function addServices()
{
$publicServices = $privateServices = $aliasServices = $synchronizers = '';
$publicServices = $privateServices = $synchronizers = '';
$definitions = $this->container->getDefinitions();
ksort($definitions);
foreach ($definitions as $id => $definition) {
Expand All @@ -647,13 +614,7 @@ private function addServices()
$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;
}

/**
Expand Down Expand Up @@ -799,6 +760,9 @@ public function __construct()
$code .= " \$this->scopeChildren = ".$this->dumpValue($this->container->getScopeChildren()).";\n";
}

$code .= $this->addMethodMap();
$code .= $this->addAliases();

$code .= <<<EOF
}
Expand Down Expand Up @@ -846,26 +810,59 @@ public function __construct()
$code .= " \$this->scopeChildren = array();\n";
}

// build method map
$code .= " \$this->methodMap = array(\n";
$definitions = $this->container->getDefinitions();
$code .= $this->addMethodMap();
$code .= $this->addAliases();

$code .= <<<EOF
}
EOF;

return $code;
}

/**
* Adds the methodMap property definition
*
* @return string
*/
private function addMethodMap()
{
if (!$definitions = $this->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";
}
$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 .= <<<EOF
return $code . " );\n";
}

EOF;
/**
* Adds the aliases property definition
*
* @return string
*/
private function addAliases()
{
if (!$aliases = $this->container->getAliases()) {
return '';
}

return $code;
$code = " \$this->aliases = array(\n";
ksort($aliases);
foreach ($aliases as $alias => $id) {
$id = (string) $id;
while (isset($aliases[$id])) {
$id = (string) $aliases[$id];
}
$code .= ' '.var_export($alias, true).' => '.var_export($id, true).",\n";
}

return $code . " );\n";
}

/**
Expand Down
11 changes: 8 additions & 3 deletions src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
View file
Expand Up @@ -15,6 +15,7 @@
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;

/**
Expand Down Expand Up @@ -182,10 +183,10 @@ private function addService($definition, $id, \DOMElement $parent)
* 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);
Expand Down Expand Up @@ -213,7 +214,11 @@ private function addServices(\DOMElement $parent)
$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);
Expand Down
6 changes: 5 additions & 1 deletion src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
View file
Expand Up @@ -182,7 +182,11 @@ private function addServices()
$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);
}

Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
View file
Expand Up @@ -120,6 +120,20 @@ public function testAddService()
}
}

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';
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php
View file
Expand Up @@ -43,6 +43,7 @@
'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'))->
Expand Down

0 comments on commit df0a02d

Please sign in to comment.