Commit 788b0e78 authored by pfrenssen's avatar pfrenssen Committed by drunken monkey

Issue #2824640 by pfrenssen, drunken monkey, sardara, idimopoulos, geek.merlin...

Issue #2824640 by pfrenssen, drunken monkey, sardara, idimopoulos, geek.merlin aka axel.rutz, borisson_, claudiu.cristea: Fixed caching of Views results.
parent 00414936
Search API 1.x, dev (xxxx-xx-xx):
---------------------------------
- #2824640 by pfrenssen, drunken monkey, sardara, idimopoulos, geek.merlin aka
axel.rutz, borisson_, claudiu.cristea: Fixed caching of Views results.
- #3001743 by drunken monkey: Adapted to use the new
ConditionInterface::alwaysFalse() method.
- #3098722 by drunken monkey, milindk: Increased the Core version requirement to
......
......@@ -5,4 +5,4 @@ package: Search
core: 8.x
configure: search_api.overview
dependencies:
- drupal:system (>=8.7)
- drupal:system (>=8.7.4)
......@@ -257,4 +257,20 @@ interface DatasourceInterface extends IndexPluginInterface {
*/
public function getFieldDependencies(array $fields);
/**
* Returns the list cache contexts associated with this datasource.
*
* List cache contexts ensure that if items from a datasource are included in
* a list that any caches containing this list are varied as necessary. For
* example a view might contain a number of items from this datasource that
* are visible only by users that have a certain role. These list cache
* contexts will ensure that separate cached versions exist for users with
* this role and without it. These contexts should be included whenever a list
* is rendered that contains items from this datasource.
*
* @return string[]
* The list cache contexts associated with this datasource.
*/
public function getListCacheContexts();
}
......@@ -164,4 +164,11 @@ abstract class DatasourcePluginBase extends IndexPluginBase implements Datasourc
return [];
}
/**
* {@inheritdoc}
*/
public function getListCacheContexts() {
return [];
}
}
......@@ -4,6 +4,7 @@ namespace Drupal\search_api\Plugin\search_api\datasource;
use Drupal\Component\Utility\Crypt;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Entity\ContentEntityInterface;
......@@ -1159,4 +1160,13 @@ class ContentEntity extends DatasourcePluginBase implements EntityDatasourceInte
return $valid_ids;
}
/**
* {@inheritdoc}
*/
public function getListCacheContexts() {
$contexts = parent::getListCacheContexts();
$entity_list_contexts = $this->getEntityType()->getListCacheContexts();
return Cache::mergeContexts($entity_list_contexts, $contexts);
}
}
......@@ -173,8 +173,7 @@ trait SearchApiCachePluginTrait {
*/
public function generateResultsKey() {
if (!isset($this->resultsKey)) {
$query = $this->getQuery()->getSearchApiQuery();
$query->preExecute();
$this->getQuery()->getSearchApiQuery()->preExecute();
$view = $this->getView();
$build_info = $view->build_info;
......@@ -188,6 +187,9 @@ trait SearchApiCachePluginTrait {
],
];
// Vary the results key by the cache contexts of the display handler.
// These cache contexts are calculated when the view is saved in the Views
// UI and stored in the view config entity.
$display_handler_cache_contexts = $this->displayHandler
->getCacheMetadata()
->getCacheContexts();
......
......@@ -78,8 +78,14 @@ class SearchApiTagCache extends Tag {
*/
public function getCacheTags() {
$tags = $this->view->storage->getCacheTags();
// Add the list cache tag of the search index, so that the view will be
// invalidated whenever the index is updated.
$tag = 'search_api_list:' . $this->getQuery()->getIndex()->id();
$tags = Cache::mergeTags([$tag], $tags);
// Also add the cache tags of the index itself, so that the view will be
// invalidated if the configuration of the index changes.
$index_tags = $this->getQuery()->getIndex()->getCacheTagsToInvalidate();
$tags = Cache::mergeTags($index_tags, $tags);
return $tags;
}
......
......@@ -3,6 +3,8 @@
namespace Drupal\search_api\Plugin\views\query;
use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheableDependencyInterface;
use Drupal\Core\Database\Query\ConditionInterface;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
......@@ -260,7 +262,7 @@ class SearchApiQuery extends QueryPluginBase {
if ($display_type === 'rest_export') {
$display_type = 'rest';
}
$this->query->setSearchId("views_$display_type:" . $view->id() . '__' . $view->current_display);
$this->query->setSearchId("views_$display_type:" . $view->id() . '__' . $display->display['id']);
$this->query->setOption('search_api_view', $view);
}
catch (\Exception $e) {
......@@ -701,6 +703,49 @@ class SearchApiQuery extends QueryPluginBase {
}
}
/**
* {@inheritdoc}
*/
public function getCacheContexts() {
$query = $this->getSearchApiQuery();
if ($query instanceof CacheableDependencyInterface) {
return $query->getCacheContexts();
}
// We are not returning the cache contexts from the parent class since these
// are based on the default SQL storage from Views, while our results are
// coming from the search engine.
return [];
}
/**
* {@inheritdoc}
*/
public function getCacheTags() {
$tags = parent::getCacheTags();
$query = $this->getSearchApiQuery();
if ($query instanceof CacheableDependencyInterface) {
$tags = Cache::mergeTags($query->getCacheTags(), $tags);
}
return $tags;
}
/**
* {@inheritdoc}
*/
public function getCacheMaxAge() {
$max_age = parent::getCacheMaxAge();
$query = $this->getSearchApiQuery();
if ($query instanceof CacheableDependencyInterface) {
$max_age = Cache::mergeMaxAges($query->getCacheMaxAge(), $max_age);
}
return $max_age;
}
/**
* Retrieves the conditions placed on this query.
*
......
......@@ -3,6 +3,9 @@
namespace Drupal\search_api\Query;
use Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
use Drupal\Core\Cache\RefinableCacheableDependencyTrait;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
......@@ -19,9 +22,10 @@ use Drupal\search_api\Utility\QueryHelperInterface;
/**
* Provides a standard implementation for a Search API query.
*/
class Query implements QueryInterface {
class Query implements QueryInterface, RefinableCacheableDependencyInterface {
use StringTranslationTrait;
use RefinableCacheableDependencyTrait;
use DependencySerializationTrait {
__sleep as traitSleep;
__wakeup as traitWakeup;
......@@ -748,6 +752,52 @@ class Query implements QueryInterface {
return $this->originalQuery ?: clone $this;
}
/**
* {@inheritdoc}
*/
public function getCacheContexts() {
// Call the pre-execute method to ensure that processors and modules have
// had the chance to alter the query and modify the cacheability metadata.
$this->preExecute();
$contexts = $this->cacheContexts;
foreach ($this->getIndex()->getDatasources() as $datasource) {
$contexts = Cache::mergeContexts($datasource->getListCacheContexts(), $contexts);
}
return $contexts;
}
/**
* {@inheritdoc}
*/
public function getCacheTags() {
// Call the pre-execute method to ensure that processors and modules have
// had the chance to alter the query and modify the cacheability metadata.
$this->preExecute();
$tags = $this->cacheTags;
// If the configuration of the search index changes we should invalidate the
// views that show results from this index.
$index_tags = $this->getIndex()->getCacheTagsToInvalidate();
$tags = Cache::mergeTags($index_tags, $tags);
return $tags;
}
/**
* {@inheritdoc}
*/
public function getCacheMaxAge() {
// Call the pre-execute method to ensure that processors and modules have
// had the chance to alter the query and modify the cacheability metadata.
$this->preExecute();
return $this->cacheMaxAge;
}
/**
* {@inheritdoc}
*/
......
......@@ -307,9 +307,6 @@ interface QueryInterface extends ConditionSetInterface {
* This method should always be called by execute() and contain all necessary
* operations that have to be execute before the query is passed to the
* server's search() method.
*
* @throws \Drupal\search_api\SearchApiException
* Thrown if any wrong options were discovered.
*/
public function preExecute();
......
......@@ -14,7 +14,7 @@ use Drupal\node\NodeInterface;
function search_api_test_node_grants(AccountInterface $account, $op) {
$grants = [];
if (\Drupal::state()->get('search_api_test_add_node_access_grant', TRUE)) {
if (\Drupal::state()->get('search_api_test_add_node_access_grant', FALSE)) {
$grants['search_api_test'] = [$account->id()];
}
......@@ -27,7 +27,7 @@ function search_api_test_node_grants(AccountInterface $account, $op) {
function search_api_test_node_access_records(NodeInterface $node) {
$grants = [];
if (\Drupal::state()->get('search_api_test_add_node_access_grant', TRUE)) {
if (\Drupal::state()->get('search_api_test_add_node_access_grant', FALSE)) {
$grants[] = [
'realm' => 'search_api_test',
'gid' => $node->getOwnerId(),
......
id: test_node_index
name: 'Test node index'
description: 'An index of node entities used for testing'
read_only: false
field_settings:
status:
label: Published
datasource_id: 'entity:node'
property_path: status
type: boolean
dependencies:
module:
- node
title:
label: Title
datasource_id: 'entity:node'
property_path: title
type: string
dependencies:
module:
- node
processor_settings:
add_url: { }
aggregated_field: { }
rendered_item: { }
options:
cron_limit: -1
index_directly: false
datasource_settings:
'entity:node':
bundles:
default: true
selected: { }
languages:
default: true
selected: { }
tracker_settings:
default:
indexing_order: fifo
server: database_search_server
status: true
langcode: en
dependencies:
config:
- search_api.server.database_search_server
module:
- node
- search_api
id: database_search_server
name: 'Database search server'
description: 'A server used for testing'
backend: search_api_db
backend_config:
database: 'default:default'
min_chars: 3
matching: words
status: true
langcode: en
dependencies:
module:
- search_api_db
langcode: en
status: true
dependencies:
config:
- search_api.index.test_node_index
module:
- search_api
id: search_api_test_node_view
label: 'Search API test node view'
module: views
description: ''
tag: ''
base_table: search_api_index_test_node_index
base_field: search_api_id
core: 8.x
display:
default:
display_plugin: default
id: default
display_title: Master
position: 0
display_options:
access:
type: none
options: { }
cache:
type: search_api_tag
options: { }
query:
type: search_api_query
options:
bypass_access: false
skip_access: false
preserve_facet_query_args: false
exposed_form:
type: basic
options:
submit_button: Search
reset_button: false
reset_button_label: Reset
exposed_sorts_label: 'Sort by'
expose_sort_order: true
sort_asc_label: Asc
sort_desc_label: Desc
pager:
type: full
options:
items_per_page: 10
offset: 0
id: 0
total_pages: null
expose:
items_per_page: false
items_per_page_label: 'Items per page'
items_per_page_options: '5, 10, 20, 40, 60'
items_per_page_options_all: false
items_per_page_options_all_label: '- All -'
offset: false
offset_label: Offset
tags:
previous: ‹‹
next: ››
style:
type: default
row:
type: fields
fields:
title:
id: title
table: search_api_index_test_node_index
field: title
plugin_id: search_api_field
filters: { }
sorts: { }
title: 'Search API test node view'
header: { }
footer: { }
empty: { }
relationships: { }
arguments: { }
display_extenders: { }
cache_metadata:
max-age: -1
contexts:
- 'languages:language_content'
- 'languages:language_interface'
- url.query_args
- 'user.node_grants:view'
tags:
- 'config:search_api.index.test_node_index'
page_1:
display_plugin: page
id: page_1
display_title: Page
position: 1
display_options:
display_extenders: { }
path: search-api-test-node-view
cache_metadata:
max-age: -1
contexts:
- 'languages:language_content'
- 'languages:language_interface'
- url.query_args
- 'user.node_grants:view'
tags:
- 'config:search_api.index.test_node_index'
type: module
name: 'Search API node indexing test'
description: 'Test module for testing indexing of nodes in Search API.'
package: 'Search API'
dependencies:
- search_api:search_api_db
core: 8.x
hidden: true
......@@ -5,6 +5,7 @@
* Contains hook implementations for the Search API Views Test module.
*/
use Drupal\Core\Cache\RefinableCacheableDependencyInterface;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityListBuilder;
use Drupal\search_api\Query\QueryInterface;
......@@ -12,14 +13,26 @@ use Drupal\search_api\Query\QueryInterface;
/**
* Implements hook_search_api_query_alter().
*
* Prints the contents of the "search_api_retrieved_field_values" query option
* to the page (if present) so it can be checked by the testing code.
* - Prints the contents of the "search_api_retrieved_field_values" query option
* to the page (if present) so it can be checked by the testing code.
* - Optionally alters the query to include custom cacheability metadata, so
* that we can test if modules can alter the cacheability of search queries.
*/
function search_api_test_views_search_api_query_alter(QueryInterface $query) {
$fields = $query->getOption('search_api_retrieved_field_values');
if ($fields) {
\Drupal::messenger()->addStatus("'" . implode("' '", $fields) . "'");
}
$alter_cache_metadata = \Drupal::state()
->get('search_api_test_views.alter_query_cacheability_metadata', FALSE);
if ($alter_cache_metadata
&& $query instanceof RefinableCacheableDependencyInterface) {
// Alter in some imaginary cacheability metadata for testing.
$query->addCacheContexts(['search_api_test_context']);
$query->addCacheTags(['search_api:test_tag']);
$query->mergeCacheMaxAge(100);
}
}
/**
......
......@@ -48,6 +48,9 @@ class ContentAccessTest extends ProcessorTestBase {
public function setUp($processor = NULL) {
parent::setUp('content_access');
// Activate our custom grant.
\Drupal::state()->set('search_api_test_add_node_access_grant', TRUE);
// Create a node type for testing.
$type = NodeType::create(['type' => 'page', 'name' => 'page']);
$type->save();
......
This diff is collapsed.
<?php
namespace Drupal\Tests\search_api\Kernel\Views;
use Drupal\Core\Cache\Context\CacheContextsManager;
use Drupal\Core\Config\Config;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\KernelTests\KernelTestBase;
use Drupal\views\ViewExecutable;
/**
* Tests that cacheability metadata is included when Views config is exported.
*
* @group search_api
*/
class ViewsCacheabilityMetadataExportTest extends KernelTestBase {
/**
* The ID of the view used in the test.
*/
const TEST_VIEW_ID = 'search_api_test_node_view';
/**
* The display IDs used in the test.
*
* @var string[]
*/
protected static $testViewDisplayIds = ['default', 'page_1'];
/**
* The entity type manager.
*
* @var \Drupal\Core\Entity\EntityTypeManagerInterface
*/
protected $entityTypeManager;
/**
* The service that is responsible for creating Views executable objects.
*
* @var \Drupal\views\ViewExecutableFactory
*/
protected $viewExecutableFactory;
/**
* The state service.
*
* @var \Drupal\Core\State\StateInterface
*/
protected $state;
/**
* {@inheritdoc}
*/
protected static $modules = [
'field',
'node',
'search_api',
'search_api_db',
'search_api_test_node_indexing',
'search_api_test_views',
'system',
'text',
'user',
'views',
];
/**
* {@inheritdoc}
*/
public function register(ContainerBuilder $container) {
parent::register($container);
// Use a mocked version of the cache contexts manager so we can use a mocked
// cache context "search_api_test_context" without triggering a validation
// error.
$cache_contexts_manager = $this->createMock(CacheContextsManager::class);
$cache_contexts_manager->method('assertValidTokens')->willReturn(TRUE);
$container->set('cache_contexts_manager', $cache_contexts_manager);
}
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->installEntitySchema('node');
$this->installEntitySchema('search_api_task');
$this->installConfig([
'search_api',
'search_api_test_node_indexing',
]);
$this->entityTypeManager = $this->container->get('entity_type.manager');
$this->viewExecutableFactory = $this->container->get('views.executable');
$this->state = $this->container->get('state');
}
/**
* Tests that an exported view contains the right cacheability metadata.
*/
public function testViewExport() {
$expected_cacheability_metadata = [
'contexts' => [
// Search API uses the core EntityFieldRenderer for rendering content.
// This has support for translatable content, so the result varies by
// content language.
// @see \Drupal\views\Entity\Render\EntityFieldRenderer::getCacheContexts()
'languages:language_content',
// By default, Views always adds the interface language cache context
// since it is very likely that there will be translatable strings in
// the result.
// @see \Drupal\views\Entity\View::addCacheMetadata()
'languages:language_interface',
// Our test view has a pager so we expect it to vary by query arguments.
// @see \Drupal\views\Plugin\views\pager\SqlBase::getCacheContexts()
'url.query_args',
// The test view is a listing of nodes returned as a search result. It
// is expected to have the list cache contexts of the node entity type.
// This is defined in the "list_cache_contexts" key of the node entity
// annotation.
'user.node_grants:view',
],
'tags' => [
// Our test view depends on the search index, so whenever the index
// configuration changes the cached results should be invalidated.
// @see \Drupal\search_api\Query\Query::getCacheTags()
'config:search_api.index.test_node_index',
],
// By default the result is permanently cached.
'max-age' => -1,
];
// Check that our test view has the expected cacheability metadata.
$view = $this->getView();
$this->assertViewCacheabilityMetadata($expected_cacheability_metadata, $view);
// For efficiency Views calculates the cacheability metadata whenever a view
// is saved, and includes it in the exported configuration.
// @see \Drupal\views\Entity\View::addCacheMetadata()
// Check that the exported configuration contains the expected metadata.
$view_config = $this->config('views.view.' . self::TEST_VIEW_ID);
$this->assertViewConfigCacheabilityMetadata($expected_cacheability_metadata, $view_config);
// Test that modules are able to alter the cacheability metadata. Our test
// hook implementation will alter all 3 types of metadata.
// @see search_api_test_views_search_api_query_alter()
$expected_cacheability_metadata['contexts'][] = 'search_api_test_context';
$expected_cacheability_metadata['tags'][] = 'search_api:test_tag';
$expected_cacheability_metadata['max-age'] = 100;
// Activate the alter hook and resave the view so it will recalculate the
// cacheability metadata.
$this->state->set('search_api_test_views.alter_query_cacheability_metadata', TRUE);
$view->save();
// Check that the altered metadata is now present in the view and the
// configuration.
$view = $this->getView();
$this->assertViewCacheabilityMetadata($expected_cacheability_metadata, $view);
$view_config = $this->config('views.view.' . self::TEST_VIEW_ID);
$this->assertViewConfigCacheabilityMetadata($expected_cacheability_metadata, $view_config);
}
/**
* Checks that the given view has the expected cacheability metadata.
*
* @param array $expected_cacheability_metadata
* An array of cacheability metadata that is expected to be present on the
* view.
* @param \Drupal\views\ViewExecutable $view
* The view.
*/
protected function assertViewCacheabilityMetadata(array $expected_cacheability_metadata, ViewExecutable $view) {
// Cacheability metadata is stored separately for each Views display since
// depending on how the display is configured it might have different
// caching needs. Ensure to check all displays.
foreach (self::$testViewDisplayIds as $display_id) {
$view->setDisplay($display_id);
$display = $view->getDisplay();
$actual_cacheability_metadata = $display->getCacheMetadata();
$this->assertArrayEquals($expected_cacheability_metadata['contexts'], $actual_cacheability_metadata->getCacheContexts());
$this->assertArrayEquals($expected_cacheability_metadata['tags'], $actual_cacheability_metadata->getCacheTags());
$this->assertEquals($expected_cacheability_metadata['max-age'], $actual_cacheability_metadata->getCacheMaxAge());
}
}
/**
* Checks that the given view config has the expected cacheability metadata.
*
* @param array $expected_cacheability_metadata
* An array of cacheability metadata that is expected to be present on the
* view configuration.
* @param \Drupal\Core\Config\Config $config
* The configuration to check.
*/
protected function assertViewConfigCacheabilityMetadata(array $expected_cacheability_metadata, Config $config) {