[FrameworkBundle] Fix and add tests for cache pool wiring

symfony · fabpot · Apr 7, 2016 · Apr 1, 2016 · Feb 20, 2016 · Mar 4, 2016
commit 92b1a206134d9c7c2fe1c7ffca07391a5058cb70
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php
@@ -14,7 +14,6 @@
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\DefinitionDecorator;
-use Symfony\Compo
8000
nent\DependencyInjection\Reference;
 
 /**
  * @author Nicolas Grekas <p@tchwork.com>
@@ -28,7 +27,6 @@ public function process(ContainerBuilder $container)
     {
         foreach ($container->findTaggedServiceIds('cache.pool') as $id => $tags) {
             $pool = $container->getDefinition($id);
-            $namespaceArgIndex = isset($tags[0]['namespace_arg_index']) ? $tags[0]['namespace_arg_index'] : -1;
 
             if (!$pool instanceof DefinitionDecorator) {
                 throw new \InvalidArgumentException(sprintf('Services tagged with "cache.pool" must have a parent service but "%s" has none.', $id));
@@ -39,7 +37,11 @@ public function process(ContainerBuilder $container)
             do {
                 $adapterId = $adapter->getParent();
                 $adapter = $container->getDefinition($adapterId);
-            } while ($adapter instanceof DefinitionDecorator && !$adapter->getTag('cache.adapter'));
+            } 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));
+            }
 
             $tags = $adapter->getTag('cache.adapter');
 
@@ -51,14 +53,14 @@ public function process(ContainerBuilder $container)
                 throw new \InvalidArgumentException(sprintf('Services tagged as "cache.adapter" must be abstract: "%s" is not.', $adapterId));
             }
 
-            if (0 <= $namespaceArgIndex) {
+            if (0 <= $namespaceArgIndex = $tags[0]['namespace_arg_index']) {
                 $pool->replaceArgument($namespaceArgIndex, $this->getNamespace($id));
             }
         }
     }
 
     private function getNamespace($id)
     {
-        return substr(str_replace('/', '-', base64_encode(md5('symfony.'.$id, true)), 0, 10));
+        return substr(str_replace('/', '-', base64_encode(md5('symfony.'.$id, true))), 0, 10);
     }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -557,18 +557,16 @@ private function addCacheSection(ArrayNodeDefinition $rootNode)
                     ->info('Cache configuration')
                     ->fixXmlConfig('pool')
                     ->children()
-                        ->arrayNode('pool')
+                        ->arrayNode('pools')
                             ->useAttributeAsKey('name')
                             ->prototype('array')
-                                ->beforeNormalization()
-                                ->end()
                                 ->children()
                                     ->enumNode('type')
                                         ->info('The cache pool type (one of "apcu", "doctrine", "psr6" or "filesystem")')
                                         ->isRequired()
                                         ->values(array('apcu', 'doctrine', 'psr6', 'filesystem'))
                                     ->end()
-                                    ->integerNode('default_lifetime')->default(0)->end()
+                                    ->integerNode('default_lifetime')->defaultValue(0)->end()
                                     ->scalarNode('cache_provider_service')->defaultNull()->end()
                                     ->scalarNode('directory')->defaultNull()->end()
                                 ->end()

diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -139,7 +139,7 @@ public function load(array $configs, ContainerBuilder $container)
         }
 
         if (isset($config['cache'])) {
-            $this->registerCacheConfiguration($config['cache'], $container);
+            $this->registerCacheConfiguration($config['cache'], $container, $loader);
         }
 
         $loader->load('debug_prod.xml');
@@ -1022,11 +1022,11 @@ private function registerPropertyInfoConfiguration(array $config, ContainerBuild
 
     private function registerCacheConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader)
     {
-        if (!empty($config['pool'])) {
+        if (!empty($config['pools'])) {
             $loader->load('cache_adapters.xml');
         }
 
-        foreach ($config['pool'] as $name => $poolConfig) {
+        foreach ($config['pools'] as $name => $poolConfig) {
             $poolDefinition = new DefinitionDecorator('cache.adapter.'.$poolConfig['type']);
             $poolDefinition->replaceArgument(1, $poolConfig['default_lifetime']);
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_adapters.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_adapters.xml
@@ -7,27 +7,27 @@
     <services>
 
         <service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">
-            <tag name="cache.adapter" namespace-arg-index="0"></tag>
+            <tag name="cache.adapter" namespace-arg-index="0" />
             <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>
+            <tag name="cache.adapter" namespace-arg-index="2" />
             <argument /> <!-- doctrine provider service -->
             <argument /> <!-- default lifetime -->
             <argument /> <!-- namespace -->
         </service>
 
         <service id="cache.adapter.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true">
-            <tag name="cache.adapter" namespace-arg-index="2"></tag>
+            <tag name="cache.adapter" namespace-arg-index="2" />
             <argument /> <!-- PSR-6 provider service -->
             <argument /> <!-- default lifetime -->
             <argument /> <!-- namespace -->
         </service>
 
         <service id="cache.adapter.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true">
-            <tag name="cache.adapter" namespace-arg-index="2"></tag>
+            <tag name="cache.adapter" namespace-arg-index="2" />
             <argument>%kernel.cache_dir%</argument>
             <argument /> <!-- default lifetime -->
             <argument /> <!-- namespace -->

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/CachePoolPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/CachePoolPassTest.php
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php
@@ -0,0 +1,108 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler;
+
+use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\CachePoolPass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\DefinitionDecorator;
+
+class CachePoolPassTest extends \PHPUnit_Framework_TestCase
+{
+    private $cachePoolPass;
+
+    protected function setUp()
+    {
+        $this->cachePoolPass = new CachePoolPass();
+    }
+
+    public function testNamespaceArgumentIsReplaced()
+    {
+        $container = new Contain
D95F
erBuilder();
+        $adapter = new Definition();
+        $adapter->setAbstract(true);
+        $adapter->addTag('cache.adapter', array('namespace_arg_index' => 0));
+        $container->setDefinition('app.cache_adapter', $adapter);
+        $cachePool = new DefinitionDecorator('app.cache_adapter');
+        $cachePool->addArgument(null);
+        $cachePool->addTag('cache.pool');
+        $container->setDefinition('app.cache_pool', $cachePool);
+
+        $this->cachePoolPass->process($container);
+
+        $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()
+    {
+        $container = new ContainerBuilder();
+        $cachePool = new Definition();
+        $cachePool->addTag('cache.pool');
+        $container->setDefinition('app.cache_pool', $cachePool);
+
+        $this->cachePoolPass->process($container);
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage Services tagged with "cache.pool" must have a parent service tagged with "cache.adapter" but "app.cache_pool" has none.
+     */
+    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()
+    {
+        $container = new ContainerBuilder();
+        $adapter = new Definition();
+        $adapter->setAbstract(true);
+        $adapter->addTag('cache.adapter');
+        $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');
+        $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
@@ -2,26 +2,25 @@
 
 $container->loadFromExtension('framework', array(
     'cache' => array(
-        'adapters' => array(
+        'pools' => array(
             'foo' => array(
                 'type' => 'apcu',
-                'options' => array(
-                    'default_lifetime' => 30,
-                ),
+                'default_lifetime' => 30,
             ),
             'bar' => array(
                 'type' => 'doctrine',
-                'options' => array(
-                    'default_lifetime' => 5,
-                    'cache_provider_service' => 'app.doctrine_cache_provider',
-                ),
+                'default_lifetime' => 5,
+                'cache_provider_service' => 'app.doctrine_cache_provider',
             ),
             'baz' => array(
                 'type' => 'filesystem',
-                'options' => array(
-                    'default_lifetime' => 7,
-                    'directory' => 'app/cache/psr',
-                ),
+                'default_lifetime' => 7,
+                'directory' => 'app/cache/psr',
+            ),
+            'foobar' => array(
+                'type' => 'psr6',
+                'default_lifetime' => 10,
+                'cache_provider_service' => 'app.cache_pool',
             ),
         ),
     ),

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml
@@ -7,9 +7,10 @@
 
     <framework:config>
         <framework:cache>
-            <framework:adapter name="foo" type="apcu" default-lifetime="30" />
-            <framework:adapter name="bar" type="doctrine" default-lifetime="5" cache-provider-service="app.doctrine_cache_provider" />
-            <framework:adapter name="baz" type="filesystem" default-lifetime="7" directory="app/cache/psr" />
+            <framework:pool name="foo" type="apcu" default-lifetime="30" />
+            <framework:pool name="bar" type="doctrine" default-lifetime="5" cache-provider-service="app.doctrine_cache_provider" />
+            <framework:pool name="baz" type="filesystem" default-lifetime="7" directory="app/cache/psr" />
+            <framework:pool name="foobar" type="psr6" default-lifetime="10" cache-provider-service="app.cache_pool" />
         </framework:cache>
     </framework:config>
 </container>