Issue #2433591 by dawehner, plach, YesCT, Wim Leers: Views using pagers should...

Issue #2433591 by dawehner, plach, YesCT, Wim Leers: Views using pagers should specify a cache context

core/modules/node/src/Tests/Views/FrontPageTest.php

@@ -250,6 +250,7 @@ protected function assertFrontPageViewCacheTags($do_assert_views_caches) {
       'user.permissions',
       // Default cache contexts of the renderer.
       'theme',
+      'url.query_args.pagers:0',
     ];
 
     // Test before there are any nodes.

core/modules/views/src/Plugin/views/cache/CachePluginBase.php

@@ -229,7 +229,7 @@ public function cacheGet($type) {
             // Load entities for each result.
             $this->view->query->loadEntities($this->view->result);
             $this->view->total_rows = $cache->data['total_rows'];
-            $this->view->setCurrentPage($cache->data['current_page']);
+            $this->view->setCurrentPage($cache->data['current_page'], TRUE);
             $this->view->execute_time = 0;
             return TRUE;
           }

core/modules/views/src/Plugin/views/pager/SqlBase.php

@@ -388,7 +388,7 @@ public function isCacheable() {
    * {@inheritdoc}
    */
   public function getCacheContexts() {
-    $contexts = [];
+    $contexts = ['url.query_args.pagers:' . $this->options['id']];
     if ($this->options['expose']['items_per_page']) {
       $contexts[] = 'url.query_args:items_per_page';
     }

core/modules/views/src/Tests/Handler/FieldWebTest.php

@@ -75,6 +75,7 @@ public function testClickSorting() {
     $expected_contexts = [
       'languages:language_interface',
       'theme',
+      'url.query_args.pagers:0',
       'url.query_args:order',
       'url.query_args:sort',
     ];

core/modules/views/src/Tests/Plugin/ExposedFormTest.php

@@ -206,6 +206,7 @@ public function testExposedSortAndItemsPerPage() {
       'languages:language_interface',
       'entity_test_view_grants',
       'theme',
+      'url.query_args.pagers:0',
       'url.query_args:items_per_page',
       'url.query_args:offset',
       'url.query_args:sort_order',

core/modules/views/src/Tests/Plugin/PagerKernelTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\views\Tests\Plugin\PagerKernelTest.
+ */
+
+namespace Drupal\views\Tests\Plugin;
+
+use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\views\Tests\ViewUnitTestBase;
+use Drupal\views\Views;
+
+/**
+ * Tests pager related APIs.
+ *
+ * @group views
+ */
+class PagerKernelTest extends ViewUnitTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $testViews = ['test_pager_full'];
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['user', 'node'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp($import_test_views = TRUE) {
+    parent::setUp($import_test_views);
+
+    $this->installEntitySchema('node');
+    $this->installEntitySchema('user');
+  }
+
+  /**
+   * Tests pager related setter methods on ViewExecutable.
+   *
+   * @see \Drupal\views\ViewExecutable::setItemsPerPage
+   * @see \Drupal\views\ViewExecutable::setOffset
+   * @see \Drupal\views\ViewExecutable::setCurrentPage
+   */
+  public function testSetPagerMethods() {
+    $view = Views::getView('test_pager_full');
+    $output = $view->preview();
+
+    \Drupal::service('renderer')->renderPlain($output);
+    $this->assertIdentical(CacheBackendInterface::CACHE_PERMANENT, $output['#cache']['max-age']);
+
+    foreach (['setItemsPerPage', 'setOffset', 'setCurrentPage'] as $method) {
+      // Without $keep_cacheablity.
+      $view = Views::getView('test_pager_full');
+      $view->setDisplay('default');
+      $view->{$method}(1);
+      $output = $view->preview();
+
+      \Drupal::service('renderer')->renderPlain($output);
+      $this->assertIdentical(0, $output['#cache']['max-age'], 'Max age set to 0 without $keep_cacheablity.');
+
+      // With $keep_cacheablity.
+      $view = Views::getView('test_pager_full');
+      $view->setDisplay('default');
+      $view->{$method}(1, TRUE);
+      $output = $view->preview();
+
+      \Drupal::service('renderer')->renderPlain($output);
+      $this->assertIdentical(CacheBackendInterface::CACHE_PERMANENT, $output['#cache']['max-age'], 'Max age kept on -1 with $keep_cacheablity.');
+    }
+
+  }
+
+}

core/modules/views/src/Tests/Plugin/PagerTest.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\views\Tests\Plugin;
 
+use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait;
 use Drupal\views\Views;
 use Drupal\language\Entity\ConfigurableLanguage;
 
@@ -17,6 +18,8 @@
  */
 class PagerTest extends PluginTestBase {
 
+  use AssertPageCacheContextsAndTagsTrait;
+
   /**
    * Views used by this test.
    *
@@ -255,6 +258,10 @@ public function testNormalPager() {
     $this->executeView($view);
     $this->assertEqual($view->pager->getItemsPerPage(), 0);
     $this->assertEqual(count($view->result), 11);
+
+    // Test pager cache contexts.
+    $this->drupalGet('test_pager_full');
+    $this->assertCacheContexts(['languages:language_interface', 'theme', 'timezone', 'url.query_args.pagers:0', 'user.node_grants:view']);
   }
 
   /**

core/modules/views/src/Tests/RenderCacheIntegrationTest.php

@@ -222,7 +222,7 @@ public function testViewAddCacheMetadata() {
     $view = View::load('test_display');
     $view->save();
 
-    $this->assertEqual(['languages:' . LanguageInterface::TYPE_CONTENT, 'languages:' . LanguageInterface::TYPE_INTERFACE, 'user.node_grants:view'], $view->getDisplay('default')['cache_metadata']['contexts']);
+    $this->assertEqual(['languages:' . LanguageInterface::TYPE_CONTENT, 'languages:' . LanguageInterface::TYPE_INTERFACE, 'url.query_args.pagers:0', 'user.node_grants:view'], $view->getDisplay('default')['cache_metadata']['contexts']);
   }
 
 }

core/modules/views/src/ViewExecutable.php

@@ -491,10 +491,22 @@ public function setArguments(array $args) {
 
   /**
    * Change/Set the current page for the pager.
+   *
+   * @param int $page
+   *   The current page.
+   * @param bool $keep_cacheability
+   *   (optional) Keep the cacheability. By default we mark the view as not
+   *   cacheable. The reason for this parameter is that we do not know what the
+   *   passed in value varies by. For example, it could be per role. Defaults to
+   *   FALSE.
    */
-  public function setCurrentPage($page) {
+  public function setCurrentPage($page, $keep_cacheability = FALSE) {
     $this->current_page = $page;
 
+    if (!$keep_cacheability) {
+      $this->element['#cache']['max-age'] = 0;
+    }
+
     // If the pager is already initialized, pass it through to the pager.
     if (!empty($this->pager)) {
       return $this->pager->setCurrentPage($page);
@@ -531,14 +543,26 @@ public function getItemsPerPage() {
 
   /**
    * Set the items per page on the pager.
+   *
+   * @param int $items_per_page
+   *   The items per page.
+   * @param bool $keep_cacheability
+   *   (optional) Keep the cacheability. By default we mark the view as not
+   *   cacheable. The reason for this parameter is that we do not know what the
+   *   passed in value varies by. For example, it could be per role. Defaults to
+   *   FALSE.
    */
-  public function setItemsPerPage($items_per_page) {
+  public function setItemsPerPage($items_per_page, $keep_cacheability = FALSE) {
     $this->items_per_page = $items_per_page;
 
     // If the pager is already initialized, pass it through to the pager.
     if (!empty($this->pager)) {
       $this->pager->setItemsPerPage($items_per_page);
     }
+
+    if (!$keep_cacheability) {
+      $this->element['#cache']['max-age'] = 0;
+    }
   }
 
   /**
@@ -557,14 +581,26 @@ public function getOffset() {
 
   /**
    * Set the offset on the pager.
+   *
+   * @param int $offset
+   *   The pager offset.
+   * @param bool $keep_cacheability
+   *   (optional) Keep the cacheability. By default we mark the view as not
+   *   cacheable. The reason for this parameter is that we do not know what the
+   *   passed in value varies by. For example, it could be per role. Defaults to
+   *   FALSE.
    */
-  public function setOffset($offset) {
+  public function setOffset($offset, $keep_cacheability = FALSE) {
     $this->offset = $offset;
 
     // If the pager is already initialized, pass it through to the pager.
     if (!empty($this->pager)) {
       $this->pager->setOffset($offset);
     }
+
+    if (!$keep_cacheability) {
+      $this->element['#cache']['max-age'] = 0;
+    }
   }
 
   /**
@@ -2363,7 +2399,7 @@ public function unserialize($serialized) {
 
     $this->setDisplay($current_display);
     $this->setArguments($args);
-    $this->setCurrentPage($current_page);
+    $this->setCurrentPage($current_page, TRUE);
     $this->setExposedInput($exposed_input);
     $this->exposed_data = $exposed_data;
     $this->exposed_raw_input = $exposed_raw_input;