Issue #2719721 by kristiaanvandeneynde, jhodgdon, BramDriesen, pameeela,...

Issue #2719721 by kristiaanvandeneynde, jhodgdon, BramDriesen, pameeela, catch, Wim Leers, Berdir, alexpott, neclimdul, acbramley, benjy, cilefen, mxr576, rothlive, andrewmacpherson: BreadcrumbBuilder::applies() mismatch with cacheability metadata

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

@@ -14,16 +14,28 @@ interface BreadcrumbBuilderInterface {
    *
    * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
    *   The current route match.
+   * phpcs:disable Drupal.Commenting
+   * @todo Uncomment new method parameters before drupal:12.0.0, see
+   *   https://www.drupal.org/project/drupal/issues/3459277.
+   *
+   * @param \Drupal\Core\Cache\CacheableMetadata $cacheable_metadata
+   *   The cacheable metadata to add to if your check varies by or depends
+   *   on something. Anything you specify here does not have to be repeated in
+   *   the build() method as it will be merged in automatically.
+   * phpcs:enable
    *
    * @return bool
    *   TRUE if this builder should be used or FALSE to let other builders
    *   decide.
    */
-  public function applies(RouteMatchInterface $route_match);
+  public function applies(RouteMatchInterface $route_match /* , CacheableMetadata $cacheable_metadata */);
 
   /**
    * Builds the breadcrumb.
    *
+   * There is no need to add any cacheable metadata that was already added in
+   * applies() as that will be automatically added for you.
+   *
    * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
    *   The current route match.
    *

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

@@ -2,6 +2,7 @@
 
 namespace Drupal\Core\Breadcrumb;
 
+use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Routing\RouteMatchInterface;
 
@@ -62,7 +63,7 @@ public function addBuilder(BreadcrumbBuilderInterface $builder, $priority) {
   /**
    * {@inheritdoc}
    */
-  public function applies(RouteMatchInterface $route_match) {
+  public function applies(RouteMatchInterface $route_match, ?CacheableMetadata $cacheable_metadata = NULL) {
     return TRUE;
   }
 
@@ -70,12 +71,13 @@ public function applies(RouteMatchInterface $route_match) {
    * {@inheritdoc}
    */
   public function build(RouteMatchInterface $route_match) {
+    $cacheable_metadata = new CacheableMetadata();
     $breadcrumb = new Breadcrumb();
     $context = ['builder' => NULL];
     // Call the build method of registered breadcrumb builders,
     // until one of them returns an array.
     foreach ($this->getSortedBuilders() as $builder) {
-      if (!$builder->applies($route_match)) {
+      if (!$builder->applies($route_match, $cacheable_metadata)) {
         // The builder does not apply, so we continue with the other builders.
         continue;
       }
@@ -84,6 +86,7 @@ public function build(RouteMatchInterface $route_match) {
 
       if ($breadcrumb instanceof Breadcrumb) {
         $context['builder'] = $builder;
+        $breadcrumb->addCacheableDependency($cacheable_metadata);
         break;
       }
       else {

core/modules/comment/src/CommentBreadcrumbBuilder.php

@@ -4,6 +4,7 @@
 
 use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface;
 use Drupal\Core\Breadcrumb\Breadcrumb;
+use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Link;
 use Drupal\Core\Routing\RouteMatchInterface;
@@ -35,7 +36,10 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager) {
   /**
    * {@inheritdoc}
    */
-  public function applies(RouteMatchInterface $route_match) {
+  public function applies(RouteMatchInterface $route_match, ?CacheableMetadata $cacheable_metadata = NULL) {
+    // @todo Remove null safe operator in Drupal 12.0.0, see
+    //   https://www.drupal.org/project/drupal/issues/3459277.
+    $cacheable_metadata?->addCacheContexts(['route']);
     return $route_match->getRouteName() == 'comment.reply' && $route_match->getParameter('entity');
   }
 
@@ -44,6 +48,8 @@ public function applies(RouteMatchInterface $route_match) {
    */
   public function build(RouteMatchInterface $route_match) {
     $breadcrumb = new Breadcrumb();
+    // @todo Remove in Drupal 12.0.0, will be added from ::applies(). See
+    //   https://www.drupal.org/project/drupal/issues/3459277
     $breadcrumb->addCacheContexts(['route']);
     $breadcrumb->addLink(Link::createFromRoute($this->t('Home'), '<front>'));
 

core/modules/help/src/HelpBreadcrumbBuilder.php

@@ -4,6 +4,7 @@
 
 use Drupal\Core\Breadcrumb\Breadcrumb;
 use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface;
+use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Core\Link;
 use Drupal\Core\Routing\RouteMatchInterface;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
@@ -19,7 +20,10 @@ class HelpBreadcrumbBuilder implements BreadcrumbBuilderInterface {
   /**
    * {@inheritdoc}
    */
-  public function applies(RouteMatchInterface $route_match) {
+  public function applies(RouteMatchInterface $route_match, ?CacheableMetadata $cacheable_metadata = NULL) {
+    // @todo Remove null safe operator in Drupal 12.0.0, see
+    //   https://www.drupal.org/project/drupal/issues/3459277.
+    $cacheable_metadata?->addCacheContexts(['route']);
     return $route_match->getRouteName() == 'help.help_topic';
   }
 

core/modules/system/src/PathBasedBreadcrumbBuilder.php

@@ -6,6 +6,7 @@
 use Drupal\Core\Access\AccessManagerInterface;
 use Drupal\Core\Breadcrumb\Breadcrumb;
 use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface;
+use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Controller\TitleResolverInterface;
 use Drupal\Core\Link;
@@ -134,7 +135,7 @@ public function __construct(RequestContext $context, AccessManagerInterface $acc
   /**
    * {@inheritdoc}
    */
-  public function applies(RouteMatchInterface $route_match) {
+  public function applies(RouteMatchInterface $route_match, ?CacheableMetadata $cacheable_metadata = NULL) {
     return TRUE;
   }
 

core/modules/taxonomy/src/TermBreadcrumbBuilder.php

@@ -4,6 +4,7 @@
 
 use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface;
 use Drupal\Core\Breadcrumb\Breadcrumb;
+use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Core\Entity\EntityRepositoryInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Link;
@@ -46,7 +47,10 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Ent
   /**
    * {@inheritdoc}
    */
-  public function applies(RouteMatchInterface $route_match) {
+  public function applies(RouteMatchInterface $route_match, ?CacheableMetadata $cacheable_metadata = NULL) {
+    // @todo Remove null safe operator in Drupal 12.0.0, see
+    //   https://www.drupal.org/project/drupal/issues/3459277.
+    $cacheable_metadata?->addCacheContexts(['route']);
     return $route_match->getRouteName() == 'entity.taxonomy_term.canonical'
       && $route_match->getParameter('taxonomy_term') instanceof TermInterface;
   }
@@ -74,8 +78,8 @@ public function build(RouteMatchInterface $route_match) {
       $breadcrumb->addLink(Link::createFromRoute($term->getName(), 'entity.taxonomy_term.canonical', ['taxonomy_term' => $term->id()]));
     }
 
-    // This breadcrumb builder is based on a route parameter, and hence it
-    // depends on the 'route' cache context.
+    // @todo Remove in Drupal 12.0.0, will be added from ::applies(). See
+    //   https://www.drupal.org/project/drupal/issues/3459277
     $breadcrumb->addCacheContexts(['route']);
 
     return $breadcrumb;

core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php

@@ -171,11 +171,11 @@ public function testAnonymous(): void {
     $recorded_queries = $performance_data->getQueries();
     $this->assertSame($expected_queries, $recorded_queries);
     $this->assertSame(12, $performance_data->getQueryCount());
-    $this->assertSame(76, $performance_data->getCacheGetCount());
-    $this->assertSame(15, $performance_data->getCacheSetCount());
+    $this->assertSame(78, $performance_data->getCacheGetCount());
+    $this->assertSame(16, $performance_data->getCacheSetCount());
     $this->assertSame(0, $performance_data->getCacheDeleteCount());
-    $this->assertSame(21, $performance_data->getCacheTagChecksumCount());
-    $this->assertSame(33, $performance_data->getCacheTagIsValidCount());
+    $this->assertSame(22, $performance_data->getCacheTagChecksumCount());
+    $this->assertSame(32, $performance_data->getCacheTagIsValidCount());
     $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());
   }