[FrameworkBundle] Add cache adapters in semantic configuration #18625

tgalopin · Apr 23, 2016

Q	A
Branch?	3.1
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

Following other PRs about the integration of the Cache component into the FrameworkBundle, this PR proposes the possibility to define and overide adapters from the semantic configuration.

There are three main reasons why I think we should be able to configure adapters from the config_*.yml files:

to be consistent with cache pools
because it's very useful to be able to override local/shared adapters dependeing on the environment
because it's easier to understand the Cache components concepts in simple YAML configuration than with services (currently, to override adapters, we need to define services).

Here is an example of how we could configure every shared cache of the application to use redis instead of the filesystem with this proposal:

framework:    
    cache:
        adapters:
            shared:
                parent:               cache.adapter.redis
                provider:             app.redis_connection

By default, the FrameworkBundle defines some useful adapters and pools. Using this proposal configuration, we can represent these adapters and pools like these:

framework:
    #...

    cache:
        adapters:
            apcu:
                parent:               ~
                default_lifetime:     ~
                provider:             ~
            doctrine:
                parent:               ~
                default_lifetime:     ~
                provider:             ~
            filesystem:
                parent:               ~
                default_lifetime:     ~
                provider:             ~
            psr6:
                parent:               ~
                default_lifetime:     ~
                provider:             ~
            redis:
                parent:               ~
                default_lifetime:     ~
                provider:             ~
            local:
                parent:               cache.adapter.filesystem
                default_lifetime:     ~
                provider:             ~
            shared:
                parent:               cache.adapter.filesystem
                default_lifetime:     ~
                provider:             ~
        pools:
            validator:
                adapter:              cache.adapter.local
                public:               false
                default_lifetime:     ~
                provider:             ~
                namespace:            ~
                clearer:              cache.default_pools_clearer
            serializer:
                adapter:              cache.adapter.local
                public:               false
                default_lifetime:     ~
                provider:             ~
                namespace:            ~
                clearer:              cache.default_pools_clearer
            app:
                adapter:              cache.adapter.shared
                public:               false
                default_lifetime:     ~
                provider:             ~
                namespace:            ~
                clearer:              cache.default_pools_clearer

And every adapter could be overriden by using the key parent or by using the same name as the parent adapter (if you redefine the redis adapter to use a different provider, it will override the parent definition).

This PR also removes the local and shared pool for a single app pool. This is done to avoid confusion for users between the local/shared adapters and the local/shared pools and because app cache will almost always be shared.

nicolas-grekas · Apr 24, 2016

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

-        </service>
-
-        <service id="cache.pool.local" parent="cache.adapter.local">
+        <service id="cache.pool.app" parent="cache.adapter.shared">


@javiereguiluz WDYT about this one? We'd like to provide a preconfigured cache pool for user's need, so that one can inject it without much configuration. Currently we have two of these (cache.pool.local and cache.pool.shared).
The proposal here is to remove cache.pool.local and rename cache.pool.shared to cache.pool.app.
Which means we have to questions:

OK for removing the local pool and keep only the shared one?

is the cache.pool.app name OK or do we have a better idea (e.g. is cache.pool.user better)?

At first glance, maybe this should be called cache.pool.default I don't like the .app suffix because all the pools are for the app and because the app. convention is commonly used as a prefix for user's stuff.

But the pool we're talking about is only for user's stuff :)

nicolas-grekas · Apr 25, 2016

👍 with a few more tests, must be on 3.1 or will be much harder to change afterwards.

xabbuh · Apr 25, 2016

We need to update the XML schema definition.

tgalopin · Apr 27, 2016

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

@@ -58,5 +39,20 @@
            <argument /> <!-- default lifetime -->
        </service>

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


I put the adapters here to follow the reading order (as in the configuration, the extension, etc.)

nicolas-grekas · Apr 28, 2016

Included in #18667
Thank you @tgalopin for working on this!

