Skip to content

Commit 44a109f

Browse files
nicolas-grekasnicolas-grekas
committed
[FrameworkBundle] use abstract cache.pool decoration and aliases
1 parent bf53e88 commit 44a109f

File tree

10 files changed

+116
-131
lines changed

10 files changed

+116
-131
lines changed

‎DependencyInjection/Compiler/CachePoolPass.php‎

Lines changed: 26 additions & 24 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 <[email protected]>
@@ -25,36 +26,37 @@ class CachePoolPass implements CompilerPassInterface
2526
*/
2627
public function process(ContainerBuilder $container)
2728
{
29+
$attributes = array(
30+
'provider_service',
31+
'namespace',
32+
'default_lifetime',
33+
'directory',
34+
);
2835
foreach ($container->findTaggedServiceIds('cache.pool') as $id => $tags) {
29-
$pool = $container->getDefinition($id);
36+
$adapter = $pool = $container->getDefinition($id);
37+
$tags[0] += array('namespace' => $this->getNamespace($id));
3038

31-
if (!$pool instanceof DefinitionDecorator) {
32-
throw new \InvalidArgumentException(sprintf('Services tagged with "cache.pool" must have a parent service but "%s" has none.', $id));
39+
while ($adapter instanceof DefinitionDecorator) {
40+
$adapter = $container->findDefinition($adapter->getParent());
41+
if ($t = $adapter->getTag('cache.pool')) {
42+
$tags[0] += $t[0];
43+
}
3344
}
34-
35-
$adapter = $pool;
36-
37-
do {
38-
$adapterId = $adapter->getParent();
39-
$adapter = $container->getDefinition($adapterId);
40-
} while ($adapter instanceof DefinitionDecorator && !$adapter->hasTag('cache.adapter'));
41-
42-
if (!$adapter->hasTag('cache.adapter')) {
43-
throw new \InvalidArgumentException(sprintf('Services tagged with "cache.pool" must have a parent service tagged with "cache.adapter" but "%s" has none.', $id));
45+
if ($pool->isAbstract()) {
46+
continue;
4447
}
45-
46-
$tags = $adapter->getTag('cache.adapter');
47-
48-
if (!isset($tags[0]['namespace_arg_index'])) {
49-
throw new \InvalidArgumentException(sprintf('Invalid "cache.adapter" tag for service "%s": attribute "namespace_arg_index" is missing.', $adapterId));
48+
if (isset($tags[0]['provider_service']) && is_string($tags[0]['provider_service'])) {
49+
$tags[0]['provider_service'] = new Reference($tags[0]['provider_service']);
5050
}
51-
52-
if (!$adapter->isAbstract()) {
53-
throw new \InvalidArgumentException(sprintf('Services tagged as "cache.adapter" must be abstract: "%s" is not.', $adapterId));
51+
$i = 0;
52+
foreach ($attributes as $attr) {
53+
if (isset($tags[0][$attr])) {
54+
$pool->replaceArgument($i++, $tags[0][$attr]);
55+
unset($tags[0][$attr]);
56+
}
5457
}
55-
56-
if (0 <= $namespaceArgIndex = $tags[0]['namespace_arg_index']) {
57-
$pool->replaceArgument($namespaceArgIndex, $this->getNamespace($id));
58+
if (!empty($tags[0])) {
59+
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]))));
5860
}
5961
}
6062
}

‎DependencyInjection/Configuration.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -561,13 +561,13 @@ private function addCacheSection(ArrayNodeDefinition $rootNode)
561561
->useAttributeAsKey('name')
562562
->prototype('array')
563563
->children()
564-
->enumNode('type')
565-
->info('The cache pool type (one of "apcu", "doctrine", "psr6" or "filesystem")')
566-
->isRequired()
567-
->values(array('apcu', 'doctrine', 'psr6', 'filesystem'))
564+
->scalarNode('adapter_service')
565+
->info('The cache pool service to use as template definition.')
566+
->defaultValue('cache.adapter.default')
568567
->end()
569-
->integerNode('default_lifetime')->defaultValue(0)->end()
570-
->scalarNode('cache_provider_service')->defaultNull()->end()
568+
->booleanNode('public')->defaultFalse()->end()
569+
->integerNode('default_lifetime')->defaultNull()->end()
570+
->scalarNode('provider_service')->defaultNull()->end()
571571
->scalarNode('directory')->defaultNull()->end()
572572
->end()
573573
->end()

