From 02fd55286f29b838e0fd24a0064b173815a2e4a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Thu, 13 Jun 2019 11:23:44 +0200 Subject: [PATCH 1/3] Adding a failing test for Psr16Adapter + namespace --- .../Cache/Tests/Adapter/Psr16AdapterTest.php | 17 +++++++++++++++++ src/Symfony/Component/Cache/composer.json | 1 + 2 files changed, 18 insertions(+) diff --git a/src/Symfony/Component/Cache/Tests/Adapter/Psr16AdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/Psr16AdapterTest.php index 19907ddf7873..e67a9962ec6d 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/Psr16AdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/Psr16AdapterTest.php @@ -14,6 +14,7 @@ use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Component\Cache\Adapter\Psr16Adapter; use Symfony\Component\Cache\Psr16Cache; +use Symfony\Component\Debug\BufferingLogger; /** * @group time-sensitive @@ -28,4 +29,20 @@ public function createCachePool($defaultLifetime = 0) { return new Psr16Adapter(new Psr16Cache(new FilesystemAdapter()), '', $defaultLifetime); } + + public function testValidCacheKeyWithNamespace() + { + $logger = new BufferingLogger(); + $cache = new Psr16Adapter(new Psr16Cache(new FilesystemAdapter()), 'some_namespace', 0); + $cache->setLogger($logger); + $this->assertSame('foo', $cache->get('my_key', function () { + return 'foo'; + })); + $logs = $logger->cleanLogs(); + foreach ($logs as $log) { + if ('warning' === $log[0] || 'error' === $log[0]) { + $this->fail('An error was triggered while caching key with a namespace: '.$log[1]); + } + } + } } diff --git a/src/Symfony/Component/Cache/composer.json b/src/Symfony/Component/Cache/composer.json index 05bed1cf7548..d3ab17689cdb 100644 --- a/src/Symfony/Component/Cache/composer.json +++ b/src/Symfony/Component/Cache/composer.json @@ -35,6 +35,7 @@ "predis/predis": "~1.1", "psr/simple-cache": "^1.0", "symfony/config": "~4.2", + "symfony/debug": "^4", "symfony/dependency-injection": "~3.4|~4.1", "symfony/var-dumper": "^4.1.1" }, From 693f88961cc0ba61d60fb65f62f6ef83e9c1f09c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Thu, 13 Jun 2019 11:32:56 +0200 Subject: [PATCH 2/3] Changing namespace separator to _ to match PSR-16 allowed keys --- src/Symfony/Component/Cache/Adapter/AbstractAdapter.php | 2 +- .../Cache/Tests/Adapter/MaxIdLengthAdapterTest.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php index 883a94ba9bae..6bfcb99ed2dc 100644 --- a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php @@ -34,7 +34,7 @@ abstract class AbstractAdapter implements AdapterInterface, CacheInterface, Logg protected function __construct(string $namespace = '', int $defaultLifetime = 0) { - $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).':'; + $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).'_'; if (null !== $this->maxIdLength && \strlen($namespace) > $this->maxIdLength - 24) { throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s")', $this->maxIdLength - 24, \strlen($namespace), $namespace)); } diff --git a/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php index fec985e6da99..f18437a29b49 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php @@ -26,8 +26,8 @@ public function testLongKey() $cache->expects($this->exactly(2)) ->method('doHave') ->withConsecutive( - [$this->equalTo('----------:nWfzGiCgLczv3SSUzXL3kg:')], - [$this->equalTo('----------:---------------------------------------')] + [$this->equalTo('----------_nWfzGiCgLczv3SSUzXL3kg:')], + [$this->equalTo('----------_---------------------------------------')] ); $cache->hasItem(str_repeat('-', 40)); @@ -46,7 +46,7 @@ public function testLongKeyVersioning() $reflectionMethod->setAccessible(true); // No versioning enabled - $this->assertEquals('--------------------------:------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); + $this->assertEquals('--------------------------_------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 23)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 40)]))); @@ -56,7 +56,7 @@ public function testLongKeyVersioning() $reflectionProperty->setValue($cache, true); // Versioning enabled - $this->assertEquals('--------------------------:1/------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); + $this->assertEquals('--------------------------_1/------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 23)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 40)]))); From f5474609cac2fd350ece31a18da787072be324e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Thu, 13 Jun 2019 15:13:21 +0200 Subject: [PATCH 3/3] Using configurable NS_SEPARATOR to avoid changing keys for other implementations --- src/Symfony/Component/Cache/Adapter/AbstractAdapter.php | 4 +++- src/Symfony/Component/Cache/Adapter/Psr16Adapter.php | 2 ++ .../Cache/Tests/Adapter/MaxIdLengthAdapterTest.php | 8 ++++---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php index 6bfcb99ed2dc..51c4b6664c74 100644 --- a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php @@ -29,12 +29,14 @@ abstract class AbstractAdapter implements AdapterInterface, CacheInterface, Logg use AbstractAdapterTrait; use ContractsTrait; + protected const NS_SEPARATOR = ':'; + private static $apcuSupported; private static $phpFilesSupported; protected function __construct(string $namespace = '', int $defaultLifetime = 0) { - $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).'_'; + $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).static::NS_SEPARATOR; if (null !== $this->maxIdLength && \strlen($namespace) > $this->maxIdLength - 24) { throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s")', $this->maxIdLength - 24, \strlen($namespace), $namespace)); } diff --git a/src/Symfony/Component/Cache/Adapter/Psr16Adapter.php b/src/Symfony/Component/Cache/Adapter/Psr16Adapter.php index a14ee082be1e..ffe68b7c7a5b 100644 --- a/src/Symfony/Component/Cache/Adapter/Psr16Adapter.php +++ b/src/Symfony/Component/Cache/Adapter/Psr16Adapter.php @@ -25,6 +25,8 @@ class Psr16Adapter extends AbstractAdapter implements PruneableInterface, Resett { use ProxyTrait; + protected const NS_SEPARATOR = '_'; + private $miss; public function __construct(CacheInterface $pool, string $namespace = '', int $defaultLifetime = 0) diff --git a/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php index f18437a29b49..fec985e6da99 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php @@ -26,8 +26,8 @@ public function testLongKey() $cache->expects($this->exactly(2)) ->method('doHave') ->withConsecutive( - [$this->equalTo('----------_nWfzGiCgLczv3SSUzXL3kg:')], - [$this->equalTo('----------_---------------------------------------')] + [$this->equalTo('----------:nWfzGiCgLczv3SSUzXL3kg:')], + [$this->equalTo('----------:---------------------------------------')] ); $cache->hasItem(str_repeat('-', 40)); @@ -46,7 +46,7 @@ public function testLongKeyVersioning() $reflectionMethod->setAccessible(true); // No versioning enabled - $this->assertEquals('--------------------------_------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); + $this->assertEquals('--------------------------:------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 23)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 40)]))); @@ -56,7 +56,7 @@ public function testLongKeyVersioning() $reflectionProperty->setValue($cache, true); // Versioning enabled - $this->assertEquals('--------------------------_1/------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); + $this->assertEquals('--------------------------:1/------------', $reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)])); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 12)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 23)]))); $this->assertLessThanOrEqual(50, \strlen($reflectionMethod->invokeArgs($cache, [str_repeat('-', 40)])));