From f2f86f1675bc4c9c5996337f42bdc742d3699697 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 16 Nov 2016 06:31:44 -0500 Subject: [PATCH] [FrameworkBundle][DI] Resolve cache pool adapters in CachePoolPass --- .../Compiler/CachePoolPass.php | 30 ++++- .../FrameworkBundle/FrameworkBundle.php | 2 +- .../Compiler/CachePoolPassTest.php | 4 +- .../ResolveDefinitionTemplatesPass.php | 90 +-------------- .../DependencyInjection/ContainerBuilder.php | 11 +- .../DefinitionDecorator.php | 103 ++++++++++++++++++ .../UnresolvedServiceDefinitionException.php | 21 ++++ .../Tests/ContainerBuilderTest.php | 2 +- 8 files changed, 163 insertions(+), 100 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Exception/UnresolvedServiceDefinitionException.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php index c856c0206a5d7..c3f46d972f487 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php @@ -18,6 +18,7 @@ use Symfony\Component\DependencyInjection\DefinitionDecorator; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; /** * @author Nicolas Grekas @@ -37,6 +38,7 @@ public function process(ContainerBuilder $container) } } + $resolvedPools = array(); $aliases = $container->getAliases(); $attributes = array( 'provider', @@ -44,15 +46,12 @@ public function process(ContainerBuilder $container) 'default_lifetime', ); foreach ($container->findTaggedServiceIds('cache.pool') as $id => $tags) { - $adapter = $pool = $container->getDefinition($id); + $pool = $container->getDefinition($id); if ($pool->isAbstract()) { continue; } - while ($adapter instanceof DefinitionDecorator) { - $adapter = $container->findDefinition($adapter->getParent()); - if ($t = $adapter->getTag('cache.pool')) { - $tags[0] += $t[0]; - } + if ($pool instanceof DefinitionDecorator) { + $resolvedPools[$id] = $pool = $this->resolveAdapters($container, $pool, $id, $tags); } if (!isset($tags[0]['namespace'])) { $tags[0]['namespace'] = $this->getNamespace($namespaceSuffix, $id); @@ -85,6 +84,9 @@ public function process(ContainerBuilder $container) $pool->addTag('cache.pool', array('clearer' => $clearer)); } } + foreach ($resolvedPools as $id => $pool) { + $container->setDefinition($id, $pool); + } } private function getNamespace($namespaceSuffix, $id) @@ -92,6 +94,22 @@ private function getNamespace($namespaceSuffix, $id) return substr(str_replace('/', '-', base64_encode(hash('sha256', $id.$namespaceSuffix, true))), 0, 10); } + private function resolveAdapters(ContainerBuilder $container, DefinitionDecorator $pool, $id, array &$tags) + { + if (!$container->has($parent = $pool->getParent())) { + throw new RuntimeException(sprintf('Service "%s": Parent definition "%s" does not exist.', $id, $parent)); + } + $adapter = $container->findDefinition($parent); + if ($t = $adapter->getTag('cache.pool')) { + $tags[0] += $t[0]; + } + if ($adapter instanceof DefinitionDecorator) { + $adapter = $this->resolveAdapters($container, $adapter, $parent, $tags); + } + + return $pool->resolveChanges($adapter); + } + /** * @internal */ diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index 72873aa6e97e8..9486aeecb7bf1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -93,7 +93,7 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new SerializerPass()); $container->addCompilerPass(new PropertyInfoPass()); $container->addCompilerPass(new ControllerArgumentValueResolverPass()); - $container->addCompilerPass(new CachePoolPass()); + $container->addCompilerPass(new CachePoolPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 1); $container->addCompilerPass(new ValidateWorkflowsPass()); $container->addCompilerPass(new CachePoolClearerPass(), PassConfig::TYPE_AFTER_REMOVING); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php index 53cb95837f7e8..29fb39ed91af8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php @@ -41,7 +41,8 @@ public function testNamespaceArgumentIsReplaced() $this->cachePoolPass->process($container); - $this->assertSame('kRFqMp5odS', $cachePool->getArgument(0)); + $this->assertSame(Definition::class, get_class($container->getDefinition('app.cache_pool'))); + $this->assertSame('kRFqMp5odS', $container->getDefinition('app.cache_pool')->getArgument(0)); } public function testArgsAreReplaced() @@ -75,6 +76,7 @@ public function testThrowsExceptionWhenCachePoolTagHasUnknownAttributes() $adapter = new Definition(); $adapter->setAbstract(true); $adapter->addTag('cache.pool'); + $adapter->addArgument(null); $container->setDefinition('app.cache_adapter', $adapter); $cachePool = new DefinitionDecorator('app.cache_adapter'); $cachePool->addTag('cache.pool', array('foobar' => 123)); diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php index a755fd61e12d7..84f3d6de84bd0 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php @@ -124,95 +124,7 @@ private function doResolveDefinition(ContainerBuilder $container, DefinitionDeco } $this->compiler->addLogMessage($this->formatter->formatResolveInheritance($this, $this->currentId, $parent)); - $def = new Definition(); - - // merge in parent definition - // purposely ignored attributes: abstract, tags - $def->setClass($parentDef->getClass()); - $def->setArguments($parentDef->getArguments()); - $def->setMethodCalls($parentDef->getMethodCalls()); - $def->setProperties($parentDef->getProperties()); - $def->setAutowiringTypes($parentDef->getAutowiringTypes()); - if ($parentDef->isDeprecated()) { - $def->setDeprecated(true, $parentDef->getDeprecationMessage('%service_id%')); - } - $def->setFactory($parentDef->getFactory()); - $def->setConfigurator($parentDef->getConfigurator()); - $def->setFile($parentDef->getFile()); - $def->setPublic($parentDef->isPublic()); - $def->setLazy($parentDef->isLazy()); - $def->setAutowired($parentDef->isAutowired()); - - // overwrite with values specified in the decorator - $changes = $definition->getChanges(); - if (isset($changes['class'])) { - $def->setClass($definition->getClass()); - } - if (isset($changes['factory'])) { - $def->setFactory($definition->getFactory()); - } - if (isset($changes['configurator'])) { - $def->setConfigurator($definition->getConfigurator()); - } - if (isset($changes['file'])) { - $def->setFile($definition->getFile()); - } - if (isset($changes['public'])) { - $def->setPublic($definition->isPublic()); - } - if (isset($changes['lazy'])) { - $def->setLazy($definition->isLazy()); - } - if (isset($changes['deprecated'])) { - $def->setDeprecated($definition->isDeprecated(), $definition->getDeprecationMessage('%service_id%')); - } - if (isset($changes['autowire'])) { - $def->setAutowired($definition->isAutowired()); - } - if (isset($changes['decorated_service'])) { - $decoratedService = $definition->getDecoratedService(); - if (null === $decoratedService) { - $def->setDecoratedService($decoratedService); - } else { - $def->setDecoratedService($decoratedService[0], $decoratedService[1], $decoratedService[2]); - } - } - - // merge arguments - foreach ($definition->getArguments() as $k => $v) { - if (is_numeric($k)) { - $def->addArgument($v); - continue; - } - - if (0 !== strpos($k, 'index_')) { - throw new RuntimeException(sprintf('Invalid argument key "%s" found.', $k)); - } - - $index = (int) substr($k, strlen('index_')); - $def->replaceArgument($index, $v); - } - - // merge properties - foreach ($definition->getProperties() as $k => $v) { - $def->setProperty($k, $v); - } - - // append method calls - if (count($calls = $definition->getMethodCalls()) > 0) { - $def->setMethodCalls(array_merge($def->getMethodCalls(), $calls)); - } - - // merge autowiring types - foreach ($definition->getAutowiringTypes() as $autowiringType) { - $def->addAutowiringType($autowiringType); - } - - // these attributes are always taken from the child - $def->setAbstract($definition->isAbstract()); - $def->setShared($definition->isShared()); - $def->setTags($definition->getTags()); - return $def; + return $definition->resolveChanges($parentDef); } } diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 8dca75a3a9eec..d159a2b951708 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -20,6 +20,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException; use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException; use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; +use Symfony\Component\DependencyInjection\Exception\UnresolvedServiceDefinitionException; use Symfony\Component\DependencyInjection\Extension\ExtensionInterface; use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag; use Symfony\Component\Config\Resource\FileResource; @@ -424,7 +425,7 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV } if (!isset($this->definitions[$id]) && isset($this->aliasDefinitions[$id])) { - return $this->get($this->aliasDefinitions[$id]); + return $this->get($this->aliasDefinitions[$id], $invalidBehavior); } try { @@ -441,6 +442,12 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV try { $service = $this->createService($definition, $id); + } catch (UnresolvedServiceDefinitionException $e) { + if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $invalidBehavior) { + return; + } + + throw $e; } finally { unset($this->loading[$id]); } @@ -836,7 +843,7 @@ public function findDefinition($id) private function createService(Definition $definition, $id, $tryProxy = true) { if ('Symfony\Component\DependencyInjection\Definition' !== get_class($definition)) { - throw new RuntimeException(sprintf('Constructing service "%s" from a %s is not supported at build time.', $id, get_class($definition))); + throw new UnresolvedServiceDefinitionException(sprintf('Constructing service "%s" from a %s is not supported at build time.', $id, get_class($definition))); } if ($definition->isSynthetic()) { diff --git a/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php index f17657a3a1b07..36af273a70567 100644 --- a/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php +++ b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php @@ -13,6 +13,7 @@ use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; /** * This definition decorates another definition. @@ -196,4 +197,106 @@ public function replaceArgument($index, $value) return $this; } + + /** + * Creates a new Definition by merging the current decorator with the given parent definition. + * + * @return Definition + */ + public function resolveChanges(Definition $parentDef) + { + if ($parentDef instanceof self) { + throw new InvalidArgumentException('$parenfDef must be a resolved Definition.'); + } + $def = new Definition(); + + // merge in parent definition + // purposely ignored attributes: abstract, tags + $def->setClass($parentDef->getClass()); + $def->setArguments($parentDef->getArguments()); + $def->setMethodCalls($parentDef->getMethodCalls()); + $def->setProperties($parentDef->getProperties()); + $def->setAutowiringTypes($parentDef->getAutowiringTypes()); + if ($parentDef->isDeprecated()) { + $def->setDeprecated(true, $parentDef->getDeprecationMessage('%service_id%')); + } + $def->setFactory($parentDef->getFactory()); + $def->setConfigurator($parentDef->getConfigurator()); + $def->setFile($parentDef->getFile()); + $def->setPublic($parentDef->isPublic()); + $def->setLazy($parentDef->isLazy()); + $def->setAutowired($parentDef->isAutowired()); + + // overwrite with values specified in the decorator + $changes = $this->getChanges(); + if (isset($changes['class'])) { + $def->setClass($this->getClass()); + } + if (isset($changes['factory'])) { + $def->setFactory($this->getFactory()); + } + if (isset($changes['configurator'])) { + $def->setConfigurator($this->getConfigurator()); + } + if (isset($changes['file'])) { + $def->setFile($this->getFile()); + } + if (isset($changes['public'])) { + $def->setPublic($this->isPublic()); + } + if (isset($changes['lazy'])) { + $def->setLazy($this->isLazy()); + } + if (isset($changes['deprecated'])) { + $def->setDeprecated($this->isDeprecated(), $this->getDeprecationMessage('%service_id%')); + } + if (isset($changes['autowire'])) { + $def->setAutowired($this->isAutowired()); + } + if (isset($changes['decorated_service'])) { + $decoratedService = $this->getDecoratedService(); + if (null === $decoratedService) { + $def->setDecoratedService($decoratedService); + } else { + $def->setDecoratedService($decoratedService[0], $decoratedService[1], $decoratedService[2]); + } + } + + // merge arguments + foreach ($this->getArguments() as $k => $v) { + if (is_numeric($k)) { + $def->addArgument($v); + continue; + } + + if (0 !== strpos($k, 'index_')) { + throw new RuntimeException(sprintf('Invalid argument key "%s" found.', $k)); + } + + $index = (int) substr($k, strlen('index_')); + $def->replaceArgument($index, $v); + } + + // merge properties + foreach ($this->getProperties() as $k => $v) { + $def->setProperty($k, $v); + } + + // append method calls + if ($calls = $this->getMethodCalls()) { + $def->setMethodCalls(array_merge($def->getMethodCalls(), $calls)); + } + + // merge autowiring types + foreach ($this->getAutowiringTypes() as $autowiringType) { + $def->addAutowiringType($autowiringType); + } + + // these attributes are always taken from the child + $def->setAbstract($this->isAbstract()); + $def->setShared($this->isShared()); + $def->setTags($this->getTags()); + + return $def; + } } diff --git a/src/Symfony/Component/DependencyInjection/Exception/UnresolvedServiceDefinitionException.php b/src/Symfony/Component/DependencyInjection/Exception/UnresolvedServiceDefinitionException.php new file mode 100644 index 0000000000000..e94d2282195cf --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Exception/UnresolvedServiceDefinitionException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Exception; + +/** + * This exception is thrown when an unresolved service definition is requested. + * + * @author Nicolas Grekas + */ +class UnresolvedServiceDefinitionException extends RuntimeException +{ +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index 2dcc5d1e24d94..a0d934ffece04 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -414,7 +414,7 @@ public function testResolveServices() } /** - * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedException \Symfony\Component\DependencyInjection\Exception\UnresolvedServiceDefinitionException * @expectedExceptionMessage Constructing service "foo" from a Symfony\Component\DependencyInjection\DefinitionDecorator is not supported at build time. */ public function testResolveServicesWithDecoratedDefinition()