diff --git a/core/core.services.yml b/core/core.services.yml index 6954b56..1de7bcb 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -101,6 +101,13 @@ services: factory_method: get factory_service: cache_factory arguments: [discovery] + cache.url_generator: + class: Drupal\Core\Cache\CacheBackendInterface + tags: + - { name: cache.bin } + factory_method: get + factory_service: cache_factory + arguments: [url_generator] page_cache_request_policy: class: Drupal\Core\PageCache\DefaultRequestPolicy tags: @@ -493,11 +500,19 @@ services: - [setFinalMatcher, ['@router.matcher.final_matcher']] tags: - { name: service_collector, tag: route_filter, call: addRouteFilter } - url_generator: + url_generator.uncached: class: Drupal\Core\Routing\UrlGenerator + public: false arguments: ['@router.route_provider', '@path_processor_manager', '@route_processor_manager', '@config.factory', '@settings', '@logger.channel.default', '@request_stack'] calls: - [setContext, ['@?router.request_context']] + url_generator: + class: Drupal\Core\Routing\CachedUrlGenerator + arguments: ['@url_generator.uncached', '@cache.url_generator', '@language_manager', '@router.route_provider'] + calls: + - [prepareCache, ['@request_stack']] + tags: + - { name: needs_destruction } unrouted_url_assembler: class: Drupal\Core\Utility\UnroutedUrlAssembler arguments: ['@request_stack', '@config.factory' ] diff --git a/core/lib/Drupal/Core/Routing/CachedUrlGenerator.php b/core/lib/Drupal/Core/Routing/CachedUrlGenerator.php new file mode 100644 index 0000000..7cf60d5 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/CachedUrlGenerator.php @@ -0,0 +1,255 @@ +urlGenerator = $url_generator; + $this->cache = $cache; + $this->languageManager = $language_manager; + $this->routeProvider = $route_provider; + } + + /** + * {@inheritdoc} + */ + public function clearCache() { + $this->cachedUrls = array(); + $this->cache->delete($this->cacheKey); + } + + /** + * Writes the cache of generated URLs. + */ + protected function writeCache() { + if ($this->cacheNeedsWriting && !empty($this->cachedUrls) && !empty($this->cacheKey)) { + // Set the URL cache to expire in 24 hours. + $expire = REQUEST_TIME + (60 * 60 * 24); + $this->cache->set($this->cacheKey, $this->cachedUrls, $expire); + } + } + + /** + * {@inheritdoc} + */ + public function generate($name, $parameters = array(), $absolute = FALSE) { + $options = array(); + // We essentially inline the implementation from the Drupal UrlGenerator + // and avoid setting $options so that we increase the likelihood of caching. + if ($absolute) { + $options['absolute'] = $absolute; + } + return $this->generateFromRoute($name, $parameters, $options); + } + + /** + * {@inheritdoc} + */ + public function generateFromPath($path = NULL, $options = array()) { + $key = self::PATH_CACHE_PREFIX . hash('sha256', $path . serialize($options)); + if (!isset($this->cachedUrls[$key])) { + $this->cachedUrls[$key] = $this->urlGenerator->generateFromPath($path, $options); + $this->cacheNeedsWriting = TRUE; + } + return $this->cachedUrls[$key]; + } + + /** + * {@inheritdoc} + */ + public function generateFromRoute($name, $parameters = array(), $options = array()) { + // In some cases $name may be a Route object, rather than a string. + $key = self::ROUTE_CACHE_PREFIX . hash('sha256', serialize($name) . serialize($options) . serialize($parameters)); + if (!isset($this->cachedUrls[$key])) { + $generated_url = $this->urlGenerator->generateFromRoute($name, $parameters, $options); + $route = $this->getRoute($name); + if ($route->hasOption('_cacheable') && !$route->getOption('_cacheable')) { + return $generated_url; + } + else { + $this->cachedUrls[$key] = $generated_url; + $this->cacheNeedsWriting = TRUE; + } + } + return $this->cachedUrls[$key]; + } + + /** + * Find the route using the provided route name. + * + * @param string $name + * The route name to fetch + * + * @return \Symfony\Component\Routing\Route + * The found route. + * + * @throws \Symfony\Component\Routing\Exception\RouteNotFoundException + * Thrown if there is no route with that name in this repository. + * + * @see \Drupal\Core\Routing\RouteProviderInterface + */ + protected function getRoute($name) { + if ($name instanceof SymfonyRoute) { + $route = $name; + } + elseif (NULL === $route = clone $this->routeProvider->getRouteByName($name)) { + throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name)); + } + return $route; + } + + /** + * Based on the current request load the appropiate set of cached URLs. + * + * @param \Symfony\Component\HttpFoundation\RequestStack $requestStack + */ + public function prepareCache(RequestStack $requestStack) { + $this->cacheKey = $requestStack->getCurrentRequest()->attributes->get('_system_path'); + if (!RequestHelper::isCleanUrl($requestStack->getCurrentRequest())){ + $this->cacheKey .= '::' . $requestStack->getCurrentRequest()->getScriptName(); + } + // Only multilingual sites have language dependant URLs. + if ($this->languageManager->isMultilingual()) { + $this->cacheKey .= '::' . $this->languageManager->getCurrentLanguage(Language::TYPE_URL)->getId(); + } + + $cached = $this->cache->get($this->cacheKey); + if ($cached) { + $this->cachedUrls = $cached->data; + } + } + + /** + * {@inheritdoc} + */ + public function supports($name) { + return $this->urlGenerator->supports($name); + } + + /** + * {@inheritdoc} + */ + public function getRouteDebugMessage($name, array $parameters = array()) { + return $this->urlGenerator->getRouteDebugMessage($name, $parameters); + } + + /** + * {@inheritdoc} + */ + public function destruct() { + $this->writeCache(); + } + + /** + * {@inheritdoc} + */ + public function setContext(SymfonyRequestContext $context) { + $this->urlGenerator->setContext($context); + } + + /** + * {@inheritdoc} + */ + public function getContext() { + return $this->urlGenerator->getContext(); + } + + /** + * {@inheritdoc} + */ + public function getPathFromRoute($name, $parameters = array()) { + return $this->urlGenerator->getPathFromRoute($name, $parameters); + } + + +} diff --git a/core/lib/Drupal/Core/Routing/CachedUrlGeneratorInterface.php b/core/lib/Drupal/Core/Routing/CachedUrlGeneratorInterface.php new file mode 100644 index 0000000..d9bbbf3 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/CachedUrlGeneratorInterface.php @@ -0,0 +1,22 @@ +drupalPostForm(NULL, $edit, t('Save configuration')); $this->rebuildContainer(); + // Clear cached urls in the local process too. + $this->container->get('url_generator')->clearCache(); $this->assertOptionSelected('edit-site-default-language', 'fr', 'Default language updated.'); $this->assertUrl(\Drupal::url('system.regional_settings', [], ['absolute' => TRUE, 'langcode' => 'fr']), [], 'Correct page redirection.'); diff --git a/core/modules/language/src/Tests/LanguageListTest.php b/core/modules/language/src/Tests/LanguageListTest.php index c27f981..8cdefd0 100644 --- a/core/modules/language/src/Tests/LanguageListTest.php +++ b/core/modules/language/src/Tests/LanguageListTest.php @@ -53,6 +53,8 @@ function testLanguageList() { 'direction' => Language::DIRECTION_LTR, ); $this->drupalPostForm('admin/config/regional/language/add', $edit, t('Add custom language')); + // Clear cached urls in the local process too. + $this->container->get('url_generator')->clearCache(); $this->assertUrl(\Drupal::url('language.admin_overview', [], ['absolute' => TRUE])); $this->assertRaw('"edit-languages-' . $langcode .'-weight"', 'Language code found.'); $this->assertText(t($name), 'Test language added.'); @@ -69,6 +71,8 @@ function testLanguageList() { 'site_default_language' => $langcode, ); $this->drupalPostForm(NULL, $edit, t('Save configuration')); + // Clear cached urls in the local process too. + $this->container->get('url_generator')->clearCache(); $this->rebuildContainer(); $this->assertNoOptionSelected('edit-site-default-language', 'en', 'Default language updated.'); $this->assertUrl(\Drupal::url('system.regional_settings', [], ['absolute' => TRUE, 'language' => $language])); @@ -96,6 +100,8 @@ function testLanguageList() { ); $this->drupalPostForm($path, $edit, t('Save configuration')); $this->rebuildContainer(); + // Clear cached urls in the local process too. + $this->container->get('url_generator')->clearCache(); // Ensure 'delete' link works. $this->drupalGet('admin/config/regional/language'); $this->clickLink(t('Delete')); @@ -117,6 +123,8 @@ function testLanguageList() { $this->drupalGet('admin/config/regional/language/delete/' . $langcode); $this->assertResponse(404, 'Language no longer found.'); + // Clear cached urls in the local process too. + $this->container->get('url_generator')->clearCache(); // Delete French. $this->drupalPostForm('admin/config/regional/language/delete/fr', array(), t('Delete')); // Make sure the "language_count" state has been updated correctly. diff --git a/core/modules/shortcut/shortcut.routing.yml b/core/modules/shortcut/shortcut.routing.yml index d8d8ea2..4a0258c 100644 --- a/core/modules/shortcut/shortcut.routing.yml +++ b/core/modules/shortcut/shortcut.routing.yml @@ -30,8 +30,11 @@ entity.shortcut_set.edit_form: requirements: _entity_access: 'shortcut_set.update' +# @todo Make this route cacheable once https://drupal.org/node/2351015 is commited. shortcut.link_add_inline: path: '/admin/config/user-interface/shortcut/manage/{shortcut_set}/add-link-inline' + options: + _cacheable: false defaults: _controller: 'Drupal\shortcut\Controller\ShortcutSetController::addShortcutLinkInline' requirements: diff --git a/core/modules/system/src/Form/RegionalForm.php b/core/modules/system/src/Form/RegionalForm.php index 2d3934f..b40cfb2 100644 --- a/core/modules/system/src/Form/RegionalForm.php +++ b/core/modules/system/src/Form/RegionalForm.php @@ -8,9 +8,11 @@ namespace Drupal\system\Form; use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Form\ConfigFormBase; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Locale\CountryManagerInterface; -use Drupal\Core\Form\ConfigFormBase; +use Drupal\Core\Routing\CachedUrlGeneratorInterface; +use Drupal\Core\Routing\UrlGeneratorInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -26,16 +28,26 @@ class RegionalForm extends ConfigFormBase { protected $countryManager; /** + * The URL generator. + * + * @var \Drupal\Core\Routing\UrlGeneratorInterface + */ + protected $urlGenerator; + + /** * Constructs a RegionalForm object. * * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The factory for configuration objects. * @param \Drupal\Core\Locale\CountryManagerInterface $country_manager * The country manager. + * @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator + * The url generator. */ - public function __construct(ConfigFactoryInterface $config_factory, CountryManagerInterface $country_manager) { + public function __construct(ConfigFactoryInterface $config_factory, CountryManagerInterface $country_manager, UrlGeneratorInterface $url_generator) { parent::__construct($config_factory); $this->countryManager = $country_manager; + $this->urlGenerator = $url_generator; } /** @@ -44,7 +56,8 @@ public function __construct(ConfigFactoryInterface $config_factory, CountryManag public static function create(ContainerInterface $container) { return new static( $container->get('config.factory'), - $container->get('country_manager') + $container->get('country_manager'), + $container->get('url_generator') ); } @@ -152,6 +165,10 @@ public function submitForm(array &$form, FormStateInterface $form_state) { ->set('timezone.user.default', $form_state->getValue('user_default_timezone')) ->save(); + if ($this->urlGenerator instanceof CachedUrlGeneratorInterface) { + $this->urlGenerator->clearCache(); + } + parent::submitForm($form, $form_state); } diff --git a/core/modules/system/system.routing.yml b/core/modules/system/system.routing.yml index fc05a95..a4e4d4f 100644 --- a/core/modules/system/system.routing.yml +++ b/core/modules/system/system.routing.yml @@ -380,9 +380,13 @@ system.theme_settings_theme: '': path: '' + options: + _cacheable: false '': path: '' + options: + _cacheable: false system.modules_uninstall: path: '/admin/modules/uninstall' diff --git a/core/tests/Drupal/Tests/Core/Routing/CachedUrlGeneratorTest.php b/core/tests/Drupal/Tests/Core/Routing/CachedUrlGeneratorTest.php new file mode 100644 index 0000000..27c262d --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Routing/CachedUrlGeneratorTest.php @@ -0,0 +1,256 @@ +urlGenerator = $this->getMock('Drupal\Core\Routing\UrlGeneratorInterface'); + $this->cache = $this->getMock('Drupal\Core\Cache\CacheBackendInterface'); + $this->languageManager = $this->getMock('Drupal\Core\Language\LanguageManagerInterface'); + $this->routeProvider = $this->getMock('Symfony\Cmf\Component\Routing\RouteProviderInterface'); + $this->route = $this->getMockBuilder('Symfony\Component\Routing\Route') + ->disableOriginalConstructor() + ->getMock(); + $this->routeProvider->expects($this->any()) + ->method('getRouteByName') + ->willReturn($this->route); + $this->cachedUrlGenerator = new CachedUrlGenerator($this->urlGenerator, $this->cache, $this->languageManager, $this->routeProvider); + } + + /** + * Tests the generate method. + * + * @see \Drupal\Core\Routing\CachedUrlGenerator::generate() + */ + public function testGenerate() { + $this->urlGenerator->expects($this->once()) + ->method('generateFromRoute') + ->with('test_route') + ->will($this->returnValue('test-route-1')); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromRoute('test_route')); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromRoute('test_route')); + } + + /** + * Tests the generate method with the same route name but different parameters. + * + * @see \Drupal\Core\Routing\CachedUrlGenerator::generate() + */ + public function testGenerateWithDifferentParameters() { + $this->urlGenerator->expects($this->exactly(2)) + ->method('generateFromRoute') + ->will($this->returnValueMap(array( + array('test_route', array('key' => 'value1'), array(), 'test-route-1/value1'), + array('test_route', array('key' => 'value2'), array(), 'test-route-1/value2'), + ))); + $this->assertEquals('test-route-1/value1', $this->cachedUrlGenerator->generate('test_route', array('key' => 'value1'))); + $this->assertEquals('test-route-1/value1', $this->cachedUrlGenerator->generate('test_route', array('key' => 'value1'))); + $this->assertEquals('test-route-1/value2', $this->cachedUrlGenerator->generate('test_route', array('key' => 'value2'))); + $this->assertEquals('test-route-1/value2', $this->cachedUrlGenerator->generate('test_route', array('key' => 'value2'))); + } + + /** + * Tests the generateFromPath method. + * + * @see \Drupal\Core\Routing\CachedUrlGenerator::generateFromPath() + */ + public function testGenerateFromPath() { + $this->urlGenerator->expects($this->once()) + ->method('generateFromPath') + ->with('test-route-1') + ->will($this->returnValue('test-route-1')); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromPath('test-route-1')); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromPath('test-route-1')); + } + + /** + * Tests the generate method with the same path but different options + * + * @see \Drupal\Core\Routing\CachedUrlGenerator::generateFromPath() + */ + public function testGenerateFromPathWithDifferentParameters() { + $this->urlGenerator->expects($this->exactly(2)) + ->method('generateFromPath') + ->will($this->returnValueMap(array( + array('test-route-1', array('absolute' => TRUE), 'http://localhost/test-route-1'), + array('test-route-1', array('absolute' => FALSE), 'test-route-1'), + ))); + $this->assertEquals('http://localhost/test-route-1', $this->cachedUrlGenerator->generateFromPath('test-route-1', array('absolute' => TRUE))); + $this->assertEquals('http://localhost/test-route-1', $this->cachedUrlGenerator->generateFromPath('test-route-1', array('absolute' => TRUE))); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromPath('test-route-1', array('absolute' => FALSE))); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromPath('test-route-1', array('absolute' => FALSE))); + } + + + /** + * Tests the generateFromRoute method. + * + * @see \Drupal\Core\Routing\CachedUrlGenerator::generateFromRoute() + */ + public function testGenerateFromRoute() { + $this->urlGenerator->expects($this->once()) + ->method('generateFromRoute') + ->with('test_route') + ->will($this->returnValue('test-route-1')); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromRoute('test_route')); + $this->assertEquals('test-route-1', $this->cachedUrlGenerator->generateFromRoute('test_route')); + } + + /** + * Tests the generateFromRoute method with the same path, different options. + * + * @see \Drupal\Core\Routing\CachedUrlGenerator::generateFromRoute() + */ + public function testGenerateFromRouteWithDifferentParameters() { + $this->urlGenerator->expects($this->exactly(4)) + ->method('generateFromRoute') + ->will($this->returnValueMap(array( + array('test_route', array('key' => 'value1'), array(), 'test-route-1/value1'), + array('test_route', array('key' => 'value1'), array('absolute' => TRUE), 'http://localhost/test-route-1/value1'), + array('test_route', array('key' => 'value2'), array(), 'test-route-1/value2'), + array('test_route', array('key' => 'value2'), array('absolute' => TRUE), 'http://localhost/test-route-1/value2'), + ))); + $this->assertEquals('test-route-1/value1', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value1'))); + $this->assertEquals('test-route-1/value1', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value1'))); + $this->assertEquals('http://localhost/test-route-1/value1', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value1'), array('absolute' => TRUE))); + $this->assertEquals('http://localhost/test-route-1/value1', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value1'), array('absolute' => TRUE))); + $this->assertEquals('test-route-1/value2', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value2'))); + $this->assertEquals('test-route-1/value2', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value2'))); + $this->assertEquals('http://localhost/test-route-1/value2', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value2'), array('absolute' => TRUE))); + $this->assertEquals('http://localhost/test-route-1/value2', $this->cachedUrlGenerator->generateFromRoute('test_route', array('key' => 'value2'), array('absolute' => TRUE))); + } + + /** + * Tests that prepareCache's treatment of routes is case sensitive. + * i.e. Different caches are accessed for similar but distinctly named + * requests. + * + * For example, these URLs + * + * "/PAGE/A" + * "/PAGE/a" + * + * are not the same and that when processing the request the accumulation of + * generated URLs should be cached separately. + * + * @covers \Drupal\Core\Routing\CachedUrlGenerator::prepareCache() + */ + public function testPrepareCache(){ + + // Attach a monitor to capture the number of unique caches accessed. + $this->cache->expects($this->any()) + ->method('set') + ->will($this->returnCallback(array($this, 'logCacheIdentifiers'))); + + $requestStack = new RequestStack(); + + // For a lowercase request populate the array of URLs and then cache it. + $request1 = Request::create('/page/a'); + $request1->attributes->set('_system_path', '/page/a'); + $requestStack->push($request1); + $this->cachedUrlGenerator->prepareCache($requestStack); + $this->cachedUrlGenerator->generateFromRoute('test-route-1'); + $this->cachedUrlGenerator->destruct(); // write to cache + + // For a uppercase request populate the array of URLs and then cache it. + $request2 = Request::create('/PAGE/A'); + $request2->attributes->set('_system_path', '/PAGE/A'); + $requestStack->push($request2); + $this->cachedUrlGenerator->prepareCache($requestStack); + $this->cachedUrlGenerator->generateFromRoute('test-route-1'); + $this->cachedUrlGenerator->destruct(); // write to cache. + + // Verify that two uniquely named caches have been accessed - One for each request. + $this->assertEquals(count($this->cacheIds), 2, 'Two unique caches have been stored.'); + } + + /** + * Monitor the number of distinct caches that are accessed. + * + * Build up a list of unique cache identifiers as access to them is sought. + * + * The form of the set method being mocked :- + * + * set($cid, $data, $expire = Cache::PERMANENT, array $tags = array()); + */ + public function logCacheIdentifiers() { + $arg = func_get_args(); + + // cid is insensitive to the case of the identifier. + $cid = strtolower($arg[0]); + + $this->cacheIds[$cid] = 1; + } +} +