When a referenced view does not define a empty handler (for example a little text snippet or so) the referenced view gets cached without any cache information.
This happens because of following code construct in ViewsReferenceFieldFormatter.php
if (!empty($view->result) || !empty($view->empty)) {
if ($this->getSetting('plugin_types')) {
// Add a custom template if the title is available.
$title = $view->getTitle();
if (!empty($title)) {
$elements[$delta]['title'] = [
'#theme' => 'viewsreference__view_title',
'#title' => $title,
];
}
}
$elements[$delta]['contents'] = $view->buildRenderable($display_id, $view->args, FALSE);
}
$elements with #cache information is only filled when referenced view returns results or some stuff for empty.
I think $elements should always provide some basic caching information.
| Comment | File | Size | Author |
|---|
Show commandsStart 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:
changes
Comments
Comment #2
leon kessler commentedThis bug was causing me issues with an apache solr view. Sometimes the results would be blank, and then render cache would store these as empty, and would mean that even re-indexing would not cause the cache to be cleared (had to manually clear the cache to see results in the view).
Adding a no results text did fix this. But I think it should also be fixed in code.
I'm not sure why we need a check for
if (!empty($view->result) || !empty($view->empty)) {I checked the core views block, and it doesn't do this kind of check.
Why not let the view render out normally (even without results), this way the correct caching information is added?
Patch attached removes the conditional check.
Comment #3
gambryI think this will print the title even if the view is empty?
Which is probably acceptable, as the editor can always use the "Include View Title" setting plugin and hide it.
Also someone could debate if an empty view should hide the field, which I believe is the current behaviour?
But even though, you are printing a view, which usually renders something even when empty.
Another option would be to bubble only the cacheability metadata when the view is empty, so we can still control if field should be hidden/skipped as well as bubble our metadata? A bit like EntityReferenceFormatterBase() does, which however I'm wondering why
ViewsReferenceFieldFormatterdoesn't extend it?Comment #4
seanbI like the idea of just adding the cacheability metadata. Could we do that instead?
Comment #5
cbeier commentedRerolled patch from #3.
Comment #6
RecursiveMeta commentedRe-roll patch for current version
3017716-view-without-empty-handling-breach-caching-6.patch.
Comment #7
dydave commentedHi everyone!
Thanks a lot for posting this issue and contributing a patch.
I would like to confirm the patch from #6 fixed the issue in my case:
The patch applied properly without error and fixed the problem.
Changing status to Needs review for more testing.
Thanks!
Comment #8
gambryThanks @DYdave !
However the patch still needs work as per #4 (and #3).
Comment #9
nvakenWe still experience this issue. I think that this is actually a bigger issue then originally though. We've encountered it on one of our client's site and noticed that one of the views was sporadically not rendering and was temporarily fixed by flushing the cache. After hours of debugging we noticed that the following reproduction steps will consistently reproduce it:
After noticing this, we noticed that every other site that we have with this setup, is vulnerable for this bug. The only reason we noticed it this time is because there is probably a bot of some sort that is trying to poll these out of bound page numbers.
Though still, I think this is actually a pretty big problem for this module since it's pretty easy to break views on random sites.
Comment #10
tunicThanks nielsva! We had a similar problem with a view. Randomly, it started to show no results in English (in another language worked ok). We were not able to reproduce the problem. It turns out that in our case is enough to go to a page that is out of bound for the pager to trigger the problem, as you describe. Now I can reproduce the problem:
No need to edit the node that contains the view. However, the view currently has the cache disabled (we were trying to track down the problem and we disabled it), so that may be the reason.
The view has no empty handler. After creating one, it seems to work fine, I can't reproduce the problem again.
I agree this is a major issue.
Comment #11
socialnicheguru commentedIs this related?
Comment #12
cbeier commentedRe-roll patch for current version.
@SocialNicheGuru Yes, the other issue is relevant to the control of cache metadata.
I use both patches together to solve the cache problem.
Comment #13
socialnicheguru commentedthis does not apply if I apply the other patch
Comment #14
cbccharlie commentedHello,
The current patch causes the title to be displayed even though the view has no results.
Wouldn't it be enough to return the cache tags when the view has no results instead of rendering it anyway?
I upload a new patch doing it this way.
Thanks!
Comment #15
socialnicheguru commentedThe following patches all conflict and work on the same issue and piece of code for patch in #12:
#3228829: ViewsReferenceFieldFormatter does not check for headers set to show on empty.
#3294529: ViewsReferenceFieldFormatter does not render view if it has no results
#3017716: View without empty handling could break caching
Comment #16
seanbA fix for this has been added in #2919092: Viewsreference embeds Views without cache, breaking Dynamic page cache. Could you please check the latest dev version and let me know if this issue has been resolved?
Comment #17
marthinal commentedIt works. Thanks @seanB
Comment #18
socialnicheguru commentedso should this be closed as a dupe of #2919092: Viewsreference embeds Views without cache, breaking Dynamic page cache?
Comment #21
scott_euser commentedSince the existance of the 2 formatters now (ViewsReferenceLazyFieldFormatter and ViewsReferenceFieldFormatter) this is still an issue for the latter. I have created a Merge Request and hidden the patches. The original patch only handles cache tags, but actually the full cache settings of the View should be respected. For example, when using a Search API SOLR View, if there is an exception thrown, max-age is set to 0 so next load attempts again to contact SOLR - the formatter should respect that.
So I was able to reproduce this by
Comment #22
joegraduateApplying the diff from @scott_euser's MR as a composer patch fixes this issue in my testing.
I'm attaching a static copy here so we can use this on our sites.
Comment #23
scott_euser commentedHiding the patch to avoid confusion with merge request (side note: you can download the patch to apply locally as per recommended instructions)
Comment #24
ericdsd commentedTested MR, it works like a charm. +1 for RTBC
Comment #25
hchonovThe patch here fixes the issue for me where when accessing the page with a bad / wrong query parameter for the contextual filter with cold caches the page gets cached without the view and then accessing the page again without any query parameter returns again the page without the view instead of showing the view with all available filters.
Comment #27
seanbFixed, thanks!