SimpleCacheAdapter fails to cache any item if a namespace is used

moufmouf · fabpot · commit ffd3469ddfb9 · 2019-06-14T13:16:02.000+02:00
diff --git a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php
@@ -39,7 +39,7 @@ abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface
      */
     protected function __construct($namespace = '', $defaultLifetime = 0)
     {
-        $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).':';
+        $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).static::getNsSeparator();
         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/SimpleCacheAdapter.php b/src/Symfony/Component/Cache/Adapter/SimpleCacheAdapter.php
@@ -75,4 +75,12 @@ protected function doSave(array $values, $lifetime)
     {
         return $this->pool->setMultiple($values, 0 === $lifetime ? null : $lifetime);
     }
+
+    /**
+     * @return string the namespace separator for cache keys
+     */
+    protected static function getNsSeparator()
+    {
+        return '_';
+    }
 }
diff --git a/src/Symfony/Component/Cache/Tests/Adapter/SimpleCacheAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/SimpleCacheAdapterTest.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Cache\Tests\Adapter;
 
 use Symfony\Component\Cache\Adapter\SimpleCacheAdapter;
+use Symfony\Component\Cache\Simple\ArrayCache;
 use Symfony\Component\Cache\Simple\FilesystemCache;
 
 /**
@@ -27,4 +28,14 @@ public function createCachePool($defaultLifetime = 0)
     {
         return new SimpleCacheAdapter(new FilesystemCache(), '', $defaultLifetime);
     }
+
+    public function testValidCacheKeyWithNamespace()
+    {
+        $cache = new SimpleCacheAdapter(new ArrayCache(), 'some_namespace', 0);
+        $item = $cache->getItem('my_key');
+        $item->set('someValue');
+        $cache->save($item);
+
+        $this->assertTrue($cache->getItem('my_key')->isHit(), 'Stored item is successfully retrieved.');
+    }
 }
diff --git a/src/Symfony/Component/Cache/Traits/AbstractTrait.php b/src/Symfony/Component/Cache/Traits/AbstractTrait.php
@@ -106,7 +106,7 @@ public function clear()
     {
         $this->deferred = [];
         if ($cleared = $this->versioningIsEnabled) {
-            $namespaceVersion = substr_replace(base64_encode(pack('V', mt_rand())), ':', 5);
+            $namespaceVersion = substr_replace(base64_encode(pack('V', mt_rand())), static::getNsSeparator(), 5);
             try {
                 $cleared = $this->doSave(['@'.$this->namespace => $namespaceVersion], 0);
             } catch (\Exception $e) {
@@ -235,13 +235,13 @@ private function getId($key)
         CacheItem::validateKey($key);
 
         if ($this->versioningIsEnabled && '' === $this->namespaceVersion) {
-            $this->namespaceVersion = '1:';
+            $this->namespaceVersion = '1'.static::getNsSeparator();
             try {
                 foreach ($this->doFetch(['@'.$this->namespace]) as $v) {
                     $this->namespaceVersion = $v;
                 }
-                if ('1:' === $this->namespaceVersion) {
-                    $this->namespaceVersion = substr_replace(base64_encode(pack('V', time())), ':', 5);
+                if ('1'.static::getNsSeparator() === $this->namespaceVersion) {
+                    $this->namespaceVersion = substr_replace(base64_encode(pack('V', time())), static::getNsSeparator(), 5);
                     $this->doSave(['@'.$this->namespace => $this->namespaceVersion], 0);
                 }
             } catch (\Exception $e) {
@@ -252,7 +252,7 @@ private function getId($key)
             return $this->namespace.$this->namespaceVersion.$key;
         }
         if (\strlen($id = $this->namespace.$this->namespaceVersion.$key) > $this->maxIdLength) {
-            $id = $this->namespace.$this->namespaceVersion.substr_replace(base64_encode(hash('sha256', $key, true)), ':', -(\strlen($this->namespaceVersion) + 22));
+            $id = $this->namespace.$this->namespaceVersion.substr_replace(base64_encode(hash('sha256', $key, true)), static::getNsSeparator(), -(\strlen($this->namespaceVersion) + 22));
         }
 
         return $id;
@@ -265,4 +265,12 @@ public static function handleUnserializeCallback($class)
     {
         throw new \DomainException('Class not found: '.$class);
     }
+
+    /**
+     * @return string the namespace separator for cache keys
+     */
+    protected static function getNsSeparator()
+    {
+        return ':';
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface`
`39`	`39`	`*/`
`40`	`40`	`protected function __construct($namespace = '', $defaultLifetime = 0)`
`41`	`41`	`{`
`42`		`- $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).':';`
	`42`	`+ $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).static::getNsSeparator();`
`43`	`43`	`if (null !== $this->maxIdLength && \strlen($namespace) > $this->maxIdLength - 24) {`
`44`	`44`	`throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s")', $this->maxIdLength - 24, \strlen($namespace), $namespace));`
`45`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -75,4 +75,12 @@ protected function doSave(array $values, $lifetime)`
`75`	`75`	`{`
`76`	`76`	`return $this->pool->setMultiple($values, 0 === $lifetime ? null : $lifetime);`
`77`	`77`	`}`
	`78`	`+`
	`79`	`+ /**`
	`80`	`+ * @return string the namespace separator for cache keys`
	`81`	`+ */`
	`82`	`+ protected static function getNsSeparator()`
	`83`	`+ {`
	`84`	`+ return '_';`
	`85`	`+ }`
`78`	`86`	`}`
Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ public function clear()`
`106`	`106`	`{`
`107`	`107`	`$this->deferred = [];`
`108`	`108`	`if ($cleared = $this->versioningIsEnabled) {`
`109`		`- $namespaceVersion = substr_replace(base64_encode(pack('V', mt_rand())), ':', 5);`
	`109`	`+ $namespaceVersion = substr_replace(base64_encode(pack('V', mt_rand())), static::getNsSeparator(), 5);`
`110`	`110`	`try {`
`111`	`111`	`$cleared = $this->doSave(['@'.$this->namespace => $namespaceVersion], 0);`
`112`	`112`	`} catch (\Exception $e) {`
`@@ -235,13 +235,13 @@ private function getId($key)`
`235`	`235`	`CacheItem::validateKey($key);`
`236`	`236`
`237`	`237`	`if ($this->versioningIsEnabled && '' === $this->namespaceVersion) {`
`238`		`- $this->namespaceVersion = '1:';`
	`238`	`+ $this->namespaceVersion = '1'.static::getNsSeparator();`
`239`	`239`	`try {`
`240`	`240`	`foreach ($this->doFetch(['@'.$this->namespace]) as $v) {`
`241`	`241`	`$this->namespaceVersion = $v;`
`242`	`242`	`}`
`243`		`- if ('1:' === $this->namespaceVersion) {`
`244`		`- $this->namespaceVersion = substr_replace(base64_encode(pack('V', time())), ':', 5);`
	`243`	`+ if ('1'.static::getNsSeparator() === $this->namespaceVersion) {`
	`244`	`+ $this->namespaceVersion = substr_replace(base64_encode(pack('V', time())), static::getNsSeparator(), 5);`
`245`	`245`	`$this->doSave(['@'.$this->namespace => $this->namespaceVersion], 0);`
`246`	`246`	`}`
`247`	`247`	`} catch (\Exception $e) {`
`@@ -252,7 +252,7 @@ private function getId($key)`
`252`	`252`	`return $this->namespace.$this->namespaceVersion.$key;`
`253`	`253`	`}`
`254`	`254`	`if (\strlen($id = $this->namespace.$this->namespaceVersion.$key) > $this->maxIdLength) {`
`255`		`- $id = $this->namespace.$this->namespaceVersion.substr_replace(base64_encode(hash('sha256', $key, true)), ':', -(\strlen($this->namespaceVersion) + 22));`
	`255`	`+ $id = $this->namespace.$this->namespaceVersion.substr_replace(base64_encode(hash('sha256', $key, true)), static::getNsSeparator(), -(\strlen($this->namespaceVersion) + 22));`
`256`	`256`	`}`
`257`	`257`
`258`	`258`	`return $id;`
`@@ -265,4 +265,12 @@ public static function handleUnserializeCallback($class)`
`265`	`265`	`{`
`266`	`266`	`throw new \DomainException('Class not found: '.$class);`
`267`	`267`	`}`
	`268`	`+`
	`269`	`+ /**`
	`270`	`+ * @return string the namespace separator for cache keys`
	`271`	`+ */`
	`272`	`+ protected static function getNsSeparator()`
	`273`	`+ {`
	`274`	`+ return ':';`
	`275`	`+ }`
`268`	`276`	`}`