Commit bb96982e authored by alexpott's avatar alexpott

Issue #2483183 by lauriii, martin107, Wim Leers, tim.plunkett, moshe weitzman,...

Issue #2483183 by lauriii, martin107, Wim Leers, tim.plunkett, moshe weitzman, bfr, Fabianx, dawehner: Make breadcrumb block cacheable
parent 531d0a4e
......@@ -62,6 +62,11 @@ services:
arguments: ['@request_stack']
tags:
- { name: cache.context }
cache_context.url.path:
class: Drupal\Core\Cache\Context\PathCacheContext
arguments: ['@request_stack']
tags:
- { name: cache.context }
cache_context.url.query_args:
class: Drupal\Core\Cache\Context\QueryArgsCacheContext
arguments: ['@request_stack']
......
<?php
/**
* @file
* Contains \Drupal\Core\Breadcrumb\Breadcrumb.
*/
namespace Drupal\Core\Breadcrumb;
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Link;
/**
* Used to return generated breadcrumbs with associated cacheability metadata.
*
* @todo implement RenderableInterface once https://www.drupal.org/node/2529560 lands.
*/
class Breadcrumb extends CacheableMetadata {
/**
* An ordered list of links for the breadcrumb.
*
* @var \Drupal\Core\Link[]
*/
protected $links = [];
/**
* Gets the breadcrumb links.
*
* @return \Drupal\Core\Link[]
*/
public function getLinks() {
return $this->links;
}
/**
* Sets the breadcrumb links.
*
* @param \Drupal\Core\Link[] $links
* The breadcrumb links.
*
* @return $this
*
* @throws \LogicException
* Thrown when setting breadcrumb links after they've already been set.
*/
public function setLinks(array $links) {
if (!empty($this->links)) {
throw new \LogicException('Once breadcrumb links are set, only additional breadcrumb links can be added.');
}
$this->links = $links;
return $this;
}
/**
* Appends a link to the end of the ordered list of breadcrumb links.
*
* @param \Drupal\Core\Link $link
* The link appended to the breadcrumb.
*
* @return $this
*/
public function addLink(Link $link) {
$this->links[] = $link;
return $this;
}
}
......@@ -32,9 +32,8 @@ public function applies(RouteMatchInterface $route_match);
* @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The current route match.
*
* @return \Drupal\Core\Link[]
* An array of links for the breadcrumb. Returning an empty array will
* suppress all breadcrumbs.
* @return \Drupal\Core\Breadcrumb\Breadcrumb
* A breadcrumb.
*/
public function build(RouteMatchInterface $route_match);
......
......@@ -75,7 +75,7 @@ public function applies(RouteMatchInterface $route_match) {
* {@inheritdoc}
*/
public function build(RouteMatchInterface $route_match) {
$breadcrumb = array();
$breadcrumb = new Breadcrumb();
$context = array('builder' => NULL);
// Call the build method of registered breadcrumb builders,
// until one of them returns an array.
......@@ -85,11 +85,9 @@ public function build(RouteMatchInterface $route_match) {
continue;
}
$build = $builder->build($route_match);
$breadcrumb = $builder->build($route_match);
if (is_array($build)) {
// The builder returned an array of breadcrumb links.
$breadcrumb = $build;
if ($breadcrumb instanceof Breadcrumb) {
$context['builder'] = $builder;
break;
}
......@@ -99,7 +97,7 @@ public function build(RouteMatchInterface $route_match) {
}
// Allow modules to alter the breadcrumb.
$this->moduleHandler->alter('system_breadcrumb', $breadcrumb, $route_match, $context);
// Fall back to an empty breadcrumb.
return $breadcrumb;
}
......
<?php
/**
* @file
* Contains \Drupal\Core\Cache\Context\PathCacheContext.
*/
namespace Drupal\Core\Cache\Context;
use Drupal\Core\Cache\CacheableMetadata;
/**
* Defines the PathCacheContext service, for "per URL path" caching.
*
* Cache context ID: 'url.path'.
*
* (This allows for caching relative URLs.)
*
* @see \Symfony\Component\HttpFoundation\Request::getBasePath()
* @see \Symfony\Component\HttpFoundation\Request::getPathInfo()
*/
class PathCacheContext extends RequestStackCacheContextBase implements CacheContextInterface {
/**
* {@inheritdoc}
*/
public static function getLabel() {
return t('Path');
}
/**
* {@inheritdoc}
*/
public function getContext() {
$request = $this->requestStack->getCurrentRequest();
return $request->getBasePath() . $request->getPathInfo();
}
/**
* {@inheritdoc}
*/
public function getCacheableMetadata() {
return new CacheableMetadata();
}
}
......@@ -562,12 +562,8 @@ function hook_contextual_links_plugins_alter(array &$contextual_links) {
/**
* Perform alterations to the breadcrumb built by the BreadcrumbManager.
*
* @param array $breadcrumb
* An array of breadcrumb link a tags, returned by the breadcrumb manager
* build method, for example
* @code
* array('<a href="/">Home</a>');
* @endcode
* @param \Drupal\Core\Breadcrumb\Breadcrumb $breadcrumb
* A breadcrumb object returned by BreadcrumbBuilderInterface::build().
* @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The current route match.
* @param array $context
......@@ -578,9 +574,9 @@ function hook_contextual_links_plugins_alter(array &$contextual_links) {
*
* @ingroup menu
*/
function hook_system_breadcrumb_alter(array &$breadcrumb, \Drupal\Core\Routing\RouteMatchInterface $route_match, array $context) {
function hook_system_breadcrumb_alter(\Drupal\Core\Breadcrumb\Breadcrumb &$breadcrumb, \Drupal\Core\Routing\RouteMatchInterface $route_match, array $context) {
// Add an item to the end of the breadcrumb.
$breadcrumb[] = Drupal::l(t('Text'), 'example_route_name');
$breadcrumb->addLink(Drupal::l(t('Text'), 'example_route_name'));
}
/**
......
......@@ -26,6 +26,8 @@ services:
cache_context.route.book_navigation:
class: Drupal\book\Cache\BookNavigationCacheContext
arguments: ['@request_stack']
calls:
- [setContainer, ['@service_container']]
tags:
- { name: cache.context}
......
......@@ -8,6 +8,7 @@
namespace Drupal\book;
use Drupal\Core\Access\AccessManagerInterface;
use Drupal\Core\Breadcrumb\Breadcrumb;
use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface;
use Drupal\Core\Entity\EntityManagerInterface;
use Drupal\Core\Link;
......@@ -72,6 +73,8 @@ public function applies(RouteMatchInterface $route_match) {
*/
public function build(RouteMatchInterface $route_match) {
$book_nids = array();
$breadcrumb = new Breadcrumb();
$links = array(Link::createFromRoute($this->t('Home'), '<front>'));
$book = $route_match->getParameter('node')->book;
$depth = 1;
......@@ -92,7 +95,9 @@ public function build(RouteMatchInterface $route_match) {
$depth++;
}
}
return $links;
$breadcrumb->setLinks($links);
$breadcrumb->setCacheContexts(['route.book_navigation']);
return $breadcrumb;
}
}
......@@ -24,7 +24,7 @@ class BookTest extends WebTestBase {
*
* @var array
*/
public static $modules = array('book', 'block', 'node_access_test');
public static $modules = array('book', 'block', 'node_access_test', 'book_test');
/**
* A book node.
......@@ -109,6 +109,45 @@ function createBook() {
return $nodes;
}
/**
* Tests the book navigation cache context.
*
* @see \Drupal\book\Cache\BookNavigationCacheContext
*/
public function testBookNavigationCacheContext() {
// Create a page node.
$this->drupalCreateContentType(['type' => 'page']);
$page = $this->drupalCreateNode();
// Create a book, consisting of book nodes.
$book_nodes = $this->createBook();
// Enable the debug output.
\Drupal::state()->set('book_test.debug_book_navigation_cache_context', TRUE);
$this->drupalLogin($this->bookAuthor);
// On non-node route.
$this->drupalGet('');
$this->assertRaw('[route.book_navigation]=book.none');
// On non-book node route.
$this->drupalGet($page->urlInfo());
$this->assertRaw('[route.book_navigation]=book.none');
// On book node route.
$this->drupalGet($book_nodes[0]->urlInfo());
$this->assertRaw('[route.book_navigation]=0|2|3');
$this->drupalGet($book_nodes[1]->urlInfo());
$this->assertRaw('[route.book_navigation]=0|2|3|4');
$this->drupalGet($book_nodes[2]->urlInfo());
$this->assertRaw('[route.book_navigation]=0|2|3|5');
$this->drupalGet($book_nodes[3]->urlInfo());
$this->assertRaw('[route.book_navigation]=0|2|6');
$this->drupalGet($book_nodes[4]->urlInfo());
$this->assertRaw('[route.book_navigation]=0|2|7');
}
/**
* Tests saving the book outline on an empty book.
*/
......@@ -303,7 +342,7 @@ function createBookNode($book_nid, $parent = NULL) {
static $number = 0; // Used to ensure that when sorted nodes stay in same order.
$edit = array();
$edit['title[0][value]'] = $number . ' - SimpleTest test node ' . $this->randomMachineName(10);
$edit['title[0][value]'] = str_pad($number, 2, '0', STR_PAD_LEFT) . ' - SimpleTest test node ' . $this->randomMachineName(10);
$edit['body[0][value]'] = 'SimpleTest test body ' . $this->randomMachineName(32) . ' ' . $this->randomMachineName(32);
$edit['book[bid]'] = $book_nid;
......
name: 'Book module tests'
type: module
description: 'Support module for book module testing.'
package: Testing
version: VERSION
core: 8.x
<?php
/**
* @file
* Test module for testing the book module.
*
* This module's functionality depends on the following state variables:
* - book_test.debug_book_navigation_cache_context: Used in NodeQueryAlterTest to enable the
* node_access_all grant realm.
*
* @see \Drupal\book\Tests\BookTest::testBookNavigationCacheContext()
*/
/**
* Implements hook_page_attachments().
*/
function book_test_page_attachments(array &$page) {
if (\Drupal::state()->get('book_test.debug_book_navigation_cache_context', FALSE)) {
drupal_set_message(\Drupal::service('cache_contexts_manager')->convertTokensToKeys(['route.book_navigation'])->getKeys()[0]);
}
}
......@@ -8,6 +8,7 @@
namespace Drupal\comment;
use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface;
use Drupal\Core\Breadcrumb\Breadcrumb;
use Drupal\Core\Entity\EntityManagerInterface;
use Drupal\Core\Link;
use Drupal\Core\Routing\RouteMatchInterface;
......@@ -47,16 +48,20 @@ public function applies(RouteMatchInterface $route_match) {
* {@inheritdoc}
*/
public function build(RouteMatchInterface $route_match) {
$breadcrumb = [Link::createFromRoute($this->t('Home'), '<front>')];
$breadcrumb = new Breadcrumb();
$breadcrumb->setCacheContexts(['route']);
$breadcrumb->addLink(Link::createFromRoute($this->t('Home'), '<front>'));
$entity = $route_match->getParameter('entity');
$breadcrumb[] = new Link($entity->label(), $entity->urlInfo());
$breadcrumb->addLink(new Link($entity->label(), $entity->urlInfo()));
$breadcrumb->addCacheableDependency($entity);
if (($pid = $route_match->getParameter('pid')) && ($comment = $this->storage->load($pid))) {
/** @var \Drupal\comment\CommentInterface $comment */
$breadcrumb->addCacheableDependency($comment);
// Display link to parent comment.
// @todo Clean-up permalink in https://www.drupal.org/node/2198041
$breadcrumb[] = new Link($comment->getSubject(), $comment->urlInfo());
$breadcrumb->addLink(new Link($comment->getSubject(), $comment->urlInfo()));
}
return $breadcrumb;
......
......@@ -8,6 +8,7 @@
namespace Drupal\forum\Breadcrumb;
use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface;
use Drupal\Core\Breadcrumb\Breadcrumb;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Entity\EntityManagerInterface;
use Drupal\Core\Link;
......@@ -65,14 +66,18 @@ public function __construct(EntityManagerInterface $entity_manager, ConfigFactor
* {@inheritdoc}
*/
public function build(RouteMatchInterface $route_match) {
$breadcrumb[] = Link::createFromRoute($this->t('Home'), '<front>');
$breadcrumb = new Breadcrumb();
$breadcrumb->setCacheContexts(['route']);
$links[] = Link::createFromRoute($this->t('Home'), '<front>');
$vocabulary = $this->entityManager
->getStorage('taxonomy_vocabulary')
->load($this->config->get('vocabulary'));
$breadcrumb[] = Link::createFromRoute($vocabulary->label(), 'forum.index');
$breadcrumb->addCacheableDependency($vocabulary);
$links[] = Link::createFromRoute($vocabulary->label(), 'forum.index');
return $breadcrumb;
return $breadcrumb->setLinks($links);
}
}
......@@ -27,19 +27,26 @@ public function applies(RouteMatchInterface $route_match) {
*/
public function build(RouteMatchInterface $route_match) {
$breadcrumb = parent::build($route_match);
$breadcrumb->addCacheContexts(['route']);
// Add all parent forums to breadcrumbs.
$term_id = $route_match->getParameter('taxonomy_term')->id();
/** @var \Drupal\Taxonomy\TermInterface $term */
$term = $route_match->getParameter('taxonomy_term');
$term_id = $term->id();
$breadcrumb->addCacheableDependency($term);
$parents = $this->forumManager->getParents($term_id);
if ($parents) {
foreach (array_reverse($parents) as $parent) {
if ($parent->id() != $term_id) {
$breadcrumb[] = Link::createFromRoute($parent->label(), 'forum.page', array(
$breadcrumb->addCacheableDependency($parent);
$breadcrumb->addLink(Link::createFromRoute($parent->label(), 'forum.page', [
'taxonomy_term' => $parent->id(),
));
]));
}
}
}
return $breadcrumb;
}
......
......@@ -29,18 +29,21 @@ public function applies(RouteMatchInterface $route_match) {
*/
public function build(RouteMatchInterface $route_match) {
$breadcrumb = parent::build($route_match);
$breadcrumb->addCacheContexts(['route']);
$parents = $this->forumManager->getParents($route_match->getParameter('node')->forum_tid);
if ($parents) {
$parents = array_reverse($parents);
foreach ($parents as $parent) {
$breadcrumb[] = Link::createFromRoute($parent->label(), 'forum.page',
$breadcrumb->addCacheableDependency($parent);
$breadcrumb->addLink(Link::createFromRoute($parent->label(), 'forum.page',
array(
'taxonomy_term' => $parent->id(),
)
);
));
}
}
return $breadcrumb;
}
......
......@@ -7,8 +7,10 @@
namespace Drupal\Tests\forum\Unit\Breadcrumb;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Link;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\DependencyInjection\Container;
/**
* @coversDefaultClass \Drupal\forum\Breadcrumb\ForumBreadcrumbBuilderBase
......@@ -16,6 +18,22 @@
*/
class ForumBreadcrumbBuilderBaseTest extends UnitTestCase {
/**
* {@inheritdoc}
*/
public function setUp() {
parent::setUp();
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->expects($this->any())
->method('validate_tokens');
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
}
/**
* Tests ForumBreadcrumbBuilderBase::__construct().
*
......@@ -74,16 +92,18 @@ public function testBuild() {
->disableOriginalConstructor()
->getMock();
$vocab_item = $this->getMock('Drupal\taxonomy\VocabularyInterface');
$vocab_item->expects($this->any())
->method('label')
->will($this->returnValue('Fora_is_the_plural_of_forum'));
$prophecy = $this->prophesize('Drupal\taxonomy\VocabularyInterface');
$prophecy->label()->willReturn('Fora_is_the_plural_of_forum');
$prophecy->id()->willReturn(5);
$prophecy->getCacheTags()->willReturn(['taxonomy_vocabulary:5']);
$prophecy->getCacheContexts()->willReturn([]);
$prophecy->getCacheMaxAge()->willReturn(Cache::PERMANENT);
$vocab_storage = $this->getMock('Drupal\Core\Entity\EntityStorageInterface');
$vocab_storage->expects($this->any())
->method('load')
->will($this->returnValueMap(array(
array('forums', $vocab_item),
array('forums', $prophecy->reveal()),
)));
$entity_manager = $this->getMockBuilder('Drupal\Core\Entity\EntityManagerInterface')
......@@ -128,7 +148,11 @@ public function testBuild() {
);
// And finally, the test.
$this->assertEquals($expected, $breadcrumb_builder->build($route_match));
$breadcrumb = $breadcrumb_builder->build($route_match);
$this->assertEquals($expected, $breadcrumb->getLinks());
$this->assertEquals(['route'], $breadcrumb->getCacheContexts());
$this->assertEquals(['taxonomy_vocabulary:5'], $breadcrumb->getCacheTags());
$this->assertEquals(Cache::PERMANENT, $breadcrumb->getCacheMaxAge());
}
}
......@@ -7,9 +7,11 @@
namespace Drupal\Tests\forum\Unit\Breadcrumb;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Link;
use Drupal\Tests\UnitTestCase;
use Symfony\Cmf\Component\Routing\RouteObjectInterface;
use Symfony\Component\DependencyInjection\Container;
/**
* @coversDefaultClass \Drupal\forum\Breadcrumb\ForumListingBreadcrumbBuilder
......@@ -17,6 +19,22 @@
*/
class ForumListingBreadcrumbBuilderTest extends UnitTestCase {
/**
* {@inheritdoc}
*/
public function setUp() {
parent::setUp();
$cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\Context\CacheContextsManager')
->disableOriginalConstructor()
->getMock();
$cache_contexts_manager->expects($this->any())
->method('validate_tokens');
$container = new Container();
$container->set('cache_contexts_manager', $cache_contexts_manager);
\Drupal::setContainer($container);
}
/**
* Tests ForumListingBreadcrumbBuilder::applies().
*
......@@ -105,25 +123,21 @@ public function providerTestApplies() {
*/
public function testBuild() {
// Build all our dependencies, backwards.
$term1 = $this->getMockBuilder('Drupal\taxonomy\Entity\Term')
->disableOriginalConstructor()
->getMock();
$term1->expects($this->any())
->method('label')
->will($this->returnValue('Something'));
$term1->expects($this->any())
->method('id')
->will($this->returnValue(1));
$term2 = $this->getMockBuilder('Drupal\taxonomy\Entity\Term')
->disableOriginalConstructor()
->getMock();
$term2->expects($this->any())
->method('label')
->will($this->returnValue('Something else'));
$term2->expects($this->any())
->method('id')
->will($this->returnValue(2));
$prophecy = $this->prophesize('Drupal\taxonomy\Entity\Term');
$prophecy->label()->willReturn('Something');
$prophecy->id()->willReturn(1);
$prophecy->getCacheTags()->willReturn(['taxonomy_term:1']);
$prophecy->getCacheContexts()->willReturn([]);
$prophecy->getCacheMaxAge()->willReturn(Cache::PERMANENT);
$term1 = $prophecy->reveal();
$prophecy = $this->prophesize('Drupal\taxonomy\Entity\Term');
$prophecy->label()->willReturn('Something else');
$prophecy->id()->willReturn(2);
$prophecy->getCacheTags()->willReturn(['taxonomy_term:2']);
$prophecy->getCacheContexts()->willReturn([]);
$prophecy->getCacheMaxAge()->willReturn(Cache::PERMANENT);
$term2 = $prophecy->reveal();
$forum_manager = $this->getMock('Drupal\forum\ForumManagerInterface');
$forum_manager->expects($this->at(0))
......@@ -134,15 +148,17 @@ public function testBuild() {
->will($this->returnValue(array($term1, $term2)));
// The root forum.
$vocab_item = $this->getMock('Drupal\taxonomy\VocabularyInterface');
$vocab_item->expects($this->any())
->method('label')
->will($this->returnValue('Fora_is_the_plural_of_forum'));
$prophecy = $this->prophesize('Drupal\taxonomy\VocabularyInterface');
$prophecy->label()->willReturn('Fora_is_the_plural_of_forum');
$prophecy->id()->willReturn(5);
$prophecy->getCacheTags()->willReturn(['taxonomy_vocabulary:5']);
$prophecy->getCacheContexts()->willReturn([]);
$prophecy->getCacheMaxAge()->willReturn(Cache::PERMANENT);
$vocab_storage = $this->getMock('Drupal\Core\Entity\EntityStorageInterface');
$vocab_storage->expects($this->any())
->method('load')
->will($this->returnValueMap(array(
array('forums', $vocab_item),
array('forums', $prophecy->reveal()),
)));
$entity_manager = $this->getMockBuilder('Drupal\Core\Entity\EntityManagerInterface')
......@@ -176,13 +192,13 @@ public function testBuild() {
$breadcrumb_builder->setStringTranslation($translation_manager);
// The forum listing we need a breadcrumb back from.
$forum_listing = $this->getMockBuilder('Drupal\taxonomy\Entity\Term')
->disableOriginalConstructor()
->getMock();
$forum_listing->tid = 23;
$forum_listing->expects($this->any())
->method('label')
->will($this->returnValue('You_should_not_see_this'));
$prophecy = $this->prophesize('Drupal\taxonomy\Entity\Term');
$prophecy->label()->willReturn('You_should_not_see_this');
$prophecy->id()->willReturn(23);
$prophecy->getCacheTags()->willReturn(['taxonomy_term:23']);
$prophecy->getCacheContexts()->willReturn([]);
$prophecy->getCacheMaxAge()->willReturn(Cache::PERMANENT);
$forum_listing = $prophecy->reveal();
// Our data set.
$route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface');
......@@ -197,7 +213,11 @@ public function testBuild() {
Link::createFromRoute('Fora_is_the_plural_of_forum', 'forum.index'),
Link::createFromRoute('Something', 'forum.page', array('taxonomy_term' => 1)),
);
$this->assertEquals($expected1, $breadcrumb_builder->build($route_match));
$breadcrumb = $breadcrumb_builder->build($route_match);
$this->assertEquals($expected1, $breadcrumb->getLinks());
$this->assertEquals(['route'], $breadcrumb->getCacheContexts());
$this->assertEquals(['taxonomy_term:1', 'taxonomy_term:23', 'taxonomy_vocabulary:5'], $breadcrumb->getCacheTags());
$this->assertEquals(Cache::PERMANENT, $breadcrumb->getCacheMaxAge());
// Second test.