diff --git a/core/components/pdotools/docs/changelog.txt b/core/components/pdotools/docs/changelog.txt index bc4de5a..b62d9f3 100644 --- a/core/components/pdotools/docs/changelog.txt +++ b/core/components/pdotools/docs/changelog.txt @@ -33,6 +33,7 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html) - [pdoTools3#16] [pdoCrumbs] Fixed [] operator on string &parents when showHome is enabled. - [#384] [pdoFetch] Fixed warning when preparing where/having condition for the log message. - [#384] [pdoPage] Fixed too strict page comparison in the paginator (integer vs float). +- [pdoTools3#24] [pdoTools] Snippet result cache keys include the current context so web and extra contexts no longer share one entry. ## [3.0.3-pl2] - 2026-06-23 diff --git a/core/components/pdotools/src/CoreTools.php b/core/components/pdotools/src/CoreTools.php index 7de7c48..7bf5236 100644 --- a/core/components/pdotools/src/CoreTools.php +++ b/core/components/pdotools/src/CoreTools.php @@ -12,6 +12,7 @@ use MODX\Revolution\modTemplateVar; use MODX\Revolution\Sources\modFileMediaSource; use ModxPro\PdoTools\Parsing\Fenom\Fenom; +use ModxPro\PdoTools\Support\CacheKey; class CoreTools @@ -1087,21 +1088,19 @@ public function checkPermissions(array $rows = []) */ public function getCache($options = []) { - $cacheKey = $this->getCacheKey($options); - $cacheOptions = $this->getCacheOptions($options); - - $cached = ''; - if (!empty($cacheOptions) && !empty($cacheKey) && $this->modx->getCacheManager()) { - if ($cached = $this->modx->cacheManager->get($cacheKey, $cacheOptions)) { - $this->addTime('Retrieved data from cache "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); - } else { - $this->addTime('No cached data for key "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); - } - } else { - $this->addTime('Could not check cached data for key "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); - } + return $this->readCache($this->getCacheKey($options), $this->getCacheOptions($options)); + } - return $cached; + /** + * Read a cache entry by exact key, without a context suffix. + * Used for Fenom compile dumps that are source-based, not request-based. + * + * @param string $key + * @return mixed + */ + public function getExactCache($key) + { + return $this->readCache($key, $this->getCacheOptions(['cache_key' => $key])); } @@ -1115,20 +1114,19 @@ public function getCache($options = []) */ public function setCache($data = [], $options = []) { - $cacheKey = $this->getCacheKey($options); - $cacheOptions = $this->getCacheOptions($options); - - if (!empty($cacheKey) && !empty($cacheOptions) && $this->modx->getCacheManager()) { - $this->modx->cacheManager->set( - $cacheKey, - $data, - $cacheOptions[xPDO::OPT_CACHE_EXPIRES], - $cacheOptions - ); - $this->addTime('Saved data to cache "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); - } + return $this->writeCache($this->getCacheKey($options), $data, $this->getCacheOptions($options)); + } - return $cacheKey; + /** + * Write a cache entry by exact key, without a context suffix. + * + * @param string $key + * @param mixed $data + * @return string + */ + public function setExactCache($key, $data) + { + return $this->writeCache($key, $data, $this->getCacheOptions(['cache_key' => $key])); } @@ -1235,7 +1233,7 @@ protected function getCacheOptions($options = []) * * @var mixed $options * - * @return bool|string + * @return string */ protected function getCacheKey($options = []) { @@ -1244,21 +1242,76 @@ protected function getCacheKey($options = []) } if (!empty($options['cache_key'])) { - return $options['cache_key']; + $key = $options['cache_key']; } elseif (!empty($options['cacheKey'])) { - return $options['cacheKey']; + $key = $options['cacheKey']; + } else { + $key = !empty($this->modx->resource) + ? $this->modx->resource->getCacheKey() + : ''; + if (is_array($options)) { + $options['cache_user'] = isset($options['cache_user']) + ? (int)$options['cache_user'] + : $this->modx->user->id; + } + $key .= '/' . sha1(serialize($options)); + } + + return CacheKey::withContext($key, $this->currentContextKey()); + } + + /** + * @return string + */ + protected function currentContextKey() + { + if (!is_object($this->modx->context) || empty($this->modx->context->key)) { + return ''; } - $key = !empty($this->modx->resource) - ? $this->modx->resource->getCacheKey() - : ''; - if (is_array($options)) { - $options['cache_user'] = isset($options['cache_user']) - ? (int)$options['cache_user'] - : $this->modx->user->id; + return (string)$this->modx->context->key; + } + + /** + * @param string $cacheKey + * @param array $cacheOptions + * @return mixed + */ + protected function readCache($cacheKey, array $cacheOptions) + { + $cached = ''; + if (!empty($cacheOptions) && !empty($cacheKey) && $this->modx->getCacheManager()) { + if ($cached = $this->modx->cacheManager->get($cacheKey, $cacheOptions)) { + $this->addTime('Retrieved data from cache "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); + } else { + $this->addTime('No cached data for key "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); + } + } else { + $this->addTime('Could not check cached data for key "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); } - return $key . '/' . sha1(serialize($options)); + return $cached; + } + + /** + * @param string $cacheKey + * @param mixed $data + * @param array $cacheOptions + * @return string + */ + protected function writeCache($cacheKey, $data, array $cacheOptions) + { + if (!empty($cacheKey) && !empty($cacheOptions) && $this->modx->getCacheManager()) { + $this->modx->cacheManager->set( + $cacheKey, + $data, + $cacheOptions[xPDO::OPT_CACHE_EXPIRES], + $cacheOptions + ); + $this->addTime('Saved data to cache "' . $cacheOptions[xPDO::OPT_CACHE_KEY] . '/' . $cacheKey . '"'); + } + + return $cacheKey; } diff --git a/core/components/pdotools/src/Parsing/Fenom/Fenom.php b/core/components/pdotools/src/Parsing/Fenom/Fenom.php index 386c7a4..adb5885 100644 --- a/core/components/pdotools/src/Parsing/Fenom/Fenom.php +++ b/core/components/pdotools/src/Parsing/Fenom/Fenom.php @@ -105,12 +105,10 @@ public function process($chunk, array $properties = []) /** @var \Fenom\Template $tpl */ if (!$tpl = $this->pdoTools->getStore($name, 'fenom')) { if (!empty($this->pdoTools->config('useFenomCache'))) { - $cache_options = [ - 'cache_key' => 'pdotools/' . $name, - ]; - if (!$cache = $this->pdoTools->getCache($cache_options)) { + $compileKey = 'pdotools/' . $name; + if (!$cache = $this->pdoTools->getExactCache($compileKey)) { if ($tpl = $this->_compileChunk($content, $name)) { - $this->pdoTools->setCache($tpl->getTemplateCode(), $cache_options); + $this->pdoTools->setExactCache($compileKey, $tpl->getTemplateCode()); } } else { $cache = preg_replace('#^<\?php#', '', $cache); @@ -190,7 +188,7 @@ protected function _compileChunk($content, $name = '') $this->modx->log(modX::LOG_LEVEL_ERROR, $e->getMessage()); $this->modx->log(modX::LOG_LEVEL_INFO, $content); if ($this->modx->getOption('pdotools_fenom_save_on_errors')) { - $this->pdoTools->setCache($content, ['cache_key' => 'error/' . $name]); + $this->pdoTools->setExactCache('error/' . $name, $content); } $tpl = $this->getRawTemplate()->source($name, '', false); $this->pdoTools->addTime('Can`t compile Fenom chunk with name "' . $name . '": ' . $e->getMessage()); diff --git a/core/components/pdotools/src/Support/CacheKey.php b/core/components/pdotools/src/Support/CacheKey.php new file mode 100644 index 0000000..c18812d --- /dev/null +++ b/core/components/pdotools/src/Support/CacheKey.php @@ -0,0 +1,32 @@ + $returned]) must not become …/web/web. + * + * @param string $key + * @param string $context + * @return string + */ + public static function withContext($key, $context) + { + $key = (string)$key; + $context = trim((string)$context); + if ($key === '' || $context === '') { + return $key; + } + $suffix = '/' . $context; + if (substr($key, -strlen($suffix)) === $suffix) { + return $key; + } + + return rtrim($key, '/') . $suffix; + } +} diff --git a/core/components/pdotools/tests/Stubs/ModxStub.php b/core/components/pdotools/tests/Stubs/ModxStub.php index f21eaa6..10ceba1 100644 --- a/core/components/pdotools/tests/Stubs/ModxStub.php +++ b/core/components/pdotools/tests/Stubs/ModxStub.php @@ -54,6 +54,7 @@ class modX public $user; public $context; public $resource; + public $cacheManager; /** @var \ModxPro\PdoTools\Tests\Stubs\ServiceBag */ public $services; /** @var array */ @@ -248,6 +249,8 @@ class xPDO public const LOG_LEVEL_INFO = 2; public const LOG_LEVEL_DEBUG = 3; public const OPT_CACHE_KEY = 'cache_key'; + public const OPT_CACHE_HANDLER = 'cache_handler'; + public const OPT_CACHE_EXPIRES = 'cache_expires'; } } } diff --git a/core/components/pdotools/tests/Unit/CoreTools/CacheKeyTest.php b/core/components/pdotools/tests/Unit/CoreTools/CacheKeyTest.php index 20c869f..c0e692e 100644 --- a/core/components/pdotools/tests/Unit/CoreTools/CacheKeyTest.php +++ b/core/components/pdotools/tests/Unit/CoreTools/CacheKeyTest.php @@ -9,20 +9,81 @@ class CacheKeyTest extends TestCase { - public function testExplicitCacheKeyWins(): void + public function testExplicitCacheKeyGetsContext(): void { $tools = new CoreToolsHarness($this->modx); - $this->assertSame('pdoMenu/custom', $tools->publicGetCacheKey(['cache_key' => 'pdoMenu/custom'])); - $this->assertSame('legacy', $tools->publicGetCacheKey(['cacheKey' => 'legacy'])); + $this->assertSame('pdoMenu/custom/web', $tools->publicGetCacheKey(['cache_key' => 'pdoMenu/custom'])); + $this->assertSame('legacy/web', $tools->publicGetCacheKey(['cacheKey' => 'legacy'])); } - public function testDefaultKeyIncludesUserAndSha1(): void + public function testDefaultKeyIncludesUserSha1AndContext(): void { $this->modx->user->id = 12; $tools = new CoreToolsHarness($this->modx, ['limit' => 10]); $key = $tools->publicGetCacheKey(); $this->assertIsString($key); - $this->assertMatchesRegularExpression('#^/[a-f0-9]{40}$#', $key); + $this->assertMatchesRegularExpression('#^/[a-f0-9]{40}/web$#', $key); + } + + public function testContextChangeProducesAnotherKey(): void + { + $tools = new CoreToolsHarness($this->modx); + $options = ['cache_key' => 'pdomenu/' . sha1('menu')]; + + $this->assertSame($options['cache_key'] . '/web', $tools->publicGetCacheKey($options)); + + $this->modx->context->key = 'de'; + $this->assertSame($options['cache_key'] . '/de', $tools->publicGetCacheKey($options)); + } + + public function testEmptyContextLeavesExplicitKey(): void + { + $this->modx->context->key = ''; + $tools = new CoreToolsHarness($this->modx); + $this->assertSame('pdoMenu/custom', $tools->publicGetCacheKey(['cache_key' => 'pdoMenu/custom'])); + } + + public function testReturnedKeyIsNotSuffixedTwice(): void + { + $tools = new CoreToolsHarness($this->modx); + $stored = $tools->publicGetCacheKey(['cache_key' => 'pdomenu/menu']); + $this->assertSame('pdomenu/menu/web', $stored); + $this->assertSame($stored, $tools->publicGetCacheKey(['cache_key' => $stored])); + } + + public function testSnippetCacheIsIsolatedByContext(): void + { + $this->attachCacheManager(); + $tools = new CoreToolsHarness($this->modx); + $options = ['cache_key' => 'pdomenu/menu']; + + $this->assertSame('pdomenu/menu/web', $tools->setCache(['tree' => 'web'], $options)); + $this->assertSame(['tree' => 'web'], $tools->getCache($options)); + + $this->modx->context->key = 'de'; + $this->assertEmpty($tools->getCache($options)); + $this->assertSame('pdomenu/menu/de', $tools->setCache(['tree' => 'de'], $options)); + $this->assertSame(['tree' => 'de'], $tools->getCache($options)); + + $this->modx->context->key = 'web'; + $this->assertSame(['tree' => 'web'], $tools->getCache($options)); + } + + public function testExactCacheIgnoresContext(): void + { + $this->attachCacheManager(); + $tools = new CoreToolsHarness($this->modx); + + $this->assertSame('pdotools/chunk', $tools->setExactCache('pdotools/chunk', 'compiled')); + $this->modx->context->key = 'de'; + $this->assertSame('compiled', $tools->getExactCache('pdotools/chunk')); + $this->assertSame('error/chunk', $tools->setExactCache('error/chunk', 'dump')); + $this->assertSame('dump', $tools->getExactCache('error/chunk')); + } + + private function attachCacheManager(): void + { + $this->modx->cacheManager = $this->modx->getCacheManager(); } } diff --git a/core/components/pdotools/tests/Unit/Support/CacheKeyTest.php b/core/components/pdotools/tests/Unit/Support/CacheKeyTest.php index 2937438..b58a175 100644 --- a/core/components/pdotools/tests/Unit/Support/CacheKeyTest.php +++ b/core/components/pdotools/tests/Unit/Support/CacheKeyTest.php @@ -9,14 +9,6 @@ class CacheKeyTest extends TestCase { - protected function setUp(): void - { - parent::setUp(); - if (!class_exists(CacheKey::class)) { - $this->markTestSkipped('CacheKey is not on this branch yet (pdoTools3#24).'); - } - } - public function testAppendsContextOnce(): void { $hash = sha1('menu'); @@ -26,14 +18,41 @@ public function testAppendsContextOnce(): void ); } + public function testSitemapStyleKey(): void + { + $hash = md5('sitemap'); + $this->assertSame( + 'sitemap/' . $hash . '/de', + CacheKey::withContext('sitemap/' . $hash, 'de') + ); + } + public function testIsIdempotentWhenContextAlreadyPresent(): void { $key = 'pdomenu/' . sha1('menu') . '/web'; $this->assertSame($key, CacheKey::withContext($key, 'web')); } + public function testDifferentContextsStayDistinct(): void + { + $base = 'pdomenu/' . sha1('menu'); + $this->assertSame($base . '/web', CacheKey::withContext($base, 'web')); + $this->assertSame($base . '/de', CacheKey::withContext($base, 'de')); + } + public function testEmptyContextLeavesKey(): void { $this->assertSame('pdomenu/abc', CacheKey::withContext('pdomenu/abc', '')); + $this->assertSame('pdomenu/abc', CacheKey::withContext('pdomenu/abc', ' ')); + } + + public function testEmptyKeyStaysEmpty(): void + { + $this->assertSame('', CacheKey::withContext('', 'web')); + } + + public function testStripsTrailingSlashBeforeSuffix(): void + { + $this->assertSame('pdomenu/abc/web', CacheKey::withContext('pdomenu/abc/', 'web')); } }