‎DependencyInjection/FrameworkExtension.php‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,20 +1023,15 @@ private function registerPropertyInfoConfiguration(array $config, ContainerBuild
10231023
private function registerCacheConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader)
10241024
{
10251025
if (!empty($config['pools'])) {
1026-
$loader->load('cache_adapters.xml');
1026+
$loader->load('cache_pools.xml');
10271027
}
10281028

10291029
foreach ($config['pools'] as $name => $poolConfig) {
1030-
$poolDefinition = new DefinitionDecorator('cache.adapter.'.$poolConfig['type']);
1031-
$poolDefinition->replaceArgument(1, $poolConfig['default_lifetime']);
1030+
$poolDefinition = new DefinitionDecorator($poolConfig['adapter_service']);
1031+
$poolDefinition->setPublic($poolConfig['public']);
1032+
unset($poolConfig['adapter_service'], $poolConfig['public']);
10321033

1033-
if ('doctrine' === $poolConfig['type'] || 'psr6' === $poolConfig['type']) {
1034-
$poolDefinition->replaceArgument(0, new Reference($poolConfig['cache_provider_service']));
1035-
} elseif ('filesystem' === $poolConfig['type'] && isset($poolConfig['directory'][0])) {
1036-
$poolDefinition->replaceArgument(0, $poolConfig['directory']);
1037-
}
1038-
1039-
$poolDefinition->addTag('cache.pool');
1034+
$poolDefinition->addTag('cache.pool', $poolConfig);
10401035
$container->setDefinition('cache.pool.'.$name, $poolDefinition);
10411036
}
10421037
}

‎Resources/config/cache_adapters.xml‎ renamed to ‎Resources/config/cache_pools.xml‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,33 @@
66

77
<services>
88

9+
<service id="cache.adapter.default" alias="cache.adapter.filesystem" />
10+
911
<service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">
10-
<tag name="cache.adapter" namespace-arg-index="0" />
12+
<tag name="cache.pool" />
1113
<argument /> <!-- namespace -->
1214
<argument /> <!-- default lifetime -->
1315
</service>
1416

1517
<service id="cache.adapter.doctrine" class="Symfony\Component\Cache\Adapter\DoctrineAdapter" abstract="true">
16-
<tag name="cache.adapter" namespace-arg-index="2" />
17-
<argument /> <!-- doctrine provider service -->
18-
<argument /> <!-- default lifetime -->
18+
<tag name="cache.pool" />
19+
<argument /> <!-- Doctrine provider service -->
1920
<argument /> <!-- namespace -->
21+
<argument /> <!-- default lifetime -->
2022
</service>
2123

2224
<service id="cache.adapter.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true">
23-
<tag name="cache.adapter" namespace-arg-index="2" />
25+
<tag name="cache.pool" />
2426
<argument /> <!-- PSR-6 provider service -->
25-
<argument /> <!-- default lifetime -->
2627
<argument /> <!-- namespace -->
28+
<argument /> <!-- default lifetime -->
2729
</service>
2830

2931
<service id="cache.adapter.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true">
30-
<tag name="cache.adapter" namespace-arg-index="2" />
31-
<argument>%kernel.cache_dir%</argument>
32-
<argument /> <!-- default lifetime -->
32+
<tag name="cache.pool" />
3333
<argument /> <!-- namespace -->
34+
<argument /> <!-- default lifetime -->
35+
<argument>%kernel.cache_dir%</argument>
3436
</service>
3537

