Add support for "Search API (tags based)" caching in Views

If they seem to work, that would be great.

The facetblock is not cached, see \Drupal\facets\Plugin\Block\FacetBlock::getCacheMaxAge, so that behavior keeps working, but just for this block.
Also, we currently disable caches for the views facets are linked to: modules/globalignore/facets/src/Form/FacetSettingsForm.php:307

c/p fail :(

I updated the patch to be compatible with the last release (1.4)

Thanks for this patch!

So just to clarify for others:
Currently, facets, in their readme strongly recommends to disable the views cache.
Facets even disables the cache when a facet is saved. See #2663552: Facets do not display after page refresh
This patch changes that logic slightly and will first check if the Views cache is none, or any search_api_* ViewsCache plugin, and if so it won't change the Views cache.
This allows us to use search_api_tag for example which I'm currently using.
Haven't noticed any issues so far.

I think this is more of a feature request.

@DamienMcKenna yes, a very important feature, as some other modules may work properly only with cache enabled.

Reroll on top of 8.x-1.x-be2b9dd9a
Also works with 1.6.

I tried to add facets into the cache key of the Search API tag-based plugin and it seems to work fine.
And I removed extra preExecute() of the search query as it decreases performance, previously it used for cache key, but in #2624472: Make sure that cache tags of index and server are added to search result pages it was removed.

Sorry, my mistake, the patch I made should be uploaded in search_api module, not facets, moved to #3197050: Facet conditions not added to SearchApiCachePluginTrait::generateResultsKey causing caching issue for AJAX view.

Hiding patch 12 since it doesn't belong here.

We were using https://www.drupal.org/files/issues/2019-05-31/2939710_1.patch, but that now fails with the most recent version of Facets. Should we use https://www.drupal.org/files/issues/2018-04-13/2939710_0.patch ?

It looks like https://www.drupal.org/files/issues/2020-11-12/support_views_cache-29397... is working.

I'd humbly recommend that, if we're modifying views, let's modify the view to use Search API Tags Caching instead of none.

On my mind, we can more in terms of caching here, as facets with Views View as a source can rely on the Views cache plugins.
Views cache plugins have everything for it Drupal\views\Plugin\views\cache\CachePluginBase: getCacheMaxAge, getCacheTags + each views view display has its own \Drupal\views\Plugin\views\DisplayPluginBase::getCacheMetadata() that also can be injected as a facets cache metadata.

So if someone wants to disable or configure cache for the facet he/she can go and do it via configuring facet source view cache plugin:
- None - facets wont be cached as maxAge will be set to 0.
- Drupal\search_api\Plugin\views\cache\SearchApiTagCache - maxAge is permanent but it will be invalidated by cache tag system
- Drupal\search_api\Plugin\views\cache\SearchApiTimeCache - well, name speaks for it self.

Am I missing something: strategy above seems to be obvious but not yet implemented?

Don't have good ideas for the tests though.

• I assume that only functional test cases have to be covered.
• There are around ~30 classes that covers max-age:0 scenarios.

Last statement means that we need to extend each of them, that use facets_search_api_dependency as a dependency, and extend it with cache enable logic:

• Turn on page_cache and set system.performace, max-age => 1 day
• Configure search view from the facets_search_api_dependency to use search_api_cache tag.
• Modify each test to generate page cache before parent test will be performed
• Review each of them.

But this will double the "phpunit docroot/modules/contrib/facets" execution time.

Will be glad to here ideas from the community.

We need to verify that facets functionality works correctly/as expected for the views that have search_api_tag enabled as cache plugin.
Adding a patch that contains only cache plugin change to verify that.

My fault, I didn't realize that the base branch will be current issue branch...
Previous test is not "clean". This one reverts everything what have been done in this branch and changes cache plugin.

Hiding wrong #24 patch.
#23 identified several issues that can be divided in two groups:

1. testAndOrFacet, testMinimumAmount, testExcludeFacetDate, testExcludeFacet
2. testViewsCacheDisable

First group modifies a query object sent to Search API, and for some reason it doesn't invalidate search API query cache e.g.:

1. Query was created and cached with "And" operator
2. User changes facet preferences and set this operator to "Or"
3. Search API query doesn't know about this change and delivers old results

Second group is a side effect of a patch: it expects views to be not cacheable.

Latest changes:

• Merged latest dev into issue branch
• Addressed issues identified https://www.drupal.org/project/facets/issues/2939710#comment-14469475, however solution is not straightforward and related to the one that posted in this thread by @kiselevat https://www.drupal.org/project/search_api/issues/3197050. I've added logic to clear facet source plugin cache whenever user changes facet configuration. But I think that search_api need to provide a better way.
• Extended Integration tests and enabled website cache + reconfigured on the fly default search views view to use Search API cache tag approach.

Looks good to me, but tests needs to be revised/reviewed, unfortunately I don't have a lot of experience with facets, so looking for help here.

It should be some kind of tradition: every once in the while, someone, tries to enable Facet caching.
Several things that are missing though:

1. update hook, to inject facets metadata into search api views display: will add it shortly
2. Tests
   • I tested cachebility metadata of the facet block with 3 plugins: none, search_api_tag and search_api_time. Made sure that there are proper max ages, and it didn't break existing facet approach e.g. max-age:0 with disabled caching.
   • Tested FacetManager alterQuery cache metadata.
   • Ran Integration test but with enabled views caching, to make sure that facet core functionality works in the same way as with disabled cache..

Of course there are many things that I've missed, will be glad if someone would give me a heads-up.
Note that facets wont work for views with enabled search_api_time cache and that don't have "url" or "url.query_args" contexts(there is no exposed form or pager). Search API related issue https://www.drupal.org/project/search_api/issues/3197050 should address this alongside with small performance issues related to re-saving a view each time when facet gets updated.

Cleanup tests - remove UI parts, and make sure that all tests runs for anonymous users,
Included build, sort, post_query processers cacheability into render array.

1. +++ b/facets.install
   @@ -216,3 +217,37 @@ function facets_update_8009() {
   +    return t(
   ...
   +    return t('There are no views with search API cache plugins and facets in the same time, so nothing has been updated.');
   

   Don't translate the message here. Otherwise these strings will remain in the database forever.

2. +++ b/src/Processor/ProcessorPluginBase.php
   @@ -116,4 +118,25 @@ class ProcessorPluginBase extends PluginBase implements ProcessorInterface {
   +    return Cache::PERMANENT;
   

   Shouldn't be "0", no caching be the default?

Hi @mkalkbrenner I've addressed issue with an install and t() usage.
Just to open a discussion regarding ProcessorPluginBase::getCacheMaxAge(), in my mind:

• Facet plugins provided by the module from the box should support the caching.
• In case another module extends base plugin logic, and can't support cache it should explicitly disabled it. I've added a test sort plugin FpcSortRandomProcessor and a test case testBuildCacheabilityMetadata that covers it.
• To tell the true it was much easier to enable it in the base class, instead of going into the each plugin.

But again if you will tell that this is wrong, then I'll move it on the plugin level.
One more thing that I would like to mention: I noticed that cache will work correctly only with native Facets block, and will break logic for every other module. However it can be easily fixed if we will inject FacetSource cache a bit earlier, for instance in the processFacets stage, in this way we can be sure that ::returnProcessedFacet(), ::returnBuiltFacet() and ::build() will contain all necessary cache metadata.

Meanwhile I rebased issue branch with the latest 2.0.x, so it's up to date.

I agree that ideally caching should work out of the box. But if you have an asynchronous search backend like Elastic or Solr where indexing takes some time and you cache too early with CACHE_PERMANENT, users will have to delete their entire cache to get the expected result.

So maybe setting it to 15 minutes instead of permanent might be a good solution. Or even better a configurable default.

In this approach I completely rely on the Source Search Plugin, by design it is SearchApi, that provides cache result mechanism:

• cache tag: whenever item is added to the index search_api_list_{INDEX_NAME} cache will be invalidated, thus facets results will be invalidated
• cache time: well don't need to explain anything, can be set to the 15 minutes
• no cache: views results and facets results will not be cached at all.

In the patch there is a fallback for the source plugins that don't implementCacheableDependencyInterface - facets cache for such sources will be set to 0.

Basically the way how facets results will be cached: time based, tag based or even will not be cached is controlled by the Facet Source plugin. And user can navigate to the Search View and set time based cache 15 minutes.

I can easily imagine this situation and bumped few times in it my self: index has finished, however search view displays old results and you need to wait until "Solr" backend will index/process all items. But in this case developer should set time based cached for the view or even turn it off completely, otherwise both view and facets attached to it will have issues.

Don't get me wrong. I like the patch at I'll merge it.
The only thing I'm concerned about is the CACHE_PERMANENT default for Processors.
I assume that there're custom processors out there. At least I know some.
Having them caching permanently instead of time based or not at all just because of a module update seems wrong to me. I consider that a BC break.
So I still recommend to set the default to 0 or a short lifetime.

Hi @mkalkbrenner can you provide an example of such case? Having it, will help me to get into the issue you are talking about. Maybe I could add more tests that will proof or will show to me where I'm wrong.

Meanwhile I've moved source plugin cache dependency from block into updateResult, which will allow other modules to not care about it and use FacetManager to get all required cacheabillity.

At Jartech we currently use 5 custom processors for facets.
One translates nummeric status codes into human readable words in different languages.
These values are fetched from a legacy system via API. The API call results are already cached for 15 Minutes, because new status codes and translations could be added there.
If I accept this patch and don't adjust the code of the processor, this caching will be converted into a permanent cache because the API is not called again for the same result.

From my point of view 3rd party developers should take actions to leverage the new caching features. But they should not be forced to do it in the context of a module update to avoid bugs.

I reviewed the patch again. I think the default for max lifetime for processors should be "0". And the processors included in facets should explicitly declare CACHE_PERMANENT if appropriate.

Hi @mkalkbrenner thanks for the heads up, it makes sense. Then lets agree on the next steps:
- use UncacheableDependencyTrait for the ProcessorPluginBase or dropCacheableDependencyInterface interface from it, which one do you prefer?
- add UnchangingCacheableDependencyTrait to almost all plugins defined in the Facet core.

Another things that has to be mentioned here:
- currently this patch doesn't cover facet_summary module, should it be part of this issue or new ticket should be created?
- patch for the search_api that lets to inject cache dependency into a query was merged recently and it is available only in dev. So we have a dependency here and to tell the true I don't know how to manage it, except wait for the stable release of SAPI and then add requirement in facet module.

- use UncacheableDependencyTrait for the ProcessorPluginBase or dropCacheableDependencyInterface interface from it, which one do you prefer?

Use UncacheableDependencyTrait and add a comment that no caching is the defensive default but most processors should be perfectly able to cache permanent.

add UnchangingCacheableDependencyTrait to almost all plugins defined in the Facet core.

yes, if the correct cache tags are declared.

currently this patch doesn't cover facet_summary module, should it be part of this issue or new ticket should be created

If it doesn't break with the new approach, I think caching should be addressed in a separate issue.

patch for the search_api that lets to inject cache dependency into a query was merged recently and it is available only in dev. So we have a dependency here and to tell the true I don't know how to manage it, except wait for the stable release of SAPI and then add requirement in facet module.

raise the dependency for search_api in composer.json to "^1.24 | dev-1.x". I'll talk to Thomas to create a new release soon.

Great work!
I decided to merge the current version as it is. This will ease testing and integration into existing setups and leave some time to do so before the upcoming release.
any issue we find now should be treated as bug report.

Hi @mkalkbrenner, it seems that there is an issue with "author by" can you have a look?
Meanwhile I'l start to work on facet summary.
Once again, thank you for the review!

Created #3281382: fine tune processor cache meta data.

We have a critical issue: #3281408: New Search API Tag based caching potentially breaks install from existing config

Thanks to everyone that worked getting this rolled out. I wanted to drop an edge case here for others that may be experiencing an issue I came across:

If you embed your view in a template like using twig_tweak (e.g. {{ drupal_view('my_view','block_1', term.id) }}), caching on the view will prevent your facets from being displayed. Moving the view earlier in the page building cycle, like normal block placement, resolves the issue and views caching + facets works as it should.