Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[FrameworkBundle] Semantic config for app/system/pool caches #18667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 4, 2016
Merged

[FrameworkBundle] Semantic config for app/system/pool caches #18667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
80a5508
[FrameworkBundle] Add cache adapters in semantic configuration
tgalopin Apr 23, 2016
a2a567d
[FrameworkBundle] Simplify config for app/system/pool caches
nicolas-grekas Apr 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions 6 UPGRADE-3.1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ FrameworkBundle
cache service. If you are using `serializer.mapping.cache.apc`, use
`serializer.mapping.cache.doctrine.apc` instead.

* The `framework.serializer.cache` option has been deprecated. Configure a cache pool
called `serializer` under `framework.cache.pools` instead.
* The `framework.serializer.cache` option has been deprecated. Configure the
`cache.serializer` service under `framework.cache.pools` instead.

Before:

Expand All @@ -110,7 +110,7 @@ FrameworkBundle
framework:
cache:
pools:
serializer:
cache.serializer:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we repeat the cache. prefix here. Imo this looks weird as it repeats what we already know from the config tree (I mean we are configuring the cache, why do we want to reflect that in the name here too?)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because I've been told that Symfony core prefers explicit over implicit & "magic" conventions. This in fact is much more flexible and doesn't force any service prefix. See examples above where an app.doctrine_cache pool is created. We don't have to teach any special convention here. Just give a service name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that we want the service to be cache.serializer. But imo we should then add the cache. prefix here: https://github.com/nicolas-grekas/symfony/blob/cache-config/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L1059

