mirror of
https://github.com/twigphp/Twig.git
synced 2026-09-15 20:06:31 +00:00
Fabien Potencier
4de6bc3b06
This PR was merged into the 3.x branch.
Discussion
----------
Fix wrapping the Twig cache pool in a second tag aware adapter
`twig/extra-bundle` wires the `{% cache %}` pool like this:
```php
->set('twig.cache', TagAwareAdapter::class)
->args([$service('.twig.cache.inner')])
->set('.twig.cache.inner')
->parent('cache.app')
->tag('cache.pool', ['name' => 'twig.cache'])
```
`.twig.cache.inner` is a child of `cache.app`, so when an application configures
`framework.cache.app` with a natively tag aware adapter (`cache.adapter.redis_tag_aware`,
or the valkey, pdo and mongodb variants), the child pool is *already* a
`TagAwareAdapterInterface` and `twig.cache` wraps it in a second `TagAwareAdapter`.
Two nested `TagAwareAdapter`s never read back what they wrote:
```php
$pool = new TagAwareAdapter(new TagAwareAdapter(new ArrayAdapter()));
$item = $pool->getItem('k');
$item->set('value')->tag('t1');
$pool->save($item);
var_dump($pool->getItem('k')->isHit()); // bool(false)
```
So every `{% cache %}` block silently misses, on every request. This was reported here as
twigphp/Twig#3636 (the workaround in that thread is to redefine `twig.cache` and
`.twig.cache.inner` by hand as a `RedisTagAwareAdapter`), and again on the Symfony side as
symfony/symfony#54339 by `@rpkamp`. Symfony first tried to fix it in symfony/symfony#57927 by
deprecating passing a tag aware pool to `TagAwareAdapter`, but `@keulinho`'s analysis in
symfony/symfony#58830 showed that the deprecated thing was not the culprit, so that
deprecation was reverted in symfony/symfony#58950 and the defect was left where it is: in
this bundle. Symfony's own cache wiring already gets this right, it aliases
`cache.app.taggable` to `cache.app` when the configured adapter is natively tag aware
instead of decorating it.
## The fix
A small compiler pass resolves the parent chain of `.twig.cache.inner` and, when the
adapter it ends up on implements `TagAwareAdapterInterface`, drops the decorator and
aliases `twig.cache` to the pool. Nothing changes for the common case of a plain
`cache.app`, where the decorator is what makes the pool taggable and stays in place.
I picked this over declaring the pool through `framework.cache.pools` (with `tags: true`,
which would let Symfony make the same decision) because the pool is only registered when
the `cache` extension is enabled, and reproducing that condition inside a `prepend()` call
means processing the bundle's own configuration before `load()` runs. The compiler pass
stays entirely inside the bundle and only acts when `twig.cache` exists.
Detecting tag awareness from the resolved adapter class rather than from a list of adapter
ids keeps this working across `^5.4|^6.4|^7.0|^8.0` even though the set of natively tag
aware adapters grew over that range, and it also covers a `cache.app` overridden with a
custom tag aware adapter.
Two constraints are covered by the tests: `twig.cache` keeps its own namespace instead of
collapsing into the application pool, and the three argument aliases
(`TagAwareCacheInterface $twigCache`, `CacheInterface $twigCache`,
`CacheItemPoolInterface $twigCache`) keep resolving.
## Verification
Compiled containers with `framework.cache.app` left at its default and set to
`cache.adapter.redis_tag_aware`, before and after the patch:
| `framework.cache.app` | before | after |
| --- | --- | --- |
| default (filesystem) | `TagAwareAdapter(FilesystemAdapter)` | unchanged |
| `cache.adapter.redis_tag_aware` | `TagAwareAdapter(RedisTagAwareAdapter)` | `RedisTagAwareAdapter` |
| `cache.adapter.valkey_tag_aware` | `TagAwareAdapter(RedisTagAwareAdapter)` | `RedisTagAwareAdapter` |
| `cache.adapter.redis` | `TagAwareAdapter(RedisAdapter)` | unchanged |
In the fixed tag aware case the pool namespace stays distinct from `cache.app`'s.
`extra/twig-extra-bundle` test suite, on Symfony 8.2-dev and on framework-bundle 6.4:
```
PHPUnit 9.6.36 by Sebastian Bergmann and contributors.
Testing
................... 19 / 19 (100%)
Time: 00:00.534, Memory: 28.00 MB
OK (19 tests, 100 assertions)
```
The two new tests fail without the pass:
```
1) TwigCachePoolPassTest::testThePoolIsNotDecoratedWhenTheAppAdapterIsTagAware
Failed asserting that two strings are identical.
-'@.twig.cache.inner'
+'Symfony\Component\Cache\Adapter\TagAwareAdapter'
```
Commits
-------
bd939c8c3a Fix wrapping the Twig cache pool in a second tag aware adapter
Twig, the flexible, fast, and secure template language for PHP
==============================================================
Twig is a template language for PHP.
Twig uses a syntax similar to the Django and Jinja template languages which
inspired the Twig runtime environment.
Sponsors
--------
.. raw:: html
<a href="https://docs.blackfire.io/introduction?utm_source=twig&utm_medium=github_readme&utm_campaign=logo">
<img src="https://static.blackfire.io/assets/intemporals/logo/png/blackfire-io_secondary_horizontal_transparent.png?1" width="255px" alt="Blackfire.io">
</a>
More Information
----------------
Read the `documentation`_ for more information.
.. _documentation: https://twig.symfony.com/documentation