Define block cacheability metadata [#2581629]

Currently, a FacetBlock defines it's cache:max-age as 0 to prevent cacheing. We should be defining correct cacheability metadata instead.

Cache Contexts
I think it should be enough to vary by the current url. For example, when using the UrlProcessorQueryString url processor, it should be enough to define the cache context for the query url argument. (url.query_args:query). When using a pretty paths url processor I think we should probably vary by the entire url (url). Varying by the entire url will work for the UrlProcessorQueryString as well though and could be good enough for a first implementation.

Cache Tags
We should define the correct cache tags for the contents of the block as well; there's 2 options to go with here.

Add cache tags for all entities listed in the facet block. (e.g. term:4, term:6, term:10
Add 1 cache tag for the searcher and invalidate that tag when changing the searcher (e.g. search_api_index:2)

The first option is the best one to implement but is probably the hardest one to do as well.

Comments

Comment #1

7 October 2015 at 08:54

borisson_ created an issue. See original summary.

Comment #2

strykaizer

Dutch

Belgium

commented 7 October 2015 at 11:43

[13:32:23] borisson_: we can not cache on the terms (for a term facet, that is) only
[13:32:44] borisson_: since we also need to know all the content, terms can disappear if no content is available anymore
[13:33:29] borisson_: so point 1 should either be extended to the content too (which imo is not possible, since setting 1 million cachetags is prolly a bad idea), or is not an option
[13:34:12] borisson_: we *might* be able to just use the entity_list cachetag, not sure but that tag *might* invalidate everytime 1 entity is changed/inserted/dleeted
[13:35:51] StryKaizer: I think listing the term cache id's is enough. Because the cache will vary per url. And if no content is available anymore for that term then the content will have been updated
[13:36:13] StryKaizer: and updating a node should clear all related tags as well.
[13:37:09] borisson_: hmm, ic, if updating a node purges term:1 tag too, you are right
[13:37:29] borisson_: in that case we SHOULD use point 1, and no other solution ;)
[13:37:45] rteijeiro (~rteijeiro@alv-global.tietoenator.com) joined the channel
[13:38:09] If I'm understanding this discussion correctly (haven't read the issue, I think): Shouldn't the facets just depend on the Search API's cache tags? I.e., invaldiate the cached facet information as soon as new content gets indexed?
[13:38:22] (And of course the taxonomy terms, since their names might change.)
[13:38:34] drunken_monkey: that does sounds valid :)
[13:39:20] borisson_: are you sure cache varies per url? Since I think thats what we should avoid (if somebody alters another facet, we dont want the "Tags" facet to get loaded again, having the same values)
[13:39:28] borisson_: so we want to use placeholders, I guess
[13:39:49] We just don't have that in the Search API itself, atm. Here is the issue: https://www.drupal.org/node/2463715
[13:40:14] I guess vary on the search query?
[13:40:19] drunken_monkey: thx for the link!
[13:40:23] And use the search query's cache tags?
[13:40:27] (Once those are there-.)
[13:40:37] StryKaizer: we can make cache vary per url, that is in that issue.
[13:41:12] But, in theory, an identical query will lead to identical facets, even if on two completely different pages?
[13:41:46] But the whole thing's very complicated in any case.
[13:41:50] drunken_monkey: in theory, yes.
[13:42:09] borisson_: it would be cool if /search/tag=1&anotherfacet=2 and /search/tag=1&anotherfacet=3 does not load the tag facet twice, which we can do using placeholders
[13:42:33] If not essential, shouldn't we maybe just wait with implementing caching and operate on the assumption that searches and facets are not cacheable? Or is it important to bake in right at the beginning?
[13:42:34] rteijeiro (~rteijeiro@alv-global.tietoenator.com) left IRC (Ping timeout: 260 seconds)
[13:42:44] drunken_monkey: yes, definitly :)
[13:42:51] drunken_monkey: optimise later
[13:42:53] (Once everything else works correctly, it's way easier to see if introducing caching broke something.)
[13:43:02] drunken_monkey: totally agree