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

merge-requests/8696/head
Alex Pott 2024-07-08 08:33:00 +01:00
parent 82a5a70103
commit a75df83c57
No known key found for this signature in database
GPG Key ID: BDA67E7EE836E5CE
7 changed files with 43 additions and 13 deletions

View File

@ -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.
*

View File

@ -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 @@ class BreadcrumbManager implements ChainBreadcrumbBuilderInterface {
/**
* {@inheritdoc}
*/
public function applies(RouteMatchInterface $route_match) {
public function applies(RouteMatchInterface $route_match, ?CacheableMetadata $cacheable_metadata = NULL) {
return TRUE;
}
@ -70,12 +71,13 @@ class BreadcrumbManager implements ChainBreadcrumbBuilderInterface {
* {@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 @@ class BreadcrumbManager implements ChainBreadcrumbBuilderInterface {
if ($breadcrumb instanceof Breadcrumb) {
$context['builder'] = $builder;
$breadcrumb->addCacheableDependency($cacheable_metadata);
break;
}
else {

View File

@ -4,6 +4,7 @@ namespace Drupal\comment;
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 @@ class CommentBreadcrumbBuilder 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() == 'comment.reply' && $route_match->getParameter('entity');
}
@ -44,6 +48,8 @@ class CommentBreadcrumbBuilder implements BreadcrumbBuilderInterface {
*/
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>'));

View File

@ -4,6 +4,7 @@ namespace Drupal\help;
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';
}

View File

@ -6,6 +6,7 @@ use Drupal\Component\Utility\Unicode;
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 @@ class PathBasedBreadcrumbBuilder implements BreadcrumbBuilderInterface {
/**
* {@inheritdoc}
*/
public function applies(RouteMatchInterface $route_match) {
public function applies(RouteMatchInterface $route_match, ?CacheableMetadata $cacheable_metadata = NULL) {
return TRUE;
}

View File

@ -4,6 +4,7 @@ namespace Drupal\taxonomy;
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 @@ class TermBreadcrumbBuilder 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() == 'entity.taxonomy_term.canonical'
&& $route_match->getParameter('taxonomy_term') instanceof TermInterface;
}
@ -74,8 +78,8 @@ class TermBreadcrumbBuilder implements BreadcrumbBuilderInterface {
$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;

View File

@ -171,11 +171,11 @@ class StandardPerformanceTest extends PerformanceTestBase {
$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());
}