From 8f0729767610930cca902778ea0c3a820bd42c90 Mon Sep 17 00:00:00 2001
From: webchick <drupal@webchick.net>
Date: Thu, 16 Apr 2015 09:45:19 -0700
Subject: [PATCH] Issue #2375689 by Arla, Wim Leers, dawehner, tim.plunkett,
 Berdir: BlockBase::blockAccess() should return AccessResult instead of a bool

---
 core/lib/Drupal/Core/Block/BlockBase.php      | 22 ++++-------
 .../src/Plugin/Block/AggregatorFeedBlock.php  |  3 +-
 .../src/Plugin/Block/TestAccessBlock.php      |  3 +-
 .../Plugin/Block/TestBlockInstantiation.php   |  3 +-
 .../src/Plugin/Block/BlockContentBlock.php    | 38 +++++++++++++++++--
 .../forum/src/Plugin/Block/ForumBlockBase.php |  3 +-
 .../help/src/Plugin/Block/HelpBlock.php       |  8 +++-
 .../src/Plugin/Block/LanguageBlock.php        |  4 +-
 .../node/src/Plugin/Block/SyndicateBlock.php  |  3 +-
 .../search/src/Plugin/Block/SearchBlock.php   |  4 +-
 .../src/Plugin/Block/ShortcutsBlock.php       |  3 +-
 .../Plugin/Block/StatisticsPopularBlock.php   | 12 +++---
 .../src/Plugin/Block/RedirectFormBlock.php    |  8 ----
 .../user/src/Plugin/Block/UserLoginBlock.php  |  7 +++-
 .../views/src/Plugin/Block/ViewsBlockBase.php |  9 ++++-
 15 files changed, 88 insertions(+), 42 deletions(-)

diff --git a/core/lib/Drupal/Core/Block/BlockBase.php b/core/lib/Drupal/Core/Block/BlockBase.php
index 7ec2e6cffe16..262fd0c55dbb 100644
--- a/core/lib/Drupal/Core/Block/BlockBase.php
+++ b/core/lib/Drupal/Core/Block/BlockBase.php
@@ -121,34 +121,28 @@ public function calculateDependencies() {
    * {@inheritdoc}
    */
   public function access(AccountInterface $account, $return_as_object = FALSE) {
-    // @todo Remove self::blockAccess() and force individual plugins to return
-    //   their own AccessResult logic. Until that is done in
-    //   https://www.drupal.org/node/2375689 the access will be set uncacheable.
-    if ($this->blockAccess($account)) {
-      $access = AccessResult::allowed();
-    }
-    else {
-      $access = AccessResult::forbidden();
-    }
-
-    $access->setCacheMaxAge(0);
+    $access = $this->blockAccess($account);
     return $return_as_object ? $access : $access->isAllowed();
   }
 
   /**
    * Indicates whether the block should be shown.
    *
+   * Blocks with specific access checking should override this method rather
+   * than access(), in order to avoid repeating the handling of the
+   * $return_as_object argument.
+   *
    * @param \Drupal\Core\Session\AccountInterface $account
    *   The user session for which to check access.
    *
-   * @return bool
-   *   TRUE if the block should be shown, or FALSE otherwise.
+   * @return \Drupal\Core\Access\AccessResult
+   *   The access result.
    *
    * @see self::access()
    */
   protected function blockAccess(AccountInterface $account) {
     // By default, the block is visible.
-    return TRUE;
+    return AccessResult::allowed();
   }
 
   /**
diff --git a/core/modules/aggregator/src/Plugin/Block/AggregatorFeedBlock.php b/core/modules/aggregator/src/Plugin/Block/AggregatorFeedBlock.php
index f49abd5f465c..fcc25848a17b 100644
--- a/core/modules/aggregator/src/Plugin/Block/AggregatorFeedBlock.php
+++ b/core/modules/aggregator/src/Plugin/Block/AggregatorFeedBlock.php
@@ -9,6 +9,7 @@
 
 use Drupal\aggregator\FeedStorageInterface;
 use Drupal\aggregator\ItemStorageInterface;
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Entity\Query\QueryInterface;
@@ -104,7 +105,7 @@ public function defaultConfiguration() {
    */
   protected function blockAccess(AccountInterface $account) {
     // Only grant access to users with the 'access news feeds' permission.
-    return $account->hasPermission('access news feeds');
+    return AccessResult::allowedIfHasPermission($account, 'access news feeds');
   }
 
   /**
diff --git a/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestAccessBlock.php b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestAccessBlock.php
index 1aa7d6b0c37b..873a77b5e451 100644
--- a/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestAccessBlock.php
+++ b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestAccessBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\block_test\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
@@ -62,7 +63,7 @@ public static function create(ContainerInterface $container, array $configuratio
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $this->state->get('test_block_access', FALSE);
+    return $this->state->get('test_block_access', FALSE) ? AccessResult::allowed() : AccessResult::forbidden();
   }
 
   /**
diff --git a/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestBlockInstantiation.php b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestBlockInstantiation.php
index e9b4144b1ed5..9ca1503accf8 100644
--- a/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestBlockInstantiation.php
+++ b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestBlockInstantiation.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\block_test\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Session\AccountInterface;
@@ -34,7 +35,7 @@ public function defaultConfiguration() {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access content');
+    return AccessResult::allowedIfHasPermission($account, 'access content');
   }
 
   /**
diff --git a/core/modules/block_content/src/Plugin/Block/BlockContentBlock.php b/core/modules/block_content/src/Plugin/Block/BlockContentBlock.php
index 152572327aa9..b4596f198394 100644
--- a/core/modules/block_content/src/Plugin/Block/BlockContentBlock.php
+++ b/core/modules/block_content/src/Plugin/Block/BlockContentBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\block_content\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Block\BlockManagerInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
@@ -50,6 +51,13 @@ class BlockContentBlock extends BlockBase implements ContainerFactoryPluginInter
    */
   protected $account;
 
+  /**
+   * The block content entity.
+   *
+   * @var \Drupal\block_content\BlockContentInterface
+   */
+  protected $blockContent;
+
   /**
    * Constructs a new BlockContentBlock.
    *
@@ -129,22 +137,46 @@ public function blockSubmit($form, FormStateInterface $form_state) {
     $this->blockManager->clearCachedDefinitions();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function blockAccess(AccountInterface $account) {
+    if ($this->getEntity()) {
+      return $this->getEntity()->access('view', $account, TRUE);
+    }
+    return AccessResult::forbidden();
+  }
+
   /**
    * {@inheritdoc}
    */
   public function build() {
-    $uuid = $this->getDerivativeId();
-    if ($block = $this->entityManager->loadEntityByUuid('block_content', $uuid)) {
+    if ($block = $this->getEntity()) {
       return $this->entityManager->getViewBuilder($block->getEntityTypeId())->view($block, $this->configuration['view_mode']);
     }
     else {
       return array(
         '#markup' => $this->t('Block with uuid %uuid does not exist. <a href="!url">Add custom block</a>.', array(
-          '%uuid' => $uuid,
+          '%uuid' => $this->getDerivativeId(),
           '!url' => $this->urlGenerator->generate('block_content.add_page')
         )),
         '#access' => $this->account->hasPermission('administer blocks')
       );
     }
   }
+
+  /**
+   * Loads the block content entity of the block.
+   *
+   * @return \Drupal\block_content\BlockContentInterface|null
+   *   The block content entity.
+   */
+  protected function getEntity() {
+    $uuid = $this->getDerivativeId();
+    if (!isset($this->blockContent)) {
+      $this->blockContent = $this->entityManager->loadEntityByUuid('block_content', $uuid);
+    }
+    return $this->blockContent;
+  }
+
 }
diff --git a/core/modules/forum/src/Plugin/Block/ForumBlockBase.php b/core/modules/forum/src/Plugin/Block/ForumBlockBase.php
index c48767cf7868..5e8fbebc553b 100644
--- a/core/modules/forum/src/Plugin/Block/ForumBlockBase.php
+++ b/core/modules/forum/src/Plugin/Block/ForumBlockBase.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\forum\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Session\AccountInterface;
@@ -59,7 +60,7 @@ public function defaultConfiguration() {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access content');
+    return AccessResult::allowedIfHasPermission($account, 'access content');
   }
 
   /**
diff --git a/core/modules/help/src/Plugin/Block/HelpBlock.php b/core/modules/help/src/Plugin/Block/HelpBlock.php
index 968fca60b797..7b86f9fc400a 100644
--- a/core/modules/help/src/Plugin/Block/HelpBlock.php
+++ b/core/modules/help/src/Plugin/Block/HelpBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\help\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
@@ -96,7 +97,12 @@ public static function create(ContainerInterface $container, array $configuratio
    */
   protected function blockAccess(AccountInterface $account) {
     $this->help = $this->getActiveHelp($this->request);
-    return (bool) $this->help;
+    if ($this->help) {
+      return AccessResult::allowed();
+    }
+    else {
+      return AccessResult::forbidden();
+    }
   }
 
   /**
diff --git a/core/modules/language/src/Plugin/Block/LanguageBlock.php b/core/modules/language/src/Plugin/Block/LanguageBlock.php
index 7aa9bb880803..3d31770b48f8 100644
--- a/core/modules/language/src/Plugin/Block/LanguageBlock.php
+++ b/core/modules/language/src/Plugin/Block/LanguageBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\language\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Path\PathMatcherInterface;
 use Drupal\Core\Session\AccountInterface;
@@ -80,7 +81,8 @@ public static function create(ContainerInterface $container, array $configuratio
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $this->languageManager->isMultilingual();
+    $access = $this->languageManager->isMultilingual() ? AccessResult::allowed() : AccessResult::forbidden();
+    return $access->addCacheTags(['config:configurable_language_list']);
   }
 
   /**
diff --git a/core/modules/node/src/Plugin/Block/SyndicateBlock.php b/core/modules/node/src/Plugin/Block/SyndicateBlock.php
index f2a6a3e93f2b..9e5834b71de0 100644
--- a/core/modules/node/src/Plugin/Block/SyndicateBlock.php
+++ b/core/modules/node/src/Plugin/Block/SyndicateBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\node\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Form\FormStateInterface;
@@ -36,7 +37,7 @@ public function defaultConfiguration() {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access content');
+    return AccessResult::allowedIfHasPermission($account, 'access content');
   }
 
   /**
diff --git a/core/modules/search/src/Plugin/Block/SearchBlock.php b/core/modules/search/src/Plugin/Block/SearchBlock.php
index 48a2b54aaa03..5bdfce34612e 100644
--- a/core/modules/search/src/Plugin/Block/SearchBlock.php
+++ b/core/modules/search/src/Plugin/Block/SearchBlock.php
@@ -7,10 +7,10 @@
 
 namespace Drupal\search\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Block\BlockBase;
 use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\HttpFoundation\Request;
 
 /**
  * Provides a 'Search form' block.
@@ -27,7 +27,7 @@ class SearchBlock extends BlockBase {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('search content');
+    return AccessResult::allowedIfHasPermission($account, 'search content');
   }
 
   /**
diff --git a/core/modules/shortcut/src/Plugin/Block/ShortcutsBlock.php b/core/modules/shortcut/src/Plugin/Block/ShortcutsBlock.php
index 9396f8220bc3..379d74d85531 100644
--- a/core/modules/shortcut/src/Plugin/Block/ShortcutsBlock.php
+++ b/core/modules/shortcut/src/Plugin/Block/ShortcutsBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\shortcut\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Session\AccountInterface;
 
@@ -34,7 +35,7 @@ public function build() {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access shortcuts');
+    return AccessResult::allowedIfHasPermission($account, 'access shortcuts');
   }
 
 }
diff --git a/core/modules/statistics/src/Plugin/Block/StatisticsPopularBlock.php b/core/modules/statistics/src/Plugin/Block/StatisticsPopularBlock.php
index e8c2fbb6607b..6dd6f7fccda9 100644
--- a/core/modules/statistics/src/Plugin/Block/StatisticsPopularBlock.php
+++ b/core/modules/statistics/src/Plugin/Block/StatisticsPopularBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\statistics\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Session\AccountInterface;
@@ -57,22 +58,23 @@ public function defaultConfiguration() {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
+    $access = AccessResult::allowedIfHasPermission($account, 'access content');
     if ($account->hasPermission('access content')) {
       $daytop = $this->configuration['top_day_num'];
       if (!$daytop || !($result = statistics_title_list('daycount', $daytop)) || !($this->day_list = node_title_list($result, $this->t("Today's:")))) {
-        return FALSE;
+        return AccessResult::forbidden()->inheritCacheability($access);
       }
       $alltimetop = $this->configuration['top_all_num'];
       if (!$alltimetop || !($result = statistics_title_list('totalcount', $alltimetop)) || !($this->all_time_list = node_title_list($result, $this->t('All time:')))) {
-        return FALSE;
+        return AccessResult::forbidden()->inheritCacheability($access);
       }
       $lasttop = $this->configuration['top_last_num'];
       if (!$lasttop || !($result = statistics_title_list('timestamp', $lasttop)) || !($this->last_list = node_title_list($result, $this->t('Last viewed:')))) {
-        return FALSE;
+        return AccessResult::forbidden()->inheritCacheability($access);
       }
-      return TRUE;
+      return $access;
     }
-    return FALSE;
+    return AccessResult::forbidden()->inheritCacheability($access);
   }
 
   /**
diff --git a/core/modules/system/tests/modules/form_test/src/Plugin/Block/RedirectFormBlock.php b/core/modules/system/tests/modules/form_test/src/Plugin/Block/RedirectFormBlock.php
index f02b523f7afe..0b10289e4f8b 100644
--- a/core/modules/system/tests/modules/form_test/src/Plugin/Block/RedirectFormBlock.php
+++ b/core/modules/system/tests/modules/form_test/src/Plugin/Block/RedirectFormBlock.php
@@ -10,7 +10,6 @@
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Form\FormBuilderInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
-use Drupal\Core\Session\AccountInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -63,13 +62,6 @@ public static function create(ContainerInterface $container, array $configuratio
     );
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function blockAccess(AccountInterface $account) {
-    return TRUE;
-  }
-
   /**
    * {@inheritdoc}
    */
diff --git a/core/modules/user/src/Plugin/Block/UserLoginBlock.php b/core/modules/user/src/Plugin/Block/UserLoginBlock.php
index 3f28d3c331c0..564870e41808 100644
--- a/core/modules/user/src/Plugin/Block/UserLoginBlock.php
+++ b/core/modules/user/src/Plugin/Block/UserLoginBlock.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\user\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Routing\RedirectDestinationTrait;
 use Drupal\Core\Routing\RouteMatchInterface;
@@ -76,7 +77,11 @@ public static function create(ContainerInterface $container, array $configuratio
    */
   protected function blockAccess(AccountInterface $account) {
     $route_name = $this->routeMatch->getRouteName();
-    return ($account->isAnonymous() && !in_array($route_name, array('user.register', 'user.login', 'user.logout')));
+    if ($account->isAnonymous() && !in_array($route_name, array('user.register', 'user.login', 'user.logout'))) {
+      return AccessResult::allowed()
+        ->addCacheContexts(['route', 'user.roles:anonymous']);
+    }
+    return AccessResult::forbidden();
   }
 
   /**
diff --git a/core/modules/views/src/Plugin/Block/ViewsBlockBase.php b/core/modules/views/src/Plugin/Block/ViewsBlockBase.php
index 953dfae3abcc..b87711fc4939 100644
--- a/core/modules/views/src/Plugin/Block/ViewsBlockBase.php
+++ b/core/modules/views/src/Plugin/Block/ViewsBlockBase.php
@@ -7,6 +7,7 @@
 
 namespace Drupal\views\Plugin\Block;
 
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Block\BlockBase;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
@@ -93,7 +94,13 @@ public static function create(ContainerInterface $container, array $configuratio
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $this->view->access($this->displayID);
+    if ($this->view->access($this->displayID)) {
+      $access = AccessResult::allowed();
+    }
+    else {
+      $access = AccessResult::forbidden();
+    }
+    return $access;
   }
 
   /**
-- 
GitLab