[FrameworkBundle] use abstract cache.pool decoration and aliases

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 <p@tchwork.com>
@@ -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) {
+            while ($adapter instanceof DefinitionDecorator) {
-                throw new \InvalidArgumentException(sprintf('Services tagged with "cache.pool" must have a parent service but "%s" has none.', $id));
+                $adapter = $container->findDefinition($adapter->getParent());
+                if ($t = $adapter->getTag('cache.pool')) {
+                    $tags[0] += $t[0];
+                }
             }
-
+            if ($pool->isAbstract()) {
-            $adapter = $pool;
+                continue;
-
-            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 (isset($tags[0]['provider_service']) && is_string($tags[0]['provider_service'])) {
-            $tags = $adapter->getTag('cache.adapter');
+                $tags[0]['provider_service'] = new Reference($tags[0]['provider_service']);
-
-            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));
             }
-
+            $i = 0;
-            if (!$adapter->isAbstract()) {
+            foreach ($attributes as $attr) {
-                throw new \InvalidArgumentException(sprintf('Services tagged as "cache.adapter" must be abstract: "%s" is not.', $adapterId));
+                if (isset($tags[0][$attr])) {
+                    $pool->replaceArgument($i++, $tags[0][$attr]);
+                    unset($tags[0][$attr]);
+                }
             }
-
+            if (!empty($tags[0])) {
-            if (0 <= $namespaceArgIndex = $tags[0]['namespace_arg_index']) {
+                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]))));
-                $pool->replaceArgument($namespaceArgIndex, $this->getNamespace($id));
             }
         }
     }

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -561,13 +561,13 @@ class Configuration implements ConfigurationInterface
                             ->useAttributeAsKey('name')
                             ->prototype('array')
                                 ->children()
-                                    ->enumNode('type')
+                                    ->scalarNode('adapter_service')
-                                        ->info('The cache pool type (one of "apcu", "doctrine", "psr6" or "filesystem")')
+                                        ->info('The cache pool service to use as template definition.')
-                                        ->isRequired()
+                                        ->defaultValue('cache.adapter.default')
-                                        ->values(array('apcu', 'doctrine', 'psr6', 'filesystem'))
                                     ->end()
-                                    ->integerNode('default_lifetime')->defaultValue(0)->end()
+                                    ->booleanNode('public')->defaultFalse()->end()
-                                    ->scalarNode('cache_provider_service')->defaultNull()->end()
+                                    ->integerNode('default_lifetime')->defaultNull()->end()
+                                    ->scalarNode('provider_service')->defaultNull()->end()
                                     ->scalarNode('directory')->defaultNull()->end()
                                 ->end()
                             ->end()

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 = new DefinitionDecorator($poolConfig['adapter_service']);
-            $poolDefinition->replaceArgument(1, $poolConfig['default_lifetime']);
+            $poolDefinition->setPublic($poolConfig['public']);
+            unset($poolConfig['adapter_service'], $poolConfig['public']);
 
-            if ('doctrine' === $poolConfig['type'] || 'psr6' === $poolConfig['type']) {
+            $poolDefinition->addTag('cache.pool', $poolConfig);
-                $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');
             $container->setDefinition('cache.pool.'.$name, $poolDefinition);
         }
     }

src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_pools.xml

@@ -6,31 +6,33 @@
 
     <services>
 
+        <service id="cache.adapter.default" alias="cache.adapter.filesystem" />
+
         <service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">
-            <tag name="cache.adapter" namespace-arg-index="0" />
+            <tag name="cache.pool" />
             <argument /> <!-- namespace -->
             <argument /> <!-- default lifetime -->
         </service>
 
         <service id="cache.adapter.doctrine" class="Symfony\Component\Cache\Adapter\DoctrineAdapter" abstract="true">
-            <tag name="cache.adapter" namespace-arg-index="2" />
+            <tag name="cache.pool" />
-            <argument /> <!-- doctrine provider service -->
+            <argument /> <!-- Doctrine provider service -->
-            <argument /> <!-- default lifetime -->
             <argument /> <!-- namespace -->
+            <argument /> <!-- default lifetime -->
         </service>
 
         <service id="cache.adapter.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true">
-            <tag name="cache.adapter" namespace-arg-index="2" />
+            <tag name="cache.pool" />
             <argument /> <!-- PSR-6 provider service -->
