From e242632ca19cc2ac63c7d52ec7c79f50896251f0 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org> Date: Wed, 13 May 2015 12:08:57 +0100 Subject: [PATCH] Issue #2450897 by plach, epari.siva, dawehner, jibran, Fabianx: Cache Field views row output --- core/lib/Drupal/Core/Render/RenderCache.php | 24 +- core/lib/Drupal/Core/Render/Renderer.php | 12 +- .../Drupal/Core/Render/RendererInterface.php | 8 +- .../src/Tests/Views/CommentUserNameTest.php | 2 + .../contact/src/Access/ContactPageAccess.php | 19 +- .../contact/src/Tests/ContactPersonalTest.php | 4 +- .../src/Tests/Views/ContactLinkTest.php | 3 + .../views.view.test_contact_link.yml | 19 +- .../src/Tests/Views/ViewsIntegrationTest.php | 1 + .../src/Tests/Views/HandlerFieldFieldTest.php | 1 + .../simpletest/src/UserCreationTrait.php | 217 ++++++++ core/modules/simpletest/src/WebTestBase.php | 155 +----- .../src/Plugin/views/field/BulkForm.php | 16 +- .../src/Tests/Entity/EntityOperationsTest.php | 5 +- .../src/Tests/Views/TermNameFieldTest.php | 3 +- core/modules/views/src/Entity/View.php | 10 + .../Plugin/views/cache/CachePluginBase.php | 69 +++ .../views/src/Plugin/views/field/Counter.php | 2 + .../src/Plugin/views/field/EntityLink.php | 4 +- .../views/field/FieldHandlerInterface.php | 20 + .../Plugin/views/field/FieldPluginBase.php | 27 +- .../views/src/Plugin/views/field/LinkBase.php | 11 +- .../field/UncacheableFieldHandlerTrait.php | 65 +++ .../Plugin/views/style/StylePluginBase.php | 91 +++- .../Tests/Entity/RowEntityRenderersTest.php | 1 + .../src/Tests/Handler/FieldCounterTest.php | 1 + .../src/Tests/Handler/FieldEntityLinkTest.php | 19 +- .../src/Tests/Plugin/RowRenderCacheTest.php | 178 +++++++ .../views.view.test_row_render_cache.yml | 503 ++++++++++++++++++ .../src/Unit/Plugin/field/CounterTest.php | 34 +- .../Drupal/Tests/Core/Render/RendererTest.php | 69 +++ 31 files changed, 1378 insertions(+), 215 deletions(-) create mode 100644 core/modules/simpletest/src/UserCreationTrait.php create mode 100644 core/modules/views/src/Plugin/views/field/UncacheableFieldHandlerTrait.php create mode 100644 core/modules/views/src/Tests/Plugin/RowRenderCacheTest.php create mode 100644 core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml diff --git a/core/lib/Drupal/Core/Render/RenderCache.php b/core/lib/Drupal/Core/Render/RenderCache.php index 3f206a38c3c0..aa071cc9be05 100644 --- a/core/lib/Drupal/Core/Render/RenderCache.php +++ b/core/lib/Drupal/Core/Render/RenderCache.php @@ -295,7 +295,7 @@ protected function createCacheID(array $elements) { * {@inheritdoc} */ public function getCacheableRenderArray(array $elements) { - return [ + $data = [ '#markup' => $elements['#markup'], '#attached' => $elements['#attached'], '#post_render_cache' => $elements['#post_render_cache'], @@ -305,6 +305,28 @@ public function getCacheableRenderArray(array $elements) { 'max-age' => $elements['#cache']['max-age'], ], ]; + + // Preserve cacheable items if specified. If we are preserving any cacheable + // children of the element, we assume we are only interested in their + // individual markup and not the parent's one, thus we empty it to minimize + // the cache entry size. + if (!empty($elements['#cache_properties']) && is_array($elements['#cache_properties'])) { + $data['#cache_properties'] = $elements['#cache_properties']; + // Extract all the cacheable items from the element using cache + // properties. + $cacheable_items = array_intersect_key($elements, array_flip($elements['#cache_properties'])); + $cacheable_children = Element::children($cacheable_items); + if ($cacheable_children) { + $data['#markup'] = ''; + // Cache only cacheable children's markup. + foreach ($cacheable_children as $key) { + $cacheable_items[$key] = ['#markup' => $cacheable_items[$key]['#markup']]; + } + } + $data += $cacheable_items; + } + + return $data; } } diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index b969a518b48d..827330f99674 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -9,12 +9,11 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\NestedArray; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Cache\Cache; -use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Controller\ControllerResolverInterface; use Drupal\Core\Theme\ThemeManagerInterface; -use Drupal\Component\Utility\SafeMarkup; /** * Turns a render array into a HTML string. @@ -180,7 +179,16 @@ protected function doRender(&$elements, $is_root_call = FALSE) { if ($is_root_call) { $this->processPostRenderCache($elements); } + // Mark the element markup as safe. If we have cached children, we need + // to mark them as safe too. The parent markup contains the child + // markup, so if the parent markup is safe, then the markup of the + // individual children must be safe as well. $elements['#markup'] = SafeMarkup::set($elements['#markup']); + if (!empty($elements['#cache_properties'])) { + foreach (Element::children($cached_element) as $key) { + SafeMarkup::set($cached_element[$key]['#markup']); + } + } // The render cache item contains all the bubbleable rendering metadata // for the subtree. $this->updateStack($elements); diff --git a/core/lib/Drupal/Core/Render/RendererInterface.php b/core/lib/Drupal/Core/Render/RendererInterface.php index 0af066e28a32..d407f0f49da5 100644 --- a/core/lib/Drupal/Core/Render/RendererInterface.php +++ b/core/lib/Drupal/Core/Render/RendererInterface.php @@ -7,8 +7,6 @@ namespace Drupal\Core\Render; -use Drupal\Core\Cache\CacheableDependencyInterface; - /** * Defines an interface for turning a render array into a string. */ @@ -249,6 +247,12 @@ public function renderPlain(&$elements); * and written to cache using the value of $pre_bubbling_cid as the cache * ID. This ensures the pre-bubbling ("wrong") cache ID redirects to the * post-bubbling ("right") cache ID. + * - If this element also has #cache_properties defined, all the array items + * matching the specified property names will be cached along with the + * element markup. If properties include children names, the system + * assumes only children's individual markup is relevant and ignores the + * parent markup. This approach is normally not needed and should be + * adopted only when dealing with very advanced use cases. * - If this element has an array of #post_render_cache functions defined, * or any of its children has (which we would know thanks to the stack * having been updated just before the render caching step), they are diff --git a/core/modules/comment/src/Tests/Views/CommentUserNameTest.php b/core/modules/comment/src/Tests/Views/CommentUserNameTest.php index 2c64bb290b1a..bf06be66f549 100644 --- a/core/modules/comment/src/Tests/Views/CommentUserNameTest.php +++ b/core/modules/comment/src/Tests/Views/CommentUserNameTest.php @@ -152,6 +152,8 @@ public function testUsername() { $account_switcher->switchTo(new AnonymousUserSession()); $executable = Views::getView($view_id); + $executable->storage->invalidateCaches(); + $build = $executable->preview(); $this->setRawContent($renderer->render($build)); diff --git a/core/modules/contact/src/Access/ContactPageAccess.php b/core/modules/contact/src/Access/ContactPageAccess.php index 0d6ffcb3daf6..6b7878462a29 100644 --- a/core/modules/contact/src/Access/ContactPageAccess.php +++ b/core/modules/contact/src/Access/ContactPageAccess.php @@ -62,16 +62,17 @@ public function access(UserInterface $user, AccountInterface $account) { // Anonymous users cannot have contact forms. if ($contact_account->isAnonymous()) { - return AccessResult::forbidden()->cachePerPermissions(); + return AccessResult::forbidden(); } - // Users may not contact themselves. + // Users may not contact themselves by default, hence this requires user + // granularity for caching. + $access = AccessResult::neutral()->cachePerUser(); if ($account->id() == $contact_account->id()) { - return AccessResult::forbidden()->cachePerUser(); + return $access; } // User administrators should always have access to personal contact forms. - $access = AccessResult::neutral()->cachePerPermissions(); $permission_access = AccessResult::allowedIfHasPermission($account, 'administer users'); if ($permission_access->isAllowed()) { return $access->orIf($permission_access); @@ -83,14 +84,12 @@ public function access(UserInterface $user, AccountInterface $account) { return $access; } - // Load preference of the requested user. + // Forbid access if the requested user has disabled their contact form. $account_data = $this->userData->get('contact', $contact_account->id(), 'enabled'); - if (isset($account_data)) { - // Forbid access if the requested user has disabled their contact form. - if (empty($account_data)) { - return $access; - } + if (isset($account_data) && !$account_data) { + return $access; } + // If the requested user did not save a preference yet, deny access if the // configured default is disabled. $contact_settings = $this->configFactory->get('contact.settings'); diff --git a/core/modules/contact/src/Tests/ContactPersonalTest.php b/core/modules/contact/src/Tests/ContactPersonalTest.php index a586580ea431..102452e1145f 100644 --- a/core/modules/contact/src/Tests/ContactPersonalTest.php +++ b/core/modules/contact/src/Tests/ContactPersonalTest.php @@ -152,13 +152,13 @@ function testPersonalContactAccess() { // Test that anonymous users can access admin user's contact form. $this->drupalGet('user/' . $this->adminUser->id() . '/contact'); $this->assertResponse(200); - $this->assertCacheContext('user.permissions'); + $this->assertCacheContext('user'); // Revoke the personal contact permission for the anonymous user. user_role_revoke_permissions(RoleInterface::ANONYMOUS_ID, array('access user contact forms')); $this->drupalGet('user/' . $this->contactUser->id() . '/contact'); $this->assertResponse(403); - $this->assertCacheContext('user.permissions'); + $this->assertCacheContext('user'); $this->drupalGet('user/' . $this->adminUser->id() . '/contact'); $this->assertResponse(403); diff --git a/core/modules/contact/src/Tests/Views/ContactLinkTest.php b/core/modules/contact/src/Tests/Views/ContactLinkTest.php index 320deb073b52..4c82d1878d4a 100644 --- a/core/modules/contact/src/Tests/Views/ContactLinkTest.php +++ b/core/modules/contact/src/Tests/Views/ContactLinkTest.php @@ -7,6 +7,7 @@ namespace Drupal\contact\Tests\Views; +use Drupal\Core\Cache\Cache; use Drupal\views\Tests\ViewTestBase; use Drupal\views\Tests\ViewTestData; use Drupal\user\Entity\User; @@ -84,6 +85,8 @@ public function testContactLink() { // Disable contact link for no_contact. $this->userData->set('contact', $no_contact_account->id(), 'enabled', FALSE); + // @todo Remove cache invalidation in https://www.drupal.org/node/2477903. + Cache::invalidateTags($no_contact_account->getCacheTags()); $this->drupalGet('test-contact-link'); $this->assertContactLinks($accounts, array('root', 'admin')); } diff --git a/core/modules/contact/tests/modules/contact_test_views/test_views/views.view.test_contact_link.yml b/core/modules/contact/tests/modules/contact_test_views/test_views/views.view.test_contact_link.yml index 28b74c84eeda..339f4ff09461 100644 --- a/core/modules/contact/tests/modules/contact_test_views/test_views/views.view.test_contact_link.yml +++ b/core/modules/contact/tests/modules/contact_test_views/test_views/views.view.test_contact_link.yml @@ -3,12 +3,13 @@ status: true dependencies: module: - contact + - user id: test_contact_link label: test_contact_link module: views description: '' tag: '' -base_table: users +base_table: users_field_data base_field: uid core: 8.x display: @@ -69,7 +70,7 @@ display: fields: name: id: name - table: users + table: users_field_data field: name label: '' alter: @@ -111,7 +112,7 @@ display: filters: status: value: true - table: users + table: users_field_data field: status id: status expose: @@ -127,10 +128,22 @@ display: empty: { } relationships: { } arguments: { } + display_extenders: { } + cache_metadata: + contexts: + - 'languages:language_content' + - 'languages:language_interface' + cacheable: false page_1: display_plugin: page id: page_1 display_title: Page position: 1 display_options: + display_extenders: { } path: test-contact-link + cache_metadata: + contexts: + - 'languages:language_content' + - 'languages:language_interface' + cacheable: false diff --git a/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php b/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php index e934e0f5bab5..c7f37975d4e2 100644 --- a/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php +++ b/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php @@ -100,6 +100,7 @@ public function testIntegration() { // Disable replacing variables and check that the tokens aren't replaced. $view->destroy(); + $view->storage->invalidateCaches(); $view->initHandlers(); $this->executeView($view); $view->initStyle(); diff --git a/core/modules/field/src/Tests/Views/HandlerFieldFieldTest.php b/core/modules/field/src/Tests/Views/HandlerFieldFieldTest.php index 415b831c796e..f3be4294713b 100644 --- a/core/modules/field/src/Tests/Views/HandlerFieldFieldTest.php +++ b/core/modules/field/src/Tests/Views/HandlerFieldFieldTest.php @@ -117,6 +117,7 @@ protected function setUp() { * The view to add field data to. */ protected function prepareView(ViewExecutable $view) { + $view->storage->invalidateCaches(); $view->initDisplay(); foreach ($this->fieldStorages as $field_storage) { $field_name = $field_storage->getName(); diff --git a/core/modules/simpletest/src/UserCreationTrait.php b/core/modules/simpletest/src/UserCreationTrait.php new file mode 100644 index 000000000000..92262cf4d810 --- /dev/null +++ b/core/modules/simpletest/src/UserCreationTrait.php @@ -0,0 +1,217 @@ +<?php + +/** + * @file + * Contains \Drupal\simpletest\UserCreationTrait. + */ + +namespace Drupal\simpletest; + +use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Session\AccountInterface; +use Drupal\user\Entity\Role; +use Drupal\user\Entity\User; +use Drupal\user\RoleInterface; + +/** + * Provides methods to create additional test users and switch the currently + * logged in one. + * + * This trait is meant to be used only by test classes extending + * \Drupal\simpletest\TestBase. + */ +trait UserCreationTrait { + + /** + * Switch the current logged in user. + * + * @param \Drupal\Core\Session\AccountInterface $account + * The user account object. + */ + protected function setCurrentUser(AccountInterface $account) { + \Drupal::currentUser()->setAccount($account); + } + + /** + * Create a user with a given set of permissions. + * + * @param array $permissions + * Array of permission names to assign to user. Note that the user always + * has the default permissions derived from the "authenticated users" role. + * @param string $name + * The user name. + * @param bool $admin + * (optional) Whether the user should be an administrator + * with all the available permissions. + * + * @return \Drupal\user\Entity\User|false + * A fully loaded user object with pass_raw property, or FALSE if account + * creation fails. + */ + protected function createUser(array $permissions = array(), $name = NULL, $admin = FALSE) { + // Create a role with the given permission set, if any. + $rid = FALSE; + if ($permissions) { + $rid = $this->createRole($permissions); + if (!$rid) { + return FALSE; + } + } + + // Create a user assigned to that role. + $edit = array(); + $edit['name'] = !empty($name) ? $name : $this->randomMachineName(); + $edit['mail'] = $edit['name'] . '@example.com'; + $edit['pass'] = user_password(); + $edit['status'] = 1; + if ($rid) { + $edit['roles'] = array($rid); + } + + if ($admin) { + $edit['roles'][] = $this->createAdminRole(); + } + + $account = User::create($edit); + $account->save(); + + $this->assertTrue($account->id(), SafeMarkup::format('User created with name %name and pass %pass', array('%name' => $edit['name'], '%pass' => $edit['pass'])), 'User login'); + if (!$account->id()) { + return FALSE; + } + + // Add the raw password so that we can log in as this user. + $account->pass_raw = $edit['pass']; + return $account; + } + + /** + * Creates an administrative role. + * + * @param string $rid + * (optional) The role ID (machine name). Defaults to a random name. + * @param string $name + * (optional) The label for the role. Defaults to a random string. + * @param integer $weight + * (optional) The weight for the role. Defaults NULL so that entity_create() + * sets the weight to maximum + 1. + * + * @return string + * Role ID of newly created role, or FALSE if role creation failed. + */ + protected function createAdminRole($rid = NULL, $name = NULL, $weight = NULL) { + $rid = $this->createRole([], $rid, $name, $weight); + if ($rid) { + /** @var \Drupal\user\RoleInterface $role */ + $role = Role::load($rid); + $role->setIsAdmin(TRUE); + $role->save(); + } + return $rid; + } + + /** + * Creates a role with specified permissions. + * + * @param array $permissions + * Array of permission names to assign to role. + * @param string $rid + * (optional) The role ID (machine name). Defaults to a random name. + * @param string $name + * (optional) The label for the role. Defaults to a random string. + * @param integer $weight + * (optional) The weight for the role. Defaults NULL so that entity_create() + * sets the weight to maximum + 1. + * + * @return string + * Role ID of newly created role, or FALSE if role creation failed. + */ + protected function createRole(array $permissions, $rid = NULL, $name = NULL, $weight = NULL) { + // Generate a random, lowercase machine name if none was passed. + if (!isset($rid)) { + $rid = strtolower($this->randomMachineName(8)); + } + // Generate a random label. + if (!isset($name)) { + // In the role UI role names are trimmed and random string can start or + // end with a space. + $name = trim($this->randomString(8)); + } + + // Check the all the permissions strings are valid. + if (!$this->checkPermissions($permissions)) { + return FALSE; + } + + // Create new role. + $role = Role::create(array( + 'id' => $rid, + 'label' => $name, + )); + if (isset($weight)) { + $role->set('weight', $weight); + } + $result = $role->save(); + + $this->assertIdentical($result, SAVED_NEW, SafeMarkup::format('Created role ID @rid with name @name.', array( + '@name' => var_export($role->label(), TRUE), + '@rid' => var_export($role->id(), TRUE), + )), 'Role'); + + if ($result === SAVED_NEW) { + // Grant the specified permissions to the role, if any. + if (!empty($permissions)) { + $this->grantPermissions($role, $permissions); + $assigned_permissions = Role::load($role->id())->getPermissions(); + $missing_permissions = array_diff($permissions, $assigned_permissions); + if (!$missing_permissions) { + $this->pass(SafeMarkup::format('Created permissions: @perms', array('@perms' => implode(', ', $permissions))), 'Role'); + } + else { + $this->fail(SafeMarkup::format('Failed to create permissions: @perms', array('@perms' => implode(', ', $missing_permissions))), 'Role'); + } + } + return $role->id(); + } + else { + return FALSE; + } + } + + /** + * Checks whether a given list of permission names is valid. + * + * @param array $permissions + * The permission names to check. + * + * @return bool + * TRUE if the permissions are valid, FALSE otherwise. + */ + protected function checkPermissions(array $permissions) { + $available = array_keys(\Drupal::service('user.permissions')->getPermissions()); + $valid = TRUE; + foreach ($permissions as $permission) { + if (!in_array($permission, $available)) { + $this->fail(SafeMarkup::format('Invalid permission %permission.', array('%permission' => $permission)), 'Role'); + $valid = FALSE; + } + } + return $valid; + } + + /** + * Grant permissions to a user role. + * + * @param \Drupal\user\RoleInterface $role + * The ID of a user role to alter. + * @param array $permissions + * (optional) A list of permission names to grant. + */ + protected function grantPermissions(RoleInterface $role, array $permissions) { + foreach ($permissions as $permission) { + $role->grantPermission($permission); + } + $role->trustData()->save(); + } + +} diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index fe96f9fdbffa..50f95331db1f 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -7,32 +7,26 @@ namespace Drupal\simpletest; +use Drupal\block\Entity\Block; use Drupal\Component\FileCache\FileCacheFactory; use Drupal\Component\Serialization\Json; use Drupal\Component\Serialization\Yaml; -use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Html; use Drupal\Component\Utility\NestedArray; -use Drupal\Core\Cache\Cache; use Drupal\Component\Utility\SafeMarkup; -use Drupal\Core\DependencyInjection\YamlFileLoader; -use Drupal\Core\DrupalKernel; +use Drupal\Core\Cache\Cache; use Drupal\Core\Database\Database; -use Drupal\Core\Database\ConnectionNotDefinedException; +use Drupal\Core\DrupalKernel; use Drupal\Core\Entity\EntityInterface; -use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Render\Element; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\AnonymousUserSession; use Drupal\Core\Session\UserSession; use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\PublicStream; -use Drupal\Core\Datetime\DrupalDateTime; -use Drupal\block\Entity\Block; -use Drupal\node\Entity\NodeType; use Drupal\Core\Url; +use Drupal\node\Entity\NodeType; use Symfony\Component\HttpFoundation\Request; -use Drupal\user\Entity\Role; /** * Test case for typical Drupal tests. @@ -43,6 +37,12 @@ abstract class WebTestBase extends TestBase { use AssertContentTrait; + use UserCreationTrait { + createUser as drupalCreateUser; + createRole as drupalCreateRole; + createAdminRole as drupalCreateAdminRole; + } + /** * The profile to install as a basis for testing. * @@ -520,141 +520,6 @@ protected function drupalCompareFiles($file1, $file2) { } } - /** - * Create a user with a given set of permissions. - * - * @param array $permissions - * Array of permission names to assign to user. Note that the user always - * has the default permissions derived from the "authenticated users" role. - * @param string $name - * The user name. - * - * @return \Drupal\user\Entity\User|false - * A fully loaded user object with pass_raw property, or FALSE if account - * creation fails. - */ - protected function drupalCreateUser(array $permissions = array(), $name = NULL) { - // Create a role with the given permission set, if any. - $rid = FALSE; - if ($permissions) { - $rid = $this->drupalCreateRole($permissions); - if (!$rid) { - return FALSE; - } - } - - // Create a user assigned to that role. - $edit = array(); - $edit['name'] = !empty($name) ? $name : $this->randomMachineName(); - $edit['mail'] = $edit['name'] . '@example.com'; - $edit['pass'] = user_password(); - $edit['status'] = 1; - if ($rid) { - $edit['roles'] = array($rid); - } - - $account = entity_create('user', $edit); - $account->save(); - - $this->assertTrue($account->id(), SafeMarkup::format('User created with name %name and pass %pass', array('%name' => $edit['name'], '%pass' => $edit['pass'])), 'User login'); - if (!$account->id()) { - return FALSE; - } - - // Add the raw password so that we can log in as this user. - $account->pass_raw = $edit['pass']; - return $account; - } - - /** - * Creates a role with specified permissions. - * - * @param array $permissions - * Array of permission names to assign to role. - * @param string $rid - * (optional) The role ID (machine name). Defaults to a random name. - * @param string $name - * (optional) The label for the role. Defaults to a random string. - * @param integer $weight - * (optional) The weight for the role. Defaults NULL so that entity_create() - * sets the weight to maximum + 1. - * - * @return string - * Role ID of newly created role, or FALSE if role creation failed. - */ - protected function drupalCreateRole(array $permissions, $rid = NULL, $name = NULL, $weight = NULL) { - // Generate a random, lowercase machine name if none was passed. - if (!isset($rid)) { - $rid = strtolower($this->randomMachineName(8)); - } - // Generate a random label. - if (!isset($name)) { - // In the role UI role names are trimmed and random string can start or - // end with a space. - $name = trim($this->randomString(8)); - } - - // Check the all the permissions strings are valid. - if (!$this->checkPermissions($permissions)) { - return FALSE; - } - - // Create new role. - $role = entity_create('user_role', array( - 'id' => $rid, - 'label' => $name, - )); - if (!is_null($weight)) { - $role->set('weight', $weight); - } - $result = $role->save(); - - $this->assertIdentical($result, SAVED_NEW, SafeMarkup::format('Created role ID @rid with name @name.', array( - '@name' => var_export($role->label(), TRUE), - '@rid' => var_export($role->id(), TRUE), - )), 'Role'); - - if ($result === SAVED_NEW) { - // Grant the specified permissions to the role, if any. - if (!empty($permissions)) { - user_role_grant_permissions($role->id(), $permissions); - $assigned_permissions = Role::load($role->id())->getPermissions(); - $missing_permissions = array_diff($permissions, $assigned_permissions); - if (!$missing_permissions) { - $this->pass(SafeMarkup::format('Created permissions: @perms', array('@perms' => implode(', ', $permissions))), 'Role'); - } - else { - $this->fail(SafeMarkup::format('Failed to create permissions: @perms', array('@perms' => implode(', ', $missing_permissions))), 'Role'); - } - } - return $role->id(); - } - else { - return FALSE; - } - } - - /** - * Checks whether a given list of permission names is valid. - * - * @param array $permissions - * The permission names to check. - * - * @return bool - * TRUE if the permissions are valid, FALSE otherwise. - */ - protected function checkPermissions(array $permissions) { - $available = array_keys(\Drupal::service('user.permissions')->getPermissions()); - $valid = TRUE; - foreach ($permissions as $permission) { - if (!in_array($permission, $available)) { - $this->fail(SafeMarkup::format('Invalid permission %permission.', array('%permission' => $permission)), 'Role'); - $valid = FALSE; - } - } - return $valid; - } - /** * Log in a user with the internal browser. * diff --git a/core/modules/system/src/Plugin/views/field/BulkForm.php b/core/modules/system/src/Plugin/views/field/BulkForm.php index 6c72c6fd9ada..5587ffa838e0 100644 --- a/core/modules/system/src/Plugin/views/field/BulkForm.php +++ b/core/modules/system/src/Plugin/views/field/BulkForm.php @@ -12,11 +12,11 @@ use Drupal\Core\Routing\RedirectDestinationTrait; use Drupal\views\Plugin\views\display\DisplayPluginBase; use Drupal\views\Plugin\views\field\FieldPluginBase; +use Drupal\views\Plugin\views\field\UncacheableFieldHandlerTrait; use Drupal\views\Plugin\views\style\Table; use Drupal\views\ResultRow; use Drupal\views\ViewExecutable; use Symfony\Component\DependencyInjection\ContainerInterface; -use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; /** * Defines a actions-based bulk operation form element. @@ -26,6 +26,7 @@ class BulkForm extends FieldPluginBase { use RedirectDestinationTrait; + use UncacheableFieldHandlerTrait; /** * The action storage. @@ -134,13 +135,6 @@ public function validateOptionsForm(&$form, FormStateInterface $form_state) { $form_state->setValue(array('options', 'selected_actions'), array_values(array_filter($selected_actions))); } - /** - * {@inheritdoc} - */ - public function render(ResultRow $values) { - return '<!--form-item-' . $this->options['id'] . '--' . $values->index . '-->'; - } - /** * {@inheritdoc} */ @@ -157,6 +151,12 @@ public function preRender(&$values) { } } + /** + * {@inheritdoc} + */ + public function getValue(ResultRow $row, $field = NULL) { + return '<!--form-item-' . $this->options['id'] . '--' . $row->index . '-->'; + } /** * Form constructor for the bulk form. diff --git a/core/modules/system/src/Tests/Entity/EntityOperationsTest.php b/core/modules/system/src/Tests/Entity/EntityOperationsTest.php index fbd77dfdd4d0..c7aeb7c3c929 100644 --- a/core/modules/system/src/Tests/Entity/EntityOperationsTest.php +++ b/core/modules/system/src/Tests/Entity/EntityOperationsTest.php @@ -48,13 +48,14 @@ public function testEntityOperationAlter() { /** * {@inheritdoc} */ - protected function drupalCreateRole(array $permissions, $rid = NULL, $name = NULL, $weight = NULL) { + protected function createRole(array $permissions, $rid = NULL, $name = NULL, $weight = NULL) { // WebTestBase::drupalCreateRole() by default uses random strings which may // include HTML entities for the entity label. Since in this test the entity // label is used to generate a link, and AssertContentTrait::assertLink() is // not designed to deal with links potentially containing HTML entities this // causes random failures. Use a random HTML safe string instead. $name = $name ?: $this->randomMachineName(); - return parent::drupalCreateRole($permissions, $rid, $name, $weight); + return parent::createRole($permissions, $rid, $name, $weight); } + } diff --git a/core/modules/taxonomy/src/Tests/Views/TermNameFieldTest.php b/core/modules/taxonomy/src/Tests/Views/TermNameFieldTest.php index 6a2d65c509d3..35aa4485f9cf 100644 --- a/core/modules/taxonomy/src/Tests/Views/TermNameFieldTest.php +++ b/core/modules/taxonomy/src/Tests/Views/TermNameFieldTest.php @@ -43,12 +43,11 @@ public function testTermNameField() { $view = Views::getView('test_taxonomy_term_name'); $display =& $view->storage->getDisplay('default'); $display['display_options']['fields']['name']['convert_spaces'] = TRUE; - + $view->storage->invalidateCaches(); $this->executeView($view); $this->assertEqual(str_replace(' ', '-', $this->term1->getName()), $view->getStyle()->getField(0, 'name')); $this->assertEqual($this->term2->getName(), $view->getStyle()->getField(1, 'name')); - } } diff --git a/core/modules/views/src/Entity/View.php b/core/modules/views/src/Entity/View.php index b7188c5b71cc..6708d5da76c4 100644 --- a/core/modules/views/src/Entity/View.php +++ b/core/modules/views/src/Entity/View.php @@ -344,6 +344,7 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) { // @todo Remove if views implements a view_builder controller. views_invalidate_cache(); + $this->invalidateCaches(); // Rebuild the router if this is a new view, or it's status changed. if (!isset($this->original) || ($this->status() != $this->original->status())) { @@ -459,4 +460,13 @@ public function __sleep() { return $keys; } + /** + * Invalidates cache tags. + */ + public function invalidateCaches() { + // Invalidate cache tags for cached rows. + $tags = $this->getCacheTags(); + \Drupal::service('cache_tags.invalidator')->invalidateTags($tags); + } + } diff --git a/core/modules/views/src/Plugin/views/cache/CachePluginBase.php b/core/modules/views/src/Plugin/views/cache/CachePluginBase.php index a92955789fff..bf5eb2a89936 100644 --- a/core/modules/views/src/Plugin/views/cache/CachePluginBase.php +++ b/core/modules/views/src/Plugin/views/cache/CachePluginBase.php @@ -12,6 +12,7 @@ use Drupal\Core\Render\RendererInterface; use Drupal\views\Plugin\views\PluginBase; use Drupal\Core\Database\Query\Select; +use Drupal\views\ResultRow; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -425,6 +426,74 @@ protected function prepareViewResult(array $result) { public function alterCacheMetadata(&$is_cacheable, array &$cache_contexts) { } + /** + * Returns the row cache tags. + * + * @param ResultRow $row + * A result row. + * + * @return string[] + * The row cache tags. + */ + public function getRowCacheTags(ResultRow $row) { + $tags = !empty($row->_entity) ? $row->_entity->getCacheTags() : []; + + if (!empty($row->_relationship_entities)) { + foreach ($row->_relationship_entities as $entity) { + $tags = Cache::mergeTags($tags, $entity->getCacheTags()); + } + } + + return $tags; + } + + /** + * Returns the row cache keys. + * + * @param \Drupal\views\ResultRow $row + * A result row. + * + * @return string[] + * The row cache keys. + */ + public function getRowCacheKeys(ResultRow $row) { + return [ + 'views', + 'fields', + $this->view->id(), + $this->view->current_display, + $this->getRowId($row), + ]; + } + + /** + * Returns a unique identifier for the specified row. + * + * @param \Drupal\views\ResultRow $row + * A result row. + * + * @return string + * The row identifier. + */ + public function getRowId(ResultRow $row) { + // Here we compute a unique identifier for the row by computing the hash of + // its data. We exclude the current index, since the same row could have a + // different result index depending on the user permissions. We exclude also + // entity data, since serializing entity objects is very expensive. Instead + // we include entity cache tags, which are enough to identify all the + // entities associated with the row. + $row_data = array_diff_key((array) $row, array_flip(['index', '_entity', '_relationship_entities'])) + $this->getRowCacheTags($row); + + // This ensures that we get a unique identifier taking field handler access + // into account: users having access to different sets of fields will get + // different row identifiers. + $field_ids = array_keys($this->view->field); + $row_data += array_flip($field_ids); + + // Finally we compute a hash of row data and return it as row identifier. + return hash('sha256', serialize($row_data)); + } + } /** diff --git a/core/modules/views/src/Plugin/views/field/Counter.php b/core/modules/views/src/Plugin/views/field/Counter.php index a524222fc5a6..0dc9884ea96a 100644 --- a/core/modules/views/src/Plugin/views/field/Counter.php +++ b/core/modules/views/src/Plugin/views/field/Counter.php @@ -19,6 +19,8 @@ */ class Counter extends FieldPluginBase { + use UncacheableFieldHandlerTrait; + /** * {@inheritdoc} */ diff --git a/core/modules/views/src/Plugin/views/field/EntityLink.php b/core/modules/views/src/Plugin/views/field/EntityLink.php index 4d07c95bab33..4a1bc8badf4e 100644 --- a/core/modules/views/src/Plugin/views/field/EntityLink.php +++ b/core/modules/views/src/Plugin/views/field/EntityLink.php @@ -21,8 +21,8 @@ class EntityLink extends LinkBase { /** * {@inheritdoc} */ - public function render(ResultRow $values) { - return $this->getEntity($values) ? parent::render($values) : ''; + public function render(ResultRow $row) { + return $this->getEntity($row) ? parent::render($row) : []; } /** diff --git a/core/modules/views/src/Plugin/views/field/FieldHandlerInterface.php b/core/modules/views/src/Plugin/views/field/FieldHandlerInterface.php index b85676e9fb83..9cb1684a8eda 100644 --- a/core/modules/views/src/Plugin/views/field/FieldHandlerInterface.php +++ b/core/modules/views/src/Plugin/views/field/FieldHandlerInterface.php @@ -173,6 +173,26 @@ public function preRender(&$values); */ public function render(ResultRow $values); + /** + * Runs after every field has been rendered. + * + * This is meant to be used mainly to deal with field handlers whose output + * cannot be cached at row level but can be cached at display level. The + * typical example is the row counter. For completely uncacheable field output + * #post_render_cache should be used. + * + * @param \Drupal\views\ResultRow $row + * An array of all ResultRow objects returned from the query. + * @param $output + * The field rendered output. + * + * @return string[] + * An associative array of post-render token values keyed by placeholder. + * + * @see \Drupal\views\Plugin\views\field\UncacheableFieldHandlerTrait + */ + public function postRender(ResultRow $row, $output); + /** * Renders a field using advanced settings. * diff --git a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php index ecb5270861ba..fc40295938a3 100644 --- a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php @@ -1107,6 +1107,16 @@ public function render(ResultRow $values) { return $this->sanitizeValue($value); } + /** + * {@inheritdoc} + */ + public function postRender(ResultRow $row, $output) { + // Make sure the last rendered value is available also when this is + // retrieved from cache. + $this->last_render = $output; + return []; + } + /** * {@inheritdoc} */ @@ -1518,11 +1528,14 @@ public function getRenderTokens($item) { // Now add replacements for our fields. foreach ($this->displayHandler->getHandlers('field') as $field => $handler) { + /** @var static $handler */ + $placeholder = $handler->getFieldTokenPlaceholder(); + if (isset($handler->last_render)) { - $tokens["{{ $field }}"] = $handler->last_render; + $tokens[$placeholder] = $handler->last_render; } else { - $tokens["{{ $field }}"] = ''; + $tokens[$placeholder] = ''; } // We only use fields up to (and including) this one. @@ -1541,6 +1554,16 @@ public function getRenderTokens($item) { return $tokens; } + /** + * Returns a token placeholder for the current field. + * + * @return string + * A token placeholder. + */ + protected function getFieldTokenPlaceholder() { + return '{{ ' . $this->options['id'] . ' }}'; + } + /** * Recursive function to add replacements for nested query string parameters. * diff --git a/core/modules/views/src/Plugin/views/field/LinkBase.php b/core/modules/views/src/Plugin/views/field/LinkBase.php index cac7e825fbf8..abc505b2a2d0 100644 --- a/core/modules/views/src/Plugin/views/field/LinkBase.php +++ b/core/modules/views/src/Plugin/views/field/LinkBase.php @@ -8,7 +8,10 @@ namespace Drupal\views\Plugin\views\field; use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Access\AccessResultInterface; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Render\BubbleableMetadata; use Drupal\Core\Routing\RedirectDestinationTrait; use Drupal\views\ResultRow; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -102,6 +105,7 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) { parent::buildOptionsForm($form, $form_state); // The path is set by ::renderLink() so we do not allow to set it. + $form['alter'] += ['path' => [], 'query' => [], 'external' => []]; $form['alter']['path'] += ['#access' => FALSE]; $form['alter']['query'] += ['#access' => FALSE]; $form['alter']['external'] += ['#access' => FALSE]; @@ -124,8 +128,11 @@ public function query() { /** * {@inheritdoc} */ - public function render(ResultRow $values) { - return $this->checkUrlAccess($values)->isAllowed() ? $this->renderLink($values) : ''; + public function render(ResultRow $row) { + $access = $this->checkUrlAccess($row); + $build = ['#markup' => $access->isAllowed() ? $this->renderLink($row) : '']; + BubbleableMetadata::createFromObject($access)->applyTo($build); + return $build; } /** diff --git a/core/modules/views/src/Plugin/views/field/UncacheableFieldHandlerTrait.php b/core/modules/views/src/Plugin/views/field/UncacheableFieldHandlerTrait.php new file mode 100644 index 000000000000..c0aa8ee60ef8 --- /dev/null +++ b/core/modules/views/src/Plugin/views/field/UncacheableFieldHandlerTrait.php @@ -0,0 +1,65 @@ +<?php + +/** + * @file + * Contains \Drupal\views\Plugin\views\field\UncacheableFieldHandlerTrait. + */ + +namespace Drupal\views\Plugin\views\field; + +use Drupal\views\ResultRow; + +/** + * Trait encapsulating the logic for uncacheable field handlers. + */ +trait UncacheableFieldHandlerTrait { + + /** + * {@inheritdoc} + * + * @see \Drupal\views\Plugin\views\Field\FieldHandlerInterface::render() + */ + public function render(ResultRow $row) { + return $this->getFieldTokenPlaceholder(); + } + + /** + * {@inheritdoc} + * + * @see \Drupal\views\Plugin\views\Field\FieldHandlerInterface::postRender() + */ + public function postRender(ResultRow $row, $output) { + $placeholder = $this->getFieldTokenPlaceholder(); + $value = $this->doRender($row); + $this->last_render = str_replace($placeholder, $value, $output); + return [$placeholder => $value]; + } + + /** + * {@inheritdoc} + * + * @see \Drupal\views\Plugin\views\Field\FieldPluginBase::getFieldTokenPlaceholder() + */ + abstract protected function getFieldTokenPlaceholder(); + + /** + * Actually renders the field markup. + * + * @param \Drupal\views\ResultRow $row + * A result row. + * + * @return string + * The field markup. + */ + protected function doRender(ResultRow $row) { + return $this->getValue($row); + } + + /** + * {@inheritdoc} + * + * @see \Drupal\views\Plugin\views\Field\FieldHandlerInterface::getValue() + */ + abstract protected function getValue(ResultRow $row, $field = NULL); + +} diff --git a/core/modules/views/src/Plugin/views/style/StylePluginBase.php b/core/modules/views/src/Plugin/views/style/StylePluginBase.php index c07a5b8f3a79..8fa46177be5b 100644 --- a/core/modules/views/src/Plugin/views/style/StylePluginBase.php +++ b/core/modules/views/src/Plugin/views/style/StylePluginBase.php @@ -8,9 +8,11 @@ namespace Drupal\views\Plugin\views\style; use Drupal\Component\Utility\Html; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Form\FormStateInterface; -use Drupal\views\Plugin\views\PluginBase; +use Drupal\Core\Render\Element; use Drupal\views\Plugin\views\display\DisplayPluginBase; +use Drupal\views\Plugin\views\PluginBase; use Drupal\views\Plugin\views\wizard\WizardInterface; use Drupal\views\ViewExecutable; @@ -637,26 +639,97 @@ protected function renderFields(array $result) { } if (!isset($this->rendered_fields)) { - $this->rendered_fields = array(); + $this->rendered_fields = []; $this->view->row_index = 0; - $keys = array_keys($this->view->field); + $field_ids = array_keys($this->view->field); + + // Only tokens relating to field handlers preceding the one we invoke + // ::getRenderTokens() on are returned, so here we need to pick the last + // available field handler. + $render_tokens_field_id = end($field_ids); // If all fields have a field::access FALSE there might be no fields, so // there is no reason to execute this code. - if (!empty($keys)) { - foreach ($result as $count => $row) { - $this->view->row_index = $count; - foreach ($keys as $id) { - $this->rendered_fields[$count][$id] = $this->view->field[$id]->theme($row); + if (!empty($field_ids)) { + $renderer = $this->getRenderer(); + /** @var \Drupal\views\Plugin\views\cache\CachePluginBase $cache_plugin */ + $cache_plugin = $this->view->display_handler->getPlugin('cache'); + + /** @var \Drupal\views\ResultRow $row */ + foreach ($result as $index => $row) { + $this->view->row_index = $index; + + // Here we implement render caching for result rows. Since we never + // build a render array for single rows, given that style templates + // need individual field markup to support proper theming, we build + // a raw render array containing all field render arrays and cache it. + // This allows us to cache the markup of the various children, that is + // individual fields, which is then available for style template + // preprocess functions, later in the rendering workflow. + // @todo Fetch all the available cached row items in one single cache + // get operation, once https://www.drupal.org/node/2453945 is fixed. + $data = [ + '#pre_render' => [[$this, 'elementPreRenderRow']], + '#row' => $row, + '#cache' => [ + 'keys' => $cache_plugin->getRowCacheKeys($row), + 'tags' => $cache_plugin->getRowCacheTags($row), + ], + '#cache_properties' => $field_ids, + ]; + $renderer->addCacheableDependency($data, $this->view->storage); + $renderer->render($data); + + // Extract field output from the render array and post process it. + $fields = $this->view->field; + $rendered_fields = &$this->rendered_fields[$index]; + $post_render_tokens = []; + foreach ($field_ids as $id) { + $rendered_fields[$id] = $data[$id]['#markup']; + $tokens = $fields[$id]->postRender($row, $rendered_fields[$id]); + if ($tokens) { + $post_render_tokens += $tokens; + } } - $this->rowTokens[$count] = $this->view->field[$id]->getRenderTokens(array()); + // Populate row tokens. + $this->rowTokens[$index] = $this->view->field[$render_tokens_field_id]->getRenderTokens([]); + + // Replace post-render tokens. + if ($post_render_tokens) { + $placeholders = array_keys($post_render_tokens); + $values = array_values($post_render_tokens); + foreach ($this->rendered_fields[$index] as &$rendered_field) { + $rendered_field = str_replace($placeholders, $values, $rendered_field); + SafeMarkup::set($rendered_field); + } + } } } + unset($this->view->row_index); } } + /** + * #pre_render callback for view row field rendering. + * + * @see self::render() + * + * @param array $data + * The element to #pre_render + * + * @return array + * The processed element. + */ + public function elementPreRenderRow(array $data) { + // Render row fields. + foreach ($this->view->field as $id => $field) { + $data[$id] = ['#markup' => $field->theme($data['#row'])]; + } + return $data; + } + /** * Gets a rendered field. * diff --git a/core/modules/views/src/Tests/Entity/RowEntityRenderersTest.php b/core/modules/views/src/Tests/Entity/RowEntityRenderersTest.php index 1d837763c59e..6a8a12d78be0 100644 --- a/core/modules/views/src/Tests/Entity/RowEntityRenderersTest.php +++ b/core/modules/views/src/Tests/Entity/RowEntityRenderersTest.php @@ -211,6 +211,7 @@ protected function checkLanguageRenderers($display, $values) { */ protected function assertTranslations($display, $renderer_id, array $expected, $message = '', $group = 'Other') { $view = Views::getView('test_entity_row_renderers'); + $view->storage->invalidateCaches(); $view->setDisplay($display); $view->getDisplay()->setOption('rendering_language', $renderer_id); $view->preview(); diff --git a/core/modules/views/src/Tests/Handler/FieldCounterTest.php b/core/modules/views/src/Tests/Handler/FieldCounterTest.php index 1290e676845d..fd17ffd902f6 100644 --- a/core/modules/views/src/Tests/Handler/FieldCounterTest.php +++ b/core/modules/views/src/Tests/Handler/FieldCounterTest.php @@ -57,6 +57,7 @@ function testSimple() { $counter = $view->style_plugin->getField(2, 'counter'); $this->assertEqual($counter, 3, format_string('Make sure the expected number (@expected) patches with the rendered number (@counter)', array('@expected' => 3, '@counter' => $counter))); $view->destroy(); + $view->storage->invalidateCaches(); $view->setDisplay(); $rand_start = rand(5, 10); diff --git a/core/modules/views/src/Tests/Handler/FieldEntityLinkTest.php b/core/modules/views/src/Tests/Handler/FieldEntityLinkTest.php index 08c6ae7567c1..9076edf73a3d 100644 --- a/core/modules/views/src/Tests/Handler/FieldEntityLinkTest.php +++ b/core/modules/views/src/Tests/Handler/FieldEntityLinkTest.php @@ -7,11 +7,9 @@ namespace Drupal\views\Tests\Handler; -use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Session\AccountInterface; use Drupal\entity_test\Entity\EntityTest; -use Drupal\user\Entity\Role; -use Drupal\user\Entity\User; +use Drupal\simpletest\UserCreationTrait; use Drupal\views\Tests\ViewUnitTestBase; use Drupal\views\Views; @@ -22,6 +20,8 @@ */ class FieldEntityLinkTest extends ViewUnitTestBase { + use UserCreationTrait; + /** * Views used by this test. * @@ -54,20 +54,11 @@ protected function setUpFixtures() { // Create some test entities. for ($i = 0; $i < 5; $i++) { - EntityTest::create(array( - 'name' => $this->randomString(), - ))->save(); + EntityTest::create(['name' => $this->randomString()])->save(); } - // Create an admin role. - $role = Role::create(['id' => $this->randomMachineName()]); - $role->setIsAdmin(TRUE); - $role->save(); - // Create and admin user. - $this->adminUser = User::create(['name' => $this->randomString()]); - $this->adminUser->addRole($role->id()); - $this->adminUser->save(); + $this->adminUser = $this->createUser([], FALSE, TRUE); } /** diff --git a/core/modules/views/src/Tests/Plugin/RowRenderCacheTest.php b/core/modules/views/src/Tests/Plugin/RowRenderCacheTest.php new file mode 100644 index 000000000000..7c570a6ba8a1 --- /dev/null +++ b/core/modules/views/src/Tests/Plugin/RowRenderCacheTest.php @@ -0,0 +1,178 @@ +<?php + +/** + * @file + * Contains \Drupal\views\Tests\Plugin\RowRenderCacheTest. + */ + +namespace Drupal\views\Tests\Plugin; + +use Drupal\Core\Session\AccountInterface; +use Drupal\node\Entity\Node; +use Drupal\node\Entity\NodeType; +use Drupal\node\NodeInterface; +use Drupal\simpletest\UserCreationTrait; +use Drupal\views\Tests\ViewUnitTestBase; +use Drupal\views\Views; + +/** + * Tests row render caching. + * + * @group views + */ +class RowRenderCacheTest extends ViewUnitTestBase { + + use UserCreationTrait; + + /** + * Modules to enable. + * + * @var array + */ + public static $modules = array('user', 'node'); + + /** + * Views used by this test. + * + * @var array + */ + public static $testViews = array('test_row_render_cache'); + + /** + * An editor user account. + * + * @var \Drupal\user\UserInterface + */ + protected $editorUser; + + /** + * A power user account. + * + * @var \Drupal\user\UserInterface + */ + protected $powerUser; + + /** + * A regular user account. + * + * @var \Drupal\user\UserInterface + */ + protected $regularUser; + + /** + * {@inheritdoc} + */ + protected function setUpFixtures() { + parent::setUpFixtures(); + + $this->installEntitySchema('user'); + $this->installEntitySchema('node'); + $this->installSchema('node', 'node_access'); + + $type = NodeType::create(['type' => 'test']); + $type->save(); + + $this->editorUser = $this->createUser(['bypass node access']); + $this->powerUser = $this->createUser(['access content', 'create test content', 'edit own test content', 'delete own test content']); + $this->regularUser = $this->createUser(['access content']); + + // Create some test entities. + for ($i = 0; $i < 5; $i++) { + Node::create(['title' => 'b' . $this->randomMachineName(), 'type' => 'test'])->save(); + } + + // Create a power user node. + Node::create(['title' => 'z' . $this->randomMachineName(), 'uid' => $this->powerUser->id(), 'type' => 'test'])->save(); + } + + /** + * Test complex field rewriting and uncacheable field handlers. + */ + public function testAdvancedCaching() { + // Test that row field output is actually cached and with the proper cache + // contexts. + $this->doTestRenderedOutput($this->editorUser); + $this->doTestRenderedOutput($this->editorUser, TRUE); + $this->doTestRenderedOutput($this->powerUser); + $this->doTestRenderedOutput($this->powerUser, TRUE); + $this->doTestRenderedOutput($this->regularUser); + $this->doTestRenderedOutput($this->regularUser, TRUE); + + // Alter the result set order and check that counter is still working + // correctly. + $this->doTestRenderedOutput($this->editorUser); + /** @var \Drupal\node\NodeInterface $node */ + $node = Node::load(6); + $node->setTitle('a' . $this->randomMachineName()); + $node->save(); + $this->doTestRenderedOutput($this->editorUser); + } + + /** + * Check whether the rendered output matches expectations. + * + * @param \Drupal\Core\Session\AccountInterface $account + * The user account to tests rendering with. + * @param bool $check_cache + * (optional) Whether explicitly test render cache entries. + */ + protected function doTestRenderedOutput(AccountInterface $account, $check_cache = FALSE) { + $this->setCurrentUser($account); + $view = Views::getView('test_row_render_cache'); + $view->setDisplay(); + $view->preview(); + + /** @var \Drupal\Core\Render\RenderCacheInterface $render_cache */ + $render_cache = $this->container->get('render_cache'); + + /** @var \Drupal\views\Plugin\views\cache\CachePluginBase $cache_plugin */ + $cache_plugin = $view->display_handler->getPlugin('cache'); + + // Retrieve nodes and sort them in alphabetical order to match view results. + $nodes = Node::loadMultiple(); + usort($nodes, function (NodeInterface $a, NodeInterface $b) { + return strcmp($a->label(), $b->label()); + }); + + $index = 0; + foreach ($nodes as $node) { + $nid = $node->id(); + $access = $node->access('update'); + + $counter = $index + 1; + $expected = "$nid: $counter (just in case: $nid)"; + $counter_output = $view->style_plugin->getField($index, 'counter'); + $this->assertEqual($counter_output, $expected); + + $node_url = $node->url(); + $expected = "<a href=\"$node_url\"><span class=\"da-title\">{$node->label()}</span> <span class=\"counter\">$counter_output</span></a>"; + $output = $view->style_plugin->getField($index, 'title'); + $this->assertEqual($output, $expected); + + $expected = $access ? "<a href=\"$node_url/edit?destination=/\" hreflang=\"en\">edit</a>" : ""; + $output = $view->style_plugin->getField($index, 'edit_node'); + $this->assertEqual($output, $expected); + + $expected = $access ? "<a href=\"$node_url/delete?destination=/\" hreflang=\"en\">delete</a>" : ""; + $output = $view->style_plugin->getField($index, 'delete_node'); + $this->assertEqual($output, $expected); + + $expected = $access ? " <div class=\"dropbutton-wrapper\"><div class=\"dropbutton-widget\"><ul class=\"dropbutton\">" . + "<li class=\"edit\"><a href=\"$node_url/edit?destination=/\" hreflang=\"en\">Edit</a></li>" . + "<li class=\"delete\"><a href=\"$node_url/delete?destination=/\" hreflang=\"en\">Delete</a></li>" . + "</ul></div></div>" : ""; + $output = $view->style_plugin->getField($index, 'operations'); + $this->assertEqual($output, $expected); + + if ($check_cache) { + $keys = $cache_plugin->getRowCacheKeys($view->result[$index]); + $user_context = !$account->hasPermission('edit any test content') ? 'user' : 'user.permissions'; + $element = $render_cache->get(['#cache' => ['keys' => $keys, 'contexts' => ['languages:language_interface', 'theme', $user_context]]]); + $this->assertTrue($element); + } + + $index++; + } + } + +} diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml new file mode 100644 index 000000000000..d0d5ebfca46c --- /dev/null +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_row_render_cache.yml @@ -0,0 +1,503 @@ +langcode: en +status: true +dependencies: + config: + - system.menu.tools + module: + - node +id: test_row_render_cache +label: 'Test render cache' +module: views +description: '' +tag: '' +base_table: node_field_data +base_field: nid +core: 8.x +display: + default: + display_plugin: default + id: default + display_title: Master + position: 0 + display_options: + access: + type: none + options: { } + cache: + type: none + options: { } + query: + type: views_query + options: + disable_sql_rewrite: false + distinct: false + replica: false + query_comment: '' + query_tags: { } + exposed_form: + type: basic + options: + submit_button: Apply + 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, 25, 50' + items_per_page_options_all: false + items_per_page_options_all_label: '- All -' + offset: false + offset_label: Offset + tags: + previous: '‹ previous' + next: 'next ›' + first: '« first' + last: 'last »' + quantity: 9 + style: + type: table + options: + grouping: { } + row_class: '' + default_row_class: true + override: true + sticky: false + caption: '' + summary: '' + description: '' + columns: + counter: counter + title: title + edit_node: edit_node + delete_node: delete_node + operations: operations + info: + counter: + sortable: false + default_sort_order: asc + align: '' + separator: '' + empty_column: false + responsive: '' + title: + sortable: true + default_sort_order: asc + align: '' + separator: '' + empty_column: false + responsive: '' + edit_node: + sortable: false + default_sort_order: asc + align: '' + separator: '' + empty_column: false + responsive: '' + delete_node: + sortable: false + default_sort_order: asc + align: '' + separator: '' + empty_column: false + responsive: '' + operations: + sortable: false + default_sort_order: asc + align: '' + separator: '' + empty_column: false + responsive: '' + default: '-1' + empty_table: false + row: + type: fields + options: + inline: { } + separator: '' + hide_empty: false + default_field_elements: true + fields: + nid: + id: nid + table: node + field: nid + relationship: none + group_type: group + admin_label: '' + label: '' + exclude: true + alter: + alter_text: false + text: '' + make_link: false + path: '' + absolute: false + external: false + replace_spaces: false + path_case: none + trim_whitespace: false + alt: '' + rel: '' + link_class: '' + prefix: '' + suffix: '' + target: '' + nl2br: false + max_length: 0 + word_boundary: true + ellipsis: true + more_link: false + more_link_text: '' + more_link_path: '' + strip_tags: false + trim: false + preserve_tags: '' + html: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: false + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_empty: false + empty_zero: false + hide_alter_empty: true + click_sort_column: value + type: number_integer + settings: + thousand_separator: '' + prefix_suffix: true + group_column: value + group_columns: { } + group_rows: true + delta_limit: 0 + delta_offset: 0 + delta_reversed: false + delta_first_last: false + multi_type: separator + separator: ', ' + field_api_classes: false + entity_type: node + entity_field: nid + plugin_id: field + counter: + id: counter + table: views + field: counter + relationship: none + group_type: group + admin_label: '' + label: Index + exclude: false + alter: + alter_text: true + text: '{{ nid }}: {{ counter }} (just in case: {{ nid }})' + make_link: false + path: '' + absolute: false + external: false + replace_spaces: false + path_case: none + trim_whitespace: false + alt: '' + rel: '' + link_class: '' + prefix: '' + suffix: '' + target: '' + nl2br: false + max_length: 0 + word_boundary: true + ellipsis: true + more_link: false + more_link_text: '' + more_link_path: '' + strip_tags: false + trim: false + preserve_tags: '' + html: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: false + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_empty: false + empty_zero: false + hide_alter_empty: true + counter_start: 1 + plugin_id: counter + title: + id: title + table: node_field_data + field: title + relationship: none + group_type: group + admin_label: '' + label: Title + exclude: false + alter: + alter_text: true + text: '<span class="da-title">{{ title }}</span> <span class="counter">{{ counter }}</span>' + make_link: true + path: 'node/{{ nid }}' + absolute: false + external: false + replace_spaces: false + path_case: none + trim_whitespace: false + alt: '' + rel: '' + link_class: '' + prefix: '' + suffix: '' + target: '' + nl2br: false + max_length: 0 + word_boundary: false + ellipsis: false + more_link: false + more_link_text: '' + more_link_path: '' + strip_tags: false + trim: false + preserve_tags: '' + html: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: true + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_empty: false + empty_zero: false + hide_alter_empty: true + click_sort_column: value + type: string + settings: + link_to_entity: false + group_column: value + group_columns: { } + group_rows: true + delta_limit: 0 + delta_offset: 0 + delta_reversed: false + delta_first_last: false + multi_type: separator + separator: ', ' + field_api_classes: false + entity_type: node + entity_field: title + plugin_id: field + edit_node: + id: edit_node + table: node + field: edit_node + relationship: none + group_type: group + admin_label: '' + label: '' + exclude: false + alter: + alter_text: false + text: '' + make_link: false + path: '' + absolute: false + external: false + replace_spaces: false + path_case: none + trim_whitespace: false + alt: '' + rel: '' + link_class: '' + prefix: '' + suffix: '' + target: '' + nl2br: false + max_length: 0 + word_boundary: true + ellipsis: true + more_link: false + more_link_text: '' + more_link_path: '' + strip_tags: false + trim: false + preserve_tags: '' + html: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: false + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_empty: false + empty_zero: false + hide_alter_empty: true + text: '' + entity_type: node + plugin_id: entity_link_edit + delete_node: + id: delete_node + table: node + field: delete_node + relationship: none + group_type: group + admin_label: '' + label: '' + exclude: false + alter: + alter_text: false + text: '' + make_link: false + path: '' + absolute: false + external: false + replace_spaces: false + path_case: none + trim_whitespace: false + alt: '' + rel: '' + link_class: '' + prefix: '' + suffix: '' + target: '' + nl2br: false + max_length: 0 + word_boundary: true + ellipsis: true + more_link: false + more_link_text: '' + more_link_path: '' + strip_tags: false + trim: false + preserve_tags: '' + html: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: false + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_empty: false + empty_zero: false + hide_alter_empty: true + text: '' + entity_type: node + plugin_id: entity_link_delete + operations: + id: operations + table: node + field: operations + relationship: none + group_type: group + admin_label: '' + label: Operations + exclude: false + alter: + alter_text: false + text: '' + make_link: false + path: '' + absolute: false + external: false + replace_spaces: false + path_case: none + trim_whitespace: false + alt: '' + rel: '' + link_class: '' + prefix: '' + suffix: '' + target: '' + nl2br: false + max_length: 0 + word_boundary: true + ellipsis: true + more_link: false + more_link_text: '' + more_link_path: '' + strip_tags: false + trim: false + preserve_tags: '' + html: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: true + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_empty: false + empty_zero: false + hide_alter_empty: true + destination: true + entity_type: node + plugin_id: entity_operations + filters: + status: + value: true + table: node_field_data + field: status + plugin_id: boolean + entity_type: node + entity_field: status + id: status + expose: + operator: '' + group: 1 + sorts: + title: + id: title + table: node_field_data + field: title + relationship: none + group_type: group + admin_label: '' + order: ASC + exposed: false + expose: + label: '' + entity_type: node + entity_field: title + plugin_id: standard + header: { } + footer: { } + empty: { } + relationships: { } + arguments: { } + display_extenders: { } + cache_metadata: + contexts: + - 'languages:language_content' + - 'languages:language_interface' + - 'user.node_grants:view' + cacheable: false diff --git a/core/modules/views/tests/src/Unit/Plugin/field/CounterTest.php b/core/modules/views/tests/src/Unit/Plugin/field/CounterTest.php index ab181c1699b6..5a68eb204d87 100644 --- a/core/modules/views/tests/src/Unit/Plugin/field/CounterTest.php +++ b/core/modules/views/tests/src/Unit/Plugin/field/CounterTest.php @@ -97,7 +97,6 @@ protected function setUp() { $this->definition = array('title' => 'counter field', 'plugin_type' => 'field'); } - /** * Provides some row index to test. * @@ -130,8 +129,8 @@ public function testSimpleCounter($i) { '@expected' => $expected, '@counter' => $counter ))); - $counter = $counter_handler->render($this->testData[$i]); - $this->assertEquals($expected, $counter_handler->render($this->testData[$i]), SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( + $counter = $this->renderCounter($counter_handler, $this->testData[$i]); + $this->assertEquals($expected, $counter, SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( '@expected' => $expected, '@counter' => $counter ))); @@ -162,8 +161,8 @@ public function testCounterRandomStart($i) { '@expected' => $expected, '@counter' => $counter ))); - $counter = $counter_handler->render($this->testData[$i]); - $this->assertEquals($expected, $counter_handler->render($this->testData[$i]), SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( + $counter = $this->renderCounter($counter_handler, $this->testData[$i]); + $this->assertEquals($expected, $counter, SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( '@expected' => $expected, '@counter' => $counter ))); @@ -197,8 +196,8 @@ public function testCounterRandomPagerOffset($i) { '@expected' => $expected, '@counter' => $counter ))); - $counter = $counter_handler->render($this->testData[$i]); - $this->assertEquals($expected, $counter_handler->render($this->testData[$i]), SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( + $counter = $this->renderCounter($counter_handler, $this->testData[$i]); + $this->assertEquals($expected, $counter, SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( '@expected' => $expected, '@counter' => $counter ))); @@ -236,11 +235,28 @@ public function testCounterSecondPage($i) { '@expected' => $expected, '@counter' => $counter ))); - $counter = $counter_handler->render($this->testData[$i]); - $this->assertEquals($expected, $counter_handler->render($this->testData[$i]), SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( + $counter = $this->renderCounter($counter_handler, $this->testData[$i]); + $this->assertEquals($expected, $counter, SafeMarkup::format('The expected number (@expected) patches with the rendered number (@counter) failed', array( '@expected' => $expected, '@counter' => $counter ))); } + /** + * Renders the counter field handler. + * + * @param \Drupal\views\Plugin\views\field\Counter $handler + * The counter handler. + * @param \Drupal\views\ResultRow $row + * A result row. + * + * @return string + * The counter rendered markup. + */ + protected function renderCounter(Counter $handler, ResultRow $row) { + $markup = $handler->render($row); + $handler->postRender($row, $markup); + return $handler->last_render; + } + } diff --git a/core/tests/Drupal/Tests/Core/Render/RendererTest.php b/core/tests/Drupal/Tests/Core/Render/RendererTest.php index e8368b230dd5..298a743d6987 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererTest.php +++ b/core/tests/Drupal/Tests/Core/Render/RendererTest.php @@ -621,6 +621,75 @@ public function providerTestRenderCacheMaxAge() { ]; } + /** + * Tests that #cache_properties are properly handled. + * + * @param array $expected_results + * An associative array of expected results keyed by property name. + * + * @covers ::render + * @covers ::doRender + * @covers \Drupal\Core\Render\RenderCache::get + * @covers \Drupal\Core\Render\RenderCache::set + * @covers \Drupal\Core\Render\RenderCache::createCacheID + * @covers \Drupal\Core\Render\RenderCache::getCacheableRenderArray + * + * @dataProvider providerTestRenderCacheProperties + */ + public function testRenderCacheProperties(array $expected_results) { + $this->setUpRequest(); + $this->setupMemoryCache(); + + $element = $original = [ + '#cache' => [ + 'keys' => ['render_cache_test'], + ], + // Collect expected property names. + '#cache_properties' => array_keys(array_filter($expected_results)), + 'child1' => ['#markup' => 1], + 'child2' => ['#markup' => 2], + '#custom_property' => ['custom_value'], + ]; + $this->renderer->render($element); + + $cache = $this->cacheFactory->get('render'); + $data = $cache->get('render_cache_test:en:stark')->data; + + // Check that parent markup is ignored when caching children's markup. + $this->assertEquals($data['#markup'] === '', (bool) Element::children($data)); + + // Check that the element properties are cached as specified. + foreach ($expected_results as $property => $expected) { + $cached = !empty($data[$property]); + $this->assertEquals($cached, (bool) $expected); + // Check that only the #markup key is preserved for children. + if ($cached) { + $this->assertArrayEquals($data[$property], $original[$property]); + } + } + } + + /** + * Data provider for ::testRenderCacheProperties(). + * + * @return array + * An array of associative arrays of expected results keyed by property + * name. + */ + public function providerTestRenderCacheProperties() { + return [ + [[]], + [['child1' => 0, 'child2' => 0, '#custom_property' => 0]], + [['child1' => 0, 'child2' => 0, '#custom_property' => 1]], + [['child1' => 0, 'child2' => 1, '#custom_property' => 0]], + [['child1' => 0, 'child2' => 1, '#custom_property' => 1]], + [['child1' => 1, 'child2' => 0, '#custom_property' => 0]], + [['child1' => 1, 'child2' => 0, '#custom_property' => 1]], + [['child1' => 1, 'child2' => 1, '#custom_property' => 0]], + [['child1' => 1, 'child2' => 1, '#custom_property' => 1]], + ]; + } + /** * @covers ::addCacheableDependency * -- GitLab