Fixed Unable to extend \XF\PageCache

Affected version
2.2.4

Jake B.

Well-known member
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:

PHP:
/** @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

Well-known member
I guess you can try subscribe on app_pub_setup / app_setup event and just extend config part pageCache.onSetup:
PHP:
/** @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.

Well-known member
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:

Code:
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

Well-known member
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.

Well-known member
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

XenForo bug fixer bot
Staff member
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

XenForo developer
Staff member
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.
 
Top