Issue #2575827 by msuthars, paulocs, ayushmishra206, hitesh-jain, znerol, mindbet, longwave, alexpott, dawehner: Use RouteMatch instead of request attributes in book navigation block

merge-requests/25/head
catch 2020-11-03 10:50:54 +00:00
parent e18d1d1098
commit a8ef4362e6
4 changed files with 51 additions and 23 deletions

View File

@ -25,7 +25,7 @@ services:
- { name: access_check, applies_to: _access_book_removable } - { name: access_check, applies_to: _access_book_removable }
cache_context.route.book_navigation: cache_context.route.book_navigation:
class: Drupal\book\Cache\BookNavigationCacheContext class: Drupal\book\Cache\BookNavigationCacheContext
arguments: ['@request_stack'] arguments: ['@current_route_match']
calls: calls:
- [setContainer, ['@service_container']] - [setContainer, ['@service_container']]
tags: tags:

View File

@ -2,11 +2,13 @@
namespace Drupal\book\Cache; namespace Drupal\book\Cache;
use Drupal\Core\DependencyInjection\DeprecatedServicePropertyTrait;
use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Cache\Context\CacheContextInterface; use Drupal\Core\Cache\Context\CacheContextInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\node\NodeInterface;
use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait; use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\HttpFoundation\RequestStack;
/** /**
* Defines the book navigation cache context service. * Defines the book navigation cache context service.
@ -23,22 +25,32 @@ use Symfony\Component\HttpFoundation\RequestStack;
class BookNavigationCacheContext implements CacheContextInterface, ContainerAwareInterface { class BookNavigationCacheContext implements CacheContextInterface, ContainerAwareInterface {
use ContainerAwareTrait; use ContainerAwareTrait;
use DeprecatedServicePropertyTrait;
/** /**
* The request stack. * The current route match.
* *
* @var \Symfony\Component\HttpFoundation\RequestStack * @var \Drupal\Core\Routing\RouteMatchInterface
*/ */
protected $requestStack; protected $routeMatch;
/**
* {@inheritdoc}
*/
protected $deprecatedProperties = ['request_stack' => 'request_stack'];
/** /**
* Constructs a new BookNavigationCacheContext service. * Constructs a new BookNavigationCacheContext service.
* *
* @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The request stack. * The current route match.
*/ */
public function __construct(RequestStack $request_stack) { public function __construct($route_match) {
$this->requestStack = $request_stack; if (!$route_match instanceof RouteMatchInterface) {
@trigger_error('Passing the request_stack service to ' . __METHOD__ . '() is deprecated in drupal:9.2.0 and will be removed before drupal:10.0.0. The parameter should be an instance of \Drupal\Core\Routing\RouteMatchInterface instead.', E_USER_DEPRECATED);
$route_match = \Drupal::routeMatch();
}
$this->routeMatch = $route_match;
} }
/** /**
@ -54,8 +66,9 @@ class BookNavigationCacheContext implements CacheContextInterface, ContainerAwar
public function getContext() { public function getContext() {
// Find the current book's ID. // Find the current book's ID.
$current_bid = 0; $current_bid = 0;
if ($node = $this->requestStack->getCurrentRequest()->get('node')) { $node = $this->routeMatch->getParameter('node');
$current_bid = empty($node->book['bid']) ? 0 : $node->book['bid']; if ($node instanceof NodeInterface && !empty($node->book['bid'])) {
$current_bid = $node->book['bid'];
} }
// If we're not looking at a book node, then we're not navigating a book. // If we're not looking at a book node, then we're not navigating a book.
@ -76,7 +89,8 @@ class BookNavigationCacheContext implements CacheContextInterface, ContainerAwar
// The book active trail depends on the node and data attached to it. // The book active trail depends on the node and data attached to it.
// That information is however not stored as part of the node. // That information is however not stored as part of the node.
$cacheable_metadata = new CacheableMetadata(); $cacheable_metadata = new CacheableMetadata();
if ($node = $this->requestStack->getCurrentRequest()->get('node')) { $node = $this->routeMatch->getParameter('node');
if ($node instanceof NodeInterface) {
// If the node is part of a book then we can use the cache tag for that // If the node is part of a book then we can use the cache tag for that
// book. If not, then it can't be optimized away. // book. If not, then it can't be optimized away.
if (!empty($node->book['bid'])) { if (!empty($node->book['bid'])) {

View File

@ -9,7 +9,7 @@ use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\node\NodeInterface; use Drupal\node\NodeInterface;
use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\RequestStack; use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityStorageInterface;
/** /**
@ -24,11 +24,11 @@ use Drupal\Core\Entity\EntityStorageInterface;
class BookNavigationBlock extends BlockBase implements ContainerFactoryPluginInterface { class BookNavigationBlock extends BlockBase implements ContainerFactoryPluginInterface {
/** /**
* The request object. * The current route match.
* *
* @var \Symfony\Component\HttpFoundation\RequestStack * @var \Drupal\Core\Routing\RouteMatchInterface
*/ */
protected $requestStack; protected $routeMatch;
/** /**
* The book manager. * The book manager.
@ -53,17 +53,17 @@ class BookNavigationBlock extends BlockBase implements ContainerFactoryPluginInt
* The plugin_id for the plugin instance. * The plugin_id for the plugin instance.
* @param mixed $plugin_definition * @param mixed $plugin_definition
* The plugin implementation definition. * The plugin implementation definition.
* @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The request stack object. * The current route match.
* @param \Drupal\book\BookManagerInterface $book_manager * @param \Drupal\book\BookManagerInterface $book_manager
* The book manager. * The book manager.
* @param \Drupal\Core\Entity\EntityStorageInterface $node_storage * @param \Drupal\Core\Entity\EntityStorageInterface $node_storage
* The node storage. * The node storage.
*/ */
public function __construct(array $configuration, $plugin_id, $plugin_definition, RequestStack $request_stack, BookManagerInterface $book_manager, EntityStorageInterface $node_storage) { public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteMatchInterface $route_match, BookManagerInterface $book_manager, EntityStorageInterface $node_storage) {
parent::__construct($configuration, $plugin_id, $plugin_definition); parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->requestStack = $request_stack; $this->routeMatch = $route_match;
$this->bookManager = $book_manager; $this->bookManager = $book_manager;
$this->nodeStorage = $node_storage; $this->nodeStorage = $node_storage;
} }
@ -76,7 +76,7 @@ class BookNavigationBlock extends BlockBase implements ContainerFactoryPluginInt
$configuration, $configuration,
$plugin_id, $plugin_id,
$plugin_definition, $plugin_definition,
$container->get('request_stack'), $container->get('current_route_match'),
$container->get('book.manager'), $container->get('book.manager'),
$container->get('entity_type.manager')->getStorage('node') $container->get('entity_type.manager')->getStorage('node')
); );
@ -123,8 +123,9 @@ class BookNavigationBlock extends BlockBase implements ContainerFactoryPluginInt
public function build() { public function build() {
$current_bid = 0; $current_bid = 0;
if ($node = $this->requestStack->getCurrentRequest()->get('node')) { $node = $this->routeMatch->getParameter('node');
$current_bid = empty($node->book['bid']) ? 0 : $node->book['bid']; if ($node instanceof NodeInterface && !empty($node->book['bid'])) {
$current_bid = $node->book['bid'];
} }
if ($this->configuration['block_mode'] == 'all pages') { if ($this->configuration['block_mode'] == 'all pages') {
$book_menus = []; $book_menus = [];

View File

@ -6,6 +6,7 @@ use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\Cache;
use Drupal\Tests\BrowserTestBase; use Drupal\Tests\BrowserTestBase;
use Drupal\user\RoleInterface; use Drupal\user\RoleInterface;
use Drupal\book\Cache\BookNavigationCacheContext;
/** /**
* Create a book, add pages, and test book interface. * Create a book, add pages, and test book interface.
@ -97,6 +98,18 @@ class BookTest extends BrowserTestBase {
]); ]);
} }
/**
* Test the book navigation cache context argument deprecation.
*
* @group legacy
*/
public function testBookNavigationCacheContextDeprecatedParameter() {
$this->expectDeprecation('Passing the request_stack service to Drupal\book\Cache\BookNavigationCacheContext::__construct() is deprecated in drupal:9.2.0 and will be removed before drupal:10.0.0. The parameter should be an instance of \Drupal\Core\Routing\RouteMatchInterface instead.');
$request_stack = $this->container->get('request_stack');
$book_navigation_cache_context = new BookNavigationCacheContext($request_stack);
$this->assertNotNull($book_navigation_cache_context);
}
/** /**
* Tests the book navigation cache context. * Tests the book navigation cache context.
* *