3638
</services>

‎Resources/config/schema/symfony-1.0.xsd‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,10 @@
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="adapter-service" type="xsd:string" />
216+
<xsd:attribute name="public" type="xsd:boolean" />
216217
<xsd:attribute name="default-lifetime" type="xsd:integer" />
217-
<xsd:attribute name="cache-provider-service" type="xsd:string" />
218+
<xsd:attribute name="provider-service" type="xsd:string" />
218219
<xsd:attribute name="directory" type="xsd:string" />
219220
</xsd:complexType>
220221
</xsd:schema>

‎Tests/DependencyInjection/Compiler/CachePoolPassTest.php‎

Lines changed: 20 additions & 43 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
{
@@ -30,9 +31,10 @@ public function testNamespaceArgumentIsReplaced()
3031
$container = new ContainerBuilder();
3132
$adapter = new Definition();
3233
$adapter->setAbstract(true);
33-
$adapter->addTag('cache.adapter', array('namespace_arg_index' => 0));
34+
$adapter->addTag('cache.pool');
3435
$container->setDefinition('app.cache_adapter', $adapter);
35-
$cachePool = new DefinitionDecorator('app.cache_adapter');
36+
$container->setAlias('app.cache_adapter_alias', 'app.cache_adapter');
37+
$cachePool = new DefinitionDecorator('app.cache_adapter_alias');
3638
$cachePool->addArgument(null);
3739
$cachePool->addTag('cache.pool');
3840
$container->setDefinition('app.cache_pool', $cachePool);
@@ -42,65 +44,40 @@ public function testNamespaceArgumentIsReplaced()
4244
$this->assertSame('yRnzIIVLvL', $cachePool->getArgument(0));
4345
}
4446

45-
/**
46-
* @expectedException \InvalidArgumentException
47-
* @expectedExceptionMessage Services tagged with "cache.pool" must have a parent service but "app.cache_pool" has none.
48-
*/
49-
public function testThrowsExceptionWhenCachePoolHasNoParentDefinition()
47+
public function testArgsAreReplaced()
5048
{
5149
$container = new ContainerBuilder();
5250
$cachePool = new Definition();
53-
$cachePool->addTag('cache.pool');
51+
$cachePool->addTag('cache.pool', array(
52+
'provider_service' => 'foobar',
53+
'default_lifetime' => 3,
54+
));
55+
$cachePool->addArgument(null);
56+
$cachePool->addArgument(null);
57+
$cachePool->addArgument(null);
5458
$container->setDefinition('app.cache_pool', $cachePool);
5559

5660
$this->cachePoolPass->process($container);
57-
}
58-
59-
/**
60-
* @expectedException \InvalidArgumentException
61-
* @expectedExceptionMessage Services tagged with "cache.pool" must have a parent service tagged with "cache.adapter" but "app.cache_pool" has none.
62-
*/
63-
public function testThrowsExceptionWhenCachePoolIsNotBasedOnAdapter()
64-
{
65-
$container = new ContainerBuilder();
66-
$container->register('app.cache_adapter');
67-
$cachePool = new DefinitionDecorator('app.cache_adapter');
68-
$cachePool->addTag('cache.pool');
69-
$container->setDefinition('app.cache_pool', $cachePool);
7061

71-
$this->cachePoolPass->process($container);
62+
$this->assertInstanceOf(Reference::class, $cachePool->getArgument(0));
63+
$this->assertSame('foobar', (string) $cachePool->getArgument(0));
64+
$this->assertSame('yRnzIIVLvL', $cachePool->getArgument(1));
65+
$this->assertSame(3, $cachePool->getArgument(2));
7266
}
7367