Otherwise, the full config path we use here is framework.cache.pools.cache.serializer which imo is not really clear why I should use the cache part twice here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And by the way, adding the cache. prefix in the extension when registering the definition will also prevent us from potentially having conflicting service names because when people configure their own cache pools, they will not always think of adding the prefix here explicitly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer to be explicit. Whenever possible we try to use the full service name in configuration and that's what is done here. If the end user does not want to prefix their service name with cache., it's not a problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we then still have the point that the configured names are directly used as service ids which can easily lead to conflicts (we don't do that anywhere else, do we?).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After re-reading #18667 (comment) think we can stop the discussion here as it was based on a misunderstand on my end.

👍 for how the PR is right now.

adapter: cache.adapter.apcu

HttpKernel
Expand Down
6 changes: 3 additions & 3 deletions 6 UPGRADE-4.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ FrameworkBundle
* The service `serializer.mapping.cache.apc` has been removed; use
`serializer.mapping.cache.doctrine.apc` instead.

* The `framework.serializer.cache` option has been removed. Configure a cache pool
called `serializer` under `framework.cache.pools` instead.
* The `framework.serializer.cache` option has been removed. Configure the
`cache.serializer` service under `framework.cache.pools` instead.

Before:

Expand All @@ -97,7 +97,7 @@ FrameworkBundle
framework:
cache:
pools:
serializer:
cache.serializer:
adapter: cache.adapter.apcu
```

Expand Down
30 changes: 27 additions & 3 deletions 30 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@

namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler;

use Symfony\Component\Cache\Adapter\RedisAdapter;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\DefinitionDecorator;
use Symfony\Component\DependencyInjection\Reference;

Expand Down Expand Up @@ -44,22 +46,24 @@ public function process(ContainerBuilder $container)
if ($pool->isAbstract()) {
continue;
}
$tags[0]['namespace'] = $this->getNamespace($namespaceSuffix, isset($tags[0]['namespace']) ? $tags[0]['namespace'] : $id);
while ($adapter instanceof DefinitionDecorator) {
$adapter = $container->findDefinition($adapter->getParent());
if ($t = $adapter->getTag('cache.pool')) {
$tags[0] += $t[0];
}
}
if (!isset($tags[0]['namespace'])) {
$tags[0]['namespace'] = $this->getNamespace($namespaceSuffix, $id);
}
if (isset($tags[0]['clearer'])) {
$clearer = $container->getDefinition($tags[0]['clearer']);
} else {
$clearer = null;
}
unset($tags[0]['clearer']);

if (isset($tags[0]['provider']) && is_string($tags[0]['provider'])) {
$tags[0]['provider'] = new Reference($tags[0]['provider']);
if (isset($tags[0]['provider'])) {
$tags[0]['provider'] = new Reference(static::getServiceProvider($container, $tags[0]['provider']));
}
$i = 0;
foreach ($attributes as $attr) {
Expand All @@ -82,4 +86,24 @@ private function getNamespace($namespaceSuffix, $id)
{
return substr(str_replace('/', '-', base64_encode(md5($id.$namespaceSuffix, true))), 0, 10);
}

/**
* @internal
*/
public static function getServiceProvider(ContainerBuilder $container, $name)
{
if (0 === strpos($name, 'redis://')) {
$dsn = $name;

if (!$container->hasDefinition($name = md5($dsn))) {
$definition = new Definition(\Redis::class);
$definition->setPublic(false);
$definition->setFactory(array(RedisAdapter::class, 'createConnection'));
$definition->setArguments(array($dsn));
$container->setDefinition($name, $definition);
}
}

return $name;
}
}
26 changes: 18 additions & 8 deletions 26 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,25 +558,35 @@ private function addCacheSection(ArrayNodeDefinition $rootNode)
->addDefaultsIfNotSet()
->fixXmlConfig('pool')
->children()
->scalarNode('app')
->info('App related cache pools configuration')
->defaultValue('cache.adapter.filesystem')
->end()
->scalarNode('system')
->info('System related cache pools configuration')
->defaultValue('cache.adapter.filesystem')
->end()
->scalarNode('directory')->defaultValue('%kernel.cache_dir%/pools')->end()
->scalarNode('default_doctrine_provider')->end()
->scalarNode('default_psr6_provider')->end()
->scalarNode('default_redis_provider')->defaultValue('redis://localhost')->end()
->arrayNode('pools')
->useAttributeAsKey('name')
->prototype('array')
->children()
->scalarNode('adapter')
->info('The cache pool adapter service to use as template definition.')
->defaultValue('cache.adapter.shared')
->end()
->scalarNode('adapter')->defaultValue('cache.app')->end()
->booleanNode('public')->defaultFalse()->end()
->integerNode('default_lifetime')->end()
->scalarNode('provider')
->info('The service name to use as provider when the specified adapter needs one.')
->end()
->scalarNode('namespace')
->info('The namespace where cached items are stored. Auto-generated by default. Set to false to disable namespacing.')
->end()
->scalarNode('clearer')->defaultValue('cache.default_pools_clearer')->end()
->scalarNode('clearer')->defaultValue('cache.default_clearer')->end()
->end()
->end()
->validate()
->ifTrue(function ($v) { return isset($v['cache.app']) || isset($v['cache.system']); })
->thenInvalid('"cache.app" and "cache.system" are reserved names')
->end()
->end()
->end()
->end()
Expand Down
31 changes: 22 additions & 9 deletions 31 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('property_access.xml');

// Load Cache configuration first as it is used by other components
$loader->load('cache_pools.xml');
$loader->load('cache.xml');

$configuration = $this->getConfiguration($configs, $container);
$config = $this->processConfiguration($configuration, $configs);
Expand Down Expand Up @@ -984,7 +984,7 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
$chainLoader->replaceArgument(0, $serializerLoaders);

if (isset($config['cache']) && $config['cache']) {
@trigger_error('The "framework.serializer.cache" option is deprecated since Symfony 3.1 and will be removed in 4.0. You can configure a cache pool called "serializer" under "framework.cache.pools" instead.', E_USER_DEPRECATED);
@trigger_error('The "framework.serializer.cache" option is deprecated since Symfony 3.1 and will be removed in 4.0. Configure the "cache.serializer" service under "framework.cache.pools" instead.', E_USER_DEPRECATED);

$container->setParameter(
'serializer.mapping.cache.prefix',
Expand All @@ -999,7 +999,7 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
CacheClassMetadataFactory::class,
array(
new Reference('serializer.mapping.cache_class_metadata_factory.inner'),
new Reference('cache.pool.serializer'),
new Reference('cache.serializer'),
)
);
$cacheMetadataFactory->setPublic(false);
Expand Down Expand Up @@ -1037,13 +1037,26 @@ private function registerPropertyInfoConfiguration(array $config, ContainerBuild

private function registerCacheConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader)
{
foreach ($config['pools'] as $name => $poolConfig) {
$poolDefinition = new DefinitionDecorator($poolConfig['adapter']);
$poolDefinition->setPublic($poolConfig['public']);
unset($poolConfig['adapter'], $poolConfig['public']);
$container->getDefinition('cache.adapter.filesystem')->replaceArgument(2, $config['directory']);

$poolDefinition->addTag('cache.pool', $poolConfig);
$container->setDefinition('cache.pool.'.$name, $poolDefinition);
foreach (array('doctrine', 'psr6', 'redis') as $name) {
if (isset($config[$name = 'default_'.$name.'_provider'])) {
$container->setAlias('cache.'.$name, Compiler\CachePoolPass::getServiceProvider($container, $config[$name]));
}
}
foreach (array('app', 'system') as $name) {
$config['pools']['cache.'.$name] = array(
'adapter' => $config[$name],
'public' => true,
);
}
foreach ($config['pools'] as $name => $pool) {
$definition = new DefinitionDecorator($pool['adapter']);
$definition->setPublic($pool['public']);
unset($pool['adapter'], $pool['public']);

$definition->addTag('cache.pool', $pool);
$container->setDefinition($name, $definition);
}

$this->addClassesToCompile(array(
Expand Down
34 changes: 18 additions & 16 deletions 34 ...rkBundle/Resources/config/cache_pools.xml → ...rameworkBundle/Resources/config/cache.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,24 @@

<services>

<service id="cache.default_pools_clearer" class="Symfony\Component\HttpKernel\CacheClearer\Psr6CacheClearer" public="false">
<tag name="kernel.cache_clearer" />
</service>

<service id="cache.adapter.shared" alias="cache.adapter.filesystem" />
<service id="cache.adapter.local" alias="cache.adapter.filesystem" />

<service id="cache.pool.shared" parent="cache.adapter.shared">
<tag name="cache.pool" clearer="cache.default_pools_clearer" />
<service id="cache.app" parent="cache.adapter.filesystem">
<tag name="cache.pool" />
</service>

<service id="cache.pool.local" parent="cache.adapter.local">
<tag name="cache.pool" clearer="cache.default_pools_clearer" />
<service id="cache.system" parent="cache.adapter.filesystem">
<tag name="cache.pool" />
</service>

<service id="cache.pool.validator" parent="cache.adapter.local" public="false">
<tag name="cache.pool" clearer="cache.default_pools_clearer" />
<service id="cache.validator" parent="cache.system" public="false">
<tag name="cache.pool" />
</service>

<service id="cache.pool.serializer" parent="cache.adapter.local" public="false">
<tag name="cache.pool" clearer="cache.default_pools_clearer" />
<service id="cache.serializer" parent="cache.system" public="false">
<tag name="cache.pool" />
</service>

<service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">
<tag name="cache.pool" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- namespace -->
<argument /> <!-- default lifetime -->
Expand All @@ -39,6 +33,7 @@
</service>

<service id="cache.adapter.doctrine" class="Symfony\Component\Cache\Adapter\DoctrineAdapter" abstract="true">
<tag name="cache.pool" provider="cache.default_doctrine_provider" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- Doctrine provider service -->
<argument /> <!-- namespace -->
Expand All @@ -49,6 +44,7 @@
</service>

<service id="cache.adapter.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true">
<tag name="cache.pool" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- namespace -->
<argument /> <!-- default lifetime -->
Expand All @@ -59,20 +55,26 @@
</service>

<service id="cache.adapter.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true">
<tag name="cache.pool" provider="cache.default_psr6_provider" clearer="cache.default_clearer" />
<argument /> <!-- PSR-6 provider service -->
<argument /> <!-- namespace -->
<argument /> <!-- default lifetime -->
</service>

<service id="cache.adapter.redis" class="Symfony\Component\Cache\Adapter\RedisAdapter" abstract="true">
<tag name="cache.pool" provider="cache.default_redis_provider" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- Redis connection object -->
<argument /> <!-- Redis connection service -->
<argument /> <!-- namespace -->
<argument /> <!-- default lifetime -->
<call method="setLogger">
<argument type="service" id="logger" on-invalid="ignore" />
</call>
</service>

<service id="cache.default_clearer" class="Symfony\Component\HttpKernel\CacheClearer\Psr6CacheClearer" public="false">
<tag name="kernel.cache_clearer" />
</service>

</services>
</container>
12 changes: 9 additions & 3 deletions 12 src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,15 @@
</xsd:complexType>

<xsd:complexType name="cache">
<xsd:choice minOccurs="1" maxOccurs="unbounded">
<xsd:element name="pool" type="cache_pool" />
</xsd:choice>
<xsd:sequence>
<xsd:element name="app" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="system" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="directory" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="default-doctrine-provider" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="default-psr6-provider" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="default-redis-provider" type="xsd:string" minOccurs="0" maxOccurs="1" />
<xsd:element name="pool" type="cache_pool" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>

<xsd:complexType name="cache_pool">
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<service id="validator.mapping.class_metadata_factory" alias="validator" public="false" />

<service id="validator.mapping.cache.symfony" class="Symfony\Component\Validator\Mapping\Cache\Psr6Cache" public="false">
<argument type="service" id="cache.pool.validator" />
<argument type="service" id="cache.validator" />
</service>

<service id="validator.mapping.cache.doctrine.apc" class="Symfony\Component\Validator\Mapping\Cache\DoctrineCache" public="false">
Expand Down
4 changes: 4 additions & 0 deletions 4 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,10 @@ protected static function getBundleDefaultConfig()
),
'cache' => array(
'pools' => array(),
'app' => 'cache.adapter.filesystem',
'system' => 'cache.adapter.filesystem',
'directory' => '%kernel.cache_dir%/pools',
'default_redis_provider' => 'redis://localhost',
),
);
}
Expand Down
10 changes: 5 additions & 5 deletions 10 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/cache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@
$container->loadFromExtension('framework', array(
'cache' => array(
'pools' => array(
'foo' => array(
'cache.foo' => array(
'adapter' => 'cache.adapter.apcu',
'default_lifetime' => 30,
),
'bar' => array(
'cache.bar' => array(
'adapter' => 'cache.adapter.doctrine',
'default_lifetime' => 5,
'provider' => 'app.doctrine_cache_provider',
),
'baz' => array(
'cache.baz' => array(
'adapter' => 'cache.adapter.filesystem',
'default_lifetime' => 7,
),
'foobar' => array(
'cache.foobar' => array(
'adapter' => 'cache.adapter.psr6',
'default_lifetime' => 10,
'provider' => 'app.cache_pool',
),
'def' => array(
'cache.def' => array(
'default_lifetime' => 11,
),
),
Expand Down
10 changes: 5 additions & 5 deletions 10 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/cache.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

<framework:config>
<framework:cache>
<framework:pool name="foo" adapter="cache.adapter.apcu" default-lifetime="30" />
<framework:pool name="bar" adapter="cache.adapter.doctrine" default-lifetime="5" provider="app.doctrine_cache_provider" />
<framework:pool name="baz" adapter="cache.adapter.filesystem" default-lifetime="7" />
<framework:pool name="foobar" adapter="cache.adapter.psr6" default-lifetime="10" provider="app.cache_pool" />
<framework:pool name="def" default-lifetime="11" />
<framework:pool name="cache.foo" adapter="cache.adapter.apcu" default-lifetime="30" />
<framework:pool name="cache.bar" adapter="cache.adapter.doctrine" default-lifetime="5" provider="app.doctrine_cache_provider" />
<framework:pool name="cache.baz" adapter="cache.adapter.filesystem" default-lifetime="7" />
<framework:pool name="cache.foobar" adapter="cache.adapter.psr6" default-lifetime="10" provider="app.cache_pool" />
<framework:pool name="cache.def" default-lifetime="11" />
</framework:cache>
</framework:config>
</container>
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.