From 040f53d3e626c43f99d4b2388a5d386cf86b250a Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 13 Jun 2016 16:28:45 +0200 Subject: [PATCH] [Cache] Fix double fetch in ProxyAdapter --- .../Cache/Adapter/AbstractAdapter.php | 4 +- .../Component/Cache/Adapter/ArrayAdapter.php | 8 ++-- .../Component/Cache/Adapter/ChainAdapter.php | 2 +- .../Component/Cache/Adapter/ProxyAdapter.php | 27 +++++++------ src/Symfony/Component/Cache/CacheItem.php | 17 ++++---- .../Cache/Tests/Adapter/ProxyAdapterTest.php | 39 +++++++++++++++++++ 6 files changed, 69 insertions(+), 28 deletions(-) diff --git a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php index 01207f54e6..346a0e6088 100644 --- a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php @@ -42,7 +42,7 @@ abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface return $item; }, - $this, + null, CacheItem::class ); $this->mergeByLifetime = \Closure::bind( @@ -63,7 +63,7 @@ abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface return $byLifetime; }, - $this, + null, CacheItem::class ); } diff --git a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php index 6e55452536..974348d827 100644 --- a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php @@ -45,7 +45,7 @@ class ArrayAdapter implements AdapterInterface, LoggerAwareInterface return $item; }, - $this, + null, CacheItem::class ); } @@ -132,9 +132,9 @@ class ArrayAdapter implements AdapterInterface, LoggerAwareInterface return false; } $item = (array) $item; - $key = $item[CacheItem::CAST_PREFIX.'key']; - $value = $item[CacheItem::CAST_PREFIX.'value']; - $expiry = $item[CacheItem::CAST_PREFIX.'expiry']; + $key = $item["\0*\0key"]; + $value = $item["\0*\0value"]; + $expiry = $item["\0*\0expiry"]; if (null !== $expiry && $expiry <= time()) { $this->deleteItem($key); diff --git a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php index e8b9a43024..c731e47ddd 100644 --- a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php @@ -62,7 +62,7 @@ class ChainAdapter implements AdapterInterface $adapter->save($item); $item->defaultLifetime = $origDefaultLifetime; }, - $this, + null, CacheItem::class ); } diff --git a/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php b/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php index d4e503cf8b..c6a7efe160 100644 --- a/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php @@ -24,23 +24,28 @@ class ProxyAdapter implements AdapterInterface private $namespace; private $namespaceLen; private $createCacheItem; + private $poolHash; public function __construct(CacheItemPoolInterface $pool, $namespace = '', $defaultLifetime = 0) { $this->pool = $pool; + $this->poolHash = $poolHash = spl_object_hash($pool); $this->namespace = '' === $namespace ? '' : $this->getId($namespace); $this->namespaceLen = strlen($namespace); $this->createCacheItem = \Closure::bind( - function ($key, $value, $isHit) use ($defaultLifetime) { + function ($key, $innerItem) use ($defaultLifetime, $poolHash) { $item = new CacheItem(); $item->key = $key; - $item->value = $value; - $item->isHit = $isHit; + $item->value = $innerItem->get(); + $item->isHit = $innerItem->isHit(); $item->defaultLifetime = $defaultLifetime; + $item->innerItem = $innerItem; + $item->poolHash = $poolHash; + $innerItem->set(null); return $item; }, - $this, + null, CacheItem::class ); } @@ -53,7 +58,7 @@ class ProxyAdapter implements AdapterInterface $f = $this->createCacheItem; $item = $this->pool->getItem($this->getId($key)); - return $f($key, $item->get(), $item->isHit()); + return $f($key, $item); } /** @@ -138,12 +143,12 @@ class ProxyAdapter implements AdapterInterface return false; } $item = (array) $item; - $expiry = $item[CacheItem::CAST_PREFIX.'expiry']; - $poolItem = $this->pool->getItem($this->namespace.$item[CacheItem::CAST_PREFIX.'key']); - $poolItem->set($item[CacheItem::CAST_PREFIX.'value']); - $poolItem->expiresAt(null !== $expiry ? \DateTime::createFromFormat('U', $expiry) : null); + $expiry = $item["\0*\0expiry"]; + $innerItem = $item["\0*\0poolHash"] === $this->poolHash ? $item["\0*\0innerItem"] : $this->pool->getItem($this->namespace.$item["\0*\0key"]); + $innerItem->set($item["\0*\0value"]); + $innerItem->expiresAt(null !== $expiry ? \DateTime::createFromFormat('U', $expiry) : null); - return $this->pool->$method($poolItem); + return $this->pool->$method($innerItem); } private function generateItems($items) @@ -155,7 +160,7 @@ class ProxyAdapter implements AdapterInterface $key = substr($key, $this->namespaceLen); } - yield $key => $f($key, $item->get(), $item->isHit()); + yield $key => $f($key, $item); } } diff --git a/src/Symfony/Component/Cache/CacheItem.php b/src/Symfony/Component/Cache/CacheItem.php index e09dfafd6b..2678da4a1d 100644 --- a/src/Symfony/Component/Cache/CacheItem.php +++ b/src/Symfony/Component/Cache/CacheItem.php @@ -20,16 +20,13 @@ use Symfony\Component\Cache\Exception\InvalidArgumentException; */ final class CacheItem implements CacheItemInterface { - /** - * @internal - */ - const CAST_PREFIX = "\0Symfony\Component\Cache\CacheItem\0"; - - private $key; - private $value; - private $isHit; - private $expiry; - private $defaultLifetime; + protected $key; + protected $value; + protected $isHit; + protected $expiry; + protected $defaultLifetime; + protected $innerItem; + protected $poolHash; /** * {@inheritdoc} diff --git a/src/Symfony/Component/Cache/Tests/Adapter/ProxyAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/ProxyAdapterTest.php index 76606e256e..21f2a2bf58 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/ProxyAdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/ProxyAdapterTest.php @@ -12,8 +12,10 @@ namespace Symfony\Component\Cache\Tests\Adapter; use Cache\IntegrationTests\CachePoolTest; +use Psr\Cache\CacheItemInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\Adapter\ProxyAdapter; +use Symfony\Component\Cache\CacheItem; /** * @group time-sensitive @@ -29,4 +31,41 @@ class ProxyAdapterTest extends CachePoolTest { return new ProxyAdapter(new ArrayAdapter()); } + + /** + * @expectedException Exception + * @expectedExceptionMessage OK bar + */ + public function testProxyfiedItem() + { + $item = new CacheItem(); + $pool = new ProxyAdapter(new TestingArrayAdapter($item)); + + $proxyItem = $pool->getItem('foo'); + + $this->assertFalse($proxyItem === $item); + $pool->save($proxyItem->set('bar')); + } +} + +class TestingArrayAdapter extends ArrayAdapter +{ + private $item; + + public function __construct(CacheItemInterface $item) + { + $this->item = $item; + } + + public function getItem($key) + { + return $this->item; + } + + public function save(CacheItemInterface $item) + { + if ($item === $this->item) { + throw new \Exception('OK '.$item->get()); + } + } }