Commit 71456449 authored by alexpott's avatar alexpott
Browse files

Issue #2480811 by catch, dawehner, Crell, Berdir, amateescu: Cache incoming...

Issue #2480811 by catch, dawehner, Crell, Berdir, amateescu: Cache incoming path processing and route matching
parent 0dbb619f
......@@ -653,7 +653,7 @@ services:
arguments: ['@current_route_match']
router.route_provider:
class: Drupal\Core\Routing\RouteProvider
arguments: ['@database', '@state', '@path.current']
arguments: ['@database', '@state', '@path.current', '@cache.data', '@path_processor_manager']
tags:
- { name: event_subscriber }
- { name: backend_overridable }
......@@ -961,7 +961,7 @@ services:
class: Drupal\Core\EventSubscriber\PathSubscriber
tags:
- { name: event_subscriber }
arguments: ['@path.alias_manager', '@path_processor_manager', '@path.current']
arguments: ['@path.alias_manager', '@path.current']
route_access_response_subscriber:
class: Drupal\Core\EventSubscriber\RouteAccessResponseSubscriber
tags:
......
......@@ -21,7 +21,8 @@ class CacheRouterRebuildSubscriber implements EventSubscriberInterface {
*/
public function onRouterFinished() {
// Requested URLs that formerly gave a 403/404 may now be valid.
Cache::invalidateTags(['4xx-response']);
// Also invalidate all cached routing.
Cache::invalidateTags(['4xx-response', 'route_match']);
}
/**
......@@ -30,7 +31,7 @@ public function onRouterFinished() {
public static function getSubscribedEvents() {
$events = [];
// Act only when the router rebuild is finished.
$events[RoutingEvents::FINISHED][] = ['onRouterFinished'];
$events[RoutingEvents::FINISHED][] = ['onRouterFinished', 200];
return $events;
}
......
......@@ -86,7 +86,8 @@ protected function menuLinksRebuild() {
* {@inheritdoc}
*/
static function getSubscribedEvents() {
$events[RoutingEvents::FINISHED][] = array('onRouterRebuild', 200);
// Run after CachedRouteRebuildSubscriber.
$events[RoutingEvents::FINISHED][] = array('onRouterRebuild', 100);
return $events;
}
......
......@@ -9,10 +9,9 @@
use Drupal\Core\Path\AliasManagerInterface;
use Drupal\Core\Path\CurrentPathStack;
use Drupal\Core\PathProcessor\InboundPathProcessorInterface;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
use Symfony\Component\HttpKernel\Event\PostResponseEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
......@@ -28,13 +27,6 @@ class PathSubscriber implements EventSubscriberInterface {
*/
protected $aliasManager;
/**
* A path processor manager for resolving the system path.
*
* @var \Drupal\Core\PathProcessor\InboundPathProcessorInterface
*/
protected $pathProcessor;
/**
* The current path.
*
......@@ -46,31 +38,27 @@ class PathSubscriber implements EventSubscriberInterface {
* Constructs a new PathSubscriber instance.
*
* @param \Drupal\Core\Path\AliasManagerInterface $alias_manager
* @param \Drupal\Core\PathProcessor\InboundPathProcessorInterface $path_processor
* The alias manager.
* @param \Drupal\Core\Path\CurrentPathStack $current_path
* The current path.
*/
public function __construct(AliasManagerInterface $alias_manager, InboundPathProcessorInterface $path_processor, CurrentPathStack $current_path) {
public function __construct(AliasManagerInterface $alias_manager, CurrentPathStack $current_path) {
$this->aliasManager = $alias_manager;
$this->pathProcessor = $path_processor;
$this->currentPath = $current_path;
}
/**
* Converts the request path to a system path.
* Sets the cache key on the alias manager cache decorator.
*
* KernelEvents::CONTROLLER is used in order to be executed after routing.
*
* @param Symfony\Component\HttpKernel\Event\GetResponseEvent $event
* @param \Symfony\Component\HttpKernel\Event\FilterControllerEvent $event
* The Event to process.
*/
public function onKernelRequestConvertPath(GetResponseEvent $event) {
$request = $event->getRequest();
$path = trim($request->getPathInfo(), '/');
$path = $this->pathProcessor->processInbound($path, $request);
$this->currentPath->setPath('/' . $path, $request);
public function onKernelController(FilterControllerEvent $event) {
// Set the cache key on the alias manager cache decorator.
if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) {
$this->aliasManager->setCacheKey($path);
$this->aliasManager->setCacheKey(trim($this->currentPath->getPath($event->getRequest()), '/'));
}
}
......@@ -88,7 +76,7 @@ public function onKernelTerminate(PostResponseEvent $event) {
* An array of event listener definitions.
*/
static function getSubscribedEvents() {
$events[KernelEvents::REQUEST][] = array('onKernelRequestConvertPath', 200);
$events[KernelEvents::CONTROLLER][] = array('onKernelController', 200);
$events[KernelEvents::TERMINATE][] = array('onKernelTerminate', 200);
return $events;
}
......
......@@ -7,6 +7,7 @@
namespace Drupal\Core\Path;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Database\Connection;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\Language\LanguageInterface;
......@@ -77,6 +78,7 @@ public function save($source, $alias, $langcode = LanguageInterface::LANGCODE_NO
if ($pid) {
// @todo Switch to using an event for this instead of a hook.
$this->moduleHandler->invokeAll('path_' . $operation, array($fields));
Cache::invalidateTags(['route_match']);
return $fields;
}
return FALSE;
......@@ -110,6 +112,7 @@ public function delete($conditions) {
$deleted = $query->execute();
// @todo Switch to using an event for this instead of a hook.
$this->moduleHandler->invokeAll('path_delete', array($path));
Cache::invalidateTags(['route_match']);
return $deleted;
}
......
......@@ -7,7 +7,9 @@
namespace Drupal\Core\Routing;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Path\CurrentPathStack;
use Drupal\Core\PathProcessor\InboundPathProcessorInterface;
use Drupal\Core\State\StateInterface;
use Symfony\Cmf\Component\Routing\PagedRouteCollection;
use Symfony\Cmf\Component\Routing\PagedRouteProviderInterface;
......@@ -66,6 +68,20 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
*/
protected $currentPath;
/**
* The cache backend.
*
* @var \Drupal\Core\Cache\CacheBackendInterface
*/
protected $cache;
/**
* A path processor manager for resolving the system path.
*
* @var \Drupal\Core\PathProcessor\InboundPathProcessorInterface
*/
protected $pathProcessor;
/**
* Constructs a new PathMatcher.
*
......@@ -74,15 +90,21 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
* @param \Drupal\Core\State\StateInterface $state
* The state.
* @param \Drupal\Core\Path\CurrentPathStack $current_path
* THe current path.
* The current path.
* @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend
* The cache backend.
* @param \Drupal\Core\PathProcessor\InboundPathProcessorInterface $path_processor
* The path processor.
* @param string $table
* The table in the database to use for matching.
*/
public function __construct(Connection $connection, StateInterface $state, CurrentPathStack $current_path, $table = 'router') {
public function __construct(Connection $connection, StateInterface $state, CurrentPathStack $current_path, CacheBackendInterface $cache_backend, InboundPathProcessorInterface $path_processor, $table = 'router') {
$this->connection = $connection;
$this->state = $state;
$this->tableName = $table;
$this->currentPath = $current_path;
$this->cache = $cache_backend;
$this->pathProcessor = $path_processor;
}
/**
......@@ -107,13 +129,31 @@ public function __construct(Connection $connection, StateInterface $state, Curre
* @return \Symfony\Component\Routing\RouteCollection with all urls that
* could potentially match $request. Empty collection if nothing can
* match.
*
* @todo Should this method's found routes also be included in the cache?
*/
public function getRouteCollectionForRequest(Request $request) {
$path = $this->currentPath->getPath($request);
return $this->getRoutesByPath(rtrim($path, '/'));
// Cache both the system path as well as route parameters and matching
// routes.
$cid = 'route:' . $request->getPathInfo() . ':' . $request->getQueryString();
if ($cached = $this->cache->get($cid)) {
$this->currentPath->setPath($cached->data['path'], $request);
$request->query->replace($cached->data['query']);
return $cached->data['routes'];
}
else {
$path = trim($request->getPathInfo(), '/');
$path = $this->pathProcessor->processInbound($path, $request);
$this->currentPath->setPath('/' . $path, $request);
// Incoming path processors may also set query parameters.
$query_parameters = $request->query->all();
$routes = $this->getRoutesByPath('/' . rtrim($path, '/'));
$cache_value = [
'path' => '/' . $path,
'query' => $query_parameters,
'routes' => $routes,
];
$this->cache->set($cid, $cache_value, CacheBackendInterface::CACHE_PERMANENT, ['route_match']);
return $routes;
}
}
/**
......
......@@ -2,7 +2,7 @@
/**
* @file
* Contains \Drupal\system\EventSubscriber\ThemeSettingsCacheTag.
* Contains \Drupal\system\EventSubscriber\ConfigCacheTag.
*/
namespace Drupal\system\EventSubscriber;
......@@ -15,9 +15,9 @@
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
* A subscriber invalidating the 'rendered' cache tag when saving theme settings.
* A subscriber invalidating cache tags when system config objects are saved.
*/
class ThemeSettingsCacheTag implements EventSubscriberInterface {
class ConfigCacheTag implements EventSubscriberInterface {
/**
* The theme handler.
......@@ -34,7 +34,7 @@ class ThemeSettingsCacheTag implements EventSubscriberInterface {
protected $cacheTagsInvalidator;
/**
* Constructs a ThemeSettingsCacheTag object.
* Constructs a ConfigCacheTag object.
*
* @param \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler
* The theme handler.
......@@ -47,12 +47,19 @@ public function __construct(ThemeHandlerInterface $theme_handler, CacheTagsInval
}
/**
* Invalidate the 'rendered' cache tag whenever a theme setting is modified.
* Invalidate cache tags when particular system config objects are saved.
*
* @param \Drupal\Core\Config\ConfigCrudEvent $event
* The Event to process.
*/
public function onSave(ConfigCrudEvent $event) {
// Changing the site settings may mean a different route is selected for the
// front page. Additionally a change to the site name or similar must
// invalidate the render cache since this could be used anywhere.
if ($event->getConfig()->getName() === 'system.site') {
$this->cacheTagsInvalidator->invalidateTags(['route_match', 'rendered']);
}
// Global theme settings.
if ($event->getConfig()->getName() === 'system.theme.global') {
$this->cacheTagsInvalidator->invalidateTags(['rendered']);
......
......@@ -185,7 +185,7 @@ protected function getRequestForPath($path, array $exclude) {
}
// @todo Use the RequestHelper once https://www.drupal.org/node/2090293 is
// fixed.
$request = Request::create($this->context->getCompleteBaseUrl() . '/' . $path);
$request = Request::create('/' . $path);
// Performance optimization: set a short accept header to reduce overhead in
// AcceptHeaderMatcher when matching the request.
$request->headers->set('Accept', 'text/html');
......
......@@ -98,6 +98,10 @@ function testFormatDate() {
$this->config('system.site')->set('default_langcode', static::LANGCODE)->save();
date_default_timezone_set('America/Los_Angeles');
// Reset the language manager so new negotiations attempts will fall back on
// on the new language.
$this->container->get('language_manager')->reset();
$this->assertIdentical(format_date($timestamp, 'custom', 'l, d-M-y H:i:s T', 'America/Los_Angeles', 'en'), 'Sunday, 25-Mar-07 17:00:00 PDT', 'Test a different language.');
$this->assertIdentical(format_date($timestamp, 'custom', 'l, d-M-y H:i:s T', 'Europe/London'), 'Monday, 26-Mar-07 01:00:00 BST', 'Test a different time zone.');
$this->assertIdentical(format_date($timestamp, 'custom', 'l, d-M-y H:i:s T'), 'domingo, 25-Mar-07 17:00:00 PDT', 'Test custom date format.');
......
......@@ -7,9 +7,15 @@
namespace Drupal\system\Tests\Routing;
use Drupal\Core\Cache\MemoryBackend;
use Drupal\Core\Database\Database;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\KeyValueStore\KeyValueMemoryFactory;
use Drupal\Core\Path\CurrentPathStack;
use Drupal\Core\Routing\MatcherDumper;
use Drupal\Core\Routing\RouteProvider;
use Drupal\Core\State\State;
use Drupal\Tests\Core\Routing\RoutingFixtures;
use Drupal\simpletest\KernelTestBase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
......@@ -17,10 +23,6 @@
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Drupal\Core\Routing\RouteProvider;
use Drupal\Core\Database\Database;
use Drupal\Core\Routing\MatcherDumper;
use Drupal\Tests\Core\Routing\RoutingFixtures;
/**
* Confirm that the default route provider is working correctly.
......@@ -29,6 +31,11 @@
*/
class RouteProviderTest extends KernelTestBase {
/**
* Modules to enable.
*/
public static $modules = ['url_alter_test', 'system'];
/**
* A collection of shared fixture data for tests.
*
......@@ -50,11 +57,42 @@ class RouteProviderTest extends KernelTestBase {
*/
protected $currentPath;
/**
* The cache backend.
*
* @var \Drupal\Core\Cache\MemoryBackend
*/
protected $cache;
/**
* The inbound path processor.
*
* @var \Drupal\Core\PathProcessor\InboundPathProcessorInterface
*/
protected $pathProcessor;
protected function setUp() {
parent::setUp();
$this->fixtures = new RoutingFixtures();
$this->state = new State(new KeyValueMemoryFactory());
$this->currentPath = new CurrentPathStack(new RequestStack());
$this->cache = new MemoryBackend('data');
$this->pathProcessor = \Drupal::service('path_processor_manager');
$this->installSchema('system', 'url_alias');
}
/**
* {@inheritdoc}
*/
public function containerBuild(ContainerBuilder $container) {
parent::containerBuild($container); // TODO: Change the autogenerated stub
// Readd the incoming path alias for these tests.
if ($container->hasDefinition('path_processor_alias')) {
$definition = $container->getDefinition('path_processor_alias');
$definition->addTag('path_processor_inbound');
}
}
protected function tearDown() {
......@@ -69,7 +107,7 @@ protected function tearDown() {
public function testCandidateOutlines() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$parts = array('node', '5', 'edit');
......@@ -92,7 +130,7 @@ public function testCandidateOutlines() {
*/
function testExactPathMatch() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -116,7 +154,7 @@ function testExactPathMatch() {
*/
function testOutlinePathMatch() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -145,7 +183,7 @@ function testOutlinePathMatch() {
*/
function testOutlinePathMatchTrailingSlash() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -174,7 +212,7 @@ function testOutlinePathMatchTrailingSlash() {
*/
function testOutlinePathMatchDefaults() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -212,7 +250,7 @@ function testOutlinePathMatchDefaults() {
*/
function testOutlinePathMatchDefaultsCollision() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -251,7 +289,7 @@ function testOutlinePathMatchDefaultsCollision() {
*/
function testOutlinePathMatchDefaultsCollision2() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -290,7 +328,7 @@ function testOutlinePathMatchDefaultsCollision2() {
*/
public function testOutlinePathMatchZero() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -325,7 +363,7 @@ public function testOutlinePathMatchZero() {
*/
function testOutlinePathNoMatch() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -346,28 +384,65 @@ function testOutlinePathNoMatch() {
}
/**
* Ensures a path set on the current path services overrides the request one.
* Tests that route caching works.
*/
function testCurrentPath() {
public function testRouteCaching() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
$dumper = new MatcherDumper($connection, $this->state, 'test_routes');
$dumper->addRoutes($this->fixtures->sampleRouteCollection());
$dumper->addRoutes($this->fixtures->complexRouteCollection());
$dumper->dump();
$request = Request::create('/path/one', 'GET');
$this->currentPath->setPath('/path/two', $request);
// A simple path.
$path = '/path/add/one';
$request = Request::create($path, 'GET');
$provider->getRouteCollectionForRequest($request);
$routes_by_pattern = $provider->getRoutesByPattern('/path/two');
$routes = $provider->getRouteCollectionForRequest($request);
$this->assertEqual(array_keys($routes_by_pattern->all()), array_keys($routes->all()), 'Ensure the expected routes are found.');
$cache = $this->cache->get('route:/path/add/one:');
$this->assertEqual('/path/add/one', $cache->data['path']);
$this->assertEqual([], $cache->data['query']);
$this->assertEqual(3, count($cache->data['routes']));
foreach ($routes as $route) {
$this->assertEqual($route->getPath(), '/path/two', 'Found path has correct pattern');
}
// A path with query parameters.
$path = '/path/add/one?foo=bar';
$request = Request::create($path, 'GET');
$provider->getRouteCollectionForRequest($request);
$cache = $this->cache->get('route:/path/add/one:foo=bar');
$this->assertEqual('/path/add/one', $cache->data['path']);
$this->assertEqual(['foo' => 'bar'], $cache->data['query']);
$this->assertEqual(3, count($cache->data['routes']));
// A path with placeholders.
$path = '/path/1/one';
$request = Request::create($path, 'GET');
$provider->getRouteCollectionForRequest($request);
$cache = $this->cache->get('route:/path/1/one:');
$this->assertEqual('/path/1/one', $cache->data['path']);
$this->assertEqual([], $cache->data['query']);
$this->assertEqual(2, count($cache->data['routes']));
// A path with a path alias.
/** @var \Drupal\Core\Path\AliasStorageInterface $path_storage */
$path_storage = \Drupal::service('path.alias_storage');
$path_storage->save('path/add/one', 'path/add-one');
/** @var \Drupal\Core\Path\AliasManagerInterface $alias_manager */
$alias_manager = \Drupal::service('path.alias_manager');
$alias_manager->cacheClear();
$path = '/path/add-one';
$request = Request::create($path, 'GET');
$provider->getRouteCollectionForRequest($request);
$cache = $this->cache->get('route:/path/add-one:');
$this->assertEqual('/path/add/one', $cache->data['path']);
$this->assertEqual([], $cache->data['query']);
$this->assertEqual(3, count($cache->data['routes']));
}
/**
......@@ -375,7 +450,7 @@ function testCurrentPath() {
*/
public function testRouteByName() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
......@@ -410,7 +485,7 @@ public function testRouteByName() {
*/
public function testGetRoutesByPatternWithLongPatterns() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
// This pattern has only 3 parts, so we will get candidates, but no routes,
......@@ -468,7 +543,7 @@ public function testGetRoutesByPatternWithLongPatterns() {
*/
public function testGetRoutesPaged() {
$connection = Database::getConnection();
$provider = new RouteProvider($connection, $this->state, $this->currentPath, 'test_routes');
$provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
$this->fixtures->createTables($connection);
$dumper = new MatcherDumper($connection, $this->state, 'test_routes');
......
......@@ -42,8 +42,8 @@ services:
arguments: ['@cron', '@config.factory', '@state']
tags:
- { name: event_subscriber }
system.theme_settings_cache_tag:
class: Drupal\system\EventSubscriber\ThemeSettingsCacheTag
system.config_cache_tag:
class: Drupal\system\EventSubscriber\ConfigCacheTag
arguments: ['@theme_handler', '@cache_tags.invalidator']
tags:
- { name: event_subscriber }
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment