Merge pull request #448 from jackalope/cache-psr16-compatible

Fix PSR16 cache support: sanitize all non-guaranteed characters in key

Author: dbu

.github/workflows/test-application.yaml

@@ -75,6 +75,10 @@ jobs:
                   extensions: "pdo, pdo_sqlite, pdo_mysql, mysql, pdo_pgsql"
                   tools: 'composer:v2'
 
+            - name: Require dependencies
+              if: ${{ matrix.php-version == '8.0' }}
+              run: composer require "psr/simple-cache:2.*" --no-update --dev # This can be removed if min version of symfony/cache is ^6.0
+
             - name: Install dependencies with Composer
               uses: ramsey/composer-install@v1
               with:

CHANGELOG.md

@@ -4,6 +4,12 @@ Changelog
 1.x
 ===
 
+1.13.0
+------
+
+* Fixed cache key sanitize for PSR-16 cache.
+* Fixed not found detection for PSR-16 cache.
+
 1.12.0
 ------
 

composer.json

@@ -30,6 +30,7 @@
     },
     "require-dev": {
         "psr/log": "^1 || ^2 || ^3",
+        "psr/simple-cache": "^1.0 || ^2.0 || ^3.0",
         "phpcr/phpcr-api-tests": "2.1.22",
         "phpunit/phpunit": "8.5.21",
         "symfony/cache": "^5.4 || ^6.2"

src/Jackalope/Transport/DoctrineDBAL/CachedClient.php

@@ -51,7 +51,11 @@ public function __construct(FactoryInterface $factory, Connection $conn, array $
 
         $this->caches = $caches;
         $this->keySanitizer = static function ($cacheKey) {
-            return str_replace(' ', '_', $cacheKey);
+            return str_replace(
+                ['%', '.'],
+                ['_', '|'],
+                \urlencode($cacheKey)
+            );
         };
     }
 
@@ -259,7 +263,7 @@ public function getNode($path)
         $cacheKey = "nodes: $path, ".$this->workspaceName;
         $cacheKey = $this->sanitizeKey($cacheKey);
 
-        if (false !== ($result = $this->get('nodes', $cacheKey))) {
+        if (null !== ($result = $this->get('nodes', $cacheKey))) {
             if ('ItemNotFoundException' === $result) {
                 throw new ItemNotFoundException("Item '$path' not found in workspace '$this->workspaceName'");
             }
@@ -319,7 +323,7 @@ protected function getSystemIdForNodeUuid($uuid, $workspaceName = null)
         $cacheKey = "id: $uuid, ".$workspaceName;
         $cacheKey = $this->sanitizeKey($cacheKey);
 
-        if (false !== ($result = $this->get('nodes', $cacheKey))) {
+        if (null !== ($result = $this->get('nodes', $cacheKey))) {
             if ('false' === $result) {
                 return false;
             }
@@ -495,7 +499,7 @@ public function getNodePathForIdentifier($uuid, $workspace = null)
         $cacheKey = "nodes by uuid: $uuid, $this->workspaceName";
         $cacheKey = $this->sanitizeKey($cacheKey);
 
-        if (false !== ($result = $this->get('nodes', $cacheKey))) {
+        if (null !== ($result = $this->get('nodes', $cacheKey))) {
             if ('ItemNotFoundException' === $result) {
                 throw new ItemNotFoundException("no item found with uuid $uuid");
             }
@@ -560,7 +564,7 @@ public function getReferences($path, $name = null)
         $cacheKey = "nodes references: $path, $name, " . $this->workspaceName;
         $cacheKey = $this->sanitizeKey($cacheKey);
 
-        if (false !== ($result = $this->get('nodes', $cacheKey))) {
+        if (null !== ($result = $this->get('nodes', $cacheKey))) {
             return $result;
         }
 
@@ -608,7 +612,7 @@ public function query(Query $query)
         $cacheKey = "query: {$query->getStatement()}, {$query->getLimit()}, {$query->getOffset()}, {$query->getLanguage()}, ".$this->workspaceName;
         $cacheKey = $this->sanitizeKey($cacheKey);
 
-        if (false !== ($result = $this->get('query', $cacheKey))) {
+        if (null !== ($result = $this->get('query', $cacheKey))) {
             return $result;
         }
 
@@ -667,6 +671,12 @@ private function get(string $name, string $key)
             return $this->caches[$name]->get($key);
         }
 
-        return $this->caches[$name]->fetch($key);
+        $result = $this->caches[$name]->fetch($key);
+
+        if ($result === false) {
+            return null;
+        }
+
+        return $result;
     }
 }

tests/Jackalope/Transport/DoctrineDBAL/CachedClientFunctionalTest.php

@@ -0,0 +1,25 @@
+<?php
+
+namespace Jackalope\Transport\DoctrineDBAL;
+
+use Doctrine\DBAL\Connection;
+use Jackalope\Factory;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+use Symfony\Component\Cache\Psr16Cache;
+
+class CachedClientFunctionalTest extends ClientTest
+{
+    protected function getClient(Connection $conn)
+    {
+        $nodeCacheAdapter = new ArrayAdapter();
+        $nodeCache = new Psr16Cache($nodeCacheAdapter);
+
+        $metaCacheAdapter = new ArrayAdapter();
+        $metaCache = new Psr16Cache($metaCacheAdapter);
+
+        return new CachedClient(new Factory(), $conn, [
+            'nodes' => $nodeCache,
+            'meta' => $metaCache,
+        ]);
+    }
+}

tests/Jackalope/Transport/DoctrineDBAL/CachedClientTest.php

@@ -31,29 +31,25 @@ public function testArrayObjectIsConvertedToArray()
     public function testCacheHit()
     {
         $cache = new \stdClass();
-        $this->cacheMock->method('fetch')->with('nodes:_/test,_tests')->willReturn($cache);
+        $this->cacheMock->method('fetch')->with('nodes_3A+_2Ftest_2C+tests')->willReturn($cache);
 
         $this->assertSame($cache, $this->transport->getNode('/test'));
     }
 
     /**
-     * The default key sanitizer replaces spaces with underscores
+     * The default key sanitizer keeps the cache key compatible with PSR16
      */
     public function testDefaultKeySanitizer()
     {
-        $first = true;
-        $this->cacheMock
-            ->method('fetch')
-            ->with(self::callback(function ($arg) use (&$first) {
-                self::assertEquals($first ? 'nodetypes:_a:0:{}' : 'node_types', $arg);
-                $first = false;
+        $client = $this->getClient($this->getConnection());
+        $reflection = new \ReflectionClass($client);
+        $keySanitizerProperty = $reflection->getProperty('keySanitizer');
+        $keySanitizerProperty->setAccessible(true);
+        $defaultKeySanitizer = $keySanitizerProperty->getValue($client);
 
-                return true;
-            }));
+        $result = $defaultKeySanitizer(' :{}().@/"\\'); // not allowed PSR16 keys
 
-        /** @var CachedClient $cachedClient */
-        $cachedClient = $this->transport;
-        $cachedClient->getNodeTypes();
+        $this->assertEquals('+_3A_7B_7D_28_29|_40_2F_22_5C', $result);
     }
 
     public function testCustomkeySanitizer()