Issue #2150621 by damiankloip: Separate applies and build logic for breadcrumb builders.

core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php

@@ -12,15 +12,27 @@
  */
 interface BreadcrumbBuilderInterface {
 
+  /**
+   * Whether this breadcrumb builder should be used to build the breadcrumb.
+   *
+   * @param array $attributes
+   *   Attributes representing the current page.
+   *
+   * @return bool
+   *   TRUE if this builder should be used or FALSE to let other builders
+   *   decide.
+   */
+  public function applies(array $attributes);
+
   /**
    * Builds the breadcrumb.
    *
    * @param array $attributes
    *   Attributes representing the current page.
    *
-   * @return array|null
-   *   A render array for the breadcrumbs or NULL to let other builders decide.
-   *   Returning empty array will suppress all breadcrumbs.
+   * @return array
+   *   A render array for the breadcrumbs. Returning an empty array will
+   *   suppress all breadcrumbs.
    */
   public function build(array $attributes);
 

core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php

@@ -65,6 +65,13 @@ public function addBuilder(BreadcrumbBuilderInterface $builder, $priority) {
     $this->sortedBuilders = NULL;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function applies(array $attributes) {
+    return TRUE;
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -74,12 +81,14 @@ public function build(array $attributes) {
     // Call the build method of registered breadcrumb builders,
     // until one of them returns an array.
     foreach ($this->getSortedBuilders() as $builder) {
-      $build = $builder->build($attributes);
-      if (!isset($build)) {
-        // The builder returned NULL, so we continue with the other builders.
+      if (!$builder->applies($attributes)) {
+        // The builder does not apply, so we continue with the other builders.
         continue;
       }
-      elseif (is_array($build)) {
+
+      $build = $builder->build($attributes);
+
+      if (is_array($build)) {
         // The builder returned an array of breadcrumb links.
         $breadcrumb = $build;
         $context['builder'] = $builder;

core/modules/book/lib/Drupal/book/BookBreadcrumbBuilder.php

@@ -55,34 +55,41 @@ public function __construct(EntityManagerInterface $entity_manager, AccessManage
     $this->account = $account;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function applies(array $attributes) {
+    return !empty($attributes['node'])
+    && ($attributes['node'] instanceof NodeInterface)
+    && !empty($attributes['node']->book);
+  }
+
   /**
    * {@inheritdoc}
    */
   public function build(array $attributes) {
-    if (!empty($attributes['node']) && $attributes['node'] instanceof NodeInterface && !empty($attributes['node']->book)) {
-      $mlids = array();
-      $links = array($this->l($this->t('Home'), '<front>'));
-      $book = $attributes['node']->book;
+    $mlids = array();
+    $links = array($this->l($this->t('Home'), '<front>'));
+    $book = $attributes['node']->book;
+    $depth = 1;
+    // We skip the current node.
+    while (!empty($book['p' . ($depth + 1)])) {
+      $mlids[] = $book['p' . $depth];
+      $depth++;
+    }
+    $menu_links = $this->menuLinkStorage->loadMultiple($mlids);
+    if (count($menu_links) > 0) {
       $depth = 1;
-      // We skip the current node.
       while (!empty($book['p' . ($depth + 1)])) {
-        $mlids[] = $book['p' . $depth];
-        $depth++;
-      }
-      $menu_links = $this->menuLinkStorage->loadMultiple($mlids);
-      if (count($menu_links) > 0) {
-        $depth = 1;
-        while (!empty($book['p' . ($depth + 1)])) {
-          if (!empty($menu_links[$book['p' . $depth]]) && ($menu_link = $menu_links[$book['p' . $depth]])) {
-            if ($this->accessManager->checkNamedRoute($menu_link->route_name, $menu_link->route_parameters, $this->account)) {
-              $links[] = $this->l($menu_link->label(), $menu_link->route_name, $menu_link->route_parameters, $menu_link->options);
-            }
+        if (!empty($menu_links[$book['p' . $depth]]) && ($menu_link = $menu_links[$book['p' . $depth]])) {
+          if ($this->accessManager->checkNamedRoute($menu_link->route_name, $menu_link->route_parameters, $this->account)) {
+            $links[] = $this->l($menu_link->label(), $menu_link->route_name, $menu_link->route_parameters, $menu_link->options);
           }
-          $depth++;
         }
+        $depth++;
       }
-      return $links;
     }
+    return $links;
   }
 
 }

core/modules/comment/lib/Drupal/comment/CommentBreadcrumbBuilder.php

@@ -33,23 +33,29 @@ public function __construct(EntityManagerInterface $entity_manager) {
     $this->entityManager = $entity_manager;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function applies(array $attributes) {
+    return isset($attributes[RouteObjectInterface::ROUTE_NAME]) && $attributes[RouteObjectInterface::ROUTE_NAME] == 'comment.reply'
+    && isset($attributes['entity_type'])
+    && isset($attributes['entity_id'])
+    && isset($attributes['field_name']);
+  }
+
   /**
    * {@inheritdoc}
    */
   public function build(array $attributes) {
-    if (isset($attributes[RouteObjectInterface::ROUTE_NAME]) && $attributes[RouteObjectInterface::ROUTE_NAME] == 'comment.reply'
-      && isset($attributes['entity_type'])
-      && isset($attributes['entity_id'])
-      && isset($attributes['field_name'])
-      ) {
-      $breadcrumb[] = $this->l($this->t('Home'), '<front>');
-      $entity = $this->entityManager
-        ->getStorageController($attributes['entity_type'])
-        ->load($attributes['entity_id']);
-      $uri = $entity->uri();
-      $breadcrumb[] = l($entity->label(), $uri['path'], $uri['options']);
-      return $breadcrumb;
-    }
+    $breadcrumb = array();
+
+    $breadcrumb[] = $this->l($this->t('Home'), '<front>');
+    $entity = $this->entityManager
+      ->getStorageController($attributes['entity_type'])
+      ->load($attributes['entity_id']);
+    $uri = $entity->uri();
+    $breadcrumb[] = l($entity->label(), $uri['path'], $uri['options']);
+    return $breadcrumb;
   }
 
 }

core/modules/forum/lib/Drupal/forum/ForumBreadcrumbBuilder.php

@@ -55,20 +55,25 @@ public function __construct(EntityManagerInterface $entity_manager, ConfigFactor
     $this->forumManager = $forum_manager;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function applies(array $attributes) {
+    return !empty($attributes[RouteObjectInterface::ROUTE_NAME])
+    && (($attributes[RouteObjectInterface::ROUTE_NAME] == 'node.view' && isset($attributes['node']) && $this->forumManager->checkNodeType($attributes['node']))
+      || ($attributes[RouteObjectInterface::ROUTE_NAME] == 'forum.page' && isset($attributes['taxonomy_term']))
+    );
+  }
+
   /**
    * {@inheritdoc}
    */
   public function build(array $attributes) {
-    if (!empty($attributes[RouteObjectInterface::ROUTE_NAME])) {
-      $route_name = $attributes[RouteObjectInterface::ROUTE_NAME];
-      if ($route_name == 'node.view' && isset($attributes['node'])) {
-        if ($this->forumManager->checkNodeType($attributes['node'])) {
-          return $this->forumPostBreadcrumb($attributes['node']);
-        }
-      }
-      if ($route_name == 'forum.page' && isset($attributes['taxonomy_term'])) {
-        return $this->forumTermBreadcrumb($attributes['taxonomy_term']);
-      }
+    if ($attributes[RouteObjectInterface::ROUTE_NAME] == 'node.view') {
+      return $this->forumPostBreadcrumb($attributes['node']);
+    }
+    elseif ($attributes[RouteObjectInterface::ROUTE_NAME] == 'forum.page') {
+      return $this->forumTermBreadcrumb($attributes['taxonomy_term']);
     }
   }
 

core/modules/system/lib/Drupal/system/PathBasedBreadcrumbBuilder.php

@@ -102,6 +102,13 @@ public function __construct(Request $request, EntityManagerInterface $entity_man
     $this->titleResolver = $title_resolver;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function applies(array $attributes) {
+    return TRUE;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/taxonomy/lib/Drupal/taxonomy/TermBreadcrumbBuilder.php

@@ -15,24 +15,32 @@
  */
 class TermBreadcrumbBuilder extends BreadcrumbBuilderBase {
 
+  /**
+   * {@inheritdoc}
+   */
+  public function applies(array $attributes) {
+    return !empty($attributes[RouteObjectInterface::ROUTE_NAME])
+    && ($attributes[RouteObjectInterface::ROUTE_NAME] == 'taxonomy.term_page')
+    && ($attributes['taxonomy_term'] instanceof TermInterface);
+  }
+
   /**
    * {@inheritdoc}
    */
   public function build(array $attributes) {
-    if (!empty($attributes[RouteObjectInterface::ROUTE_NAME]) && $attributes[RouteObjectInterface::ROUTE_NAME] == 'taxonomy.term_page' && ($term = $attributes['taxonomy_term']) && $term instanceof TermInterface) {
-      // @todo This overrides any other possible breadcrumb and is a pure
-      //   hard-coded presumption. Make this behavior configurable per
-      //   vocabulary or term.
-      $breadcrumb = array();
-      while ($parents = taxonomy_term_load_parents($term->id())) {
-        $term = array_shift($parents);
-        $breadcrumb[] = $this->l($term->label(), 'taxonomy.term_page', array('taxonomy_term' => $term->id()));
-      }
-      $breadcrumb[] = $this->l($this->t('Home'), '<front>');
-      $breadcrumb = array_reverse($breadcrumb);
-
-      return $breadcrumb;
+    $term = $attributes['taxonomy_term'];
+    // @todo This overrides any other possible breadcrumb and is a pure
+    //   hard-coded presumption. Make this behavior configurable per
+    //   vocabulary or term.
+    $breadcrumb = array();
+    while ($parents = taxonomy_term_load_parents($term->id())) {
+      $term = array_shift($parents);
+      $breadcrumb[] = $this->l($term->label(), 'taxonomy.term_page', array('taxonomy_term' => $term->id()));
     }
+    $breadcrumb[] = $this->l($this->t('Home'), '<front>');
+    $breadcrumb = array_reverse($breadcrumb);
+
+    return $breadcrumb;
   }
 
 }

core/tests/Drupal/Tests/Core/Breadcrumb/BreadcrumbManagerTest.php

@@ -70,6 +70,10 @@ public function testBuildWithSingleBuilder() {
 
     $attributes = array('key' => 'value');
 
+    $builder->expects($this->once())
+      ->method('applies')
+      ->will($this->returnValue(TRUE));
+
     $builder->expects($this->once())
       ->method('build')
       ->will($this->returnValue($breadcrumb));
@@ -89,11 +93,16 @@ public function testBuildWithSingleBuilder() {
    */
   public function testBuildWithMultipleApplyingBuilders() {
     $builder1 = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface');
+    $builder1->expects($this->never())
+      ->method('applies');
     $builder1->expects($this->never())
       ->method('build');
 
     $builder2 = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface');
     $breadcrumb2 = array('<a href="/example2">Test2</a>');
+    $builder2->expects($this->once())
+      ->method('applies')
+      ->will($this->returnValue(TRUE));
     $builder2->expects($this->once())
       ->method('build')
       ->will($this->returnValue($breadcrumb2));
@@ -117,11 +126,16 @@ public function testBuildWithMultipleApplyingBuilders() {
   public function testBuildWithOneNotApplyingBuilders() {
     $builder1 = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface');
     $builder1->expects($this->once())
-      ->method('build')
-      ->will($this->returnValue(NULL));
+      ->method('applies')
+      ->will($this->returnValue(FALSE));
+    $builder1->expects($this->never())
+      ->method('build');
 
     $builder2 = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface');
     $breadcrumb2 = array('<a href="/example2">Test2</a>');
+    $builder2->expects($this->once())
+      ->method('applies')
+      ->will($this->returnValue(TRUE));
     $builder2->expects($this->once())
       ->method('build')
       ->will($this->returnValue($breadcrumb2));
@@ -146,6 +160,9 @@ public function testBuildWithOneNotApplyingBuilders() {
    */
   public function testBuildWithInvalidBreadcrumbResult() {
     $builder = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface');
+    $builder->expects($this->once())
+      ->method('applies')
+      ->will($this->returnValue(TRUE));
     $builder->expects($this->once())
       ->method('build')
       ->will($this->returnValue('invalid_result'));