7468
/**
7569
* @expectedException \InvalidArgumentException
76-
* @expectedExceptionMessage Invalid "cache.adapter" tag for service "app.cache_adapter": attribute "namespace_arg_index" is missing.
70+
* @expectedExceptionMessage Invalid "cache.pool" tag for service "app.cache_pool": accepted attributes are
7771
*/
78-
public function testThrowsExceptionWhenCacheAdapterDefinesNoNamespaceArgument()
72+
public function testThrowsExceptionWhenCachePoolTagHasUnknownAttributes()
7973
{
8074
$container = new ContainerBuilder();
8175
$adapter = new Definition();
8276
$adapter->setAbstract(true);
83-
$adapter->addTag('cache.adapter');
77+
$adapter->addTag('cache.pool');
8478
$container->setDefinition('app.cache_adapter', $adapter);
8579
$cachePool = new DefinitionDecorator('app.cache_adapter');
86-
$cachePool->addTag('cache.pool');
87-
$container->setDefinition('app.cache_pool', $cachePool);
88-
89-
$this->cachePoolPass->process($container);
90-
}
91-
92-
/**
93-
* @expectedException \InvalidArgumentException
94-
* @expectedExceptionMessage Services tagged as "cache.adapter" must be abstract: "app.cache_adapter" is not.
95-
*/
96-
public function testThrowsExceptionWhenCacheAdapterIsNotAbstract()
97-
{
98-
$container = new ContainerBuilder();
99-
$adapter = new Definition();
100-
$adapter->addTag('cache.adapter', array('namespace_arg_index' => 0));
101-
$container->setDefinition('app.cache_adapter', $adapter);
102-
$cachePool = new DefinitionDecorator('app.cache_adapter');
103-
$cachePool->addTag('cache.pool');
80+
$cachePool->addTag('cache.pool', array('foobar' => 123));
10481
$container->setDefinition('app.cache_pool', $cachePool);
10582

10683
$this->cachePoolPass->process($container);

‎Tests/DependencyInjection/Fixtures/php/cache.php‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,26 @@
44
'cache' => array(
55
'pools' => array(
66
'foo' => array(
7-
'type' => 'apcu',
7+
'adapter_service' => 'cache.adapter.apcu',
88
'default_lifetime' => 30,
99
),
1010
'bar' => array(
11-
'type' => 'doctrine',
11+
'adapter_service' => 'cache.adapter.doctrine',
1212
'default_lifetime' => 5,
13-
'cache_provider_service' => 'app.doctrine_cache_provider',
13+
'provider_service' => 'app.doctrine_cache_provider',
1414
),
1515
'baz' => array(
16-
'type' => 'filesystem',
16+
'adapter_service' => 'cache.adapter.filesystem',
1717
'default_lifetime' => 7,
1818
'directory' => 'app/cache/psr',
1919
),
2020
'foobar' => array(
21-
'type' => 'psr6',
21+
'adapter_service' => 'cache.adapter.psr6',
2222
'default_lifetime' => 10,
23-
'cache_provider_service' => 'app.cache_pool',
23+
'provider_service' => 'app.cache_pool',
24+
),
25+
'def' => array(
26+
'default_lifetime' => 11,
2427
),
2528
),
2629
),

‎Tests/DependencyInjection/Fixtures/xml/cache.xml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +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" cache-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" cache-provider-service="app.cache_pool" />
10+
<framework:pool name="foo" adapter-service="cache.adapter.apcu" default-lifetime="30" />
11+
<framework:pool name="bar" adapter-service="cache.adapter.doctrine" default-lifetime="5" provider-service="app.doctrine_cache_provider" />
12+
<framework:pool name="baz" adapter-service="cache.adapter.filesystem" default-lifetime="7" directory="app/cache/psr" />
13+
<framework:pool name="foobar" adapter-service="cache.adapter.psr6" default-lifetime="10" provider-service="app.cache_pool" />
14+
<framework:pool name="def" default-lifetime="11" />
1415
</framework:cache>
1516
</framework:config>
1617
</container>

‎Tests/DependencyInjection/Fixtures/yml/cache.yml‎

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

0 commit comments

Comments
 (0)