Problem/Motivation
Currently, the ViewsBlockBase class does not fully leverage the cache metadata (contexts, tags, max-age) provided by the associated view's display handler. This can lead to inconsistencies in cache behavior when using Views blocks, especially when the display handler defines specific cache settings.
For example:
- Cache contexts from the view are not merged into the block's cache contexts.
- Cache tags provided by the view's display handler are ignored in the block context.
- Cache max-age of the block does not reflect the display handler's cache max-age.
This limits the ability to align the block's caching behavior with that of the View, potentially resulting in outdated content being served or inefficient cache invalidation.
Steps to reproduce
1. Create a View with specific cache metadata in its display handler (e.g., cache contexts or tags).
2. Add the View as a block on a page.
3. Observe that the cache behavior of the block does not reflect the settings in the display handler.
Proposed resolution
Introduce overrides in the ViewsBlockBase class for the following methods:
getCacheContexts()getCacheTags()getCacheMaxAge()
These methods will merge the cache metadata from the parent block class with the cache metadata from the view's display handler, ensuring consistent and efficient caching.
Remaining tasks
Write unit tests to ensure the cache metadata (contexts, tags, max-age) is correctly merged between the block and the view's display handler.Done- Test scenarios where the display handler provides:
Additional cache contexts. DoneSpecific cache tags. DoneA finite or infinite cache max-age. Done
- Kernel tests. Necessary?
- Verify that cache invalidation works as expected when display handler cache metadata changes. Necessary?
User interface changes
None.
Introduced terminology
None.
API changes
No API changes. The modifications enhance the existing methods without altering their signatures or introducing breaking changes.
Data model changes
None.
Release notes snippet
The ViewsBlockBase class now merges cache metadata from the associated view's display handler for contexts, tags, and max-age. This ensures consistent cache behavior between Views blocks and their display handlers.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3493858-extend-viewsblockbase-to-2.patch | 1.28 KB | xavier.masson |
Issue fork drupal-3493858
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
xavier.massonComment #3
lendudeIs this maybe trying to fix the same problem as #2632828: Block view cachability metadata doesn't get copied in BlockView::preRender() when the block content is non-empty? Not sure, but feel very similar.
Comment #4
quietone commentedChanges are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies.
Comment #5
smustgrave commented@xavier.masson can you see if the issue mentioned by @lendude in #3 is the same?
If not the change here should be in an MR, with test coverage showing the issue.
Thanks!
Comment #6
xavier.masson@smustgrave , after reviewing the issue mentioned in comment #3, it seems to be completely different.
I attempted to create a merge request, but it seems I don’t have the necessary permissions. Regarding the tests, I’ll try to provide them when i'll have some free time.
Comment #9
vidorado commentedAdded a unit test that I believe is enough to test the funcionality.
In the "Remaining tasks" section there were kernel test and cache invalidation test suggestions, but I think they are unnecessary.
Comment #11
smustgrave commentedComments in MR.
Comment #12
vidorado commentedThanks for the quick review!
I've replied to some comments. Please, let me know if this additional comment on the issue queue was not necessary.
Comment #13
vidorado commentedAll threads resolved! :)
Comment #14
smustgrave commentedAll feedback on this one appears to be resolved.
Comment #15
quietone commentedI read the IS, comments and the MR. I didn't find any unanswered questions or other work to do.
Comment #21
longwaveThis is a nice self contained fix, backported down to 10.4.x as an eligible bug fix.
Committed and pushed 1ad501aafe7 to 11.x and 1a8a6184101 to 11.1.x and 766f32639c9 to 10.5.x and 09158e668be to 10.4.x. Thanks!
Comment #22
wim leersWOW! How did we miss this before 8.0.0 shipped?! And especially: I'm shocked that this did not become a priority years ago 😮
Very nice — I bet this fixes many subtle confusing problems on many sites, and it does so in a backwards compatible way! Impressive 👏
Comment #24
mxr576Bubbling up cacheabilty information from the display handler may not be enough, the cacheability information from the View entity should be also incorporated - at least cache tags, that includes the View config entity's own cache tag - more details in #2887144-44: Views exposed form block options are not updated immediately when adding additional sorts, filters, etc (Caching?)
Let me also deeplink the failing test which should prove a cache invalidation issue still exists: https://git.drupalcode.org/issue/drupal-2887144/-/pipelines/500971/test_...)