Skip to content

Commit

Permalink
[DI] Skip hot_path tag for deprecated services as their class might a…
Browse files Browse the repository at this point in the history 
…lso be
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Nov 19, 2017
1 parent 219d710 commit 1775bba
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 15 deletions.
6 changes: 3 additions & 3 deletions src/Symfony/Component/DependencyInjection/Compiler/ResolveHotPathPass.php
View file
Expand Up @@ -52,12 +52,12 @@ protected function processValue($value, $isRoot = false)
if ($value instanceof ArgumentInterface) {
return $value;
}
if ($value instanceof Definition && $isRoot && (isset($this->resolvedIds[$this->currentId]) || !$value->hasTag($this->tagName))) {
return $value;
if ($value instanceof Definition && $isRoot && (isset($this->resolvedIds[$this->currentId]) || !$value->hasTag($this->tagName) || $value->isDeprecated())) {
return $value->isDeprecated() ? $value->clearTag($this->tagName) : $value;
}
if ($value instanceof Reference && ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE !== $value->getInvalidBehavior() && $this->container->has($id = (string) $value)) {
$definition = $this->container->findDefinition($id);
if (!$definition->hasTag($this->tagName)) {
if (!$definition->hasTag($this->tagName) && !$definition->isDeprecated()) {
$this->resolvedIds[$id] = true;
$definition->addTag($this->tagName);
parent::processValue($definition, false);
Expand Down
21 changes: 13 additions & 8 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
View file
Expand Up @@ -271,11 +271,11 @@ private function addServiceLocalTempVariables($cId, Definition $definition, arra

array_unshift($inlinedDefinitions, $definition);

$collectLineage = $this->inlineRequires && !($this->hotPathTag && $definition->hasTag($this->hotPathTag));
$collectLineage = $this->inlineRequires && !$this->isHotPath($definition);
$isNonLazyShared = !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared();
$lineage = $calls = $behavior = array();
foreach ($inlinedDefinitions as $iDefinition) {
if ($collectLineage && $class = is_array($factory = $iDefinition->getFactory()) && is_string($factory[0]) ? $factory[0] : $iDefinition->getClass()) {
if ($collectLineage && !$iDefinition->isDeprecated() && $class = is_array($factory = $iDefinition->getFactory()) && is_string($factory[0]) ? $factory[0] : $iDefinition->getClass()) {
$this->collectLineage($class, $lineage);
}
$this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior, $isNonLazyShared);
Expand Down Expand Up @@ -763,7 +763,7 @@ private function addService($id, Definition $definition, &$file = null)
$lazyInitialization = '';
}

$asFile = $this->asFiles && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag));
$asFile = $this->asFiles && $definition->isShared() && !$this->isHotPath($definition);
$methodName = $this->generateMethodName($id);
if ($asFile) {
$file = $methodName.'.php';
Expand Down Expand Up @@ -829,7 +829,7 @@ private function addServices()
$definitions = $this->container->getDefinitions();
ksort($definitions);
foreach ($definitions as $id => $definition) {
if ($definition->isSynthetic() || ($this->asFiles && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag)))) {
if ($definition->isSynthetic() || ($this->asFiles && $definition->isShared() && !$this->isHotPath($definition))) {
continue;
}
if ($definition->isPublic()) {
Expand All @@ -847,7 +847,7 @@ private function generateServiceFiles()
$definitions = $this->container->getDefinitions();
ksort($definitions);
foreach ($definitions as $id => $definition) {
if (!$definition->isSynthetic() && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag))) {
if (!$definition->isSynthetic() && $definition->isShared() && !$this->isHotPath($definition)) {
$code = $this->addService($id, $definition, $file);
yield $file => $code;
}
Expand Down Expand Up @@ -1120,7 +1120,7 @@ private function addMethodMap()
$definitions = $this->container->getDefinitions();
ksort($definitions);
foreach ($definitions as $id => $definition) {
if (!$definition->isSynthetic() && (!$this->asFiles || !$definition->isShared() || ($this->hotPathTag && $definition->hasTag($this->hotPathTag)))) {
if (!$definition->isSynthetic() && (!$this->asFiles || !$definition->isShared() || $this->isHotPath($definition))) {
$code .= ' '.$this->export($id).' => '.$this->export($this->generateMethodName($id)).",\n";
}
}
Expand All @@ -1139,7 +1139,7 @@ private function addFileMap()
$definitions = $this->container->getDefinitions();
ksort($definitions);
foreach ($definitions as $id => $definition) {
if (!$definition->isSynthetic() && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag))) {
if (!$definition->isSynthetic() && $definition->isShared() && !$this->isHotPath($definition)) {
$code .= sprintf(" %s => __DIR__.'/%s.php',\n", $this->export($id), $this->generateMethodName($id));
}
}
Expand Down Expand Up @@ -1856,7 +1856,7 @@ private function getServiceCall($id, Reference $reference = null)
if ($definition->isShared()) {
$code = sprintf('$this->services[\'%s\'] = %s', $id, $code);
}
} elseif ($this->asFiles && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag))) {
} elseif ($this->asFiles && $definition->isShared() && !$this->isHotPath($definition)) {
$code = sprintf("\$this->load(__DIR__.'/%s.php')", $this->generateMethodName($id));
} else {
$code = sprintf('$this->%s()', $this->generateMethodName($id));
Expand Down Expand Up @@ -1988,6 +1988,11 @@ private function getExpressionLanguage()
return $this->expressionLanguage;
}

private function isHotPath(Definition $definition)
{
return $this->hotPathTag && $definition->hasTag($this->hotPathTag) && !$definition->isDeprecated();
}

private function export($value)
{
if (null !== $this->targetDirRegex && is_string($value) && preg_match($this->targetDirRegex, $value, $matches, PREG_OFFSET_CAPTURE)) {
Expand Down
14 changes: 10 additions & 4 deletions src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveHotPathPassTest.php
View file
Expand Up @@ -34,11 +34,15 @@ public function testProcess()
;

$container->register('lazy');
$container->register('bar');
$container->register('bar')->addArgument(new Reference('buz'));
$container->register('baz')->addArgument(new Reference('lazy'));
$container->register('baz')->addArgument(new Reference('lazy'));
$container->register('bar')
->addArgument(new Reference('buz'))
->addArgument(new Reference('deprec_ref_notag'));
$container->register('baz')
->addArgument(new Reference('lazy'))
->addArgument(new Reference('lazy'));
$container->register('buz');
$container->register('deprec_with_tag')->setDeprecated()->addTag('container.hot_path');
$container->register('deprec_ref_notag')->setDeprecated();

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

Expand All @@ -47,5 +51,7 @@ public function testProcess()
$this->assertTrue($container->getDefinition('buz')->hasTag('container.hot_path'));
$this->assertFalse($container->getDefinition('baz')->hasTag('container.hot_path'));
$this->assertFalse($container->getDefinition('service_container')->hasTag('container.hot_path'));
$this->assertFalse($container->getDefinition('deprec_with_tag')->hasTag('container.hot_path'));
$this->assertFalse($container->getDefinition('deprec_ref_notag')->hasTag('container.hot_path'));
}
}

0 comments on commit 1775bba

Please sign in to comment.