Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PSR16 compatibility by sanitizing Colon symbol #447

Closed
wants to merge 12 commits into from
4 changes: 4 additions & 0 deletions .github/workflows/test-application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
dbu marked this conversation as resolved.
Show resolved Hide resolved

- name: Install dependencies with Composer
uses: ramsey/composer-install@v1
with:
Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
},
"require-dev": {
"psr/log": "^1 || ^2 || ^3",
"psr/simple-cache": "^1.0 || ^2.0 || ^3.0",
dbu marked this conversation as resolved.
Show resolved Hide resolved
"phpcr/phpcr-api-tests": "2.1.22",
"phpunit/phpunit": "8.5.21",
"symfony/cache": "^5.4 || ^6.2"
Expand Down
24 changes: 17 additions & 7 deletions src/Jackalope/Transport/DoctrineDBAL/CachedClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
};
}

Expand Down Expand Up @@ -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))) {
dbu marked this conversation as resolved.
Show resolved Hide resolved
if ('ItemNotFoundException' === $result) {
throw new ItemNotFoundException("Item '$path' not found in workspace '$this->workspaceName'");
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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,
]);
}
}
22 changes: 9 additions & 13 deletions tests/Jackalope/Transport/DoctrineDBAL/CachedClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading