Skip to content

Cache ChainAdapter not working in SF 4.0.8 #26954

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.

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

Closed
tonyh79 opened this issue Apr 16, 2018 · 6 comments
Closed

Cache ChainAdapter not working in SF 4.0.8 #26954

tonyh79 opened this issue Apr 16, 2018 · 6 comments

Comments

@tonyh79
Copy link

tonyh79 commented Apr 16, 2018

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 4.0.8

I've been attempting to set up a successful ChainAdapter for days without success. I've read all the docs and looked for help online and I have not come across anyone that set up a successful ChainAdapter in the config. I asked for help in the symfony-dev slack channel and some people there said it was very likely a bug with the ChainAdapter.

When attempting to set up a Cache ChainAdapter with Apcu and Redis. I get this error:

Type error: Argument 1 passed to Symfony\Component\Cache\Adapter\ChainAdapter::__construct() must be of the type array, string given, called in /var/www/www.mysite.com/var/cache/dev/ContainerZmpvOhh/srcDevDebugProjectContainer.php on line 405
I'm using the following config:

# services.yaml
my.cache.pool.chain:
        class: 'Symfony\Component\Cache\Adapter\ChainAdapter'
        arguments:
            - ['cache.adapter.apcu', 'cache.adapter.redis']
# framework.yaml
cache:
    app: my.cache.pool.chain

I tried other variations of the config, such as:

# services.yaml
my.cache.pool.chain:
        class: 'Symfony\Component\Cache\Adapter\ChainAdapter'
        arguments:
            $adapters: ['cache.adapter.apcu', 'cache.adapter.redis']

and that gives me this other error:

OutOfBoundsException
Service "cache.app": The argument "0" doesn't exist.

I've had other people look into this and they get the same errors, so I know it's not an issue related to my specific environment.

Thank you.

@dmaicher
Copy link
Contributor

Can confirm the issue.

There is a "namespace" forced as the first argument here:

https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php#L84

Seems ArrayAdapter is excluded there already... probably then all adapters should be excluded where the first argument of the constructor is not a namespace? ping @nicolas-grekas

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 16, 2018

I suppose the current doc could be improved. Would you mind opening a PR doing so?
Here are the explanations: the framework.cache.* entries are here to define new cache pools without doing direct service configuration. Mixing both styles requires knowing how config creates services.
So, what do you want to do?
There are two errors in the example you give:

  • the chain takes concrete services as arguments, but cache.adapter.* are abstract ones.
  • the framework.cache.app config defines a new pool bound to a service named cache.app.

So, if you want to override the cache.app service, just do it by naming your chain "cache.app" (or by creating it as an alias, both should work.)
For the 1st issue, you can create pools using the config, then reference them in your chain service definition.

@tonyh79
Copy link
Author

tonyh79 commented Apr 16, 2018

Thank you for the insight. I'll be happy to open a PR to the ChainAdapter docs. I'd appreciate some additional clarification first though.

I got it working with the config below. I believe that's what you suggested in your comment. However I'm wondering if it can be simplified. Can the chain adapter be configured in framework.cache without adding a service definition? Since the only thing that service is doing is passing the apcu and redis adapters as arguments.

# framework.cache
pools:
    app.cache.apcu:
        adapter: cache.adapter.apcu
        default_lifetime: 86400
    app.cache.redis:
        adapter: cache.adapter.redis
        default_lifetime: 86400
# services.yaml
    cache.app:
        class: Symfony\Component\Cache\Adapter\ChainAdapter
        arguments:
            $adapters: ['@app.cache.apcu', '@app.cache.redis']

@nicolas-grekas
Copy link
Member

Yes, that's exactly what I suggested.

the only thing that service is doing is passing the apcu and redis adapters as arguments

You're missing the name of the pools, from which a namespace is derivated.

@tonyh79
Copy link
Author

tonyh79 commented Apr 16, 2018

I tried this config, without the service

        pools:
            app.cache.apcu:
                name: cache.app.apcu
                adapter: cache.adapter.apcu
                default_lifetime: 86400
            app.cache.redis:
                name: cache.app.redis
                adapter: cache.adapter.redis
                default_lifetime: 86400

However, when I debug the container I get two different TraceableAdapter services. e.g.

Information for Service "cache.app.apcu"
========================================
  Service ID       cache.app.apcu
  Class            Symfony\Component\Cache\Adapter\TraceableAdapter

When I manually defined the service overwriting cache.app, I was getting the expected ChainAdapter

Information for Service "cache.app"
===================================
  Service ID       cache.app
  Class            Symfony\Component\Cache\Adapter\ChainAdapter

I'm not sure if I'm missing something in my pools config. I tried using the same name for both hoping that the two adapters in the same namespace would be put together in a ChainAdapter but instead I get a DuplicateKeyException.

@nicolas-grekas
Copy link
Member

Real pool are wrapped in a TraceableAdapter to make the profiler panel work for them. All is fine :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants