From 9f0005d57f0ef916e524de81f83f1fd5799c85de Mon Sep 17 00:00:00 2001 From: effulgentsia Date: Mon, 19 Oct 2015 15:24:04 -0700 Subject: [PATCH] Issue #2582219 by catch, Berdir: Preload Cache in AliasManager can get huge --- core/lib/Drupal/Core/Path/AliasManager.php | 18 +++++----- .../Tests/Core/Path/AliasManagerTest.php | 34 +++++++------------ 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/core/lib/Drupal/Core/Path/AliasManager.php b/core/lib/Drupal/Core/Path/AliasManager.php index 9fe0e2c8b29..c201e0b1b5d 100644 --- a/core/lib/Drupal/Core/Path/AliasManager.php +++ b/core/lib/Drupal/Core/Path/AliasManager.php @@ -145,10 +145,8 @@ class AliasManager implements AliasManagerInterface, CacheDecoratorInterface { } } - if (!empty($path_lookups)) { - $twenty_four_hours = 60 * 60 * 24; - $this->cache->set($this->cacheKey, $path_lookups, $this->getRequestTime() + $twenty_four_hours); - } + $twenty_four_hours = 60 * 60 * 24; + $this->cache->set($this->cacheKey, $path_lookups, $this->getRequestTime() + $twenty_four_hours); } } @@ -175,7 +173,6 @@ class AliasManager implements AliasManagerInterface, CacheDecoratorInterface { // Look for path in storage. if ($path = $this->storage->lookupPathSource($alias, $langcode)) { $this->lookupMap[$langcode][$path] = $alias; - $this->cacheNeedsWriting = TRUE; return $path; } @@ -216,8 +213,13 @@ class AliasManager implements AliasManagerInterface, CacheDecoratorInterface { // happens if a cache key has been set. if ($this->preloadedPathLookups === FALSE) { $this->preloadedPathLookups = array(); - if ($this->cacheKey && $cached = $this->cache->get($this->cacheKey)) { - $this->preloadedPathLookups = $cached->data; + if ($this->cacheKey) { + if ($cached = $this->cache->get($this->cacheKey)) { + $this->preloadedPathLookups = $cached->data; + } + else { + $this->cacheNeedsWriting = TRUE; + } } } @@ -242,14 +244,12 @@ class AliasManager implements AliasManagerInterface, CacheDecoratorInterface { // Try to load alias from storage. if ($alias = $this->storage->lookupPathAlias($path, $langcode)) { $this->lookupMap[$langcode][$path] = $alias; - $this->cacheNeedsWriting = TRUE; return $alias; } // 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; } diff --git a/core/tests/Drupal/Tests/Core/Path/AliasManagerTest.php b/core/tests/Drupal/Tests/Core/Path/AliasManagerTest.php index 01290c42e6f..cc041ce6d72 100644 --- a/core/tests/Drupal/Tests/Core/Path/AliasManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Path/AliasManagerTest.php @@ -339,14 +339,10 @@ class AliasManagerTest extends UnitTestCase { // 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, (int) $_SERVER['REQUEST_TIME'] + (60 * 60 * 24)); + // There is already a cache entry, so this should not write out to the + // cache. + $this->cache->expects($this->never()) + ->method('set'); $this->aliasManager->writeCache(); } @@ -441,13 +437,10 @@ class AliasManagerTest extends UnitTestCase { // 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, (int) $_SERVER['REQUEST_TIME'] + (60 * 60 * 24)); + // There is already a cache entry, so this should not write out to the + // cache. + $this->cache->expects($this->never()) + ->method('set'); $this->aliasManager->writeCache(); } @@ -533,13 +526,10 @@ class AliasManagerTest extends UnitTestCase { // 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, (int) $_SERVER['REQUEST_TIME'] + (60 * 60 * 24)); + // There is already a cache entry, so this should not write out to the + // cache. + $this->cache->expects($this->never()) + ->method('set'); $this->aliasManager->writeCache(); }