-            <argument /> <!-- default lifetime -->
             <argument /> <!-- namespace -->
+            <argument /> <!-- default lifetime -->
         </service>
 
         <service id="cache.adapter.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true">
-            <tag name="cache.adapter" namespace-arg-index="2" />
+            <tag name="cache.pool" />
-            <argument>%kernel.cache_dir%</argument>
-            <argument /> <!-- default lifetime -->
             <argument /> <!-- namespace -->
+            <argument /> <!-- default lifetime -->
+            <argument>%kernel.cache_dir%</argument>
         </service>
 
     </services>

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -212,9 +212,10 @@
 
     <xsd:complexType name="cache_pool">
         <xsd:attribute name="name" type="xsd:string" use="required" />
-        <xsd:attribute name="type" type="xsd:string" use="required" />
+        <xsd:attribute name="adapter-service" type="xsd:string" />
+        <xsd:attribute name="public" type="xsd:boolean" />
         <xsd:attribute name="default-lifetime" type="xsd:integer" />
-        <xsd:attribute name="cache-provider-service" type="xsd:string" />
+        <xsd:attribute name="provider-service" type="xsd:string" />
         <xsd:attribute name="directory" type="xsd:string" />
     </xsd:complexType>
 </xsd:schema>

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));
     }
 
-    /**
+    public function testArgsAreReplaced()
-     * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage Services tagged with "cache.pool" must have a parent service but "app.cache_pool" has none.
-     */
-    public function testThrowsExceptionWhenCachePoolHasNoParentDefinition()
     {
         $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()
+    public function testThrowsExceptionWhenCachePoolTagHasUnknownAttributes()
-    {
-        $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()
     {
         $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');
+        $cachePool->addTag('cache.pool', array('foobar' => 123));
-        $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');
         $container->setDefinition('app.cache_pool', $cachePool);
 
         $this->cachePoolPass->process($container);

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,
             ),
         ),
     ),

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml

@@ -7,10 +7,11 @@
 
     <framework:config>
         <framework:cache>
-            <framework:pool name="foo" type="apcu" default-lifetime="30" />
+            <framework:pool name="foo" adapter-service="cache.adapter.apcu" default-lifetime="30" />
-            <framework:pool name="bar" type="doctrine" default-lifetime="5" cache-provider-service="app.doctrine_cache_provider" />
+            <framework:pool name="bar" adapter-service="cache.adapter.doctrine" default-lifetime="5" provider-service="app.doctrine_cache_provider" />
-            <framework:pool name="baz" type="filesystem" default-lifetime="7" directory="app/cache/psr" />
+            <framework:pool name="baz" adapter-service="cache.adapter.filesystem" default-lifetime="7" directory="app/cache/psr" />
-            <framework:pool name="foobar" type="psr6" default-lifetime="10" cache-provider-service="app.cache_pool" />
+            <framework:pool name="foobar" adapter-service="cache.adapter.psr6" default-lifetime="10" provider-service="app.cache_pool" />
+            <framework:pool name="def" default-lifetime="11" />
         </framework:cache>
     </framework:config>
 </container>

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

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, 'foo', 'cache.adapter.apcu', 30);
-        $this->assertCachePoolServiceDefinitionIsCreated($container, 'bar', 'doctrine', array('index_0' => new Reference('app.doctrine_cache_provider'), 'index_1' => 5));
+        $this->assertCachePoolServiceDefinitionIsCreated($container, 'bar', 'cache.adapter.doctrine', 5);
-        $this->assertCachePoolServiceDefinitionIsCreated($container, 'baz', 'filesystem', array('index_0' => 'app/cache/psr', 'index_1' => 7));
+        $this->assertCachePoolServiceDefinitionIsCreated($container, 'baz', 'cache.adapter.filesystem', 7);
-        $this->assertCachePoolServiceDefinitionIsCreated($container, 'foobar', 'psr6', array('index_0' => new Reference('app.cache_pool'), 'index_1' => 10));
+        $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->assertInstanceOf(DefinitionDecorator::class, $poolDefinition, sprintf('Cache pool "%s" is based on an abstract cache pool.', $name));
-        $this->assertEquals($arguments, $poolDefinition->getArguments());
 
-        $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) {
+        $tag = $poolDefinition->getTag('cache.pool');
-            case 'apcu':
+        $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));
+        $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));
-        $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.');
-        }
     }
 }