Issue #2126421 by slashrsm: Decouple \Drupal\Core\Path\AliasManager and \Drupal\Core\Path\Path.

core/core.services.yml

@@ -329,7 +329,7 @@ services:
     arguments: ['@path.alias_manager', '@cache.path']
   path.crud:
     class: Drupal\Core\Path\Path
-    arguments: ['@database', '@path.alias_manager']
+    arguments: ['@database', '@module_handler']
 # The argument to the hashing service defined in services.yml, to the
 # constructor of PhpassHashedPassword is the log2 number of iterations for
 # password stretching.

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

@@ -55,14 +55,14 @@ class AliasManagerCacheDecorator implements CacheDecoratorInterface, AliasManage
   }
 
   /**
-   * Implements \Drupal\Core\CacheDecorator\CacheDecoratorInterface::setCacheKey().
+   * {@inheritdoc}
    */
   public function setCacheKey($key) {
     $this->cacheKey = $key;
   }
 
   /**
-   * Implements \Drupal\Core\CacheDecorator\CacheDecoratorInterface::writeCache().
+   * {@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
@@ -81,7 +81,7 @@ class AliasManagerCacheDecorator implements CacheDecoratorInterface, AliasManage
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::getSystemPath().
+   * {@inheritdoc}
    */
   public function getSystemPath($path, $path_language = NULL) {
     $system_path = $this->aliasManager->getSystemPath($path, $path_language);
@@ -98,23 +98,31 @@ class AliasManagerCacheDecorator implements CacheDecoratorInterface, AliasManage
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::getPathAlias().
+   * {@inheritdoc}
    */
   public function getPathAlias($path, $path_language = NULL) {
     return $this->aliasManager->getPathAlias($path, $path_language);
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::getPathLookups().
+   * {@inheritdoc}
    */
   public function getPathLookups() {
     return $this->aliasManager->getPathLookups();
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::preloadPathLookups().
+   * {@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/EventSubscriber/PathSubscriber.php

@@ -7,7 +7,7 @@
 
 namespace Drupal\Core\EventSubscriber;
 
-use Drupal\Core\CacheDecorator\AliasManagerCacheDecorator;
+use Drupal\Core\Path\AliasManagerInterface;
 use Drupal\Core\PathProcessor\InboundPathProcessorInterface;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\HttpKernel\KernelEvents;
@@ -23,7 +23,7 @@ class PathSubscriber extends PathListenerBase implements EventSubscriberInterfac
   /**
    * The alias manager that caches alias lookups based on the request.
    *
-   * @var \Drupal\Core\CacheDecorator\AliasManagerCacheDecorator
+   * @var \Drupal\Core\Path\AliasManagerInterface
    */
   protected $aliasManager;
 
@@ -34,7 +34,7 @@ class PathSubscriber extends PathListenerBase implements EventSubscriberInterfac
    */
   protected $pathProcessor;
 
-  public function __construct(AliasManagerCacheDecorator $alias_manager, InboundPathProcessorInterface $path_processor) {
+  public function __construct(AliasManagerInterface $alias_manager, InboundPathProcessorInterface $path_processor) {
     $this->aliasManager = $alias_manager;
     $this->pathProcessor = $path_processor;
   }

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

@@ -125,7 +125,14 @@ class AliasManager implements AliasManagerInterface {
    * Implements \Drupal\Core\Path\AliasManagerInterface::cacheClear().
    */
   public function cacheClear($source = NULL) {
+    if ($source) {
+      foreach (array_keys($this->lookupMap) as $lang) {
+        $this->lookupMap[$lang][$source];
+      }
+    }
+    else {
       $this->lookupMap = array();
+    }
     $this->noSource = array();
     $this->no_aliases = array();
     $this->firstCall = TRUE;

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

@@ -54,4 +54,13 @@ interface AliasManagerInterface {
    *   An array of system paths.
    */
   public function preloadPathLookups(array $path_list);
+
+  /**
+   * Clear internal caches in alias manager.
+   *
+   * @param $source
+   *   Source path of the alias that is being inserted/updated. Can be ommitted
+   *   if entire cache needs to be flushed.
+   */
+  public function cacheClear($source = NULL);
 }

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

@@ -7,8 +7,8 @@
 
 namespace Drupal\Core\Path;
 
-use Drupal\Core\Database\Database;
 use Drupal\Core\Database\Connection;
+use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Language\Language;
 
 /**
@@ -23,22 +23,25 @@ class Path {
    */
   protected $connection;
 
+  /**
+   * The module handler.
+   *
+   * @var \Drupal\Core\Extension\ModuleHandlerInterface
+   */
+  protected $moduleHandler;
+
   /**
    * Constructs a Path CRUD object.
    *
    * @param \Drupal\Core\Database\Connection $connection
    *   A database connection for reading and writing path aliases.
    *
-   * @param \Drupal\Core\Path\AliasManager $alias_manager
+   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
-   *   An alias manager with an internal cache of stored aliases.
+   *   The module handler.
-   *
-   * @todo This class should not take an alias manager in its constructor. Once
-   *   we move to firing an event for CRUD operations instead of invoking a
-   *   hook, we can have a listener that calls cacheClear() on the alias manager.
    */
-  public function __construct(Connection $connection, AliasManager $alias_manager) {
+  public function __construct(Connection $connection, ModuleHandlerInterface $module_handler) {
     $this->connection = $connection;
-    $this->alias_manager = $alias_manager;
+    $this->moduleHandler = $module_handler;
   }
 
   /**
@@ -78,8 +81,7 @@ class Path {
         ->fields($fields);
       $pid = $query->execute();
       $fields['pid'] = $pid;
-      // @todo: Find a correct place to invoke hook_path_insert().
+      $operation = 'insert';
-      $hook = 'path_insert';
     }
     else {
       $fields['pid'] = $pid;
@@ -87,13 +89,11 @@ class Path {
         ->fields($fields)
         ->condition('pid', $pid);
       $pid = $query->execute();
-      // @todo: figure out where we can invoke hook_path_update()
+      $operation = 'update';
-      $hook = 'path_update';
     }
     if ($pid) {
       // @todo Switch to using an event for this instead of a hook.
-      module_invoke_all($hook, $fields);
+      $this->moduleHandler->invokeAll('path_' . $operation, array($fields));
-      $this->alias_manager->cacheClear();
       return $fields;
     }
     return FALSE;
@@ -138,8 +138,7 @@ class Path {
     }
     $deleted = $query->execute();
     // @todo Switch to using an event for this instead of a hook.
-    module_invoke_all('path_delete', $path);
+    $this->moduleHandler->invokeAll('path_delete', array($path));
-    $this->alias_manager->cacheClear();
     return $deleted;
   }
 }

core/modules/system/lib/Drupal/system/Tests/Path/AliasTest.php

@@ -31,10 +31,8 @@ class AliasTest extends PathUnitTestBase {
     $connection = Database::getConnection();
     $this->fixtures->createTables($connection);
 
-    //Create AliasManager and Path object.
+    //Create Path object.
-    $whitelist = new AliasWhitelist('path_alias_whitelist', $this->container->get('cache.cache'), $this->container->get('lock'), $this->container->get('state'), $connection);
+    $path = new Path($connection, $this->container->get('module_handler'));
-    $aliasManager = new AliasManager($connection, $whitelist, $this->container->get('language_manager'));
-    $path = new Path($connection, $aliasManager);
 
     $aliases = $this->fixtures->sampleUrlAliases();
 
@@ -86,9 +84,8 @@ class AliasTest extends PathUnitTestBase {
     $this->fixtures->createTables($connection);
 
     //Create AliasManager and Path object.
-    $whitelist = new AliasWhitelist('path_alias_whitelist', $this->container->get('cache.cache'), $this->container->get('lock'), $this->container->get('state'), $connection);
+    $aliasManager = $this->container->get('path.alias_manager.cached');
-    $aliasManager = new AliasManager($connection, $whitelist, $this->container->get('language_manager'));
+    $pathObject = new Path($connection, $this->container->get('module_handler'));
-    $pathObject = new Path($connection, $aliasManager);
 
     // Test the situation where the source is the same for multiple aliases.
     // Start with a language-neutral alias, which we will override.
@@ -108,6 +105,8 @@ class AliasTest extends PathUnitTestBase {
       'langcode' => 'en',
     );
     $pathObject->save($path['source'], $path['alias'], $path['langcode']);
+    // Hook that clears cache is not executed with unit tests.
+    \Drupal::service('path.alias_manager.cached')->cacheClear();
     $this->assertEqual($aliasManager->getPathAlias($path['source']), $path['alias'], 'English alias overrides language-neutral alias.');
     $this->assertEqual($aliasManager->getSystemPath($path['alias']), $path['source'], 'English source overrides language-neutral source.');
 
@@ -138,17 +137,23 @@ class AliasTest extends PathUnitTestBase {
       'langcode' => 'en',
     );
     $pathObject->save($path['source'], $path['alias'], $path['langcode']);
+    // Hook that clears cache is not executed with unit tests.
+    $aliasManager->cacheClear();
     $this->assertEqual($aliasManager->getPathAlias($path['source']), $path['alias'], 'Recently created English alias returned.');
     $this->assertEqual($aliasManager->getSystemPath($path['alias']), $path['source'], 'Recently created English source returned.');
 
     // Remove the English aliases, which should cause a fallback to the most
     // recently created language-neutral alias, 'bar'.
     $pathObject->delete(array('langcode' => 'en'));
+    // Hook that clears cache is not executed with unit tests.
+    $aliasManager->cacheClear();
     $this->assertEqual($aliasManager->getPathAlias($path['source']), 'bar', 'Path lookup falls back to recently created language-neutral alias.');
 
     // Test the situation where the alias and language are the same, but
     // the source differs. The newer alias record should be returned.
     $pathObject->save('user/2', 'bar');
+    // Hook that clears cache is not executed with unit tests.
+    $aliasManager->cacheClear();
     $this->assertEqual($aliasManager->getSystemPath('bar'), 'user/2', 'Newer alias record is returned when comparing two Language::LANGCODE_NOT_SPECIFIED paths with the same alias.');
   }
 
@@ -163,10 +168,9 @@ class AliasTest extends PathUnitTestBase {
     $memoryCounterBackend = new MemoryCounterBackend('cache');
 
     // Create AliasManager and Path object.
-
     $whitelist = new AliasWhitelist('path_alias_whitelist', $memoryCounterBackend, $this->container->get('lock'), $this->container->get('state'), $connection);
     $aliasManager = new AliasManager($connection, $whitelist, $this->container->get('language_manager'));
-    $path = new Path($connection, $aliasManager);
+    $path = new Path($connection, $this->container->get('module_handler'));
 
     // No alias for user and admin yet, so should be NULL.
     $this->assertNull($whitelist->get('user'));
@@ -178,18 +182,21 @@ class AliasTest extends PathUnitTestBase {
 
     // Add an alias for user/1, user should get whitelisted now.
     $path->save('user/1', $this->randomName());
+    $aliasManager->cacheClear();
     $this->assertTrue($whitelist->get('user'));
     $this->assertNull($whitelist->get('admin'));
     $this->assertNull($whitelist->get($this->randomName()));
 
     // Add an alias for admin, both should get whitelisted now.
     $path->save('admin/something', $this->randomName());
+    $aliasManager->cacheClear();
     $this->assertTrue($whitelist->get('user'));
     $this->assertTrue($whitelist->get('admin'));
     $this->assertNull($whitelist->get($this->randomName()));
 
     // Remove the user alias again, whitelist entry should be removed.
     $path->delete(array('source' => 'user/1'));
+    $aliasManager->cacheClear();
     $this->assertNull($whitelist->get('user'));
     $this->assertTrue($whitelist->get('admin'));
     $this->assertNull($whitelist->get($this->randomName()));

core/modules/system/lib/Drupal/system/Tests/Routing/MockAliasManager.php

@@ -60,7 +60,7 @@ class MockAliasManager implements AliasManagerInterface {
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::getSystemPath().
+   * {@inheritdoc}
    */
   public function getSystemPath($path, $path_language = NULL) {
     $language = $path_language ?: $this->defaultLanguage;
@@ -68,7 +68,7 @@ class MockAliasManager implements AliasManagerInterface {
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::getPathAlias().
+   * {@inheritdoc}
    */
   public function getPathAlias($path, $path_language = NULL) {
     $language = $path_language ?: $this->defaultLanguage;
@@ -77,16 +77,23 @@ class MockAliasManager implements AliasManagerInterface {
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::getPathLookups().
+   * {@inheritdoc}
    */
   public function getPathLookups() {
     return array_keys($this->lookedUp);
   }
 
   /**
-   * Implements \Drupal\Core\Path\AliasManagerInterface::preloadPathLookups().
+   * {@inheritdoc}
    */
   public function preloadPathLookups(array $path_list) {
     // Not needed.
   }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function cacheClear($source = NULL) {
+    // Not needed.
+  }
 }

core/modules/system/system.module

@@ -3187,3 +3187,24 @@ function system_admin_paths() {
   );
   return $paths;
 }
+
+/**
+ * Implements hook_path_update().
+ */
+function system_path_update() {
+  \Drupal::service('path.alias_manager.cached')->cacheClear();
+}
+
+/**
+ * Implements hook_path_insert().
+ */
+function system_path_insert() {
+  \Drupal::service('path.alias_manager.cached')->cacheClear();
+}
+
+/**
+ * Implements hook_path_delete().
+ */
+function system_path_delete($path) {
+  \Drupal::service('path.alias_manager.cached')->cacheClear();
+}