From 9453570a402b7e105c4c3a98d2d2cb1b0d92945e Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org> Date: Mon, 10 Aug 2015 11:53:14 +0100 Subject: [PATCH] Issue #2508445 by dawehner, martin107: Cache the query in RouteProvider::preloadRoutes() --- core/core.services.yml | 4 +- .../Drupal/Core/Routing/RoutePreloader.php | 27 ++++++++++++- .../lib/Drupal/Core/Routing/RouteProvider.php | 38 ++++++++++++++++--- .../src/Tests/Routing/RouteProviderTest.php | 34 ++++++++++------- .../Tests/Core/Routing/RoutePreloaderTest.php | 10 ++++- 5 files changed, 90 insertions(+), 23 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 8fe29d2e6c8b..bf28df9a17b5 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -699,13 +699,13 @@ services: arguments: ['@current_route_match'] router.route_provider: class: Drupal\Core\Routing\RouteProvider - arguments: ['@database', '@state', '@path.current', '@cache.data', '@path_processor_manager'] + arguments: ['@database', '@state', '@path.current', '@cache.data', '@path_processor_manager', '@cache_tags.invalidator'] tags: - { name: event_subscriber } - { name: backend_overridable } router.route_preloader: class: Drupal\Core\Routing\RoutePreloader - arguments: ['@router.route_provider', '@state'] + arguments: ['@router.route_provider', '@state', '@cache.bootstrap'] tags: - { name: 'event_subscriber' } router.matcher.final_matcher: diff --git a/core/lib/Drupal/Core/Routing/RoutePreloader.php b/core/lib/Drupal/Core/Routing/RoutePreloader.php index 23a09aa33f21..e7dbc85c1837 100644 --- a/core/lib/Drupal/Core/Routing/RoutePreloader.php +++ b/core/lib/Drupal/Core/Routing/RoutePreloader.php @@ -7,6 +7,8 @@ namespace Drupal\Core\Routing; +use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\State\StateInterface; use Symfony\Component\EventDispatcher\Event; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -43,6 +45,13 @@ class RoutePreloader implements EventSubscriberInterface { */ protected $nonAdminRoutesOnRebuild = array(); + /** + * The cache backend used to skip the state loading. + * + * @var \Drupal\Core\Cache\CacheBackendInterface + */ + protected $cache; + /** * Constructs a new RoutePreloader. * @@ -50,10 +59,12 @@ class RoutePreloader implements EventSubscriberInterface { * The route provider. * @param \Drupal\Core\State\StateInterface $state * The state key value store. + * @param \Drupal\Core\Cache\CacheBackendInterface $cache */ - public function __construct(RouteProviderInterface $route_provider, StateInterface $state) { + public function __construct(RouteProviderInterface $route_provider, StateInterface $state, CacheBackendInterface $cache) { $this->routeProvider = $route_provider; $this->state = $state; + $this->cache = $cache; } /** @@ -65,7 +76,19 @@ public function __construct(RouteProviderInterface $route_provider, StateInterfa public function onRequest(KernelEvent $event) { // Only preload on normal HTML pages, as they will display menu links. if ($this->routeProvider instanceof PreloadableRouteProviderInterface && $event->getRequest()->getRequestFormat() == 'html') { - if ($routes = $this->state->get('routing.non_admin_routes', [])) { + + // Ensure that the state query is cached to skip the database query, if + // possible. + $key = 'routing.non_admin_routes'; + if ($cache = $this->cache->get($key)) { + $routes = $cache->data; + } + else { + $routes = $this->state->get($key, []); + $this->cache->set($key, $routes, Cache::PERMANENT, ['routes']); + } + + if ($routes) { // Preload all the non-admin routes at once. $this->routeProvider->preLoadRoutes($routes); } diff --git a/core/lib/Drupal/Core/Routing/RouteProvider.php b/core/lib/Drupal/Core/Routing/RouteProvider.php index 278b486377ec..8e350ffd76ac 100644 --- a/core/lib/Drupal/Core/Routing/RouteProvider.php +++ b/core/lib/Drupal/Core/Routing/RouteProvider.php @@ -7,7 +7,9 @@ namespace Drupal\Core\Routing; +use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheBackendInterface; +use Drupal\Core\Cache\CacheTagsInvalidatorInterface; use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\PathProcessor\InboundPathProcessorInterface; use Drupal\Core\State\StateInterface; @@ -75,6 +77,13 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv */ protected $cache; + /** + * The cache tag invalidator. + * + * @var \Drupal\Core\Cache\CacheTagsInvalidatorInterface + */ + protected $cacheTagInvalidator; + /** * A path processor manager for resolving the system path. * @@ -82,6 +91,11 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv */ protected $pathProcessor; + /** + * Cache ID prefix used to load routes. + */ + const ROUTE_LOAD_CID_PREFIX = 'route_provider.route_load:'; + /** * Constructs a new PathMatcher. * @@ -95,16 +109,19 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv * The cache backend. * @param \Drupal\Core\PathProcessor\InboundPathProcessorInterface $path_processor * The path processor. + * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tag_invalidator + * The cache tag invalidator. * @param string $table - * The table in the database to use for matching. + * (Optional) The table in the database to use for matching. Defaults to 'router' */ - public function __construct(Connection $connection, StateInterface $state, CurrentPathStack $current_path, CacheBackendInterface $cache_backend, InboundPathProcessorInterface $path_processor, $table = 'router') { + public function __construct(Connection $connection, StateInterface $state, CurrentPathStack $current_path, CacheBackendInterface $cache_backend, InboundPathProcessorInterface $path_processor, CacheTagsInvalidatorInterface $cache_tag_invalidator, $table = 'router') { $this->connection = $connection; $this->state = $state; - $this->tableName = $table; $this->currentPath = $current_path; $this->cache = $cache_backend; + $this->cacheTagInvalidator = $cache_tag_invalidator; $this->pathProcessor = $path_processor; + $this->tableName = $table; } /** @@ -189,8 +206,18 @@ public function preLoadRoutes($names) { $routes_to_load = array_diff($names, array_keys($this->routes), array_keys($this->serializedRoutes)); if ($routes_to_load) { - $result = $this->connection->query('SELECT name, route FROM {' . $this->connection->escapeTable($this->tableName) . '} WHERE name IN ( :names[] )', array(':names[]' => $routes_to_load)); - $routes = $result->fetchAllKeyed(); + + $cid = static::ROUTE_LOAD_CID_PREFIX . hash('sha512', serialize($routes_to_load)); + if ($cache = $this->cache->get($cid)) { + $routes = $cache->data; + } + else { + $result = $this->connection->query('SELECT name, route FROM {' . $this->connection->escapeTable($this->tableName) . '} WHERE name IN ( :names[] )', array(':names[]' => $routes_to_load)); + $routes = $result->fetchAllKeyed(); + + $this->cache->set($cid, $routes, Cache::PERMANENT, ['routes']); + } + $this->serializedRoutes += $routes; } } @@ -339,6 +366,7 @@ public function getAllRoutes() { public function reset() { $this->routes = array(); $this->serializedRoutes = array(); + $this->cacheTagInvalidator->invalidateTags(['routes']); } /** diff --git a/core/modules/system/src/Tests/Routing/RouteProviderTest.php b/core/modules/system/src/Tests/Routing/RouteProviderTest.php index 74cbaa30e915..e8411d6483b9 100644 --- a/core/modules/system/src/Tests/Routing/RouteProviderTest.php +++ b/core/modules/system/src/Tests/Routing/RouteProviderTest.php @@ -71,6 +71,13 @@ class RouteProviderTest extends KernelTestBase { */ protected $pathProcessor; + /** + * The cache tags invalidator. + * + * @var \Drupal\Core\Cache\CacheTagsInvalidatorInterface + */ + protected $cacheTagsInvalidator; + protected function setUp() { parent::setUp(); $this->fixtures = new RoutingFixtures(); @@ -78,6 +85,7 @@ protected function setUp() { $this->currentPath = new CurrentPathStack(new RequestStack()); $this->cache = new MemoryBackend('data'); $this->pathProcessor = \Drupal::service('path_processor_manager'); + $this->cacheTagsInvalidator = \Drupal::service('cache_tags.invalidator'); $this->installSchema('system', 'url_alias'); } @@ -107,7 +115,7 @@ protected function tearDown() { public function testCandidateOutlines() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $parts = array('node', '5', 'edit'); @@ -130,7 +138,7 @@ public function testCandidateOutlines() { */ function testExactPathMatch() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -154,7 +162,7 @@ function testExactPathMatch() { */ function testOutlinePathMatch() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -183,7 +191,7 @@ function testOutlinePathMatch() { */ function testOutlinePathMatchTrailingSlash() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -212,7 +220,7 @@ function testOutlinePathMatchTrailingSlash() { */ function testOutlinePathMatchDefaults() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -250,7 +258,7 @@ function testOutlinePathMatchDefaults() { */ function testOutlinePathMatchDefaultsCollision() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -289,7 +297,7 @@ function testOutlinePathMatchDefaultsCollision() { */ function testOutlinePathMatchDefaultsCollision2() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -328,7 +336,7 @@ function testOutlinePathMatchDefaultsCollision2() { */ public function testOutlinePathMatchZero() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -363,7 +371,7 @@ public function testOutlinePathMatchZero() { */ function testOutlinePathNoMatch() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -388,7 +396,7 @@ function testOutlinePathNoMatch() { */ public function testRouteCaching() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -450,7 +458,7 @@ public function testRouteCaching() { */ public function testRouteByName() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); @@ -485,7 +493,7 @@ public function testRouteByName() { */ public function testGetRoutesByPatternWithLongPatterns() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); // This pattern has only 3 parts, so we will get candidates, but no routes, @@ -543,7 +551,7 @@ public function testGetRoutesByPatternWithLongPatterns() { */ public function testGetRoutesPaged() { $connection = Database::getConnection(); - $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes'); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); $this->fixtures->createTables($connection); $dumper = new MatcherDumper($connection, $this->state, 'test_routes'); diff --git a/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php b/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php index 189aa92fb927..92c20f9ec032 100644 --- a/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php @@ -41,13 +41,21 @@ class RoutePreloaderTest extends UnitTestCase { */ protected $preloader; + /** + * The mocked cache. + * + * @var \Drupal\Core\Cache\CacheBackendInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $cache; + /** * {@inheritdoc} */ protected function setUp() { $this->routeProvider = $this->getMock('Drupal\Core\Routing\PreloadableRouteProviderInterface'); $this->state = $this->getMock('\Drupal\Core\State\StateInterface'); - $this->preloader = new RoutePreloader($this->routeProvider, $this->state); + $this->cache = $this->getMock('Drupal\Core\Cache\CacheBackendInterface'); + $this->preloader = new RoutePreloader($this->routeProvider, $this->state, $this->cache); } /** -- GitLab