Issue #2233623 by Berdir, slashrsm, xjm: Fixed Merge AliasManagerCacheDecorator into AliasManager.

core/core.services.yml

@@ -184,7 +184,7 @@ services:
     arguments: [path_alias_whitelist, '@cache.default', '@lock', '@state', '@path.alias_storage']
   path.alias_manager:
     class: Drupal\Core\Path\AliasManager
-    arguments: ['@path.alias_storage', '@path.alias_whitelist', '@language_manager']
+    arguments: ['@path.alias_storage', '@path.alias_whitelist', '@language_manager', '@cache.data']
   http_client:
     class: Drupal\Core\Http\Client
     tags:
@@ -393,9 +393,6 @@ services:
     arguments: ['@router.builder', '@lock']
     tags:
       - { name: event_subscriber }
-  path.alias_manager.cached:
-    class: Drupal\Core\CacheDecorator\AliasManagerCacheDecorator
-    arguments: ['@path.alias_manager', '@cache.data']
   path.alias_storage:
     class: Drupal\Core\Path\AliasStorage
     arguments: ['@database', '@module_handler']
@@ -588,7 +585,7 @@ services:
     class: Drupal\Core\EventSubscriber\PathSubscriber
     tags:
       - { name: event_subscriber }
-    arguments: ['@path.alias_manager.cached', '@path_processor_manager']
+    arguments: ['@path.alias_manager', '@path_processor_manager']
   legacy_request_subscriber:
     class: Drupal\Core\EventSubscriber\LegacyRequestSubscriber
     tags:

core/includes/common.inc

@@ -887,7 +887,7 @@ function l($text, $path, array $options = array()) {
     // Add a "data-drupal-link-system-path" attribute to let the
     // drupal.active-link library know the path in a standardized manner.
     if (!isset($variables['options']['attributes']['data-drupal-link-system-path'])) {
-      $variables['options']['attributes']['data-drupal-link-system-path'] = \Drupal::service('path.alias_manager.cached')->getPathByAlias($path);
+      $variables['options']['attributes']['data-drupal-link-system-path'] = \Drupal::service('path.alias_manager')->getPathByAlias($path);
     }
   }
 

core/includes/theme.inc

@@ -1233,7 +1233,7 @@ function template_preprocess_links(&$variables) {
 
           // Add a "data-drupal-link-system-path" attribute to let the
           // drupal.active-link library know the path in a standardized manner.
-          $li_attributes['data-drupal-link-system-path'] = \Drupal::service('path.alias_manager.cached')->getPathByAlias($path);
+          $li_attributes['data-drupal-link-system-path'] = \Drupal::service('path.alias_manager')->getPathByAlias($path);
         }
 
         $item['link'] = $link_element;

core/lib/Drupal/Core/CacheDecorator/AliasManagerCacheDecorator.php

