From 6b0840d9f13d3dd88388519b82a0d2b142bea89c Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Sat, 3 Oct 2015 22:15:01 +0100 Subject: [PATCH] Issue #2464409 by borisson_, Wim Leers, jhodgdon, Fabianx, catch, swentel: Search results should bubble rendered entity cache tags and set list cache tags --- .../node/src/Plugin/Search/NodeSearch.php | 12 ++ .../src/Controller/SearchController.php | 25 +++- .../search/src/Plugin/SearchPluginBase.php | 6 +- .../src/Tests/SearchPageCacheTagsTest.php | 118 ++++++++++++++++++ .../user/src/Plugin/Search/UserSearch.php | 3 + 5 files changed, 158 insertions(+), 6 deletions(-) diff --git a/core/modules/node/src/Plugin/Search/NodeSearch.php b/core/modules/node/src/Plugin/Search/NodeSearch.php index ecd6e869f85..97ae236fb7a 100644 --- a/core/modules/node/src/Plugin/Search/NodeSearch.php +++ b/core/modules/node/src/Plugin/Search/NodeSearch.php @@ -8,6 +8,7 @@ namespace Drupal\node\Plugin\Search; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Config\Config; use Drupal\Core\Database\Connection; use Drupal\Core\Database\Query\SelectExtender; @@ -167,6 +168,8 @@ class NodeSearch extends ConfigurableSearchPluginBase implements AccessibleInter $this->renderer = $renderer; $this->account = $account; parent::__construct($configuration, $plugin_id, $plugin_definition); + + $this->addCacheTags(['node_list']); } /** @@ -333,6 +336,7 @@ class NodeSearch extends ConfigurableSearchPluginBase implements AccessibleInter // Fetch comments for snippet. $rendered = $this->renderer->renderPlain($build); + $this->addCacheableDependency(CacheableMetadata::createFromRenderArray($build)); $rendered .= ' ' . $this->moduleHandler->invoke('comment', 'node_update_index', array($node, $item->langcode)); $extra = $this->moduleHandler->invokeAll('node_search_result', array($node, $item->langcode)); @@ -354,6 +358,14 @@ class NodeSearch extends ConfigurableSearchPluginBase implements AccessibleInter 'langcode' => $node->language()->getId(), ); + $this->addCacheableDependency($node); + + // We have to separately add the node owner's cache tags because search + // module doesn't use the rendering system, it does its own rendering + // without taking cacheability metadata into account. So we have to do it + // explicitly here. + $this->addCacheableDependency($node->getOwner()); + if ($type->displaySubmitted()) { $result += array( 'user' => $this->renderer->renderPlain($username), diff --git a/core/modules/search/src/Controller/SearchController.php b/core/modules/search/src/Controller/SearchController.php index 4d839d100d3..f108bcdaac0 100644 --- a/core/modules/search/src/Controller/SearchController.php +++ b/core/modules/search/src/Controller/SearchController.php @@ -8,8 +8,10 @@ namespace Drupal\search\Controller; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Config\ConfigFactory; use Drupal\Core\Controller\ControllerBase; +use Drupal\Core\Render\RendererInterface; use Drupal\search\SearchPageInterface; use Drupal\search\SearchPageRepositoryInterface; use Psr\Log\LoggerInterface; @@ -35,6 +37,13 @@ class SearchController extends ControllerBase { */ protected $logger; + /** + * The renderer. + * + * @var \Drupal\Core\Render\RendererInterface + */ + protected $renderer; + /** * Constructs a new search controller. * @@ -42,10 +51,13 @@ class SearchController extends ControllerBase { * The search page repository. * @param \Psr\Log\LoggerInterface $logger * A logger instance. + * @param \Drupal\Core\Render\RendererInterface $renderer + * The renderer. */ - public function __construct(SearchPageRepositoryInterface $search_page_repository, LoggerInterface $logger) { + public function __construct(SearchPageRepositoryInterface $search_page_repository, LoggerInterface $logger, RendererInterface $renderer) { $this->searchPageRepository = $search_page_repository; $this->logger = $logger; + $this->renderer = $renderer; } /** @@ -54,7 +66,8 @@ class SearchController extends ControllerBase { public static function create(ContainerInterface $container) { return new static( $container->get('search.search_page_repository'), - $container->get('logger.factory')->get('search') + $container->get('logger.factory')->get('search'), + $container->get('renderer') ); } @@ -118,14 +131,16 @@ class SearchController extends ControllerBase { '#markup' => '

' . $this->t('Your search yielded no results.') . '

