From bb5bc5dbf8ea907ddd0e7d127c4b3942addb98fb Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Mon, 8 Jul 2024 08:33:00 +0100
Subject: [PATCH] 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/Breadcrumb/BreadcrumbBuilderInterface.php | 14 +++++++++++++-
.../Drupal/Core/Breadcrumb/BreadcrumbManager.php | 7 +++++--
.../comment/src/CommentBreadcrumbBuilder.php | 8 +++++++-
core/modules/help/src/HelpBreadcrumbBuilder.php | 6 +++++-
.../system/src/PathBasedBreadcrumbBuilder.php | 3 ++-
.../modules/taxonomy/src/TermBreadcrumbBuilder.php | 10 +++++++---
.../StandardPerformanceTest.php | 8 ++++----
7 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php b/core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php
index 10b9f638a8ec..5db7f9a2cd35 100644
--- a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php
+++ b/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.
*
diff --git a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php b/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php
index ff89c437fdbb..1c745ed6a262 100644
--- a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php
+++ b/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 {
diff --git a/core/modules/comment/src/CommentBreadcrumbBuilder.php b/core/modules/comment/src/CommentBreadcrumbBuilder.php
index 72291afbf10f..cec80bd99387 100644
--- a/core/modules/comment/src/CommentBreadcrumbBuilder.php
+++ b/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>'));
diff --git a/core/modules/help/src/HelpBreadcrumbBuilder.php b/core/modules/help/src/HelpBreadcrumbBuilder.php
index b5fb69925109..ce3f531042a4 100644
--- a/core/modules/help/src/HelpBreadcrumbBuilder.php
+++ b/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';
}
diff --git a/core/modules/system/src/PathBasedBreadcrumbBuilder.php b/core/modules/system/src/PathBasedBreadcrumbBuilder.php
index b28cba3ff6dc..26c3066581df 100644
--- a/core/modules/system/src/PathBasedBreadcrumbBuilder.php
+++ b/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;
}
diff --git a/core/modules/taxonomy/src/TermBreadcrumbBuilder.php b/core/modules/taxonomy/src/TermBreadcrumbBuilder.php
index 89252df4bee8..c66445cd76e5 100644
--- a/core/modules/taxonomy/src/TermBreadcrumbBuilder.php
+++ b/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;
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
index fa8ef92c0705..3a8eaaef51f9 100644
--- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
+++ b/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());
}
--
GitLab