Fix AliasManager and AliasManagerCacheDecorator

I wanted to review the patch, but I ended up trying to understand how AliasManager and AliasManagerCacheDecorator work. I talked to beejeebus, because from reading the patch, it seems that the DB cache will no longer be hit. Turns out he was very confused by the existing code also. So I went on a mission to understand what was going on. The conclusion of that mission:

1. they're nigh impossible to understand,
2. they lack test coverage (there's zero test coverage!)
3. AliasManagerCacheDecorator abuses implementation details of AliasManager to achieve a hacky, partial and undocumented performance win
4. AliasManagerCacheDecorator is probably fundamentally broken (it uses the wrong cache key to retrieve data from cache)
5. The cache key is literally the request's system path, instead of prefixing that with something, like everything else (unless it has its own cache bin, then it'd be fine — maybe that was the case before)

AliasManagerCacheDecorator wants to preload SYSTEM PATHS (i.e. alias -> source), but AliasManager wants to preload ALIASES (i.e. source -> alias)… yet AliasManagerCacheDecorator uses AliasManager's preload functionality, but then of course doesn't yield the desired result, since it's doing the opposite!
AliasManager::lookupPathAlias does:

    // During the first call to this method per language, load the expected
    // system paths for the page from cache.
    if (!empty($this->firstLookup)) {
      $this->firstLookup = FALSE;
      $this->lookupMap[$langcode] = array();
      // Load system paths from cache.
      if (!empty($this->preloadedPathLookups)) {
        // Now fetch the aliases corresponding to these system paths.
        $this->lookupMap[$langcode] = $this->storage->preloadPathAlias($this->preloadedPathLookups, $langcode);
        // Keep a record of paths with no alias to avoid querying twice.
        $this->noAliases[$langcode] = array_flip(array_diff_key($this->preloadedPathLookups, array_keys($this->lookupMap[$langcode])));
      }
    }

But AliasManager::lookupPathSource() (the inverse) also uses $this->lookupMap:

      // Look for the value $path within the cached $map
      $source = isset($this->lookupMap[$langcode]) ? array_search($path, $this->lookupMap[$langcode]) : FALSE;
      if (!$source) {
        if ($source = $this->storage->lookupPathSource($path, $langcode)) {
          $this->lookupMap[$langcode][$source] = $path;
        }

and this is how it "kind of" works, except that it then doesn't catch the case of "this already is a system path, you should be smart enough to not do a DB query for these". So I guess it's doing the best it can… but the end result is wildly confusing!

Attached is an initial patch to make it work again and document the most confusing part. beejeebus and I think these classes should pretty much be ripped apart and rewritten, if there's any hope of making this maintainable.

I apologize if I didn't explain it well or too fiercely — my head is still spinning from confusion.

(The previous patches could be applied to this also, but I think it might be better to unbreak things first?)

1. +++ b/core/lib/Drupal/Core/CacheDecorator/AliasManagerCacheDecorator.php
   @@ -88,14 +88,38 @@ public function writeCache() {
   +    static $preloaded;
   +
   +    if (!isset($preloaded)) {
   

   Do we really have a usecase for a static variable here or could we just use a plain old member variable on the object?

2. +++ b/core/lib/Drupal/Core/EventSubscriber/PathSubscriber.php
   @@ -53,7 +53,7 @@ public function onKernelRequestConvertPath(GetResponseEvent $event) {
        // Set the cache key on the alias manager cache decorator.
        if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) {
   ...
   +      $this->aliasManager->setCacheKey('aliases:' . $path);
        }
   

   Just in general I don't understand why we can't just pull that information manually from a request stack passed as dependency into the cached alias manager.

Not exactly sure what's left of this after #2233623: Merge AliasManagerCacheDecorator into AliasManager is in, but we probably still want to improve the getPathFromAlias() case there as that will still not be cached by the other issue.

If there is work remaining here, the issue summary needs updating.

The code base has drifted enough that this issue is no longer relevant. If there's other cleanup to do in AliasManager now start a new issue.