…caches (tgalopin, nicolas-grekas) This PR was merged into the 3.1-dev branch. Discussion ---------- [FrameworkBundle] Semantic config for app/system/pool caches | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18625 | License | MIT | Doc PR | - Commits ------- a2a567d [FrameworkBundle] Simplify config for app/system/pool caches 80a5508 [FrameworkBundle] Add cache adapters in semantic configuration

carsonbot added the Status: Needs Review label Apr 23, 2016

tgalopin force-pushed the cache-config branch from 71d8604 to 187e665 Compare April 23, 2016 19:53

tgalopin changed the title ~~[WIP][Cache][FrameworkBundle] Add cache adapters in semantic configuration~~ [RFC][Cache][FrameworkBundle] Add cache adapters in semantic configuration Apr 23, 2016

javiereguiluz added Feature Cache labels Apr 24, 2016

nicolas-grekas reviewed Apr 24, 2016
View reviewed changes

tgalopin force-pushed the cache-config branch from 187e665 to 4caed0b Compare April 24, 2016 23:44

nicolas-grekas added the Status: Needs Work label Apr 25, 2016

nicolas-grekas changed the title ~~[RFC][Cache][FrameworkBundle] Add cache adapters in semantic configuration~~ [Cache][FrameworkBundle] Add cache adapters in semantic configuration Apr 25, 2016

nicolas-grekas changed the title ~~[Cache][FrameworkBundle] Add cache adapters in semantic configuration~~ [FrameworkBundle] Add cache adapters in semantic configuration Apr 25, 2016

nicolas-grekas mentioned this pull request Apr 27, 2016

Do not use validator.cache and serializer.cache anymore symfony/symfony-standard#965

Merged

tgalopin force-pushed the cache-config branch 3 times, most recently from 55bfda8 to dcbffa2 Compare April 27, 2016 14:12

tgalopin reviewed Apr 27, 2016
View reviewed changes

tgalopin force-pushed the cache-config branch 3 times, most recently from 00986d1 to 959bf64 Compare April 27, 2016 19:08

[Cache][FrameworkBundle] Add cache adapters in semantic configuration

a8b39ec

tgalopin force-pushed the cache-config branch from 959bf64 to a8b39ec Compare April 28, 2016 11:41

nicolas-grekas mentioned this pull request Apr 28, 2016

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

Merged

nicolas-grekas closed this Apr 28, 2016

tgalopin deleted the cache-config branch April 28, 2016 19:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[FrameworkBundle] Add cache adapters in semantic configuration #18625

[FrameworkBundle] Add cache adapters in semantic configuration #18625

Uh oh!

tgalopin commented Apr 23, 2016 •

edited by nicolas-grekas

Loading

Uh oh!

nicolas-grekas Apr 24, 2016

Uh oh!

javiereguiluz Apr 24, 2016

Uh oh!

nicolas-grekas Apr 24, 2016

Uh oh!

nicolas-grekas commented Apr 25, 2016

Uh oh!

xabbuh commented Apr 25, 2016

Uh oh!

tgalopin Apr 27, 2016

Uh oh!

nicolas-grekas commented Apr 28, 2016

Uh oh!

Uh oh!

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

Uh oh!

[FrameworkBundle] Add cache adapters in semantic configuration #18625

[FrameworkBundle] Add cache adapters in semantic configuration #18625

Uh oh!

Conversation

tgalopin commented Apr 23, 2016 • edited by nicolas-grekas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas Apr 24, 2016

Choose a reason for hiding this comment

Uh oh!

javiereguiluz Apr 24, 2016

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Apr 24, 2016

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Apr 25, 2016

Uh oh!

xabbuh commented Apr 25, 2016

Uh oh!

tgalopin Apr 27, 2016

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Apr 28, 2016

Uh oh!

Uh oh!

tgalopin commented Apr 23, 2016 •

edited by nicolas-grekas

Loading