diff --git a/core/core.services.yml b/core/core.services.yml index 4eb462c..511436e 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -58,6 +58,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'] diff --git a/core/lib/Drupal/Core/Breadcrumb/Breadcrumb.php b/core/lib/Drupal/Core/Breadcrumb/Breadcrumb.php new file mode 100644 index 0000000..1a38f8b --- /dev/null +++ b/core/lib/Drupal/Core/Breadcrumb/Breadcrumb.php @@ -0,0 +1,62 @@ +links; + } + + /** + * Sets the breadcrumb links. + * + * @param \Drupal\Core\Link[] $links + * The breadcrumb links. + * + * @return $this + */ + public function setLinks(array $links) { + $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; + } + +} diff --git a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php b/core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php index ebdfa55..e566f54 100644 --- a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php +++ b/core/lib/Drupal/Core/Breadcrumb/BreadcrumbBuilderInterface.php @@ -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); diff --git a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php b/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php index 0015bf3..0513fcb 100644 --- a/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php +++ b/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php @@ -75,21 +75,24 @@ public function applies(RouteMatchInterface $route_match) { * {@inheritdoc} */ public function build(RouteMatchInterface $route_match) { - $breadcrumb = array(); + // Without builders return early. + $sorted_builders = $this->getSortedBuilders(); + if (empty($sorted_builders)) { + return new Breadcrumb(); + } + $context = array('builder' => NULL); // Call the build method of registered breadcrumb builders, // until one of them returns an array. - foreach ($this->getSortedBuilders() as $builder) { + foreach ($sorted_builders as $builder) { if (!$builder->applies($route_match)) { // The builder does not apply, so we continue with the other builders. 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 +102,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; } diff --git a/core/lib/Drupal/Core/Cache/Context/PathCacheContext.php b/core/lib/Drupal/Core/Cache/Context/PathCacheContext.php new file mode 100644 index 0000000..d4c3eee --- /dev/null +++ b/core/lib/Drupal/Core/Cache/Context/PathCacheContext.php @@ -0,0 +1,50 @@ +requestStack->getCurrentRequest(); + return $request->getBasePath() . $request->getPathInfo(); + } + + /** + * {@inheritdoc} + */ + public function getCacheableMetadata() { + return new CacheableMetadata(); + } + +} diff --git a/core/lib/Drupal/Core/Menu/menu.api.php b/core/lib/Drupal/Core/Menu/menu.api.php index 8550fc5..29e7eeb 100644 --- a/core/lib/Drupal/Core/Menu/menu.api.php +++ b/core/lib/Drupal/Core/Menu/menu.api.php @@ -546,12 +546,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('Home'); - * @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 @@ -562,9 +558,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')); } /** diff --git a/core/modules/book/book.services.yml b/core/modules/book/book.services.yml index 0a022a7..06affbb 100644 --- a/core/modules/book/book.services.yml +++ b/core/modules/book/book.services.yml @@ -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} diff --git a/core/modules/book/src/BookBreadcrumbBuilder.php b/core/modules/book/src/BookBreadcrumbBuilder.php index be0e63a..b1ece44 100644 --- a/core/modules/book/src/BookBreadcrumbBuilder.php +++ b/core/modules/book/src/BookBreadcrumbBuilder.php @@ -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'), '')); $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; } } diff --git a/core/modules/comment/src/CommentBreadcrumbBuilder.php b/core/modules/comment/src/CommentBreadcrumbBuilder.php index 8bc2f25..873b569 100644 --- a/core/modules/comment/src/CommentBreadcrumbBuilder.php +++ b/core/modules/comment/src/CommentBreadcrumbBuilder.php @@ -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'), '')]; + $breadcrumb = new Breadcrumb(); + $breadcrumb->setCacheContexts(['route']); + $breadcrumb->addLink(Link::createFromRoute($this->t('Home'), '')); $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; diff --git a/core/modules/forum/src/Breadcrumb/ForumBreadcrumbBuilderBase.php b/core/modules/forum/src/Breadcrumb/ForumBreadcrumbBuilderBase.php index f595ee8..f5fa2a8 100644 --- a/core/modules/forum/src/Breadcrumb/ForumBreadcrumbBuilderBase.php +++ b/core/modules/forum/src/Breadcrumb/ForumBreadcrumbBuilderBase.php @@ -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'), ''); + $breadcrumb = new Breadcrumb(); + $breadcrumb->setCacheContexts(['route']); + + $links[] = Link::createFromRoute($this->t('Home'), ''); $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); } } diff --git a/core/modules/forum/src/Breadcrumb/ForumListingBreadcrumbBuilder.php b/core/modules/forum/src/Breadcrumb/ForumListingBreadcrumbBuilder.php index 9d63772..5f52f2d 100644 --- a/core/modules/forum/src/Breadcrumb/ForumListingBreadcrumbBuilder.php +++ b/core/modules/forum/src/Breadcrumb/ForumListingBreadcrumbBuilder.php @@ -7,6 +7,7 @@ namespace Drupal\forum\Breadcrumb; +use Drupal\Core\Breadcrumb\Breadcrumb; use Drupal\Core\Link; use Drupal\Core\Routing\RouteMatchInterface; @@ -27,19 +28,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', array( 'taxonomy_term' => $parent->id(), - )); + ))); } } } + return $breadcrumb; } diff --git a/core/modules/forum/src/Breadcrumb/ForumNodeBreadcrumbBuilder.php b/core/modules/forum/src/Breadcrumb/ForumNodeBreadcrumbBuilder.php index 090f0ea..09923f3 100644 --- a/core/modules/forum/src/Breadcrumb/ForumNodeBreadcrumbBuilder.php +++ b/core/modules/forum/src/Breadcrumb/ForumNodeBreadcrumbBuilder.php @@ -29,18 +29,20 @@ 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->addLink(Link::createFromRoute($parent->label(), 'forum.page', array( 'taxonomy_term' => $parent->id(), ) - ); + )); } } + return $breadcrumb; } diff --git a/core/modules/forum/tests/src/Unit/Breadcrumb/ForumBreadcrumbBuilderBaseTest.php b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumBreadcrumbBuilderBaseTest.php index da7ff11..e171f53 100644 --- a/core/modules/forum/tests/src/Unit/Breadcrumb/ForumBreadcrumbBuilderBaseTest.php +++ b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumBreadcrumbBuilderBaseTest.php @@ -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 @@ -17,6 +19,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(). * * @covers ::__construct @@ -78,6 +96,15 @@ public function testBuild() { $vocab_item->expects($this->any()) ->method('label') ->will($this->returnValue('Fora_is_the_plural_of_forum')); + $vocab_item->expects($this->once()) + ->method('getCacheTags') + ->willReturn([]); + $vocab_item->expects($this->once()) + ->method('getCacheContexts') + ->willReturn([]); + $vocab_item->expects($this->once()) + ->method('getCacheMaxAge') + ->willReturn(Cache::PERMANENT); $vocab_storage = $this->getMock('Drupal\Core\Entity\EntityStorageInterface'); $vocab_storage->expects($this->any()) @@ -128,7 +155,7 @@ public function testBuild() { ); // And finally, the test. - $this->assertEquals($expected, $breadcrumb_builder->build($route_match)); + $this->assertEquals($expected, $breadcrumb_builder->build($route_match)->getLinks()); } } diff --git a/core/modules/forum/tests/src/Unit/Breadcrumb/ForumListingBreadcrumbBuilderTest.php b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumListingBreadcrumbBuilderTest.php index 95c670f..6331e86 100644 --- a/core/modules/forum/tests/src/Unit/Breadcrumb/ForumListingBreadcrumbBuilderTest.php +++ b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumListingBreadcrumbBuilderTest.php @@ -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 @@ -18,6 +20,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(). * * @param bool $expected @@ -114,6 +132,7 @@ public function testBuild() { $term1->expects($this->any()) ->method('id') ->will($this->returnValue(1)); + $this->addEmptyCacheableMetadata($term1); $term2 = $this->getMockBuilder('Drupal\taxonomy\Entity\Term') ->disableOriginalConstructor() @@ -124,6 +143,7 @@ public function testBuild() { $term2->expects($this->any()) ->method('id') ->will($this->returnValue(2)); + $this->addEmptyCacheableMetadata($term2); $forum_manager = $this->getMock('Drupal\forum\ForumManagerInterface'); $forum_manager->expects($this->at(0)) @@ -138,6 +158,7 @@ public function testBuild() { $vocab_item->expects($this->any()) ->method('label') ->will($this->returnValue('Fora_is_the_plural_of_forum')); + $this->addEmptyCacheableMetadata($vocab_item); $vocab_storage = $this->getMock('Drupal\Core\Entity\EntityStorageInterface'); $vocab_storage->expects($this->any()) ->method('load') @@ -183,6 +204,7 @@ public function testBuild() { $forum_listing->expects($this->any()) ->method('label') ->will($this->returnValue('You_should_not_see_this')); + $this->addEmptyCacheableMetadata($forum_listing); // Our data set. $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); @@ -197,7 +219,7 @@ 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)); + $this->assertEquals($expected1, $breadcrumb_builder->build($route_match)->getLinks()); // Second test. $expected2 = array( @@ -206,7 +228,24 @@ public function testBuild() { Link::createFromRoute('Something else', 'forum.page', array('taxonomy_term' => 2)), Link::createFromRoute('Something', 'forum.page', array('taxonomy_term' => 1)), ); - $this->assertEquals($expected2, $breadcrumb_builder->build($route_match)); + $this->assertEquals($expected2, $breadcrumb_builder->build($route_match)->getLinks()); + } + + /** + * Adds empty CacheableMetadata for objects. + * + * @param \PHPUnit_Framework_MockObject_MockObject $object + */ + protected function addEmptyCacheableMetadata(&$object) { + $object->expects($this->any()) + ->method('getCacheTags') + ->willReturn([]); + $object->expects($this->any()) + ->method('getCacheContexts') + ->willReturn([]); + $object->expects($this->any()) + ->method('getCacheMaxAge') + ->willReturn(Cache::PERMANENT); } } diff --git a/core/modules/forum/tests/src/Unit/Breadcrumb/ForumNodeBreadcrumbBuilderTest.php b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumNodeBreadcrumbBuilderTest.php index ec5dec0..2646ba1 100644 --- a/core/modules/forum/tests/src/Unit/Breadcrumb/ForumNodeBreadcrumbBuilderTest.php +++ b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumNodeBreadcrumbBuilderTest.php @@ -7,9 +7,10 @@ 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\ForumNodeBreadcrumbBuilder @@ -18,6 +19,22 @@ class ForumNodeBreadcrumbBuilderTest 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 ForumNodeBreadcrumbBuilder::applies(). * * @param bool $expected @@ -121,6 +138,7 @@ public function testBuild() { $term1->expects($this->any()) ->method('id') ->will($this->returnValue(1)); + $this->addEmptyCacheableMetadata($term1); $term2 = $this->getMockBuilder('Drupal\Core\Entity\EntityInterface') ->disableOriginalConstructor() @@ -131,6 +149,7 @@ public function testBuild() { $term2->expects($this->any()) ->method('id') ->will($this->returnValue(2)); + $this->addEmptyCacheableMetadata($term2); $forum_manager = $this->getMockBuilder('Drupal\forum\ForumManagerInterface') ->disableOriginalConstructor() @@ -146,6 +165,7 @@ public function testBuild() { $vocab_item->expects($this->any()) ->method('label') ->will($this->returnValue('Forums')); + $this->addEmptyCacheableMetadata($vocab_item); $vocab_storage = $this->getMock('Drupal\Core\Entity\EntityStorageInterface'); $vocab_storage->expects($this->any()) ->method('load') @@ -203,7 +223,7 @@ public function testBuild() { Link::createFromRoute('Forums', 'forum.index'), Link::createFromRoute('Something', 'forum.page', array('taxonomy_term' => 1)), ); - $this->assertEquals($expected1, $breadcrumb_builder->build($route_match)); + $this->assertEquals($expected1, $breadcrumb_builder->build($route_match)->getLinks()); // Second test. $expected2 = array( @@ -212,7 +232,24 @@ public function testBuild() { Link::createFromRoute('Something else', 'forum.page', array('taxonomy_term' => 2)), Link::createFromRoute('Something', 'forum.page', array('taxonomy_term' => 1)), ); - $this->assertEquals($expected2, $breadcrumb_builder->build($route_match)); + $this->assertEquals($expected2, $breadcrumb_builder->build($route_match)->getLinks()); + } + + /** + * Adds empty CacheableMetadata for objects. + * + * @param \PHPUnit_Framework_MockObject_MockObject $object + */ + protected function addEmptyCacheableMetadata(&$object) { + $object->expects($this->any()) + ->method('getCacheTags') + ->willReturn([]); + $object->expects($this->any()) + ->method('getCacheContexts') + ->willReturn([]); + $object->expects($this->any()) + ->method('getCacheMaxAge') + ->willReturn(Cache::PERMANENT); } } diff --git a/core/modules/menu_ui/menu_ui.module b/core/modules/menu_ui/menu_ui.module index f26dade..07545e8 100644 --- a/core/modules/menu_ui/menu_ui.module +++ b/core/modules/menu_ui/menu_ui.module @@ -486,14 +486,14 @@ function menu_ui_preprocess_block(&$variables) { /** * Implements hook_system_breadcrumb_alter(). */ -function menu_ui_system_breadcrumb_alter(array &$breadcrumb, RouteMatchInterface $route_match, array $context) { +function menu_ui_system_breadcrumb_alter(&$breadcrumb, RouteMatchInterface $route_match, array $context) { // Custom breadcrumb behavior for editing menu links, we append a link to // the menu in which the link is found. if (($route_match->getRouteName() == 'menu_ui.link_edit') && $menu_link = $route_match->getParameter('menu_link_plugin')) { if (($menu_link instanceof MenuLinkInterface)) { // Add a link to the menu admin screen. $menu = Menu::load($menu_link->getMenuName()); - $breadcrumb[] = Link::createFromRoute($menu->label(), 'entity.menu.edit_form', array('menu' => $menu->id())); + $breadcrumb->addLink(Link::createFromRoute($menu->label(), 'entity.menu.edit_form', array('menu' => $menu->id()))); } } } diff --git a/core/modules/node/src/Tests/NodeTranslationUITest.php b/core/modules/node/src/Tests/NodeTranslationUITest.php index c392750..20d95fd 100644 --- a/core/modules/node/src/Tests/NodeTranslationUITest.php +++ b/core/modules/node/src/Tests/NodeTranslationUITest.php @@ -33,7 +33,8 @@ class NodeTranslationUITest extends ContentTranslationUITestBase { 'route.menu_active_trails:main', 'route.menu_active_trails:tools', 'timezone', - 'user.roles' + 'user.roles', + 'url.path', ]; /** diff --git a/core/modules/system/src/PathBasedBreadcrumbBuilder.php b/core/modules/system/src/PathBasedBreadcrumbBuilder.php index c6e51af..2ee39b4 100644 --- a/core/modules/system/src/PathBasedBreadcrumbBuilder.php +++ b/core/modules/system/src/PathBasedBreadcrumbBuilder.php @@ -9,7 +9,9 @@ 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; @@ -125,6 +127,7 @@ public function applies(RouteMatchInterface $route_match) { * {@inheritdoc} */ public function build(RouteMatchInterface $route_match) { + $breadcrumb = new Breadcrumb(); $links = array(); // General path-based breadcrumbs. Use the actual request path, prior to @@ -139,17 +142,21 @@ public function build(RouteMatchInterface $route_match) { // /user is just a redirect, so skip it. // @todo Find a better way to deal with /user. $exclude['/user'] = TRUE; + // Because this breadcrumb builder is entirely path-based, vary by the 'url.path' + // cache context. + $breadcrumb->setCacheContexts(['url.path']); while (count($path_elements) > 1) { array_pop($path_elements); // Copy the path elements for up-casting. $route_request = $this->getRequestForPath('/' . implode('/', $path_elements), $exclude); if ($route_request) { $route_match = RouteMatch::createFromRequest($route_request); - $access = $this->accessManager->check($route_match, $this->currentUser); - if ($access) { + $access = $this->accessManager->check($route_match, $this->currentUser, NULL, TRUE); + // The set of breadcrumb links depends on the access result, so merge + // the access result's cacheability metadata. + $breadcrumb = $breadcrumb->merge(CacheableMetadata::createFromObject($access)); + if ($access->isAllowed()) { $title = $this->titleResolver->getTitle($route_request, $route_match->getRouteObject()); - } - if ($access) { if (!isset($title)) { // Fallback to using the raw path component as the title if the // route is missing a _title or _title_callback attribute. @@ -165,7 +172,8 @@ public function build(RouteMatchInterface $route_match) { // Add the Home link, except for the front page. $links[] = Link::createFromRoute($this->t('Home'), ''); } - return array_reverse($links); + + return $breadcrumb->setLinks(array_reverse($links)); } /** diff --git a/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php b/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php index c7629f0..40da616 100644 --- a/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php +++ b/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php @@ -77,20 +77,13 @@ public function build() { $breadcrumb = $this->breadcrumbManager->build($this->routeMatch); if (!empty($breadcrumb)) { // $breadcrumb is expected to be an array of rendered breadcrumb links. - return array( + $build = [ '#theme' => 'breadcrumb', - '#links' => $breadcrumb, - ); + '#links' => $breadcrumb->getLinks(), + ]; + $breadcrumb->applyTo($build); + return $build; } } - /** - * {@inheritdoc} - * - * @todo Make cacheable in https://www.drupal.org/node/2483183 - */ - public function getCacheMaxAge() { - return 0; - } - } diff --git a/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php b/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php index 0a00029..052f9f6 100644 --- a/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php +++ b/core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php @@ -8,11 +8,12 @@ namespace Drupal\Tests\system\Unit\Breadcrumbs; use Drupal\Core\Link; -use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultAllowed; use Drupal\Core\Session\AccountInterface; use Drupal\Core\StringTranslation\TranslationInterface; use Drupal\Core\Url; use Drupal\Core\Utility\LinkGeneratorInterface; +use Drupal\Core\Routing\RouteMatchInterface; use Drupal\system\PathBasedBreadcrumbBuilder; use Drupal\Tests\UnitTestCase; use Symfony\Cmf\Component\Routing\RouteObjectInterface; @@ -130,7 +131,7 @@ public function testBuildOnFrontpage() { ->will($this->returnValue('/')); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); - $this->assertEquals(array(), $links); + $this->assertEquals([], $links->getLinks()); } /** @@ -144,7 +145,7 @@ public function testBuildWithOnePathElement() { ->will($this->returnValue('/example')); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); - $this->assertEquals(array(0 => new Link('Home', new Url(''))), $links); + $this->assertEquals([0 => new Link('Home', new Url(''))], $links->getLinks()); } /** @@ -176,7 +177,7 @@ public function testBuildWithTwoPathElements() { $this->setupAccessManagerToAllow(); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); - $this->assertEquals(array(0 => new Link('Home', new Url('')), 1 => new Link('Example', new Url('example'))), $links); + $this->assertEquals([0 => new Link('Home', new Url('')), 1 => new Link('Example', new Url('example'))], $links->getLinks()); } /** @@ -216,11 +217,11 @@ public function testBuildWithThreePathElements() { $this->setupAccessManagerToAllow(); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); - $this->assertEquals(array( + $this->assertEquals([ new Link('Home', new Url('')), new Link('Example', new Url('example')), new Link('Bar', new Url('example_bar')), - ), $links); + ], $links->getLinks()); } /** @@ -244,7 +245,7 @@ public function testBuildWithException($exception_class, $exception_argument) { $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); // No path matched, though at least the frontpage is displayed. - $this->assertEquals(array(0 => new Link('Home', new Url(''))), $links); + $this->assertEquals([0 => new Link('Home', new Url(''))], $links->getLinks()); } /** @@ -285,7 +286,7 @@ public function testBuildWithNonProcessedPath() { $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); // No path matched, though at least the frontpage is displayed. - $this->assertEquals(array(0 => new Link('Home', new Url(''))), $links); + $this->assertEquals([0 => new Link('Home', new Url(''))], $links->getLinks()); } /** @@ -330,7 +331,7 @@ public function testBuildWithUserPath() { ->will($this->returnValue('Admin')); $links = $this->builder->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); - $this->assertEquals(array(0 => new Link('Home', new Url('')), 1 => new Link('Admin', new Url('user_page'))), $links); + $this->assertEquals([0 => new Link('Home', new Url('')), 1 => new Link('Admin', new Url('user_page'))], $links->getLinks()); } /** @@ -339,7 +340,7 @@ public function testBuildWithUserPath() { public function setupAccessManagerToAllow() { $this->accessManager->expects($this->any()) ->method('check') - ->willReturn(TRUE); + ->willReturn(new AccessResultAllowed()); } protected function setupStubPathProcessor() { diff --git a/core/modules/taxonomy/src/TermBreadcrumbBuilder.php b/core/modules/taxonomy/src/TermBreadcrumbBuilder.php index c2e47a1..ef1ec30 100644 --- a/core/modules/taxonomy/src/TermBreadcrumbBuilder.php +++ b/core/modules/taxonomy/src/TermBreadcrumbBuilder.php @@ -8,6 +8,8 @@ namespace Drupal\taxonomy; use Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface; +use Drupal\Core\Breadcrumb\Breadcrumb; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Link; use Drupal\Core\Routing\RouteMatchInterface; @@ -29,7 +31,7 @@ class TermBreadcrumbBuilder implements BreadcrumbBuilderInterface { /** * The taxonomy storage. * - * @var \Drupal\Core\Entity\EntityStorageInterface + * @var \Drupal\Core\Entity\TermStorageInterface */ protected $termStorage; @@ -56,18 +58,28 @@ public function applies(RouteMatchInterface $route_match) { * {@inheritdoc} */ public function build(RouteMatchInterface $route_match) { + $breadcrumb = new Breadcrumb(); + $breadcrumb->addLink(Link::createFromRoute($this->t('Home'), '')); $term = $route_match->getParameter('taxonomy_term'); + // Breadcrumb needs to have terms cacheable metadata as dependency even + // though its not shown on the breadcrumb because e.g. its parent might have + // changed. + $breadcrumb->addCacheableDependency($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 = $this->termStorage->loadParents($term->id())) { - $term = array_shift($parents); + $parents = $this->termStorage->loadAllParents($term->id()); + // Remove current term being accessed. + array_shift($parents); + foreach (array_reverse($parents) as $term) { $term = $this->entityManager->getTranslationFromContext($term); - $breadcrumb[] = Link::createFromRoute($term->getName(), 'entity.taxonomy_term.canonical', array('taxonomy_term' => $term->id())); + $breadcrumb->addCacheableDependency($term); + $breadcrumb->addLink(Link::createFromRoute($term->getName(), 'entity.taxonomy_term.canonical', array('taxonomy_term' => $term->id()))); } - $breadcrumb[] = Link::createFromRoute($this->t('Home'), ''); - $breadcrumb = array_reverse($breadcrumb); + + // This breadcrumb builder is based on a route parameter, and hence it + // depends on the 'route' cache context. + $breadcrumb->setCacheContexts(['route']); return $breadcrumb; } diff --git a/core/tests/Drupal/Tests/Core/Breadcrumb/BreadcrumbManagerTest.php b/core/tests/Drupal/Tests/Core/Breadcrumb/BreadcrumbManagerTest.php index a2cbbf0..4a12f93 100644 --- a/core/tests/Drupal/Tests/Core/Breadcrumb/BreadcrumbManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Breadcrumb/BreadcrumbManagerTest.php @@ -7,8 +7,10 @@ namespace Drupal\Tests\Core\Breadcrumb; +use Drupal\Core\Breadcrumb\Breadcrumb; use Drupal\Core\Breadcrumb\BreadcrumbManager; use Drupal\Tests\UnitTestCase; +use PHPUnit_Framework_Assert as Assert; /** * @coversDefaultClass \Drupal\Core\Breadcrumb\BreadcrumbManager @@ -17,6 +19,13 @@ class BreadcrumbManagerTest extends UnitTestCase { /** + * The breadcrumb object. + * + * @var \Drupal\Core\Breadcrumb\Breadcrumb + */ + protected $breadcrumb; + + /** * The tested breadcrumb manager. * * @var \Drupal\Core\Breadcrumb\BreadcrumbManager @@ -36,6 +45,7 @@ class BreadcrumbManagerTest extends UnitTestCase { protected function setUp() { $this->moduleHandler = $this->getMock('Drupal\Core\Extension\ModuleHandlerInterface'); $this->breadcrumbManager = new BreadcrumbManager($this->moduleHandler); + $this->breadcrumb = new Breadcrumb(); } /** @@ -43,7 +53,7 @@ protected function setUp() { */ public function testBuildWithoutBuilder() { $result = $this->breadcrumbManager->build($this->getMock('Drupal\Core\Routing\RouteMatchInterface')); - $this->assertEquals(array(), $result); + $this->assertEquals(array(), $result->getLinks()); } /** @@ -51,7 +61,8 @@ public function testBuildWithoutBuilder() { */ public function testBuildWithSingleBuilder() { $builder = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface'); - $breadcrumb = array('Test'); + $links = array('Test'); + $this->breadcrumb->setLinks($links); $builder->expects($this->once()) ->method('applies') @@ -59,17 +70,24 @@ public function testBuildWithSingleBuilder() { $builder->expects($this->once()) ->method('build') - ->will($this->returnValue($breadcrumb)); + ->willReturn($this->breadcrumb); $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); $this->moduleHandler->expects($this->once()) ->method('alter') - ->with('system_breadcrumb', $breadcrumb, $route_match, array('builder' => $builder)); + ->willReturnCallback(function($type, $data, $context1, $context2) use ($route_match, $links, $builder) { + Assert::assertEquals($type, 'system_breadcrumb', 'SingleTest: Module handler expected alter call to system_breadcrumb'); + $extracted_links = $data->getLinks(); + Assert::assertEquals($extracted_links, $links, 'SingleTest: Module handler was passed the correct link'); + Assert::assertEquals($context1, $route_match, 'SingleTest: Module handler was passed route_match.'); + Assert::assertEquals($context2, array('builder' => $builder), 'SingleTest: Module handler received the builder context'); + } + ); $this->breadcrumbManager->addBuilder($builder, 0); $result = $this->breadcrumbManager->build($route_match); - $this->assertEquals($breadcrumb, $result); + $this->assertEquals($links, $result->getLinks()); } /** @@ -83,25 +101,33 @@ public function testBuildWithMultipleApplyingBuilders() { ->method('build'); $builder2 = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface'); - $breadcrumb2 = array('Test2'); + $links2 = array('Test2'); + $this->breadcrumb->setLinks($links2); $builder2->expects($this->once()) ->method('applies') ->will($this->returnValue(TRUE)); $builder2->expects($this->once()) ->method('build') - ->will($this->returnValue($breadcrumb2)); + ->willReturn($this->breadcrumb); $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); $this->moduleHandler->expects($this->once()) ->method('alter') - ->with('system_breadcrumb', $breadcrumb2, $route_match, array('builder' => $builder2)); + ->willReturnCallback(function($type, $data, $context1, $context2) use ($links2, $route_match, $builder2) { + Assert::assertEquals($type, 'system_breadcrumb', 'MultipleApplying: Module handler expected alter call to system_breadcrumb'); + $extracted_links = $data->getLinks(); + Assert::assertEquals($extracted_links, $links2, 'MultipleApplying: Module handler was passed the correct link'); + Assert::assertEquals($context1, $route_match, 'MultipleApplying: Module handler was passed route_match.'); + Assert::assertEquals($context2, array('builder' => $builder2), 'MultipleApplying: Module handler received the builder context'); + } + ); $this->breadcrumbManager->addBuilder($builder1, 0); $this->breadcrumbManager->addBuilder($builder2, 10); $result = $this->breadcrumbManager->build($route_match); - $this->assertEquals($breadcrumb2, $result); + $this->assertEquals($links2, $result->getLinks()); } /** @@ -116,25 +142,33 @@ public function testBuildWithOneNotApplyingBuilders() { ->method('build'); $builder2 = $this->getMock('Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface'); - $breadcrumb2 = array('Test2'); + $links2 = ['Test2']; + $this->breadcrumb->setLinks($links2); $builder2->expects($this->once()) ->method('applies') ->will($this->returnValue(TRUE)); $builder2->expects($this->once()) ->method('build') - ->will($this->returnValue($breadcrumb2)); + ->willReturn($this->breadcrumb); $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); $this->moduleHandler->expects($this->once()) ->method('alter') - ->with('system_breadcrumb', $breadcrumb2, $route_match, array('builder' => $builder2)); + ->willReturnCallback(function($type, $data, $context1, $context2) use ($links2, $route_match, $builder2) { + Assert::assertEquals($type, 'system_breadcrumb', 'NotApplying: Module handler expected alter call to system_breadcrumb'); + $extracted_links = $data->getLinks(); + Assert::assertEquals($extracted_links, $links2, 'NotApplying: Module handler was passed the correct link'); + Assert::assertEquals($context1, $route_match, 'NotApplying: Module handler was passed route_match.'); + Assert::assertEquals($context2, array('builder' => $builder2), 'NotApplying: Module handler received the builder context'); + } + ); $this->breadcrumbManager->addBuilder($builder1, 10); $this->breadcrumbManager->addBuilder($builder2, 0); $result = $this->breadcrumbManager->build($route_match); - $this->assertEquals($breadcrumb2, $result); + $this->assertEquals($links2, $result->getLinks()); } /**