From 4740c5ccfb4f3264e974b166f0b02e6402317056 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 14 Mar 2016 10:01:08 +0100 Subject: [PATCH] [FrameworkBundle] use abstract cache.pool decoration and aliases --- .../Compiler/CachePoolPass.php | 50 +++++++------- .../DependencyInjection/Configuration.php | 12 ++-- .../FrameworkExtension.php | 15 ++--- .../{cache_adapters.xml => cache_pools.xml} | 20 +++--- .../Resources/config/schema/symfony-1.0.xsd | 5 +- .../Compiler/CachePoolPassTest.php | 65 ++++++------------- .../Fixtures/php/cache.php | 15 +++-- .../Fixtures/xml/cache.xml | 9 +-- .../Fixtures/yml/cache.yml | 14 ++-- .../FrameworkExtensionTest.php | 44 +++++++------ 10 files changed, 117 insertions(+), 132 deletions(-) rename src/Symfony/Bundle/FrameworkBundle/Resources/config/{cache_adapters.xml => cache_pools.xml} (80%) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php index 395169caac..4c56a58781 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php @@ -14,6 +14,7 @@ namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\DefinitionDecorator; +use Symfony\Component\DependencyInjection\Reference; /** * @author Nicolas Grekas @@ -25,36 +26,37 @@ class CachePoolPass implements CompilerPassInterface */ public function process(ContainerBuilder $container) { + $attributes = array( + 'provider_service', + 'namespace', + 'default_lifetime', + 'directory', + ); foreach ($container->findTaggedServiceIds('cache.pool') as $id => $tags) { - $pool = $container->getDefinition($id); + $adapter = $pool = $container->getDefinition($id); + $tags[0] += array('namespace' => $this->getNamespace($id)); - if (!$pool instanceof DefinitionDecorator) { - throw new \InvalidArgumentException(sprintf('Services tagged with "cache.pool" must have a parent service but "%s" has none.', $id)); + while ($adapter instanceof DefinitionDecorator) { + $adapter = $container->findDefinition($adapter->getParent()); + if ($t = $adapter->getTag('cache.pool')) { + $tags[0] += $t[0]; + } } - - $adapter = $pool; - - do { - $adapterId = $adapter->getParent(); - $adapter = $container->getDefinition($adapterId); - } while ($adapter instanceof DefinitionDecorator && !$adapter->hasTag('cache.adapter')); - - if (!$adapter->hasTag('cache.adapter')) { - throw new \InvalidArgumentException(sprintf('Services tagged with "cache.pool" must have a parent service tagged with "cache.adapter" but "%s" has none.', $id)); + if ($pool->isAbstract()) { + continue; } - - $tags = $adapter->getTag('cache.adapter'); - - if (!isset($tags[0]['namespace_arg_index'])) { - throw new \InvalidArgumentException(sprintf('Invalid "cache.adapter" tag for service "%s": attribute "namespace_arg_index" is missing.', $adapterId)); + if (isset($tags[0]['provider_service']) && is_string($tags[0]['provider_service'])) { + $tags[0]['provider_service'] = new Reference($tags[0]['provider_service']); } - - if (!$adapter->isAbstract()) { - throw new \InvalidArgumentException(sprintf('Services tagged as "cache.adapter" must be abstract: "%s" is not.', $adapterId)); + $i = 0; + foreach ($attributes as $attr) { + if (isset($tags[0][$attr])) { + $pool->replaceArgument($i++, $tags[0][$attr]); + unset($tags[0][$attr]); + } } - - if (0 <= $namespaceArgIndex = $tags[0]['namespace_arg_index']) { - $pool->replaceArgument($namespaceArgIndex, $this->getNamespace($id)); + if (!empty($tags[0])) { + throw new \InvalidArgumentException(sprintf('Invalid "cache.pool" tag for service "%s": accepted attributes are "provider_service", "namespace", "default_lifetime" and "directory", found "%s".', $id, implode('", "', array_keys($tags[0])))); } } } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index deb8d1e49e..20a9e8b299 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -561,13 +561,13 @@ class Configuration implements ConfigurationInterface ->useAttributeAsKey('name') ->prototype('array') ->children() - ->enumNode('type') - ->info('The cache pool type (one of "apcu", "doctrine", "psr6" or "filesystem")') - ->isRequired() - ->values(array('apcu', 'doctrine', 'psr6', 'filesystem')) + ->scalarNode('adapter_service') + ->info('The cache pool service to use as template definition.') + ->defaultValue('cache.adapter.default') ->end() - ->integerNode('default_lifetime')->defaultValue(0)->end() - ->scalarNode('cache_provider_service')->defaultNull()->end() + ->booleanNode('public')->defaultFalse()->end() + ->integerNode('default_lifetime')->defaultNull()->end() + ->scalarNode('provider_service')->defaultNull()->end() ->scalarNode('directory')->defaultNull()->end() ->end() ->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index fd1ae70347..f00c2a42ae 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1023,20 +1023,15 @@ class FrameworkExtension extends Extension private function registerCacheConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader) { if (!empty($config['pools'])) { - $loader->load('cache_adapters.xml'); + $loader->load('cache_pools.xml'); } foreach ($config['pools'] as $name => $poolConfig) { - $poolDefinition = new DefinitionDecorator('cache.adapter.'.$poolConfig['type']); - $poolDefinition->replaceArgument(1, $poolConfig['default_lifetime']); + $poolDefinition = new DefinitionDecorator($poolConfig['adapter_service']); + $poolDefinition->setPublic($poolConfig['public']); + unset($poolConfig['adapter_service'], $poolConfig['public']); - if ('doctrine' === $poolConfig['type'] || 'psr6' === $poolConfig['type']) { - $poolDefinition->replaceArgument(0, new Reference($poolConfig['cache_provider_service'])); - } elseif ('filesystem' === $poolConfig['type'] && isset($poolConfig['directory'][0])) { - $poolDefinition->replaceArgument(0, $poolConfig['directory']); - } - - $poolDefinition->addTag('cache.pool'); + $poolDefinition->addTag('cache.pool', $poolConfig); $container->setDefinition('cache.pool.'.$name, $poolDefinition); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_adapters.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_pools.xml similarity index 80% rename from src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_adapters.xml rename to src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_pools.xml index 9c49c8672d..bea42c651e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_adapters.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_pools.xml @@ -6,31 +6,33 @@ + + - + - - - + + + - + - + - - %kernel.cache_dir% - + + + %kernel.cache_dir% diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index cee9299e44..26fed333de 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -212,9 +212,10 @@ - + + - + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php index f07c04c7e0..f69c97ddc2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php @@ -15,6 +15,7 @@ use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\CachePoolPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\DefinitionDecorator; +use Symfony\Component\DependencyInjection\Reference; class CachePoolPassTest extends \PHPUnit_Framework_TestCase { @@ -30,9 +31,10 @@ class CachePoolPassTest extends \PHPUnit_Framework_TestCase $container = new ContainerBuilder(); $adapter = new Definition(); $adapter->setAbstract(true); - $adapter->addTag('cache.adapter', array('namespace_arg_index' => 0)); + $adapter->addTag('cache.pool'); $container->setDefinition('app.cache_adapter', $adapter); - $cachePool = new DefinitionDecorator('app.cache_adapter'); + $container->setAlias('app.cache_adapter_alias', 'app.cache_adapter'); + $cachePool = new DefinitionDecorator('app.cache_adapter_alias'); $cachePool->addArgument(null); $cachePool->addTag('cache.pool'); $container->setDefinition('app.cache_pool', $cachePool); @@ -42,65 +44,40 @@ class CachePoolPassTest extends \PHPUnit_Framework_TestCase $this->assertSame('yRnzIIVLvL', $cachePool->getArgument(0)); } - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Services tagged with "cache.pool" must have a parent service but "app.cache_pool" has none. - */ - public function testThrowsExceptionWhenCachePoolHasNoParentDefinition() + public function testArgsAreReplaced() { $container = new ContainerBuilder(); $cachePool = new Definition(); - $cachePool->addTag('cache.pool'); + $cachePool->addTag('cache.pool', array( + 'provider_service' => 'foobar', + 'default_lifetime' => 3, + )); + $cachePool->addArgument(null); + $cachePool->addArgument(null); + $cachePool->addArgument(null); $container->setDefinition('app.cache_pool', $cachePool); $this->cachePoolPass->process($container); + + $this->assertInstanceOf(Reference::class, $cachePool->getArgument(0)); + $this->assertSame('foobar', (string) $cachePool->getArgument(0)); + $this->assertSame('yRnzIIVLvL', $cachePool->getArgument(1)); + $this->assertSame(3, $cachePool->getArgument(2)); } /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Services tagged with "cache.pool" must have a parent service tagged with "cache.adapter" but "app.cache_pool" has none. + * @expectedExceptionMessage Invalid "cache.pool" tag for service "app.cache_pool": accepted attributes are */ - public function testThrowsExceptionWhenCachePoolIsNotBasedOnAdapter() - { - $container = new ContainerBuilder(); - $container->register('app.cache_adapter'); - $cachePool = new DefinitionDecorator('app.cache_adapter'); - $cachePool->addTag('cache.pool'); - $container->setDefinition('app.cache_pool', $cachePool); - - $this->cachePoolPass->process($container); - } - - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Invalid "cache.adapter" tag for service "app.cache_adapter": attribute "namespace_arg_index" is missing. - */ - public function testThrowsExceptionWhenCacheAdapterDefinesNoNamespaceArgument() + public function testThrowsExceptionWhenCachePoolTagHasUnknownAttributes() { $container = new ContainerBuilder(); $adapter = new Definition(); $adapter->setAbstract(true); - $adapter->addTag('cache.adapter'); + $adapter->addTag('cache.pool'); $container->setDefinition('app.cache_adapter', $adapter); $cachePool = new DefinitionDecorator('app.cache_adapter'); - $cachePool->addTag('cache.pool'); - $container->setDefinition('app.cache_pool', $cachePool); - - $this->cachePoolPass->process($container); - } - - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Services tagged as "cache.adapter" must be abstract: "app.cache_adapter" is not. - */ - public function testThrowsExceptionWhenCacheAdapterIsNotAbstract() - { - $container = new ContainerBuilder(); - $adapter = new Definition(); - $adapter->addTag('cache.adapter', array('namespace_arg_index' => 0)); - $container->setDefinition('app.cache_adapter', $adapter); - $cachePool = new DefinitionDecorator('app.cache_adapter'); - $cachePool->addTag('cache.pool'); + $cachePool->addTag('cache.pool', array('foobar' => 123)); $container->setDefinition('app.cache_pool', $cachePool); $this->cachePoolPass->process($container); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/cache.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/cache.php index 63e5441293..2cbd3b51eb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/cache.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/cache.php @@ -4,23 +4,26 @@ $container->loadFromExtension('framework', array( 'cache' => array( 'pools' => array( 'foo' => array( - 'type' => 'apcu', + 'adapter_service' => 'cache.adapter.apcu', 'default_lifetime' => 30, ), 'bar' => array( - 'type' => 'doctrine', + 'adapter_service' => 'cache.adapter.doctrine', 'default_lifetime' => 5, - 'cache_provider_service' => 'app.doctrine_cache_provider', + 'provider_service' => 'app.doctrine_cache_provider', ), 'baz' => array( - 'type' => 'filesystem', + 'adapter_service' => 'cache.adapter.filesystem', 'default_lifetime' => 7, 'directory' => 'app/cache/psr', ), 'foobar' => array( - 'type' => 'psr6', + 'adapter_service' => 'cache.adapter.psr6', 'default_lifetime' => 10, - 'cache_provider_service' => 'app.cache_pool', + 'provider_service' => 'app.cache_pool', + ), + 'def' => array( + 'default_lifetime' => 11, ), ), ), diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml index f3d26f7380..13b05f6718 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml @@ -7,10 +7,11 @@ - - - - + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/cache.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/cache.yml index 0d45b13527..f3c6048a77 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/cache.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/cache.yml @@ -2,17 +2,19 @@ framework: cache: pools: foo: - type: apcu + adapter_service: cache.adapter.apcu default_lifetime: 30 bar: - type: doctrine + adapter_service: cache.adapter.doctrine default_lifetime: 5 - cache_provider_service: app.doctrine_cache_provider + provider_service: app.doctrine_cache_provider baz: - type: filesystem + adapter_service: cache.adapter.filesystem default_lifetime: 7 directory: app/cache/psr foobar: - type: psr6 + adapter_service: cache.adapter.psr6 default_lifetime: 10 - cache_provider_service: app.cache_pool + provider_service: app.cache_pool + def: + default_lifetime: 11 diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 93478df449..6508fe005e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -575,10 +575,11 @@ abstract class FrameworkExtensionTest extends TestCase { $container = $this->createContainerFromFile('cache'); - $this->assertCachePoolServiceDefinitionIsCreated($container, 'foo', 'apcu', array('index_1' => 30), 0); - $this->assertCachePoolServiceDefinitionIsCreated($container, 'bar', 'doctrine', array('index_0' => new Reference('app.doctrine_cache_provider'), 'index_1' => 5)); - $this->assertCachePoolServiceDefinitionIsCreated($container, 'baz', 'filesystem', array('index_0' => 'app/cache/psr', 'index_1' => 7)); - $this->assertCachePoolServiceDefinitionIsCreated($container, 'foobar', 'psr6', array('index_0' => new Reference('app.cache_pool'), 'index_1' => 10)); + $this->assertCachePoolServiceDefinitionIsCreated($container, 'foo', 'cache.adapter.apcu', 30); + $this->assertCachePoolServiceDefinitionIsCreated($container, 'bar', 'cache.adapter.doctrine', 5); + $this->assertCachePoolServiceDefinitionIsCreated($container, 'baz', 'cache.adapter.filesystem', 7); + $this->assertCachePoolServiceDefinitionIsCreated($container, 'foobar', 'cache.adapter.psr6', 10); + $this->assertCachePoolServiceDefinitionIsCreated($container, 'def', 'cache.adapter.filesystem', 11); } protected function createContainer(array $data = array()) @@ -650,38 +651,39 @@ abstract class FrameworkExtensionTest extends TestCase } } - private function assertCachePoolServiceDefinitionIsCreated(ContainerBuilder $container, $name, $type, array $arguments, $namespaceArgumentIndex = null) + private function assertCachePoolServiceDefinitionIsCreated(ContainerBuilder $container, $name, $adapter, $defaultLifetime) { $id = 'cache.pool.'.$name; - $this->assertTrue($container->has($id), sprintf('Service definition "%s" for cache pool of type "%s" is registered', $id, $type)); + $this->assertTrue($container->has($id), sprintf('Service definition "%s" for cache pool of type "%s" is registered', $id, $adapter)); $poolDefinition = $container->getDefinition($id); - $this->assertInstanceOf(DefinitionDecorator::class, $poolDefinition, sprintf('Cache pool "%s" is based on an abstract cache adapter.', $name)); - $this->assertEquals($arguments, $poolDefinition->getArguments()); + $this->assertInstanceOf(DefinitionDecorator::class, $poolDefinition, sprintf('Cache pool "%s" is based on an abstract cache pool.', $name)); - $adapterDefinition = $container->getDefinition($poolDefinition->getParent()); + $this->assertTrue($poolDefinition->hasTag('cache.pool'), sprintf('Service definition "%s" is tagged with the "cache.pool" tag.', $id)); + $this->assertFalse($poolDefinition->isAbstract(), sprintf('Service definition "%s" is not abstract.', $id)); - switch ($type) { - case 'apcu': + $tag = $poolDefinition->getTag('cache.pool'); + $this->assertTrue(isset($tag[0]['default_lifetime']), 'The default lifetime is stored as an attribute of the "cache.pool" tag.'); + $this->assertSame($defaultLifetime, $tag[0]['default_lifetime'], 'The default lifetime is stored as an attribute of the "cache.pool" tag.'); + + $adapterId = $poolDefinition->getParent(); + $adapterDefinition = $container->findDefinition($adapterId); + + switch ($adapter) { + case 'cache.adapter.apcu': $this->assertSame(ApcuAdapter::class, $adapterDefinition->getClass()); break; - case 'doctrine': + case 'cache.adapter.doctrine': $this->assertSame(DoctrineAdapter::class, $adapterDefinition->getClass()); break; - case 'filesystem': + case 'cache.adapter.filesystem': $this->assertSame(FilesystemAdapter::class, $adapterDefinition->getClass()); break; } - $this->assertTrue($adapterDefinition->hasTag('cache.adapter'), sprintf('Service definition "%s" is tagged with the "cache.adapter" tag.', $id)); - - $tag = $adapterDefinition->getTag('cache.adapter'); - - if (null !== $namespaceArgumentIndex) { - $this->assertTrue(isset($tag[0]['namespace-arg-index']), 'The namespace argument index is given by the "namespace-arg-index" attribute of the "cache.adapter" tag.'); - $this->assertSame($namespaceArgumentIndex, $tag[0]['namespace-arg-index'], 'The namespace argument index is given by the "namespace-arg-index" attribute of the "cache.adapter" tag.'); - } + $this->assertTrue($adapterDefinition->hasTag('cache.pool'), sprintf('Service definition "%s" is tagged with the "cache.pool" tag.', $adapterId)); + $this->assertTrue($adapterDefinition->isAbstract(), sprintf('Service definition "%s" is abstract.', $adapterId)); } }