Skip to content

Commit b0a2860

Browse files
nicolas-grekasnicolas-grekas
committed
Simplify cache.pool wiring
1 parent fe5009c commit b0a2860

File tree

10 files changed

+92
-71
lines changed

10 files changed

+92
-71
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1515
use Symfony\Component\DependencyInjection\ContainerBuilder;
1616
use Symfony\Component\DependencyInjection\DefinitionDecorator;
17+
use Symfony\Component\DependencyInjection\Reference;
1718

1819
/**
1920
* @author Nicolas Grekas <p@tchwork.com>
@@ -27,18 +28,27 @@ public function process(ContainerBuilder $container)
2728
{
2829
foreach ($container->findTaggedServiceIds('cache.pool') as $id => $tags) {
2930
$adapter = $pool = $container->getDefinition($id);
31+
$tags[0] += array('namespace' => $this->getNamespace($id));
3032

31-
while (!isset($tags[0]['namespace_arg_index']) && $adapter instanceof DefinitionDecorator) {
33+
while ($adapter instanceof DefinitionDecorator) {
3234
$adapter = $container->findDefinition($adapter->getParent());
33-
$tags = $adapter->getTag('cache.pool');
35+
if ($t = $adapter->getTag('cache.pool')) {
36+
$tags[0] += $t[0];
37+
}
3438
}
35-
3639
if (!isset($tags[0]['namespace_arg_index'])) {
3740
throw new \InvalidArgumentException(sprintf('Invalid "cache.pool" tag for service "%s": attribute "namespace_arg_index" is missing.', $id));
3841
}
39-
40-
if (!$pool->isAbstract() && 0 <= $namespaceArgIndex = $tags[0]['namespace_arg_index']) {
41-
$pool->replaceArgument($namespaceArgIndex, $this->getNamespace($id));
42+
if ($pool->isAbstract()) {
43+
continue;
44+
}
45+
if (isset($tags[0]['provider_service']) && is_string($tags[0]['provider_service'])) {
46+
$tags[0]['provider_service'] = new Reference($tags[0]['provider_service']);
47+
}
48+
foreach ($tags[0] as $argName => $argValue) {
49+
if (isset($tags[0][$argName.'_arg_index']) && 0 <= $tags[0][$argName.'_arg_index']) {
50+
$pool->replaceArgument($tags[0][$argName.'_arg_index'], $argValue);
51+
}
4252
}
4353
}
4454
}

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -562,20 +562,14 @@ private function addCacheSection(ArrayNodeDefinition $rootNode)
562562
->prototype('array')
563563
->children()
564564
->scalarNode('type')
565-
->info('The cache pool type (one of "default", "apcu", "doctrine", "psr6", "filesystem" or any other existing pool name.)')
566-
->isRequired()
565+
->info('The cache pool service to use as template definition.')
566+
->defaultValue('cache.adapter.default')
567567
->end()
568568
->booleanNode('public')->defaultFalse()->end()
569569
->integerNode('default_lifetime')->defaultNull()->end()
570570
->scalarNode('provider_service')->defaultNull()->end()
571571
->scalarNode('directory')->defaultNull()->end()
572572
->end()
573-
->validate()
574-
->ifTrue(function ($v) {
575-
return isset($v['provider_service']) xor in_array($v['type'], array('psr6', 'doctrine'));
576-
})
577-
->thenInvalid('You must use "provider_service" with (and only with) cache types "psr6" or "doctrine".')
578-
->end()
579573
->end()
580574
->end()
581575
->end()

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,24 +1027,11 @@ private function registerCacheConfiguration(array $config, ContainerBuilder $con
10271027
}
10281028

10291029
foreach ($config['pools'] as $name => $poolConfig) {
1030-
$type = $poolConfig['type'];
1031-
if (!isset($config['pools'][$type]) && !$container->has('cache.pool.'.$type)) {
1032-
throw new \UnexpectedValueException(sprintf('Cache pool "%s" is of undefined type "%s".', $name, $type));
1033-
}
1034-
$poolDefinition = new DefinitionDecorator('cache.pool.'.$type);
1030+
$poolDefinition = new DefinitionDecorator($poolConfig['type']);
10351031
$poolDefinition->setPublic($poolConfig['public']);
1032+
unset($poolConfig['type'], $poolConfig['public']);
10361033

1037-
if (isset($poolConfig['default_lifetime'])) {
1038-
$poolDefinition->replaceArgument(1, $poolConfig['default_lifetime']);
1039-
}
1040-
1041-
if ('doctrine' === $type || 'psr6' === $type) {
1042-
$poolDefinition->replaceArgument(0, new Reference($poolConfig['provider_service']));
1043-
} elseif ('filesystem' === $type && isset($poolConfig['directory'][0])) {
1044-
$poolDefinition->replaceArgument(0, $poolConfig['directory']);
1045-
}
1046-
1047-
$poolDefinition->addTag('cache.pool');
1034+
$poolDefinition->addTag('cache.pool', $poolConfig);
10481035
$container->setDefinition('cache.pool.'.$name, $poolDefinition);
10491036
}
10501037
}

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,30 @@
66

77
<services>
88

9-
<service id="cache.pool.default" alias="cache.pool.filesystem" />
9+
<service id="cache.adapter.default" alias="cache.adapter.filesystem" />
1010

11-
<service id="cache.pool.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">
12-
<tag name="cache.pool" namespace-arg-index="0" />
11+
<service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">
12+
<tag name="cache.pool" namespace-arg-index="0" default-lifetime-arg-index="1" />
1313
<argument /> <!-- namespace -->
1414
<argument /> <!-- default lifetime -->
1515
</service>
1616

17-
<service id="cache.pool.doctrine" class="Symfony\Component\Cache\Adapter\DoctrineAdapter" abstract="true">
18-
<tag name="cache.pool" namespace-arg-index="2" />
19-
<argument /> <!-- doctrine provider service -->
17+
<service id="cache.adapter.doctrine" class="Symfony\Component\Cache\Adapter\DoctrineAdapter" abstract="true">
18+
<tag name="cache.pool" service-provider-arg-index="0" default-lifetime-arg-index="1" namespace-arg-index="2" />
19+
<argument /> <!-- Doctrine provider service -->
2020
<argument /> <!-- default lifetime -->
2121
<argument /> <!-- namespace -->
2222
</service>
2323

24-
<service id="cache.pool.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true">
25-
<tag name="cache.pool" namespace-arg-index="2" />
24+
<service id="cache.adapter.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true">
25+
<tag name="cache.pool" service-provider-arg-index="0" default-lifetime-arg-index="1" namespace-arg-index="2" />
2626
<argument /> <!-- PSR-6 provider service -->
2727
<argument /> <!-- default lifetime -->
2828
<argument /> <!-- namespace -->
2929
</service>
3030

31-
<service id="cache.pool.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true">
32-
<tag name="cache.pool" namespace-arg-index="2" />
31+
<service id="cache.adapter.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true">
32+
<tag name="cache.pool" directory-arg-index="0" default-lifetime-arg-index="1" namespace-arg-index="2" />
3333
<argument>%kernel.cache_dir%</argument>
3434
<argument /> <!-- default lifetime -->
3535
<argument /> <!-- namespace -->

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@
212212

213213
<xsd:complexType name="cache_pool">
214214
<xsd:attribute name="name" type="xsd:string" use="required" />
215-
<xsd:attribute name="type" type="xsd:string" use="required" />
215+
<xsd:attribute name="type" type="xsd:string" />
216216
<xsd:attribute name="public" type="xsd:boolean" />
217217
<xsd:attribute name="default-lifetime" type="xsd:integer" />
218218
<xsd:attribute name="provider-service" type="xsd:string" />

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPassTest.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\DependencyInjection\ContainerBuilder;
1616
use Symfony\Component\DependencyInjection\Definition;
1717
use Symfony\Component\DependencyInjection\DefinitionDecorator;
18+
use Symfony\Component\DependencyInjection\Reference;
1819

1920
class CachePoolPassTest extends \PHPUnit_Framework_TestCase
2021
{
@@ -43,6 +44,30 @@ public function testNamespaceArgumentIsReplaced()
4344
$this->assertSame('yRnzIIVLvL', $cachePool->getArgument(0));
4445
}
4546

47+
public function testArgsAreReplaced()
48+
{
49+
$container = new ContainerBuilder();
50+
$cachePool = new Definition();
51+
$cachePool->addTag('cache.pool', array(
52+
'namespace_arg_index' => 0,
53+
'provider_service_arg_index' => 1,
54+
'provider_service' => 'foobar',
55+
'default_lifetime_arg_index' => 2,
56+
'default_lifetime' => 3,
57+
));
58+
$cachePool->addArgument(null);
59+
$cachePool->addArgument(null);
60+
$cachePool->addArgument(null);
61+
$container->setDefinition('app.cache_pool', $cachePool);
62+
63+
$this->cachePoolPass->process($container);
64+
65+
$this->assertSame('yRnzIIVLvL', $cachePool->getArgument(0));
66+
$this->assertInstanceOf(Reference::class, $cachePool->getArgument(1));
67+
$this->assertSame('foobar', (string) $cachePool->getArgument(1));
68+
$this->assertSame(3, $cachePool->getArgument(2));
69+
}
70+
4671
/**
4772
* @expectedException \InvalidArgumentException
4873
* @expectedExceptionMessage Invalid "cache.pool" tag for service "app.cache_adapter": attribute "namespace_arg_index" is missing.

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/cache.php‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,25 @@
44
'cache' => array(
55
'pools' => array(
66
'foo' => array(
7-
'type' => 'apcu',
7+
'type' => 'cache.adapter.apcu',
88
'default_lifetime' => 30,
99
),
1010
'bar' => array(
11-
'type' => 'doctrine',
11+
'type' => 'cache.adapter.doctrine',
1212
'default_lifetime' => 5,
1313
'provider_service' => 'app.doctrine_cache_provider',
1414
),
1515
'baz' => array(
16-
'type' => 'filesystem',
16+
'type' => 'cache.adapter.filesystem',
1717
'default_lifetime' => 7,
1818
'directory' => 'app/cache/psr',
1919
),
2020
'foobar' => array(
21-
'type' => 'psr6',
21+
'type' => 'cache.adapter.psr6',
2222
'default_lifetime' => 10,
2323
'provider_service' => 'app.cache_pool',
2424
),
2525
'def' => array(
26-
'type' => 'default',
2726
'default_lifetime' => 11,
2827
),
2928
),

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77

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

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/cache.yml‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,19 @@ framework:
22
cache:
33
pools:
44
foo:
5-
type: apcu
5+
type: cache.adapter.apcu
66
default_lifetime: 30
77
bar:
8-
type: doctrine
8+
type: cache.adapter.doctrine
99
default_lifetime: 5
1010
provider_service: app.doctrine_cache_provider
1111
baz:
12-
type: filesystem
12+
type: cache.adapter.filesystem
1313
default_lifetime: 7
1414
directory: app/cache/psr
1515
foobar:
16-
type: psr6
16+
type: cache.adapter.psr6
1717
default_lifetime: 10
1818
provider_service: app.cache_pool
1919
def:
20-
type: default
2120
default_lifetime: 11

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -575,11 +575,11 @@ public function testCachePoolServices()
575575
{
576576
$container = $this->createContainerFromFile('cache');
577577

578-
$this->assertCachePoolServiceDefinitionIsCreated($container, 'foo', 'apcu', array('index_1' => 30), 0);
579-
$this->assertCachePoolServiceDefinitionIsCreated($container, 'bar', 'doctrine', array('index_0' => new Reference('app.doctrine_cache_provider'), 'index_1' => 5));
580-
$this->assertCachePoolServiceDefinitionIsCreated($container, 'baz', 'filesystem', array('index_0' => 'app/cache/psr', 'index_1' => 7));
581-
$this->assertCachePoolServiceDefinitionIsCreated($container, 'foobar', 'psr6', array('index_0' => new Reference('app.cache_pool'), 'index_1' => 10));
582-
$this->assertCachePoolServiceDefinitionIsCreated($container, 'def', 'filesystem', array('index_1' => 11));
578+
$this->assertCachePoolServiceDefinitionIsCreated($container, 'foo', 'cache.adapter.apcu', 30, 0);
579+
$this->assertCachePoolServiceDefinitionIsCreated($container, 'bar', 'cache.adapter.doctrine', 5);
580+
$this->assertCachePoolServiceDefinitionIsCreated($container, 'baz', 'cache.adapter.filesystem', 7);
581+
$this->assertCachePoolServiceDefinitionIsCreated($container, 'foobar', 'cache.adapter.psr6', 10);
582+
$this->assertCachePoolServiceDefinitionIsCreated($container, 'def', 'cache.adapter.filesystem', 11);
583583
}
584584

585585
protected function createContainer(array $data = array())
@@ -651,7 +651,7 @@ private function assertVersionStrategy(ContainerBuilder $container, Reference $r
651651
}
652652
}
653653

654-
private function assertCachePoolServiceDefinitionIsCreated(ContainerBuilder $container, $name, $type, array $arguments, $namespaceArgumentIndex = null)
654+
private function assertCachePoolServiceDefinitionIsCreated(ContainerBuilder $container, $name, $type, $defaultLifetime, $namespaceArgumentIndex = null)
655655
{
656656
$id = 'cache.pool.'.$name;
657657

@@ -660,30 +660,37 @@ private function assertCachePoolServiceDefinitionIsCreated(ContainerBuilder $con
660660
$poolDefinition = $container->getDefinition($id);
661661

662662
$this->assertInstanceOf(DefinitionDecorator::class, $poolDefinition, sprintf('Cache pool "%s" is based on an abstract cache pool.', $name));
663-
$this->assertEquals($arguments, $poolDefinition->getArguments());
664663

665-
$adapterDefinition = $container->findDefinition($poolDefinition->getParent());
664+
$this->assertTrue($poolDefinition->hasTag('cache.pool'), sprintf('Service definition "%s" is tagged with the "cache.pool" tag.', $id));
665+
$this->assertFalse($poolDefinition->isAbstract(), sprintf('Service definition "%s" is not abstract.', $id));
666+
667+
$tag = $poolDefinition->getTag('cache.pool');
668+
$this->assertTrue(isset($tag[0]['default_lifetime']), 'The default lifetime is stored as an attribute of the "cache.pool" tag.');
669+
$this->assertSame($defaultLifetime, $tag[0]['default_lifetime'], 'The default lifetime is stored as an attribute of the "cache.pool" tag.');
670+
671+
$adapterId = $poolDefinition->getParent();
672+
$adapterDefinition = $container->findDefinition($adapterId);
666673

667674
switch ($type) {
668-
case 'apcu':
675+
case 'cache.adapter.apcu':
669676
$this->assertSame(ApcuAdapter::class, $adapterDefinition->getClass());
670677
break;
671-
case 'doctrine':
678+
case 'cache.adapter.doctrine':
672679
$this->assertSame(DoctrineAdapter::class, $adapterDefinition->getClass());
673680
break;
674-
case 'filesystem':
681+
case 'cache.adapter.filesystem':
675682
$this->assertSame(FilesystemAdapter::class, $adapterDefinition->getClass());
676683
break;
677684
}
678685

679-
$this->assertTrue($adapterDefinition->hasTag('cache.pool'), sprintf('Service definition "%s" is tagged with the "cache.pool" tag.', $id));
680-
$this->assertTrue($adapterDefinition->isAbstract(), sprintf('Service definition "%s" is abstract.', $id));
686+
$this->assertTrue($adapterDefinition->hasTag('cache.pool'), sprintf('Service definition "%s" is tagged with the "cache.pool" tag.', $adapterId));
687+
$this->assertTrue($adapterDefinition->isAbstract(), sprintf('Service definition "%s" is abstract.', $adapterId));
681688

682689
$tag = $adapterDefinition->getTag('cache.pool');
683690

684691
if (null !== $namespaceArgumentIndex) {
685-
$this->assertTrue(isset($tag[0]['namespace-arg-index']), 'The namespace argument index is given by the "namespace-arg-index" attribute of the "cache.pool" tag.');
686-
$this->assertSame($namespaceArgumentIndex, $tag[0]['namespace-arg-index'], 'The namespace argument index is given by the "namespace-arg-index" attribute of the "cache.pool" tag.');
692+
$this->assertTrue(isset($tag[0]['namespace_arg_index']), 'The namespace argument index is given by the "namespace_arg_index" attribute of the "cache.pool" tag.');
693+
$this->assertSame($namespaceArgumentIndex, $tag[0]['namespace_arg_index'], 'The namespace argument index is given by the "namespace_arg_index" attribute of the "cache.pool" tag.');
687694
}
688695
}
689696
}

0 commit comments

Comments
 (0)