Unverified Commit b941c7c4 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #2914110 by Berdir, neclimdul, acbramley, yongt9412, Wim Leers,...

Issue #2914110 by Berdir, neclimdul, acbramley, yongt9412, Wim Leers, arpad.rozsa, kim.pepper, Fabianx, davidwhthomas, pameeela, kualee, amateescu, jibran, longwave: Shortcut hook_toolbar implementation makes all pages uncacheable
parent b01c410b
Loading
Loading
Loading
Loading
+54 −58
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@
use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Url;
use Drupal\shortcut\Entity\ShortcutSet;
@@ -246,10 +247,10 @@ function shortcut_preprocess_block(&$variables) {
 */
function shortcut_preprocess_page_title(&$variables) {
  // Only display the shortcut link if the user has the ability to edit
  // shortcuts and if the page's actual content is being shown (for example,
  // we do not want to display it on "access denied" or "page not found"
  // pages).
  if (shortcut_set_edit_access()->isAllowed() && !\Drupal::request()->attributes->has('exception')) {
  // shortcuts, the feature is enabled for the current theme and if the page's
  // actual content is being shown (for example, we do not want to display it on
  // "access denied" or "page not found" pages).
  if (shortcut_set_edit_access()->isAllowed() && theme_get_setting('third_party_settings.shortcut.module_link') && !\Drupal::request()->attributes->has('exception')) {
    $link = Url::fromRouteMatch(\Drupal::routeMatch())->getInternalPath();
    $route_match = \Drupal::routeMatch();

@@ -264,6 +265,12 @@ function shortcut_preprocess_page_title(&$variables) {

    $shortcut_set = shortcut_current_displayed_set();

    // Pages with the add or remove shortcut button need cache invalidation when
    // a shortcut is added, edited, or removed.
    $cacheability_metadata = CacheableMetadata::createFromRenderArray($variables);
    $cacheability_metadata->addCacheTags(\Drupal::entityTypeManager()->getDefinition('shortcut')->getListCacheTags());
    $cacheability_metadata->applyTo($variables);

    // Check if $link is already a shortcut and set $link_mode accordingly.
    $shortcuts = \Drupal::entityTypeManager()->getStorage('shortcut')->loadByProperties(['shortcut_set' => $shortcut_set->id()]);
    /** @var \Drupal\shortcut\ShortcutInterface $shortcut */
@@ -287,7 +294,6 @@ function shortcut_preprocess_page_title(&$variables) {
      $route_parameters = ['shortcut' => $shortcut_id];
    }

    if (theme_get_setting('third_party_settings.shortcut.module_link')) {
    $query += \Drupal::destination()->getAsArray();
    $variables['title_suffix']['add_or_remove_shortcut'] = [
      '#attached' => [
@@ -308,7 +314,6 @@ function shortcut_preprocess_page_title(&$variables) {
    ];
  }
}
}

/**
 * Implements hook_toolbar().
@@ -320,28 +325,14 @@ function shortcut_toolbar() {
  $items['shortcuts'] = [
    '#cache' => [
      'contexts' => [
        // Cacheable per user, because each user can have their own shortcut
        // set, even if they cannot create or select a shortcut set, because
        // an administrator may have assigned a non-default shortcut set.
        'user',
        'user.permissions',
      ],
    ],
  ];

  if ($user->hasPermission('access shortcuts')) {
    $links = shortcut_renderable_links();
    $shortcut_set = shortcut_current_displayed_set();
    \Drupal::service('renderer')->addCacheableDependency($items['shortcuts'], $shortcut_set);
    $configure_link = NULL;
    if (shortcut_set_edit_access($shortcut_set)->isAllowed()) {
      $configure_link = [
        '#type' => 'link',
        '#title' => t('Edit shortcuts'),
        '#url' => Url::fromRoute('entity.shortcut_set.customize_form', ['shortcut_set' => $shortcut_set->id()]),
        '#options' => ['attributes' => ['class' => ['edit-shortcuts']]],
      ];
    }
    if (!empty($links) || !empty($configure_link)) {

    $items['shortcuts'] += [
      '#type' => 'toolbar_item',
      'tab' => [
@@ -355,8 +346,14 @@ function shortcut_toolbar() {
      ],
      'tray' => [
        '#heading' => t('User-defined shortcuts'),
          'shortcuts' => $links,
          'configure' => $configure_link,
        'children' => [
          '#lazy_builder' => ['shortcut.lazy_builders:lazyLinks', []],
          '#create_placeholder' => TRUE,
          '#cache' => [
            'keys' => ['shortcut_set_toolbar_links'],
            'contexts' => ['user'],
          ],
        ],
      ],
      '#weight' => -10,
      '#attached' => [
@@ -366,7 +363,6 @@ function shortcut_toolbar() {
      ],
    ];
  }
  }

  return $items;
}
+4 −0
Original line number Diff line number Diff line
services:
  shortcut.lazy_builders:
    class: Drupal\shortcut\ShortcutLazyBuilders
    arguments: ['@renderer']
+68 −0
Original line number Diff line number Diff line
<?php

namespace Drupal\shortcut;

use Drupal\Core\Render\RendererInterface;
use Drupal\Core\Security\TrustedCallbackInterface;
use Drupal\Core\Url;

/**
 * Lazy builders for the shortcut module.
 */
class ShortcutLazyBuilders implements TrustedCallbackInterface {

  /**
   * The renderer service.
   *
   * @var \Drupal\Core\Render\RendererInterface
   */
  protected $renderer;

  /**
   * Constructs a new ShortcutLazyBuilders object.
   *
   * @param \Drupal\Core\Render\RendererInterface $renderer
   *   The renderer service.
   */
  public function __construct(RendererInterface $renderer) {
    $this->renderer = $renderer;
  }

  /**
   * {@inheritdoc}
   */
  public static function trustedCallbacks() {
    return ['lazyLinks'];
  }

  /**
   * #lazy_builder callback; builds shortcut toolbar links.
   *
   * @return array
   *   A renderable array of shortcut links.
   */
  public function lazyLinks() {
    $shortcut_set = shortcut_current_displayed_set();

    $links = shortcut_renderable_links();

    $configure_link = NULL;
    if (shortcut_set_edit_access($shortcut_set)->isAllowed()) {
      $configure_link = [
        '#type' => 'link',
        '#title' => t('Edit shortcuts'),
        '#url' => Url::fromRoute('entity.shortcut_set.customize_form', ['shortcut_set' => $shortcut_set->id()]),
        '#options' => ['attributes' => ['class' => ['edit-shortcuts']]],
      ];
    }

    $build = [
      'shortcuts' => $links,
      'configure' => $configure_link,
    ];
    $this->renderer->addCacheableDependency($build, $shortcut_set);

    return $build;
  }

}
+121 −1
Original line number Diff line number Diff line
@@ -3,7 +3,9 @@
namespace Drupal\Tests\shortcut\Functional;

use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Url;
use Drupal\shortcut\Entity\Shortcut;
use Drupal\Tests\system\Functional\Cache\AssertPageCacheContextsAndTagsTrait;
use Drupal\Tests\system\Functional\Entity\EntityCacheTagsTestBase;
use Drupal\user\Entity\Role;
use Drupal\user\RoleInterface;
@@ -14,11 +16,12 @@
 * @group shortcut
 */
class ShortcutCacheTagsTest extends EntityCacheTagsTestBase {
  use AssertPageCacheContextsAndTagsTrait;

  /**
   * {@inheritdoc}
   */
  public static $modules = ['shortcut'];
  public static $modules = ['toolbar', 'shortcut', 'test_page_test', 'block'];

  /**
   * {@inheritdoc}
@@ -73,4 +76,121 @@ public function testEntityCreation() {
    $this->assertFalse(\Drupal::cache('render')->get('foo'), 'Creating a new shortcut invalidates the cache tag of the shortcut set.');
  }

  /**
   * Tests visibility and cacheability of shortcuts in the toolbar.
   */
  public function testToolbar() {
    $this->drupalPlaceBlock('page_title_block', ['id' => 'title']);

    $test_page_url = Url::fromRoute('test_page_test.test_page');
    $this->verifyPageCache($test_page_url, 'MISS');
    $this->verifyPageCache($test_page_url, 'HIT');

    // Ensure that without enabling the shortcuts-in-page-title-link feature
    // in the theme, the shortcut_list cache tag is not added to the page.
    $this->drupalLogin($this->rootUser);
    $this->drupalGet('admin/config/system/cron');
    $expected_cache_tags = [
      'block_view',
      'config:block.block.title',
      'config:block_list',
      'config:shortcut.set.default',
      'config:system.menu.admin',
      'config:user.role.authenticated',
      'rendered',
      'user:' . $this->rootUser->id(),
    ];
    $this->assertCacheTags($expected_cache_tags);

    \Drupal::configFactory()
      ->getEditable('stark.settings')
      ->set('third_party_settings.shortcut.module_link', TRUE)
      ->save(TRUE);

    // Add cron to the default shortcut set, now the shortcut list cache tag
    // is expected.
    $this->drupalGet('admin/config/system/cron');
    $this->clickLink('Add to Default shortcuts');
    $expected_cache_tags[] = 'config:shortcut_set_list';
    $this->assertCacheTags($expected_cache_tags);

    // Verify that users without the 'access shortcuts' permission can't see the
    // shortcuts.
    $this->drupalLogin($this->drupalCreateUser(['access toolbar']));
    $this->assertNoLink('Shortcuts');
    $this->verifyDynamicPageCache($test_page_url, 'MISS');
    $this->verifyDynamicPageCache($test_page_url, 'HIT');

    // Verify that users without the 'administer site configuration' permission
    // can't see the cron shortcut but can see shortcuts toolbar tab.
    $this->drupalLogin($this->drupalCreateUser([
      'access toolbar',
      'access shortcuts',
    ]));
    $this->verifyDynamicPageCache($test_page_url, 'MISS');
    $this->verifyDynamicPageCache($test_page_url, 'HIT');
    $this->assertLink('Shortcuts');
    $this->assertNoLink('Cron');

    // Create a role with access to shortcuts as well as the necessary
    // permissions to see specific shortcuts.
    $site_configuration_role = $this->drupalCreateRole([
      'access toolbar',
      'access shortcuts',
      'administer site configuration',
      'access administration pages',
    ]);

    // Create two different users with the same role to assert that the second
    // user has a cache hit despite the user cache context, as
    // the returned cache contexts include those from lazy-builder content.
    $site_configuration_user1 = $this->drupalCreateUser();
    $site_configuration_user1->addRole($site_configuration_role);
    $site_configuration_user1->save();
    $site_configuration_user2 = $this->drupalCreateUser();
    $site_configuration_user2->addRole($site_configuration_role);
    $site_configuration_user2->save();

    $this->drupalLogin($site_configuration_user1);
    $this->verifyDynamicPageCache($test_page_url, 'MISS');
    $this->verifyDynamicPageCache($test_page_url, 'HIT');
    $this->assertCacheContexts(['user', 'url.query_args:_wrapper_format']);
    $this->assertLink('Shortcuts');
    $this->assertLink('Cron');

    $this->drupalLogin($site_configuration_user2);
    $this->verifyDynamicPageCache($test_page_url, 'HIT');
    $this->assertCacheContexts(['user', 'url.query_args:_wrapper_format']);
    $this->assertLink('Shortcuts');
    $this->assertLink('Cron');

    // Add another shortcut.
    $shortcut = Shortcut::create([
      'shortcut_set' => 'default',
      'title' => 'Llama',
      'weight' => 0,
      'link' => [['uri' => 'internal:/admin/config']],
    ]);
    $shortcut->save();

    // The shortcuts are displayed in a lazy builder, so the page is still a
    // cache HIT but shows the new shortcut immediately.
    $this->verifyDynamicPageCache($test_page_url, 'HIT');
    $this->assertLink('Cron');
    $this->assertLink('Llama');

    // Update the shortcut title and assert that it is updated.
    $shortcut->set('title', 'Alpaca');
    $shortcut->save();
    $this->verifyDynamicPageCache($test_page_url, 'HIT');
    $this->assertLink('Cron');
    $this->assertLink('Alpaca');

    // Delete the shortcut and assert that the link is gone.
    $shortcut->delete();
    $this->verifyDynamicPageCache($test_page_url, 'HIT');
    $this->assertLink('Cron');
    $this->assertNoLink('Alpaca');
  }

}
+2 −2
Original line number Diff line number Diff line
@@ -355,9 +355,9 @@ public function testAccessShortcutsPermission() {
    $this->assertNoLink('Shortcuts', 'Shortcut link not found on page.');

    // Verify that users without the 'administer site configuration' permission
    // can't see the cron shortcuts.
    // can't see the cron shortcuts but can see shortcuts.
    $this->drupalLogin($this->drupalCreateUser(['access toolbar', 'access shortcuts']));
    $this->assertNoLink('Shortcuts', 'Shortcut link not found on page.');
    $this->assertLink('Shortcuts');
    $this->assertNoLink('Cron', 'Cron shortcut link not found on page.');

    // Verify that users with the 'access shortcuts' permission can see the
Loading