@@ -1,128 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains Drupal\Core\CacheDecorator\AliasManagerCacheDecorator.
- */
-
-namespace Drupal\Core\CacheDecorator;
-
-use Drupal\Core\Cache\CacheBackendInterface;
-use Drupal\Core\Path\AliasManagerInterface;
-
-/**
- * Class used by the PathSubscriber to get the system path and cache path lookups.
- */
-class AliasManagerCacheDecorator implements CacheDecoratorInterface, AliasManagerInterface {
-
-  /**
-   * @var \Drupal\Core\Path\AliasManagerInterface
-   */
-  protected $aliasManager;
-
-  /**
-   * @var \Drupal\Core\Cache\CacheBackendInterface;
-   */
-  protected $cache;
-
-  /**
-   * The cache key to use when caching system paths.
-   *
-   * @var string
-   */
-  protected $cacheKey;
-
-  /**
-   * Holds an array of previously cached paths based on a request path.
-   *
-   * @var array
-   */
-  protected $preloadedPathLookups = array();
-
-  /**
-   * Whether the cache needs to be written.
-   *
-   * @var boolean
-   */
-  protected $cacheNeedsWriting = TRUE;
-
-  /**
-   * Constructs a \Drupal\Core\CacheDecorator\AliasManagerCacheDecorator.
-   */
-  public function __construct(AliasManagerInterface $alias_manager, CacheBackendInterface $cache) {
-    $this->aliasManager = $alias_manager;
-    $this->cache = $cache;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setCacheKey($key) {
-    $this->cacheKey = $key;
-  }
-
-  /**
-   * {@inheritdoc}
-   *
-   * Cache an array of the system paths available on each page. We assume
-   * that aliases will be needed for the majority of these paths during
-   * subsequent requests, and load them in a single query during path alias
-   * lookup.
-   */
-  public function writeCache() {
-    $path_lookups = $this->getPathLookups();
-    // 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)) {
-      // Set the path cache to expire in 24 hours.
-      $expire = REQUEST_TIME + (60 * 60 * 24);
-      $this->cache->set($this->cacheKey, $path_lookups, $expire);
-    }
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getPathByAlias($alias, $langcode = NULL) {
-    $path = $this->aliasManager->getPathByAlias($alias, $langcode);
-    // We need to pass on the list of previously cached system paths for this
-    // key to the alias manager for use in subsequent lookups.
-    $cached = $this->cache->get($path);
-    $cached_paths = array();
-    if ($cached) {
-      $cached_paths = $cached->data;
-      $this->cacheNeedsWriting = FALSE;
-    }
-    $this->preloadPathLookups($cached_paths);
-    return $path;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getAliasByPath($path, $langcode = NULL) {
-    return $this->aliasManager->getAliasByPath($path, $langcode);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getPathLookups() {
-    return $this->aliasManager->getPathLookups();
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function preloadPathLookups(array $path_list) {
-    $this->aliasManager->preloadPathLookups($path_list);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function cacheClear($source = NULL) {
-    $this->cache->delete($this->cacheKey);
-    $this->aliasManager->cacheClear($source);
-  }
-}

core/lib/Drupal/Core/Path/AliasManager.php

@@ -7,10 +7,13 @@
 
 namespace Drupal\Core\Path;
 
+use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\CacheDecorator\CacheDecoratorInterface;
 use Drupal\Core\Language\Language;
 use Drupal\Core\Language\LanguageManager;
+use Drupal\Core\Language\LanguageManagerInterface;
 
-class AliasManager implements AliasManagerInterface {
+class AliasManager implements AliasManagerInterface, CacheDecoratorInterface {
 
   /**
    * The alias storage service.
@@ -19,6 +22,27 @@ class AliasManager implements AliasManagerInterface {
    */
   protected $storage;
 
+  /**
+   * Cache backend service.
+   *
+   * @var \Drupal\Core\Cache\CacheBackendInterface;
+   */
+  protected $cache;
+
+  /**
+   * The cache key to use when caching paths.
+   *
+   * @var string
+   */
+  protected $cacheKey;
+
+  /**
+   * Whether the cache needs to be written.
+   *
+   * @var boolean
+   */
+  protected $cacheNeedsWriting = FALSE;
+
   /**
    * Language manager for retrieving the default langcode when none is specified.
    *
@@ -64,12 +88,12 @@ class AliasManager implements AliasManagerInterface {
   /**
    * Holds an array of previously looked up paths for the current request path.
    *
-   * This will only ever get populated if the alias manager is being used in
-   * the context of a request.
+   * This will only get populated if a cache key has been set, which for example
+   * happens if the alias manager is used in the context of a request.
    *
    * @var array
    */
-  protected $preloadedPathLookups = array();
+  protected $preloadedPathLookups = FALSE;
 
   /**
    * Constructs an AliasManager.
@@ -78,13 +102,52 @@ class AliasManager implements AliasManagerInterface {
    *   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) {
+  public function __construct(AliasStorageInterface $storage, AliasWhitelistInterface $whitelist, LanguageManagerInterface $language_manager, CacheBackendInterface $cache) {
     $this->storage = $storage;
     $this->languageManager = $language_manager;
     $this->whitelist = $whitelist;
+    $this->cache = $cache;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setCacheKey($key) {
+    // Prefix the cache key to avoid clashes with other caches.
+    $this->cacheKey = 'preload-paths:' . $key;
+  }
+
+  /**
+   * {@inheritdoc}
+   *
+   * Cache an array of the paths available on each page. We assume that aliases
+   * will be needed for the majority of these paths during subsequent requests,
+   * and load them in a single query during path alias lookup.
+   */
+  public function writeCache() {
+    // Check if the paths for this page were loaded from cache in this request
+    // to avoid writing to cache on every request.
+    if ($this->cacheNeedsWriting && !empty($this->cacheKey)) {
+      // Start with the preloaded path lookups, so that cached entries for other
+      // languages will not be lost.
+      $path_lookups_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]));
+        }
+      }
+
+      if (!empty($path_lookups)) {
+        $twenty_four_hours_four_hours = 60 * 60 * 24;
+        $this->cache->set($this->cacheKey, $path_lookups, REQUEST_TIME + $twenty_four_hours);
+      }
+    }
   }
 
   /**
@@ -110,12 +173,14 @@ class AliasManager implements AliasManagerInterface {
     // Look for path in storage.
     if ($path = $this->storage->lookupPathSource($alias, $langcode)) {
       $this->lookupMap[$langcode][$path] = $alias;
+      $this->cacheNeedsWriting = TRUE;
       return $path;
     }
 
     // We can't record anything into $this->lookupMap because we didn't find any
     // paths for this alias. Thus cache to $this->noPath.
     $this->noPath[$langcode][$alias] = TRUE;
+
     return $alias;
   }
 
@@ -141,11 +206,21 @@ class AliasManager implements AliasManagerInterface {
     if (empty($this->langcodePreloaded[$langcode])) {
       $this->langcodePreloaded[$langcode] = TRUE;
       $this->lookupMap[$langcode] = array();
+
+      // Load the cached paths that should be used for preloading. This only
+      // 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;
+        }
+      }
+
       // 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])));
       }
     }
 
@@ -162,12 +237,14 @@ class AliasManager implements AliasManagerInterface {
     // 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;
   }
 
@@ -187,41 +264,24 @@ class AliasManager implements AliasManagerInterface {
     $this->noAlias = array();
     $this->langcodePreloaded = array();
     $this->preloadedPathLookups = array();
+    $this->cache->delete($this->cacheKey);
     $this->pathAliasWhitelistRebuild($source);
   }
 
-  /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::getPathLookups().
-   */
-  public function getPathLookups() {
-    $current = current($this->lookupMap);
-    if ($current) {
-      return array_keys($current);
-    }
-    return array();
-  }
-
-  /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::preloadPathLookups().
-   */
-  public function preloadPathLookups(array $path_list) {
-    $this->preloadedPathLookups = $path_list;
-  }
-
   /**
    * Rebuild the path alias white list.
    *
-   * @param $source
-   *   An optional system path for which an alias is being inserted.
+   * @param string $path
+   *   An optional path for which an alias is being inserted.
    *
    * @return
    *   An array containing a white list of path aliases.
    */
-  protected function pathAliasWhitelistRebuild($source = NULL) {
-    // When paths are inserted, only rebuild the whitelist if the system path
-    // has a top level component which is not already in the whitelist.
-    if (!empty($source)) {
-      if ($this->whitelist->get(strtok($source, '/'))) {
+  protected function pathAliasWhitelistRebuild($path = NULL) {
+    // When paths are inserted, only rebuild the whitelist if the path has a top
+    // level component which is not already in the whitelist.
+    if (!empty($path)) {
+      if ($this->whitelist->get(strtok($path, '/'))) {
         return;
      }
     }

core/lib/Drupal/Core/Path/AliasManagerInterface.php

@@ -35,23 +35,6 @@ interface AliasManagerInterface {
    */
   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();
-
-  /**
-   * Preload a set of paths for bulk alias lookups.
-   *
-   * @param $path_list
-   *   An array of system paths.
-   */
-  public function preloadPathLookups(array $path_list);
-
   /**
    * Clear internal caches in alias manager.
    *

core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php

@@ -210,7 +210,7 @@ class LinkWidget extends WidgetBase {
           // If internal links are supported, look up whether the given value is
           // a path alias and store the system path instead.
           if ($this->supportsInternalLinks() && !UrlHelper::isExternal($value['url'])) {
-            $parsed_url['path'] = \Drupal::service('path.alias_manager.cached')->getPathByAlias($parsed_url['path']);
+            $parsed_url['path'] = \Drupal::service('path.alias_manager')->getPathByAlias($parsed_url['path']);
           }
 
           $url = Url::createFromPath($parsed_url['path']);

core/modules/menu_link/src/MenuLinkForm.php

@@ -62,7 +62,7 @@ class MenuLinkForm extends EntityForm {
   public static function create(ContainerInterface $container) {
     return new static(
       $container->get('entity.manager')->getStorage('menu_link'),
-      $container->get('path.alias_manager.cached'),
+      $container->get('path.alias_manager'),
       $container->get('url_generator')
     );
   }

core/modules/path/src/Tests/PathAliasTest.php

@@ -58,12 +58,12 @@ class PathAliasTest extends PathTestBase {
     \Drupal::cache('data')->deleteAll();
     // Make sure the path is not converted to the alias.
     $this->drupalGet($edit['source'], array('alias' => TRUE));
-    $this->assertTrue(\Drupal::cache('data')->get($edit['source']), 'Cache entry was created.');
+    $this->assertTrue(\Drupal::cache('data')->get('preload-paths:' . $edit['source']), 'Cache entry was created.');
 
     // Visit the alias for the node and confirm a cache entry is created.
     \Drupal::cache('data')->deleteAll();
     $this->drupalGet($edit['alias']);
-    $this->assertTrue(\Drupal::cache('data')->get($edit['source']), 'Cache entry was created.');
+    $this->assertTrue(\Drupal::cache('data')->get('preload-paths:' .  $edit['source']), 'Cache entry was created.');
   }
 
   /**

core/modules/system/src/Tests/Path/AliasTest.php

@@ -84,7 +84,7 @@ class AliasTest extends PathUnitTestBase {
     $this->fixtures->createTables($connection);
 
     //Create AliasManager and Path object.
-    $aliasManager = $this->container->get('path.alias_manager.cached');
+    $aliasManager = $this->container->get('path.alias_manager');
     $aliasStorage = new AliasStorage($connection, $this->container->get('module_handler'));
 
     // Test the situation where the source is the same for multiple aliases.
@@ -106,7 +106,7 @@ class AliasTest extends PathUnitTestBase {
     );
     $aliasStorage->save($path['source'], $path['alias'], $path['langcode']);
     // Hook that clears cache is not executed with unit tests.
-    \Drupal::service('path.alias_manager.cached')->cacheClear();
+    \Drupal::service('path.alias_manager')->cacheClear();
     $this->assertEqual($aliasManager->getAliasByPath($path['source']), $path['alias'], 'English alias overrides language-neutral alias.');
     $this->assertEqual($aliasManager->getPathByAlias($path['alias']), $path['source'], 'English source overrides language-neutral source.');
 
@@ -170,7 +170,7 @@ class AliasTest extends PathUnitTestBase {
     // Create AliasManager and Path object.
     $aliasStorage = new AliasStorage($connection, $this->container->get('module_handler'));
     $whitelist = new AliasWhitelist('path_alias_whitelist', $memoryCounterBackend, $this->container->get('lock'), $this->container->get('state'), $aliasStorage);
-    $aliasManager = new AliasManager($aliasStorage, $whitelist, $this->container->get('language_manager'));
+    $aliasManager = new AliasManager($aliasStorage, $whitelist, $this->container->get('language_manager'), $memoryCounterBackend);
 
     // No alias for user and admin yet, so should be NULL.
     $this->assertNull($whitelist->get('user'));

core/modules/system/src/Tests/Routing/MockAliasManager.php

@@ -79,20 +79,6 @@ class MockAliasManager implements AliasManagerInterface {
     return $this->aliases[$path][$langcode];
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function getPathLookups() {
-    return array_keys($this->lookedUp);
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function preloadPathLookups(array $path_list) {
-    // Not needed.
-  }
-
   /**
    * {@inheritdoc}
    */

core/modules/system/system.module

@@ -1717,19 +1717,19 @@ function theme_system_config_form($variables) {
  * Implements hook_path_update().
  */
 function system_path_update() {
-  \Drupal::service('path.alias_manager.cached')->cacheClear();
+  \Drupal::service('path.alias_manager')->cacheClear();
 }
 
 /**
  * Implements hook_path_insert().
  */
 function system_path_insert() {
-  \Drupal::service('path.alias_manager.cached')->cacheClear();
+  \Drupal::service('path.alias_manager')->cacheClear();
 }
 
 /**
  * Implements hook_path_delete().
  */
 function system_path_delete($path) {
-  \Drupal::service('path.alias_manager.cached')->cacheClear();
+  \Drupal::service('path.alias_manager')->cacheClear();
 }

core/modules/views/src/Plugin/views/argument_default/Raw.php

@@ -56,7 +56,7 @@ class Raw extends ArgumentDefaultPluginBase {
       $configuration,
       $plugin_id,
       $plugin_definition,
-      $container->get('path.alias_manager.cached')
+      $container->get('path.alias_manager')
     );
   }
 

core/modules/views_ui/views_ui.module

@@ -320,7 +320,7 @@ function views_ui_views_analyze(ViewExecutable $view) {
       continue;
     }
     if ($display->hasPath() && $path = $display->getOption('path')) {
-      $normal_path = \Drupal::service('path.alias_manager.cached')->getPathByAlias($path);
+      $normal_path = \Drupal::service('path.alias_manager')->getPathByAlias($path);
       if ($path != $normal_path) {
         $ret[] = Analyzer::formatMessage(t('You have configured display %display with a path which is an path alias as well. This might lead to unwanted effects so better use an internal path.', array('%display' => $display->display['display_title'])), 'warning');
       }