[FrameworkBundle] alias cache.app.taggable to cache.app if using cache.adapter.redis_tag_aware
#44673
Conversation
e4077bb to
9676bd6
Compare
|
Isn't that a bugfix? |
| @@ -37,9 +36,6 @@ | |||
| ->public() | |||
| ->tag('cache.pool', ['clearer' => 'cache.app_clearer']) | |||
|
|
|||
| ->set('cache.app.taggable', TagAwareAdapter::class) | |||
There was a problem hiding this comment.
I'd prefer keeping this definition as is
There was a problem hiding this comment.
And just change to an alias if applicable in the extension?
| @@ -2103,6 +2103,15 @@ private function registerCacheConfiguration(array $config, ContainerBuilder $con | |||
| 'tags' => false, | |||
| ]; | |||
| } | |||
|
|
|||
| if ('cache.adapter.redis_tag_aware' === $config['app']) { | |||
There was a problem hiding this comment.
In theory, any pool can inherit from redis_tag_aware and require making cache.app an alias
There was a problem hiding this comment.
If I understand you correctly, the same could be said for the non-app cache pools below, correct? (cache.adapter.redis_tag_aware is hard-coded)
There was a problem hiding this comment.
absolutely, recursively also, since pools can derivate from other pools.
There was a problem hiding this comment.
Ultimately, I don't think there shouldn't be any redis-specific check here at all. As discussed in slack, we should be using a pass to determine if the adapter used is already tag aware and alias/decorate based on that.
What do you think about matching what we do for custom pools now for 5.3 as a bug fix, then adding a more robust system in 6.1?
There was a problem hiding this comment.
but the alias is created in the DI extension - the pass doesn't know about them
There was a problem hiding this comment.
Think I finally understand and I believe #44682 solves.
I was in doubt so I called this a feature. Target 5.3 and call it a bug fix? |
|
Wrapping a tagaware in a tagaware looks like a bug to me :) |
|
Closing in favour of #44682. |
…` if using `cache.adapter.redis_tag_aware` (kbond) This PR was merged into the 5.3 branch. Discussion ---------- [FrameworkBundle] alias `cache.app.taggable` to `cache.app` if using `cache.adapter.redis_tag_aware` | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | n/a | License | MIT | Doc PR | n/a When using `cache.adapter.redis_tag_aware` for your `cache.app`, `cache.app.taggable` is unnecessarily decorated with `TagAwareAdapter`. This sets `cache.app.taggable` as an alias to `cache.app` in this case. Alternative to #44673. Commits ------- 4b54aa0 alias `cache.app.taggable` to `cache.app` if using `cache.adapter.redis_tag_aware`
When using
cache.adapter.redis_tag_awarefor yourcache.app,cache.app.taggableis unnecessarily decorated.