', ), '#list_type' => 'ol', - '#cache' => array( - 'tags' => $entity->getCacheTags(), - ), '#context' => array( 'plugin' => $plugin->getPluginId(), ), ); + $this->renderer->addCacheableDependency($build, $entity); + if ($plugin instanceof CacheableDependencyInterface) { + $this->renderer->addCacheableDependency($build, $plugin); + } + // If this plugin uses a search index, then also add the cache tag tracking // that search index, so that cached search result pages are invalidated // when necessary. diff --git a/core/modules/search/src/Plugin/SearchPluginBase.php b/core/modules/search/src/Plugin/SearchPluginBase.php index ee24ea77735..2f2cd14b873 100644 --- a/core/modules/search/src/Plugin/SearchPluginBase.php +++ b/core/modules/search/src/Plugin/SearchPluginBase.php @@ -7,6 +7,8 @@ namespace Drupal\search\Plugin; +use Drupal\Core\Cache\RefinableCacheableDependencyInterface; +use Drupal\Core\Cache\RefinableCacheableDependencyTrait; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Plugin\PluginBase; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; @@ -16,7 +18,9 @@ use Symfony\Component\DependencyInjection\ContainerInterface; /** * Defines a base class for plugins wishing to support search. */ -abstract class SearchPluginBase extends PluginBase implements ContainerFactoryPluginInterface, SearchInterface { +abstract class SearchPluginBase extends PluginBase implements ContainerFactoryPluginInterface, SearchInterface, RefinableCacheableDependencyInterface { + + use RefinableCacheableDependencyTrait; /** * The keywords to use in a search. diff --git a/core/modules/search/src/Tests/SearchPageCacheTagsTest.php b/core/modules/search/src/Tests/SearchPageCacheTagsTest.php index cd5db681455..6a747181bd4 100644 --- a/core/modules/search/src/Tests/SearchPageCacheTagsTest.php +++ b/core/modules/search/src/Tests/SearchPageCacheTagsTest.php @@ -7,6 +7,8 @@ namespace Drupal\search\Tests; +use Drupal\Core\Cache\Cache; + /** * Tests the search_page entity cache tags on the search results pages. * @@ -44,6 +46,8 @@ class SearchPageCacheTagsTest extends SearchTestBase { // Create a node and update the search index. $this->node = $this->drupalCreateNode(['title' => 'bike shed shop']); + $this->node->setOwner($this->searchingUser); + $this->node->save(); $this->container->get('plugin.manager.search')->createInstance('node_search')->updateIndex(); search_update_totals(); } @@ -58,6 +62,7 @@ class SearchPageCacheTagsTest extends SearchTestBase { $this->drupalGet('search/node'); $this->assertCacheTag('config:search.page.node_search'); $this->assertCacheTag('search_index:node_search'); + $this->assertCacheTag('node_list'); // Node search results. $edit = array(); @@ -67,6 +72,10 @@ class SearchPageCacheTagsTest extends SearchTestBase { $this->assertCacheTag('config:search.page.node_search'); $this->assertCacheTag('search_index'); $this->assertCacheTag('search_index:node_search'); + $this->assertCacheTag('node:1'); + $this->assertCacheTag('user:2'); + $this->assertCacheTag('rendered'); + $this->assertCacheTag('node_list'); // Updating a node should invalidate the search plugin's index cache tag. $this->node->title = 'bike shop'; @@ -76,6 +85,10 @@ class SearchPageCacheTagsTest extends SearchTestBase { $this->assertCacheTag('config:search.page.node_search'); $this->assertCacheTag('search_index'); $this->assertCacheTag('search_index:node_search'); + $this->assertCacheTag('node:1'); + $this->assertCacheTag('user:2'); + $this->assertCacheTag('rendered'); + $this->assertCacheTag('node_list'); // Deleting a node should invalidate the search plugin's index cache tag. $this->node->delete(); @@ -84,10 +97,12 @@ class SearchPageCacheTagsTest extends SearchTestBase { $this->assertCacheTag('config:search.page.node_search'); $this->assertCacheTag('search_index'); $this->assertCacheTag('search_index:node_search'); + $this->assertCacheTag('node_list'); // Initial page for searching users. $this->drupalGet('search/user'); $this->assertCacheTag('config:search.page.user_search'); + $this->assertCacheTag('user_list'); $this->assertNoCacheTag('search_index'); $this->assertNoCacheTag('search_index:user_search'); @@ -95,8 +110,111 @@ class SearchPageCacheTagsTest extends SearchTestBase { $edit['keys'] = $this->searchingUser->getUsername(); $this->drupalPostForm('search/user', $edit, t('Search')); $this->assertCacheTag('config:search.page.user_search'); + $this->assertCacheTag('user_list'); + $this->assertCacheTag('user:2'); $this->assertNoCacheTag('search_index'); $this->assertNoCacheTag('search_index:user_search'); } + /** + * Tests the presence of expected cache tags with referenced entities. + */ + public function testSearchTagsBubbling() { + + // Install field UI and entity reference modules. + $this->container->get('module_installer')->install(['field_ui', 'entity_reference']); + $this->resetAll(); + + + // Creates a new content type that will have an entity reference. + $type_name = 'entity_reference_test'; + $type = $this->drupalCreateContentType(['name' => $type_name, 'type' => $type_name]); + + $bundle_path = 'admin/structure/types/manage/' . $type->id(); + + // Create test user. + $admin_user = $this->drupalCreateUser([ + 'access content', + 'create ' . $type_name . ' content', + 'administer node fields', + 'administer node display', + + ]); + $this->drupalLogin($admin_user); + + // First step: 'Add new field' on the 'Manage fields' page. + $this->drupalGet($bundle_path . '/fields/add-field'); + $this->drupalPostForm(NULL, [ + 'label' => 'Test label', + 'field_name' => 'test__ref', + 'new_storage_type' => 'entity_reference', + ], t('Save and continue')); + + // Second step: 'Field settings' form. + $this->drupalPostForm(NULL, [], t('Save field settings')); + + // Create a new node of our newly created node type and fill in the entity + // reference field. + $edit = [ + 'title[0][value]' => 'Llama shop', + 'field_test__ref[0][target_id]' => $this->node->getTitle() + ]; + $this->drupalPostForm('node/add/' . $type->id(), $edit, t('Save')); + + // Test that the value of the entity reference field is shown. + $this->drupalGet('node/2'); + $this->assertText('bike shed shop'); + + // Refresh the search index. + $this->container->get('plugin.manager.search')->createInstance('node_search')->updateIndex(); + search_update_totals(); + + // Login with searching user again. + $this->drupalLogin($this->searchingUser); + + // Default search cache tags. + $default_search_tags = [ + 'config:search.page.node_search', + 'search_index', + 'search_index:node_search', + 'rendered', + 'node_list', + ]; + + // Node search results for shop, should return node:1 (bike shed shop) and + // node:2 (Llama shop). The related authors cache tags should be visible as + // well. + $edit = array(); + $edit['keys'] = 'shop'; + $this->drupalPostForm('search/node', $edit, t('Search')); + $this->assertText('bike shed shop'); + $this->assertText('Llama shop'); + $expected_cache_tags = Cache::mergeTags($default_search_tags, [ + 'node:1', + 'user:2', + 'node:2', + 'user:3', + 'node_view', + 'config:filter.format.plain_text', + ]); + $cache_tags = $this->drupalGetHeader('X-Drupal-Cache-Tags'); + $this->assertEqual(explode(' ', $cache_tags), $expected_cache_tags); + + // Only get the new node in the search results, should result in node:1, + // node:2 and user:3 as cache tags even though only node:1 is shown. This is + // because node:2 is reference in node:1 as an entity reference. + $edit = array(); + $edit['keys'] = 'Llama'; + $this->drupalPostForm('search/node', $edit, t('Search')); + $this->assertText('Llama shop'); + $expected_cache_tags = Cache::mergeTags($default_search_tags, [ + 'node:1', + 'node:2', + 'user:3', + 'node_view', + ]); + $cache_tags = $this->drupalGetHeader('X-Drupal-Cache-Tags'); + $this->assertEqual(explode(' ', $cache_tags), $expected_cache_tags); + } + } diff --git a/core/modules/user/src/Plugin/Search/UserSearch.php b/core/modules/user/src/Plugin/Search/UserSearch.php index a0fb3b49f0d..df357326be9 100644 --- a/core/modules/user/src/Plugin/Search/UserSearch.php +++ b/core/modules/user/src/Plugin/Search/UserSearch.php @@ -93,6 +93,8 @@ class UserSearch extends SearchPluginBase implements AccessibleInterface { $this->moduleHandler = $module_handler; $this->currentUser = $current_user; parent::__construct($configuration, $plugin_id, $plugin_definition); + + $this->addCacheTags(['user_list']); } /** @@ -154,6 +156,7 @@ class UserSearch extends SearchPluginBase implements AccessibleInterface { if ($this->currentUser->hasPermission('administer users')) { $result['title'] .= ' (' . $account->getEmail() . ')'; } + $this->addCacheableDependency($account); $results[] = $result; }