diff --git a/core/lib/Drupal/Core/Path/AliasManager.php b/core/lib/Drupal/Core/Path/AliasManager.php index 212b360..e448222 100644 --- a/core/lib/Drupal/Core/Path/AliasManager.php +++ b/core/lib/Drupal/Core/Path/AliasManager.php @@ -11,6 +11,7 @@ use Drupal\Core\CacheDecorator\CacheDecoratorInterface; use Drupal\Core\Language\Language; use Drupal\Core\Language\LanguageManager; +use Drupal\Core\Language\LanguageManagerInterface; class AliasManager implements AliasManagerInterface, CacheDecoratorInterface { @@ -40,7 +41,7 @@ class AliasManager implements AliasManagerInterface, CacheDecoratorInterface { * * @var boolean */ - protected $cacheNeedsWriting = TRUE; + protected $cacheNeedsWriting = FALSE; /** * Language manager for retrieving the default langcode when none is specified. @@ -101,12 +102,12 @@ class AliasManager implements AliasManagerInterface, CacheDecoratorInterface { * The alias storage service. * @param \Drupal\Core\Path\AliasWhitelistInterface $whitelist * The whitelist implementation to use. - * @param \Drupal\Core\Language\LanguageManager $language_manager + * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager * The language manager. * @param \Drupal\Core\Cache\CacheBackendInterface $cache * Cache backend. */ - public function __construct(AliasStorageInterface $storage, AliasWhitelistInterface $whitelist, LanguageManager $language_manager, CacheBackendInterface $cache) { + public function __construct(AliasStorageInterface $storage, AliasWhitelistInterface $whitelist, LanguageManagerInterface $language_manager, CacheBackendInterface $cache) { $this->storage = $storage; $this->languageManager = $language_manager; $this->whitelist = $whitelist; @@ -128,7 +129,15 @@ public function setCacheKey($key) { * and load them in a single query during path alias lookup. */ public function writeCache() { - $path_lookups = $this->getPathLookups(); + // Start with the preloaded path lookups, so that cached entries for other + // languages will not be lost. + $path_lookups = $this->preloadedPathLookups ?: array(); + foreach ($this->lookupMap as $langcode => $lookups) { + $path_lookups[$langcode] = array_keys($lookups); + if (!empty($this->noAlias[$langcode])) { + $path_lookups[$langcode] = array_merge($path_lookups[$langcode], array_keys($this->noAlias[$langcode])); + } + } // Check if the system paths for this page were loaded from cache in this // request to avoid writing to cache on every request. if ($this->cacheNeedsWriting && !empty($path_lookups) && !empty($this->cacheKey)) { @@ -155,7 +164,6 @@ public function getPathByAlias($alias, $langcode = NULL) { $cached = $this->cache->get($this->cacheKey); if ($cached) { $this->preloadedPathLookups = $cached->data; - $this->cacheNeedsWriting = FALSE; } } @@ -206,10 +214,10 @@ public function getAliasByPath($path, $langcode = NULL) { $this->langcodePreloaded[$langcode] = TRUE; $this->lookupMap[$langcode] = array(); // Load paths from cache. - if (!empty($this->preloadedPathLookups)) { - $this->lookupMap[$langcode] = $this->storage->preloadPathAlias($this->preloadedPathLookups, $langcode); + if (!empty($this->preloadedPathLookups[$langcode])) { + $this->lookupMap[$langcode] = $this->storage->preloadPathAlias($this->preloadedPathLookups[$langcode], $langcode); // Keep a record of paths with no alias to avoid querying twice. - $this->noAlias[$langcode] = array_flip(array_diff_key($this->preloadedPathLookups, array_keys($this->lookupMap[$langcode]))); + $this->noAlias[$langcode] = array_flip(array_diff_key($this->preloadedPathLookups[$langcode], array_keys($this->lookupMap[$langcode]))); } } @@ -233,6 +241,7 @@ public function getAliasByPath($path, $langcode = NULL) { // We can't record anything into $this->lookupMap because we didn't find any // aliases for this path. Thus cache to $this->noAlias. $this->noAlias[$langcode][$path] = TRUE; + $this->cacheNeedsWriting = TRUE; return $path; } @@ -257,17 +266,6 @@ public function cacheClear($source = NULL) { } /** - * Implements \Drupal\Core\Path\AliasManagerInterface::getPathLookups(). - */ - public function getPathLookups() { - $current = current($this->lookupMap); - if ($current) { - return array_keys($current); - } - return array(); - } - - /** * Rebuild the path alias white list. * * @param string $path diff --git a/core/lib/Drupal/Core/Path/AliasManagerInterface.php b/core/lib/Drupal/Core/Path/AliasManagerInterface.php index 6757b02..2338846 100644 --- a/core/lib/Drupal/Core/Path/AliasManagerInterface.php +++ b/core/lib/Drupal/Core/Path/AliasManagerInterface.php @@ -36,15 +36,6 @@ public function getPathByAlias($alias, $langcode = NULL); public function getAliasByPath($path, $langcode = NULL); /** - * Returns an array of system paths that have been looked up. - * - * @return array - * An array of all system paths that have been looked up during the current - * request. - */ - public function getPathLookups(); - - /** * Clear internal caches in alias manager. * * @param $source diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MockAliasManager.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MockAliasManager.php index 5337099..6c3839d 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/MockAliasManager.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MockAliasManager.php @@ -82,20 +82,6 @@ public function getAliasByPath($path, $langcode = NULL) { /** * {@inheritdoc} */ - public function getPathLookups() { - return array_keys($this->lookedUp); - } - - /** - * {@inheritdoc} - */ - public function preloadPathLookups(array $path_list) { - // Not needed. - } - - /** - * {@inheritdoc} - */ public function cacheClear($source = NULL) { // Not needed. } diff --git a/core/tests/Drupal/Tests/Core/Path/AliasManagerTest.php b/core/tests/Drupal/Tests/Core/Path/AliasManagerTest.php new file mode 100644 index 0000000..a51327e --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Path/AliasManagerTest.php @@ -0,0 +1,536 @@ + 'Url object (external)', + 'description' => 'Tests the \Drupal\Core\Url class with external paths.', + 'group' => 'Routing', + ); + } + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + $this->aliasStorage = $this->getMock('Drupal\Core\Path\AliasStorageInterface'); + $this->aliasWhitelist = $this->getMock('Drupal\Core\Path\AliasWhitelistInterface'); + $this->languageManager = $this->getMock('Drupal\Core\Language\LanguageManagerInterface'); + $this->cache = $this->getMock('Drupal\Core\Cache\CacheBackendInterface'); + + $this->aliasManager = new AliasManager($this->aliasStorage, $this->aliasWhitelist, $this->languageManager, $this->cache); + + } + + /** + * Tests the getPathByAlias method for an alias that have no matching path. + * + * @covers ::getPathByAlias() + */ + public function testGetPathByAliasNoMatch() { + $alias = $this->randomName(); + + $language = new Language(array('id' => 'en')); + + $this->languageManager->expects($this->any()) + ->method('getCurrentLanguage') + ->with(LanguageInterface::TYPE_URL) + ->will($this->returnValue($language)); + + $this->aliasStorage->expects($this->once()) + ->method('lookupPathSource') + ->with($alias, $language->getId()) + ->will($this->returnValue(NULL)); + + $this->assertEquals($alias, $this->aliasManager->getPathByAlias($alias)); + // Call it twice to test the static cache. + $this->assertEquals($alias, $this->aliasManager->getPathByAlias($alias)); + } + + /** + * Tests the getPathByAlias method for an alias that have a matching path. + * + * @covers ::getPathByAlias() + */ + public function testGetPathByAliasNatch() { + $alias = $this->randomName(); + $path = $this->randomName(); + + $language = $this->setUpCurrentLanguage(); + + $this->aliasStorage->expects($this->once()) + ->method('lookupPathSource') + ->with($alias, $language->getId()) + ->will($this->returnValue($path)); + + $this->assertEquals($path, $this->aliasManager->getPathByAlias($alias)); + // Call it twice to test the static cache. + $this->assertEquals($path, $this->aliasManager->getPathByAlias($alias)); + } + + /** + * Tests the getPathByAlias method when a langcode is passed explicitly. + * + * @covers ::getPathByAlias() + */ + public function testGetPathByAliasLangcode() { + $alias = $this->randomName(); + $path = $this->randomName(); + + $this->languageManager->expects($this->never()) + ->method('getCurrentLanguage'); + + $this->aliasStorage->expects($this->once()) + ->method('lookupPathSource') + ->with($alias, 'de') + ->will($this->returnValue($path)); + + $this->assertEquals($path, $this->aliasManager->getPathByAlias($alias, 'de')); + // Call it twice to test the static cache. + $this->assertEquals($path, $this->aliasManager->getPathByAlias($alias, 'de')); + } + + + /** + * Tests the getAliasByPath method for a path that is not in the whitelist. + * + * @covers ::getAliasByPath() + */ + public function testGetAliasByPathWhitelist() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + + $this->setUpCurrentLanguage(); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(FALSE)); + + // The whitelist returns FALSE for that path part, so the storage should + // never be called. + $this->aliasStorage->expects($this->never()) + ->method('lookupPathAlias'); + + $this->assertEquals($path, $this->aliasManager->getAliasByPath($path)); + } + + /** + * Tests the getAliasByPath method for a path that has no matching alias. + * + * @covers ::getAliasByPath() + */ + public function testGetAliasByPathNoMatch() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + + $language = $this->setUpCurrentLanguage(); + + $this->aliasManager->setCacheKey($this->cacheKey); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(TRUE)); + + $this->aliasStorage->expects($this->once()) + ->method('lookupPathAlias') + ->with($path, $language->getId()) + ->will($this->returnValue(NULL)); + + $this->assertEquals($path, $this->aliasManager->getAliasByPath($path)); + // Call it twice to test the static cache. + $this->assertEquals($path, $this->aliasManager->getAliasByPath($path)); + + // This needs to write out the cache. + $this->cache->expects($this->once()) + ->method('set') + ->with($this->cacheKey, array($language->getId() => array($path)), REQUEST_TIME + (60 * 60 * 24)); + + $this->aliasManager->writeCache(); + } + + /** + * Tests the getAliasByPath method for a path that has a matching alias. + * + * @covers ::getAliasByPath() + * @covers ::writeCache() + */ + public function testGetAliasByPathMatch() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + $alias = $this->randomName(); + + $language = $this->setUpCurrentLanguage(); + + $this->aliasManager->setCacheKey($this->cacheKey); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(TRUE)); + + $this->aliasStorage->expects($this->once()) + ->method('lookupPathAlias') + ->with($path, $language->getId()) + ->will($this->returnValue($alias)); + + $this->assertEquals($alias, $this->aliasManager->getAliasByPath($path)); + // Call it twice to test the static cache. + $this->assertEquals($alias, $this->aliasManager->getAliasByPath($path)); + + // This needs to write out the cache. + $this->cache->expects($this->once()) + ->method('set') + ->with($this->cacheKey, array($language->getId() => array($path)), REQUEST_TIME + (60 * 60 * 24)); + + $this->aliasManager->writeCache(); + } + + /** + * Tests the getAliasByPath method for a path that is preloaded. + * + * @covers ::getAliasByPath() + * @covers ::writeCache() + */ + public function testGetAliasByPathCachedMatch() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + $alias = $this->randomName(); + + $language = $this->setUpCurrentLanguage(); + + $cached_paths = array($language->getId() => array($path)); + $this->cache->expects($this->once()) + ->method('get') + ->with($this->cacheKey) + ->will($this->returnValue((object) array('data' => $cached_paths))); + + // Simulate a request so that the preloaded paths are fetched. + $this->aliasManager->setCacheKey($this->cacheKey); + $this->aliasManager->getPathByAlias('alias'); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(TRUE)); + + $this->aliasStorage->expects($this->once()) + ->method('preloadPathAlias') + ->with($cached_paths[$language->getId()], $language->getId()) + ->will($this->returnValue(array($path => $alias))); + + // LookupPathAlias should not be called. + $this->aliasStorage->expects($this->never()) + ->method('lookupPathAlias'); + + $this->assertEquals($alias, $this->aliasManager->getAliasByPath($path)); + // Call it twice to test the static cache. + $this->assertEquals($alias, $this->aliasManager->getAliasByPath($path)); + + // This must not write to the cache again. + $this->cache->expects($this->never()) + ->method('set'); + $this->aliasManager->writeCache(); + } + + /** + * Tests the getAliasByPath cache when a different language is requested. + * + * @covers ::getAliasByPath() + * @covers ::writeCache() + */ + public function testGetAliasByPathCachedMissLanguage() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + $alias = $this->randomName(); + + $language = $this->setUpCurrentLanguage(); + $cached_language = new Language(array('id' => 'de')); + + $cached_paths = array($cached_language->getId() => array($path)); + $this->cache->expects($this->once()) + ->method('get') + ->with($this->cacheKey) + ->will($this->returnValue((object) array('data' => $cached_paths))); + + // Simulate a request so that the preloaded paths are fetched. + $this->aliasManager->setCacheKey($this->cacheKey); + $this->aliasManager->getPathByAlias('alias'); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(TRUE)); + + // The requested language is different than the cached, so this will + // need to load. + $this->aliasStorage->expects($this->never()) + ->method('preloadPathAlias'); + $this->aliasStorage->expects($this->once()) + ->method('lookupPathAlias') + ->with($path, $language->getId()) + ->will($this->returnValue($alias)); + + $this->assertEquals($alias, $this->aliasManager->getAliasByPath($path)); + // Call it twice to test the static cache. + $this->assertEquals($alias, $this->aliasManager->getAliasByPath($path)); + + // This needs to write out the cache. + $expected_new_cache = array( + $cached_language->getId() => array($path), + $language->getId() => array($path), + ); + $this->cache->expects($this->once()) + ->method('set') + ->with($this->cacheKey, $expected_new_cache, REQUEST_TIME + (60 * 60 * 24)); + $this->aliasManager->writeCache(); + } + + /** + * Tests the getAliasByPath cache with a preloaded path without alias. + * + * @covers ::getAliasByPath() + * @covers ::writeCache() + */ + public function testGetAliasByPathCachedMissNoAlias() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + $cached_path = $this->randomName(); + $cached_alias = $this->randomName(); + + $language = $this->setUpCurrentLanguage(); + + $cached_paths = array($language->getId() => array($cached_path, $path)); + $this->cache->expects($this->once()) + ->method('get') + ->with($this->cacheKey) + ->will($this->returnValue((object) array('data' => $cached_paths))); + + // Simulate a request so that the preloaded paths are fetched. + $this->aliasManager->setCacheKey($this->cacheKey); + $this->aliasManager->getPathByAlias('alias'); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(TRUE)); + + $this->aliasStorage->expects($this->once()) + ->method('preloadPathAlias') + ->with($cached_paths[$language->getId()], $language->getId()) + ->will($this->returnValue(array($cached_path => $cached_alias))); + + // LookupPathAlias() should not be called. + $this->aliasStorage->expects($this->never()) + ->method('lookupPathAlias'); + + $this->assertEquals($path, $this->aliasManager->getAliasByPath($path)); + // Call it twice to test the static cache. + $this->assertEquals($path, $this->aliasManager->getAliasByPath($path)); + + // This must not write to the cache again. + $this->cache->expects($this->never()) + ->method('set'); + $this->aliasManager->writeCache(); + } + + /** + * Tests the getAliasByPath cache with an unpreloaded path without alias. + * + * @covers ::getAliasByPath() + * @covers ::writeCache() + */ + public function testGetAliasByPathUncachedMissNoAlias() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + $cached_path = $this->randomName(); + $cached_alias = $this->randomName(); + + $language = $this->setUpCurrentLanguage(); + + $cached_paths = array($language->getId() => array($cached_path)); + $this->cache->expects($this->once()) + ->method('get') + ->with($this->cacheKey) + ->will($this->returnValue((object) array('data' => $cached_paths))); + + // Simulate a request so that the preloaded paths are fetched. + $this->aliasManager->setCacheKey($this->cacheKey); + $this->aliasManager->getPathByAlias('alias'); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(TRUE)); + + $this->aliasStorage->expects($this->once()) + ->method('preloadPathAlias') + ->with($cached_paths[$language->getId()], $language->getId()) + ->will($this->returnValue(array($cached_path => $cached_alias))); + + $this->aliasStorage->expects($this->once()) + ->method('lookupPathAlias') + ->with($path, $language->getId()) + ->will($this->returnValue(NULL)); + + $this->assertEquals($path, $this->aliasManager->getAliasByPath($path)); + // Call it twice to test the static cache. + $this->assertEquals($path, $this->aliasManager->getAliasByPath($path)); + + // This needs to write out the cache. + $expected_new_cache = array( + $language->getId() => array($cached_path, $path), + ); + $this->cache->expects($this->once()) + ->method('set') + ->with($this->cacheKey, $expected_new_cache, REQUEST_TIME + (60 * 60 * 24)); + $this->aliasManager->writeCache(); + } + + /** + * Tests the getAliasByPath cache with an unpreloaded path with alias. + * + * @covers ::getAliasByPath() + * @covers ::writeCache() + */ + public function testGetAliasByPathUncachedMissWithAlias() { + $path_part1 = $this->randomName(); + $path_part2 = $this->randomName(); + $path = $path_part1 . '/' . $path_part2; + $cached_path = $this->randomName(); + $cached_no_alias_path = $this->randomName(); + $cached_alias = $this->randomName(); + $new_alias = $this->randomName(); + + $language = $this->setUpCurrentLanguage(); + + $cached_paths = array($language->getId() => array($cached_path, $cached_no_alias_path)); + $this->cache->expects($this->once()) + ->method('get') + ->with($this->cacheKey) + ->will($this->returnValue((object) array('data' => $cached_paths))); + + // Simulate a request so that the preloaded paths are fetched. + $this->aliasManager->setCacheKey($this->cacheKey); + $this->aliasManager->getPathByAlias('alias'); + + $this->aliasWhitelist->expects($this->any()) + ->method('get') + ->with($path_part1) + ->will($this->returnValue(TRUE)); + + $this->aliasStorage->expects($this->once()) + ->method('preloadPathAlias') + ->with($cached_paths[$language->getId()], $language->getId()) + ->will($this->returnValue(array($cached_path => $cached_alias))); + + $this->aliasStorage->expects($this->once()) + ->method('lookupPathAlias') + ->with($path, $language->getId()) + ->will($this->returnValue($new_alias)); + + $this->assertEquals($new_alias, $this->aliasManager->getAliasByPath($path)); + // Call it twice to test the static cache. + $this->assertEquals($new_alias, $this->aliasManager->getAliasByPath($path)); + + // This needs to write out the cache. + $expected_new_cache = array( + $language->getId() => array($cached_path, $path, $cached_no_alias_path), + ); + $this->cache->expects($this->once()) + ->method('set') + ->with($this->cacheKey, $expected_new_cache, REQUEST_TIME + (60 * 60 * 24)); + $this->aliasManager->writeCache(); + } + + /** + * Sets up the current language. + * + * @return \Drupal\Core\Language\LanguageInterface + * The current language object. + */ + protected function setUpCurrentLanguage() { + $language = new Language(array('id' => 'en')); + + $this->languageManager->expects($this->any()) + ->method('getCurrentLanguage') + ->with(LanguageInterface::TYPE_URL) + ->will($this->returnValue($language)); + + return $language; + } + +}