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

@@ -121,34 +121,28 @@ abstract class BlockBase extends ContextAwarePluginBase implements BlockPluginIn
    * {@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();
   }
 
   /**

core/modules/aggregator/src/Plugin/Block/AggregatorFeedBlock.php

@@ -9,6 +9,7 @@ namespace Drupal\aggregator\Plugin\Block;
 
 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 @@ class AggregatorFeedBlock extends BlockBase implements ContainerFactoryPluginInt
    */
   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');
   }
 
   /**

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 @@ class TestAccessBlock extends BlockBase implements ContainerFactoryPluginInterfa
    * {@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();
   }
 
   /**

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 @@ class TestBlockInstantiation extends BlockBase {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access content');
+    return AccessResult::allowedIfHasPermission($account, 'access content');
   }
 
   /**

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 @@ class BlockContentBlock extends BlockBase implements ContainerFactoryPluginInter
     $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;
+  }
+
 }

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 @@ abstract class ForumBlockBase extends BlockBase {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access content');
+    return AccessResult::allowedIfHasPermission($account, 'access content');
   }
 
   /**

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 @@ class HelpBlock extends BlockBase implements ContainerFactoryPluginInterface {
    */
   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();
+    }
   }
 
   /**

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 @@ class LanguageBlock extends BlockBase implements ContainerFactoryPluginInterface
    * {@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']);
   }
 
   /**

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 @@ class SyndicateBlock extends BlockBase {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access content');
+    return AccessResult::allowedIfHasPermission($account, 'access content');
   }
 
   /**

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');
   }
 
   /**

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 @@ class ShortcutsBlock extends BlockBase {
    * {@inheritdoc}
    */
   protected function blockAccess(AccountInterface $account) {
-    return $account->hasPermission('access shortcuts');
+    return AccessResult::allowedIfHasPermission($account, 'access shortcuts');
   }
 
 }

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 @@ class StatisticsPopularBlock extends BlockBase {
    * {@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);
   }
 
   /**

core/modules/system/tests/modules/form_test/src/Plugin/Block/RedirectFormBlock.php

@@ -10,7 +10,6 @@ namespace Drupal\form_test\Plugin\Block;
 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 @@ class RedirectFormBlock extends BlockBase implements ContainerFactoryPluginInter
     );
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function blockAccess(AccountInterface $account) {
-    return TRUE;
-  }
-
   /**
    * {@inheritdoc}
    */

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 @@ class UserLoginBlock extends BlockBase implements ContainerFactoryPluginInterfac
    */
   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();
   }
 
   /**

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 @@ abstract class ViewsBlockBase extends BlockBase implements ContainerFactoryPlugi
    * {@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;
   }
 
   /**