-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] missing integration of PSR-16 simple cache #28918
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
Comments
Another idea would be create a the simple cache adapter automatically for each pool since it's already done for the predefined symfony/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache.xml Lines 14 to 16 in bcff647
|
I tend to agree with this second solution, but that would create the simple cache adapter service id based on a convention, which you tend to avoid (that's why cache pools names are exactly the service id to use). |
Agree it's not optimal.
Don't agree as you can easily have a typo in all the adapters you need to configured like injecting the wrong pool into an adapter which is hard to spot since it does not fail. |
The fact that this doesn't exist is on purpose: the more choices we provide, the more confused people are. Here, PSR 16 doesn't provide anything over PSR 6. With the new CacheInterface, PSR 16 is not the simplest alternatives if one thinks that's where it fits. |
I'm in favor of deprecating the cache.app.simple service and updating the doc to not tell about the PSR 16 interface so prominently. That's already on my 4.3 list :) |
And we can / should do that in 4.2, because that’s when the new CacheInterface will be available (which is both easy to use and powerful - best of both worlds) |
For what it worth I like the PSR-6. It's not as simple as PSR-16 but actually PSR-16 has many drawbacks (the key is duplicated in the code, and it's impossible to know if the item was hit). I always use PSR-6 and I'm teaching only this way. The new CacheInterface is nice too and I like it too :) But it's not a PSR :/ I'm not sure it's an issue.* So for me : We should not talk about PSR-16 at all and only promote PSR-6 or the CacheInterface |
I'd go one step further and deprecate all PSR 16 implementations except a PSR 6 to 16 bridge! (and remove psr/simple-cache from "require") |
It seems like symfonys PSR-6 implementation does not allow to set cache items without fetching the data first? With PSR-16 this should be possible: With |
That's correct, that's consistent with wordings in PSR-6. But actually there's a way: using the NullAdapter as a factory for items. Implementing lazy loading at the item level would make everything more complex, better not to me, especially also because saving without fetching first is a very unusual pattern. Only cache warmers may need it. |
I would not call it unusual pattern. We have data importers that basically refresh the cache. Those do not need to read the data first. And it makes a big difference if you do this for alot of items. So basically PSR-6 is unusable for this. |
It is, using the NullAdapter as a factory. |
I don't know how that should work. If I use the NullAdapter, then it also cannot save data. What do you mean? |
You get the item from the NullAdapter, and you save it in the not-null one. Provided both are Symfony adapters, that works. |
I see. But then I need to inject two caches everywhere to workaround a bad API. This is not practical and is asking for bugs. |
I understand, but I'm not sure that's a strong enough reason to keep PSR 16. The null adapter factory is a legit one, and if it allows reducing the maintenance burden, I'm willing to call it a good enough solution :) |
I'm closing this one as it seems PSR-16 integration in core in not wanted which is fine to me. For anyone interested, we just use a psr6 decorator using <?php
use Symfony\Component\Cache\Simple\Psr6Cache;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;
/**
* Creates a PSR-16 simple cache adapter for each Symfony cache pool with a ".simple" suffix.
*/
class SimpleCacheAdapterPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
foreach ($container->findTaggedServiceIds('cache.pool') as $id => $tags) {
$pool = $container->getDefinition($id);
if ($pool->isAbstract()) {
continue;
}
$definition = new Definition(Psr6Cache::class, [
new Reference($id),
]);
$container->setDefinition($id.'.simple', $definition);
}
}
} |
…s (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [Cache] fix optimizing Psr6Cache for AdapterInterface pools | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | -. As described by @Tobion in https://github.com/symfony/symfony/pull/29236/files#r234324045: > The problem I have experienced is that in dev mode the cache is decorated with a TraceableCache. This means it loses this optimization and introduces #28918 (comment) again Commits ------- b8100a9 [Cache] fix optimizing Psr6Cache for AdapterInterface pools
…che instead (nicolas-grekas) This PR was merged into the 4.3-dev branch. Discussion ---------- [Cache] deprecate all PSR-16 adapters, provide Psr16Cache instead | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - As discussed in #28918, PSR-16 implementations are now mostly useless: either PSR-6 or contracts' CacheInterface is always a better fit. Let's deprecate them all but keep only a `Psr16Cache` to turn any PSR-6 implementation to a PSR-16 one. From the changelog: * removed `psr/simple-cache` dependency, run `composer require psr/simple-cache` if you need it * deprecated all PSR-16 adapters, use `Psr16Cache` or `Symfony\Contracts\Cache\CacheInterface` implementations instead * deprecated `SimpleCacheAdapter`, use `Psr16Adapter` instead * deprecated the "Psr\SimpleCache\CacheInterface" / "cache.app.simple" service, use "Symfony\Contracts\Cache\CacheInterface" / "cache.app" instead Commits ------- 5006be6 [Cache] deprecate all PSR-16 adapters, provide Psr16Cache instead
Hello and thank you for reaching out. This tracker is meant for tracking bugs and feature requests. If you are looking for support, you'll get faster responses by using one of our official support channels:
See https://symfony.com/support for more support options. |
Description
Cache pools can easily be configured in the framework bundle with
But all the cache services created only implement PSR-6
Psr\Cache\CacheItemPoolInterface
. We would like to use PSR-16Psr\SimpleCache\CacheInterface
instead. Currently the only solution seems to be to wrap all the pools in an adapter likeThis is quite cumbersome as we have alot of pools. It would be nice if the framework bundle provides a way to use SimpleCache directly. Maybe something like the following would create PSR-16 caches instead:
The text was updated successfully, but these errors were encountered: