Issue #2722237 by webflo, dawehner, borisson_, drunken monkey, Wim Leers:...

Issue #2722237 by webflo, dawehner, borisson_, drunken monkey, Wim Leers: "Local actions" block doesn't take cache data of route access information into account

core/lib/Drupal/Core/Menu/LocalActionManager.php

@@ -196,9 +196,10 @@ public function getActionsForRoute($route_appears) {
       }
     }
     $links = [];
+    $cacheability = new CacheableMetadata();
+    $cacheability->addCacheContexts(['route']);
     /** @var $plugin \Drupal\Core\Menu\LocalActionInterface */
     foreach ($this->instances[$route_appears] as $plugin_id => $plugin) {
-      $cacheability = new CacheableMetadata();
       $route_name = $plugin->getRouteName();
       $route_parameters = $plugin->getRouteParameters($this->routeMatch);
       $access = $this->accessManager->checkNamedRoute($route_name, $route_parameters, $this->account, TRUE);
@@ -213,9 +214,8 @@ public function getActionsForRoute($route_appears) {
         '#weight' => $plugin->getWeight(),
       ];
       $cacheability->addCacheableDependency($access)->addCacheableDependency($plugin);
-      $cacheability->applyTo($links[$plugin_id]);
     }
-    $links['#cache']['contexts'][] = 'route';
+    $cacheability->applyTo($links);
 
     return $links;
   }

core/modules/system/tests/modules/menu_test/menu_test.links.action.yml

@@ -50,6 +50,18 @@ menu_test.local_action6:
   appears_on:
     - menu_test.local_action1
 
+menu_test.local_action7:
+  route_name: menu_test.local_action7
+  title: 'Local action with access'
+  appears_on:
+    - menu_test.local_action7
+
+menu_test.local_action8:
+  route_name: menu_test.local_action8
+  title: 'Local action without access'
+  appears_on:
+    - menu_test.local_action7
+
 menu_test.hidden_menu_add:
   route_name: menu_test.hidden_menu_add
   title: 'Add menu'

core/modules/system/tests/modules/menu_test/menu_test.routing.yml

@@ -125,6 +125,20 @@ menu_test.local_action6:
   requirements:
     _access: 'TRUE'
 
+menu_test.local_action7:
+  path: '/menu-test-local-action-7/cache-check'
+  defaults:
+    _controller: '\Drupal\menu_test\TestControllers::test2'
+  requirements:
+    _custom_access: '\Drupal\menu_test\Access\AccessCheck::menuLocalAction7'
+
+menu_test.local_action8:
+  path: '/menu-test-local-action-8/cache-check'
+  defaults:
+    _controller: '\Drupal\menu_test\TestControllers::test2'
+  requirements:
+    _custom_access: '\Drupal\menu_test\Access\AccessCheck::menuLocalAction8'
+
 menu_test.contextual_test:
   path: '/menu-test-contextual/default'
   defaults:

core/modules/system/tests/modules/menu_test/src/Access/AccessCheck.php

@@ -26,4 +26,18 @@ public function access() {
     return $result->setCacheMaxAge(0);
   }
 
+  /**
+   * @return \Drupal\Core\Access\AccessResultForbidden
+   */
+  public function menuLocalAction7() {
+    return AccessResult::forbidden()->addCacheTags(['menu_local_action7'])->addCacheContexts(['url.query_args:menu_local_action7']);
+  }
+
+  /**
+   * @return \Drupal\Core\Access\AccessResultAllowed
+   */
+  public function menuLocalAction8() {
+    return AccessResult::allowed()->addCacheTags(['menu_local_action8'])->addCacheContexts(['url.query_args:menu_local_action8']);
+  }
+
 }

core/tests/Drupal/KernelTests/Core/Menu/LocalActionManagerTest.php

+<?php
+
+namespace Drupal\KernelTests\Core\Menu;
+
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * Tests the local action manager.
+ *
+ * @coversDefaultClass \Drupal\Core\Menu\LocalActionManager
+ * @group Menu
+ */
+class LocalActionManagerTest extends KernelTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['menu_test', 'user', 'system'];
+
+  /**
+   * Tests the cacheability of local actions.
+   */
+  public function testCacheability() {
+    /** @var \Drupal\Core\Menu\LocalActionManager $local_action_manager */
+    $local_action_manager = \Drupal::service('plugin.manager.menu.local_action');
+    $build = [
+      '#cache' => [
+        'key' => 'foo',
+      ],
+      $local_action_manager->getActionsForRoute('menu_test.local_action7'),
+    ];
+
+    /** @var \Drupal\Core\Render\RendererInterface $renderer */
+    $renderer = \Drupal::service('renderer');
+    $renderer->renderRoot($build);
+
+    $this->assertContains('menu_local_action7', $build[0]['menu_test.local_action7']['#cache']['tags']);
+    $this->assertContains('url.query_args:menu_local_action7', $build[0]['menu_test.local_action7']['#cache']['contexts']);
+
+    $this->assertContains('menu_local_action8', $build[0]['menu_test.local_action8']['#cache']['tags']);
+    $this->assertContains('url.query_args:menu_local_action8', $build[0]['menu_test.local_action8']['#cache']['contexts']);
+
+    $this->assertContains('menu_local_action7', $build['#cache']['tags']);
+    $this->assertContains('url.query_args:menu_local_action7', $build['#cache']['contexts']);
+
+    $this->assertContains('menu_local_action8', $build['#cache']['tags']);
+    $this->assertContains('url.query_args:menu_local_action8', $build['#cache']['contexts']);
+  }
+
+}

core/tests/Drupal/Tests/Core/Menu/LocalActionManagerTest.php

@@ -13,7 +13,9 @@
 use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Access\AccessResultForbidden;
 use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\Cache\Context\CacheContextsManager;
 use Drupal\Core\Controller\ControllerResolver;
+use Drupal\Core\DependencyInjection\Container;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Language\Language;
 use Drupal\Core\Menu\LocalActionManager;
@@ -23,6 +25,7 @@
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Url;
 use Drupal\Tests\UnitTestCase;
