esmero / strawberryfield

A Field of strawberries
GNU Lesser General Public License v3.0
10 stars 5 forks source link

Add extra Caching context for breadcrumbs to avoid Drupal 10/11 changes in breadcrumb manager #328

Open DiegoPino opened 2 months ago

DiegoPino commented 2 months ago

What?

Drupal is such a hack sometimes. The story is quite simple. Breadcrumbs have two functions, one that is called to see if a Breadcrumb "applies" to the current Route and one to actual calculate the breadcrumb. Our own ADO implementation uses Node Context and the parent ADOs are extra cache tags when processing, but when the block is set without restrictions (at the block configuration level, e.g show on all pages) our "applies" function will Many times simply not generate any breadcrumbs. When that happens, the Block itself will get cached globally without any Caching context making any ADO breadcrumbs for anonymous users to end being "empty" (basically never running).

This is related to a "performance" improvement added for Drupal 10 that uses the "parent" route as cache context, which basically means, specially for "aliased ADOS... like /my-pretty-collection" to have the same cache context as the home page or anything else. Also reason why this almost never happens in the wild in our Archipelago deployments bc we put the breadcrumb block using conditionals and we don't use aliases (/do/uuid has a pre-folder named /do that aids with processing caches)

The solution (site building one) is to have a block setup that only acts on our Bundles to ensure that specific block does not share cache context with the "aliases/core" breadcrumb strategy as we normally do, but the actual solution on our side is to first always let the "apply" return TRUE... even if we know it does not.. and once we end with the actual "we can process" call, make the decision there if we try to calculate or not, but even if not, add

 $breadcrumb->addCacheContexts(['route', 'url.path', 'languages']);

Before returning the breadcrumb, overriding Drupal's new super strategy that basically breaks non-folder based caching

See as reference: https://www.drupal.org/project/drupal/issues/2719721 https://www.drupal.org/forum/support/post-installation/2024-03-15/breadcrumb-block-shows-only-homepage-link-in-drupal-10

DiegoPino commented 2 months ago

As additional reference. The Path Based Breadcrumb builder (\Drupal\system\breadcrumb) which will be called if ours does not match:

Does this:

 $breadcrumb = new Breadcrumb();
    $links = [];

    // Add the url.path.parent cache context. This code ignores the last path
    // part so the result only depends on the path parents.
    $breadcrumb->addCacheContexts(['url.path.parent', 'url.path.is_front']);

    // Do not display a breadcrumb on the frontpage.
    if ($this->pathMatcher->isFrontPage()) {
      return $breadcrumb;
    }

    // General path-based breadcrumbs. Use the actual request path, prior to
    // resolving path aliases, so the breadcrumb can be defined by simply
    // creating a hierarchy of path aliases.
    $path = trim($this->context->getPathInfo(), '/');
    $path_elements = explode('/', $path);
    $exclude = [];
    // Don't show a link to the front-page path.
    $front = $this->config->get('page.front');
    $exclude[$front] = TRUE;

Notice how the "parent" URL (in the case of an alias like /my-pretty-collection is basically the whole site) will be used as context, basically making any future processing of breadcrumbs for anonymous users to be a cached/empty response.