Problem/Motivation

The AliasPrefixList isn't cached site specifically.

Steps to reproduce

1. Clear cache.
2. Visit a site that has no aliases for paths with node prefix
3. Node aliases on other sites aren't used

Proposed resolution

cache the alias prefixes site specifically.

Issue fork sites-3601366

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

harlor created an issue. See original summary.

harlor opened merge request !3

harlor’s picture

harlor commented
Assigned: harlor » hydra
Status: Active » Needs review
harlor’s picture

harlor commented

I'm actually not sure if that's enough - likely we should also make the static cache site specific

harlor’s picture

harlor commented
Status: Needs review » Needs work
harlor’s picture

harlor commented
Status: Needs work » Needs review

hydra made their first commit to this issue’s fork.

hydra’s picture

hydra commented
Assigned: hydra » harlor

Thanks @harlor — I rebased onto current 1.x and took a closer look at the
two cache layers, because I think the key-prefixing wasn't quite covering the case you
were unsure about.

The persistent layer is solved by your per-site getCid() — each site gets
its own cache entry, which fixes the reported cold-cache contamination. The trickier part
is the in-request layer: CacheCollector loads its in-memory storage only once
per request (guarded by cacheLoaded), and loadMenuPathRoots() only
seeds when storage is empty. So when we switch site mid-request (preview, cross-site alias
generation), the switched-to site's path roots never get seeded and get()
just returns NULL — the aliases silently don't apply.

So instead of prefixing the storage keys, I went with a reload-on-switch:

  • Pin the cache id to the site whose data is actually in memory, so writes always land
    in the right entry even after the active site has changed.
  • When the active site changes within a request, flush the previous site's pending
    writes to its own entry, then drop and reload the in-memory state for the new site (which
    re-seeds its path roots).

That ends up being a bit less code than the key-prefixing version and handles the
switch case correctly. I also added a kernel test that reproduces the original bug:
a root that's unaliased on site A stays resolvable on site B even though A was cached
first, and switching back to A confirms it wasn't polluted in return.

Would be good to get your eyes on the approach since you opened it.

harlor’s picture

harlor commented

I think this approach will work - but don't we lose the static cache on every site-switch?

harlor opened merge request !13

harlor’s picture

harlor commented
Assigned: harlor » hydra

I implemented a new approach that uses a factory to use a separate instance of the AliasPrefixList for every class.

harlor changed the visibility of the branch 3601366-aliasprefixlist-cache to hidden.

harlor’s picture

harlor commented

I applied The suggested changes and rebased the branch.

  • harlor committed 0e7ec24c on 1.x 
    fix: #3601366 AliasPrefixList caches

By: harlor
By: hydra
By: antonín...
harlor’s picture

harlor commented
Assigned: hydra » Unassigned
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.