+use Prophecy\Argument;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpKernel\Controller\ArgumentResolverInterface;
@@ -113,7 +116,15 @@ protected function setUp() {
     $this->moduleHandler = $this->getMock('Drupal\Core\Extension\ModuleHandlerInterface');
     $this->cacheBackend = $this->getMock('Drupal\Core\Cache\CacheBackendInterface');
 
-    $access_result = new AccessResultForbidden();
+    $cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
+    $cache_contexts_manager->assertValidTokens(Argument::any())
+      ->willReturn(TRUE);
+
+    $container = new Container();
+    $container->set('cache_contexts_manager', $cache_contexts_manager->reveal());
+    \Drupal::setContainer($container);
+
+    $access_result = (new AccessResultForbidden())->cachePerPermissions();
     $this->accessManager = $this->getMock('Drupal\Core\Access\AccessManagerInterface');
     $this->accessManager->expects($this->any())
       ->method('checkNamedRoute')
@@ -186,6 +197,14 @@ public function testGetActionsForRoute($route_appears, array $plugin_definitions
   }
 
   public function getActionsForRouteProvider() {
+    $cache_contexts_manager = $this->prophesize(CacheContextsManager::class);
+    $cache_contexts_manager->assertValidTokens(Argument::any())
+      ->willReturn(TRUE);
+
+    $container = new Container();
+    $container->set('cache_contexts_manager', $cache_contexts_manager->reveal());
+    \Drupal::setContainer($container);
+
     // Single available and single expected plugins.
     $data[] = [
       'test_route',
@@ -201,7 +220,9 @@ public function getActionsForRouteProvider() {
       ],
       [
         '#cache' => [
-          'contexts' => ['route'],
+          'tags' => [],
+          'contexts' => ['route', 'user.permissions'],
+          'max-age' => 0,
         ],
         'plugin_id_1' => [
           '#theme' => 'menu_local_action',
@@ -210,13 +231,8 @@ public function getActionsForRouteProvider() {
             'url' => Url::fromRoute('test_route_2'),
             'localized_options' => '',
           ],
-          '#access' => AccessResult::forbidden(),
+          '#access' => AccessResult::forbidden()->cachePerPermissions(),
           '#weight' => 0,
-          '#cache' => [
-            'contexts' => [],
-            'tags' => [],
-            'max-age' => 0,
-          ],
         ],
       ],
     ];
@@ -243,7 +259,9 @@ public function getActionsForRouteProvider() {
       ],
       [
         '#cache' => [
-          'contexts' => ['route'],
+          'tags' => [],
+          'contexts' => ['route', 'user.permissions'],
+          'max-age' => 0,
         ],
         'plugin_id_1' => [
           '#theme' => 'menu_local_action',
@@ -252,13 +270,8 @@ public function getActionsForRouteProvider() {
             'url' => Url::fromRoute('test_route_2'),
             'localized_options' => '',
           ],
-          '#access' => AccessResult::forbidden(),
+          '#access' => AccessResult::forbidden()->cachePerPermissions(),
           '#weight' => 0,
-          '#cache' => [
-            'contexts' => [],
-            'tags' => [],
-            'max-age' => 0,
-          ],
         ],
       ],
     ];
@@ -286,7 +299,9 @@ public function getActionsForRouteProvider() {
       ],
       [
         '#cache' => [
-          'contexts' => ['route'],
+          'contexts' => ['route', 'user.permissions'],
+          'tags' => [],
+          'max-age' => 0,
         ],
         'plugin_id_1' => [
           '#theme' => 'menu_local_action',
@@ -295,13 +310,8 @@ public function getActionsForRouteProvider() {
             'url' => Url::fromRoute('test_route_2'),
             'localized_options' => '',
           ],
-          '#access' => AccessResult::forbidden(),
+          '#access' => AccessResult::forbidden()->cachePerPermissions(),
           '#weight' => 1,
-          '#cache' => [
-            'contexts' => [],
-            'tags' => [],
-            'max-age' => 0,
-          ],
         ],
         'plugin_id_2' => [
           '#theme' => 'menu_local_action',
@@ -310,13 +320,8 @@ public function getActionsForRouteProvider() {
             'url' => Url::fromRoute('test_route_3'),
             'localized_options' => '',
           ],
-          '#access' => AccessResult::forbidden(),
+          '#access' => AccessResult::forbidden()->cachePerPermissions(),
           '#weight' => 0,
-          '#cache' => [
-            'contexts' => [],
-            'tags' => [],
-            'max-age' => 0,
-          ],
         ],
       ],
     ];
@@ -346,7 +351,9 @@ public function getActionsForRouteProvider() {
       ],
       [
         '#cache' => [
-          'contexts' => ['route'],
+          'contexts' => ['route', 'user.permissions'],
+          'tags' => [],
+          'max-age' => 0,
         ],
         'plugin_id_1' => [
           '#theme' => 'menu_local_action',
@@ -355,13 +362,8 @@ public function getActionsForRouteProvider() {
             'url' => Url::fromRoute('test_route_2', ['test1']),
             'localized_options' => '',
           ],
-          '#access' => AccessResult::forbidden(),
+          '#access' => AccessResult::forbidden()->cachePerPermissions(),
           '#weight' => 1,
-          '#cache' => [
-            'contexts' => [],
-            'tags' => [],
-            'max-age' => 0,
-          ],
         ],
         'plugin_id_2' => [
           '#theme' => 'menu_local_action',
@@ -370,13 +372,8 @@ public function getActionsForRouteProvider() {
             'url' => Url::fromRoute('test_route_2', ['test2']),
             'localized_options' => '',
           ],
-          '#access' => AccessResult::forbidden(),
+          '#access' => AccessResult::forbidden()->cachePerPermissions(),
           '#weight' => 0,
-          '#cache' => [
-            'contexts' => [],
-            'tags' => [],
-            'max-age' => 0,
-          ],
         ],
       ],
     ];