Fixed Unable to extend \XF\PageCache

[Jake B.]

Affected version

2.2.4

We need to overwrite the cache ID for requests made by a certain user agent to prevent the page header / navigation from being rendered. I'm currently able to accomplish this by doing the following:

/** @var PageCache|null $pageCache */
$pageCache = $app->container('pageCache');
if ($pageCache) {
    try {
        $pageCache->setCacheIdGenerator(function(Request $request) {
            $options = \XF::options();

            $styleId = intval($request->getCookie('style_id', 0));
            if (!$styleId) {
                $styleId = $options->defaultStyleId;
            }
            $languageId = intval($request->getCookie('language_id', 0));
            if (!$languageId) {
                $languageId = $options->defaultLanguageId;
            }

            $uri = $request->getFullRequestUri();
            $uri = preg_replace('#(\?|&)_debug=[^&]*#', '', $uri);

            return 'audapp_page_' . sha1($uri) . '_' . strlen($uri) . "_s{$styleId}_l{$languageId}_v" . PageCache::CACHE_VERSION;
        });
    } catch (\LogicException $e) {
        // We don't want to overwhelm the logs in this case, so we'll allow it to fall back
    }
}

However, this won't work if someone else has already set a cache ID generator in another add-on, or if XF ever decides to set it themselves in the future as it throws an exception. I'm catching this Exception currently and allowing it to fall back to whatever was set previously, but that makes it so my changes no longer work. Would be much cleaner if I was able to extend the \XF\PageCache::generateCacheId method and add my prefix without the potential of running into this issue

 

[Kruzya]

I guess you can try subscribe on app_pub_setup / app_setup event and just extend config part pageCache.onSetup:

/** @var \XF\App $app */
$app->container()->extend('config', function (array $config, \XF\Container $c)
{
    /** @var \Closure|null $previousSetupClosure */
    $previousSetupClosure = $config['pageCache']['onSetup'];
    $config['pageCache']['onSetup'] = function(\XF\PageCache $pageCache) use ($previousSetupClosure)
    {
        if ($previousSetupClosure)
        {
            $previousSetupClosure($pageCache);
        }

        $pageCache->setCacheIdGenerator(/** ... */);
    };
});

I haven't tested, but it might work.

Also in this event you can just extend pageCache element, but then you need check of existing item in DI container.

 

[Jake B.]

That'd have the same issue though, the setCacheIdGenerator method can only be run once, if a generator closure is already defined it throws that LogicException, if I could extend the function I could just get the response from the parent and prepend my prefix then return it. There isn't a previous closure that would get run either, it has a default ID generator with an option to overwrite that with a closure that gets run first:

protected function generateCacheId()
{
   if ($this->cacheIdGenerator)
   {
      $generator = $this->cacheIdGenerator;
      return $generator($this->request);
   }

   $options = \XF::options();
   $request = $this->request;

   $styleId = intval($request->getCookie('style_id', 0));
   if (!$styleId)
   {
      $styleId = $options->defaultStyleId;
   }
   $languageId = intval($request->getCookie('language_id', 0));
   if (!$languageId)
   {
      $languageId = $options->defaultLanguageId;
   }

   $uri = $request->getFullRequestUri();
   $uri = preg_replace('#(\?|&)_debug=[^&]*#', '', $uri);

   return 'page_' . sha1($uri) . '_' . strlen($uri) . "_s{$styleId}_l{$languageId}_v" . self::CACHE_VERSION;
}

 

[Kruzya]

the setCacheIdGenerator method can only be run once, if a generator closure is already defined it throws that LogicException

What XF version is used? I looked at code in XF 2.2.4, and see LogicException throwing only in cases when cacheId is already generated. cacheId generates only when someone tries receive him.

 

[Jake B.]

Ah yeah, looks like you're right, it's checking $this->cacheId, had assumed it was checking $this->cacheIdGenerator but either way, same issue comes into play where an add-on could overwrite the cache generator I specified, or if XF started using this somewhere for a specific case I'd be overwriting them. Right now it's unused and I doubt any add-ons are using it, so it works and doesn't matter too much, but it'd still be much better if I could just prepend my prefix to whatever is generated in generateCacheId rather than having to generate the whole ID separately just so I can add a prefix to separate cached pages with and without my user agent

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.7).

Change log:

Introduce a code event for manipulating the current page cache ID

There may be a delay before changes are rolled out to the XenForo Community.

 

[Chris D]

Hopefully this should go some way to achieving what you want.

The class is still not extendable - we have concerns with extending some early run classes (similar with Request, Response) - but the page cache ID can be manipulated with a code event now.