From 49e9f3229c1441398f30d45393a1a99219b65f1c Mon Sep 17 00:00:00 2001 From: Philipp Kolesnikov Date: Wed, 8 Jul 2020 16:33:42 +1000 Subject: [PATCH] [Cache] Use the default expiry when saving (not when creating) items --- .../Cache/Adapter/AbstractAdapter.php | 9 +- .../Component/Cache/Adapter/ArrayAdapter.php | 9 +- .../Component/Cache/Adapter/ChainAdapter.php | 8 +- .../Component/Cache/Adapter/ProxyAdapter.php | 9 +- .../Cache/Adapter/TagAwareAdapter.php | 4 +- src/Symfony/Component/Cache/CacheItem.php | 5 +- .../Cache/Tests/Adapter/ChainAdapterTest.php | 120 +++++++++++++++++- .../Cache/Tests/Fixtures/ExternalAdapter.php | 4 +- 8 files changed, 141 insertions(+), 27 deletions(-) diff --git a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php index ce1262ae17..4022307284 100644 --- a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php @@ -48,12 +48,11 @@ abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s").', $this->maxIdLength - 24, \strlen($namespace), $namespace)); } $this->createCacheItem = \Closure::bind( - static function ($key, $value, $isHit) use ($defaultLifetime) { + static function ($key, $value, $isHit) { $item = new CacheItem(); $item->key = $key; $item->value = $value; $item->isHit = $isHit; - $item->defaultLifetime = $defaultLifetime; return $item; }, @@ -62,14 +61,16 @@ abstract class AbstractAdapter implements AdapterInterface, LoggerAwareInterface ); $getId = function ($key) { return $this->getId((string) $key); }; $this->mergeByLifetime = \Closure::bind( - static function ($deferred, $namespace, &$expiredIds) use ($getId) { + static function ($deferred, $namespace, &$expiredIds) use ($getId, $defaultLifetime) { $byLifetime = []; $now = time(); $expiredIds = []; foreach ($deferred as $key => $item) { if (null === $item->expiry) { - $byLifetime[0 < $item->defaultLifetime ? $item->defaultLifetime : 0][$getId($key)] = $item->value; + $byLifetime[0 < $defaultLifetime ? $defaultLifetime : 0][$getId($key)] = $item->value; + } elseif (0 === $item->expiry) { + $byLifetime[0][$getId($key)] = $item->value; } elseif ($item->expiry > $now) { $byLifetime[$item->expiry - $now][$getId($key)] = $item->value; } else { diff --git a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php index 4c6695e267..ff826f47a2 100644 --- a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php @@ -25,6 +25,7 @@ class ArrayAdapter implements AdapterInterface, LoggerAwareInterface, Resettable use ArrayTrait; private $createCacheItem; + private $defaultLifetime; /** * @param int $defaultLifetime @@ -32,14 +33,14 @@ class ArrayAdapter implements AdapterInterface, LoggerAwareInterface, Resettable */ public function __construct($defaultLifetime = 0, $storeSerialized = true) { + $this->defaultLifetime = $defaultLifetime; $this->storeSerialized = $storeSerialized; $this->createCacheItem = \Closure::bind( - static function ($key, $value, $isHit) use ($defaultLifetime) { + static function ($key, $value, $isHit) { $item = new CacheItem(); $item->key = $key; $item->value = $value; $item->isHit = $isHit; - $item->defaultLifetime = $defaultLifetime; return $item; }, @@ -127,8 +128,8 @@ class ArrayAdapter implements AdapterInterface, LoggerAwareInterface, Resettable return false; } } - if (null === $expiry && 0 < $item["\0*\0defaultLifetime"]) { - $expiry = time() + $item["\0*\0defaultLifetime"]; + if (null === $expiry && 0 < $this->defaultLifetime) { + $expiry = time() + $this->defaultLifetime; } $this->values[$key] = $value; diff --git a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php index c2c9f4a800..2bf89bcf96 100644 --- a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php @@ -61,14 +61,10 @@ class ChainAdapter implements AdapterInterface, PruneableInterface, ResettableIn $this->syncItem = \Closure::bind( static function ($sourceItem, $item) use ($defaultLifetime) { $item->value = $sourceItem->value; - $item->expiry = $sourceItem->expiry; $item->isHit = $sourceItem->isHit; - if (0 < $sourceItem->defaultLifetime && $sourceItem->defaultLifetime < $defaultLifetime) { - $defaultLifetime = $sourceItem->defaultLifetime; - } - if (0 < $defaultLifetime && ($item->defaultLifetime <= 0 || $defaultLifetime < $item->defaultLifetime)) { - $item->defaultLifetime = $defaultLifetime; + if (0 < $defaultLifetime) { + $item->expiresAfter($defaultLifetime); } return $item; diff --git a/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php b/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php index f574826844..c89a760ed2 100644 --- a/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php @@ -29,6 +29,7 @@ class ProxyAdapter implements AdapterInterface, PruneableInterface, ResettableIn private $namespaceLen; private $createCacheItem; private $poolHash; + private $defaultLifetime; /** * @param string $namespace @@ -40,11 +41,11 @@ class ProxyAdapter implements AdapterInterface, PruneableInterface, ResettableIn $this->poolHash = $poolHash = spl_object_hash($pool); $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace); $this->namespaceLen = \strlen($namespace); + $this->defaultLifetime = $defaultLifetime; $this->createCacheItem = \Closure::bind( - static function ($key, $innerItem) use ($defaultLifetime, $poolHash) { + static function ($key, $innerItem) use ($poolHash) { $item = new CacheItem(); $item->key = $key; - $item->defaultLifetime = $defaultLifetime; $item->poolHash = $poolHash; if (null !== $innerItem) { @@ -155,8 +156,8 @@ class ProxyAdapter implements AdapterInterface, PruneableInterface, ResettableIn } $item = (array) $item; $expiry = $item["\0*\0expiry"]; - if (null === $expiry && 0 < $item["\0*\0defaultLifetime"]) { - $expiry = time() + $item["\0*\0defaultLifetime"]; + if (null === $expiry && 0 < $this->defaultLifetime) { + $expiry = time() + $this->defaultLifetime; } if ($item["\0*\0poolHash"] === $this->poolHash && $item["\0*\0innerItem"]) { diff --git a/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php b/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php index ecdd0d6991..febe50900a 100644 --- a/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php @@ -46,7 +46,6 @@ class TagAwareAdapter implements TagAwareAdapterInterface, PruneableInterface, R $item = new CacheItem(); $item->key = $key; $item->value = $value; - $item->defaultLifetime = $protoItem->defaultLifetime; $item->expiry = $protoItem->expiry; $item->poolHash = $protoItem->poolHash; @@ -90,8 +89,7 @@ class TagAwareAdapter implements TagAwareAdapterInterface, PruneableInterface, R $this->invalidateTags = \Closure::bind( static function (AdapterInterface $tagsAdapter, array $tags) { foreach ($tags as $v) { - $v->defaultLifetime = 0; - $v->expiry = null; + $v->expiry = 0; $tagsAdapter->saveDeferred($v); } diff --git a/src/Symfony/Component/Cache/CacheItem.php b/src/Symfony/Component/Cache/CacheItem.php index b16ad0faf7..e802c30729 100644 --- a/src/Symfony/Component/Cache/CacheItem.php +++ b/src/Symfony/Component/Cache/CacheItem.php @@ -24,7 +24,6 @@ final class CacheItem implements CacheItemInterface protected $value; protected $isHit = false; protected $expiry; - protected $defaultLifetime; protected $tags = []; protected $prevTags = []; protected $innerItem; @@ -74,7 +73,7 @@ final class CacheItem implements CacheItemInterface public function expiresAt($expiration) { if (null === $expiration) { - $this->expiry = $this->defaultLifetime > 0 ? time() + $this->defaultLifetime : null; + $this->expiry = null; } elseif ($expiration instanceof \DateTimeInterface) { $this->expiry = (int) $expiration->format('U'); } else { @@ -92,7 +91,7 @@ final class CacheItem implements CacheItemInterface public function expiresAfter($time) { if (null === $time) { - $this->expiry = $this->defaultLifetime > 0 ? time() + $this->defaultLifetime : null; + $this->expiry = null; } elseif ($time instanceof \DateInterval) { $this->expiry = (int) \DateTime::createFromFormat('U', time())->add($time)->format('U'); } elseif (\is_int($time)) { diff --git a/src/Symfony/Component/Cache/Tests/Adapter/ChainAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/ChainAdapterTest.php index 5e48930dd1..be811d6fd7 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/ChainAdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/ChainAdapterTest.php @@ -27,7 +27,7 @@ class ChainAdapterTest extends AdapterTestCase { public function createCachePool($defaultLifetime = 0) { - return new ChainAdapter([new ArrayAdapter($defaultLifetime), new ExternalAdapter(), new FilesystemAdapter('', $defaultLifetime)], $defaultLifetime); + return new ChainAdapter([new ArrayAdapter($defaultLifetime), new ExternalAdapter($defaultLifetime), new FilesystemAdapter('', $defaultLifetime)], $defaultLifetime); } public function testEmptyAdaptersException() @@ -65,6 +65,124 @@ class ChainAdapterTest extends AdapterTestCase $this->assertFalse($cache->prune()); } + public function testMultipleCachesExpirationWhenCommonTtlIsNotSet() + { + if (isset($this->skippedTests[__FUNCTION__])) { + $this->markTestSkipped($this->skippedTests[__FUNCTION__]); + } + + $adapter1 = new ArrayAdapter(4); + $adapter2 = new ArrayAdapter(2); + + $cache = new ChainAdapter([$adapter1, $adapter2]); + + $cache->save($cache->getItem('key')->set('value')); + + $item = $adapter1->getItem('key'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value', $item->get()); + + $item = $adapter2->getItem('key'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value', $item->get()); + + sleep(2); + + $item = $adapter1->getItem('key'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value', $item->get()); + + $item = $adapter2->getItem('key'); + $this->assertFalse($item->isHit()); + + sleep(2); + + $item = $adapter1->getItem('key'); + $this->assertFalse($item->isHit()); + + $adapter2->save($adapter2->getItem('key1')->set('value1')); + + $item = $cache->getItem('key1'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value1', $item->get()); + + sleep(2); + + $item = $adapter1->getItem('key1'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value1', $item->get()); + + $item = $adapter2->getItem('key1'); + $this->assertFalse($item->isHit()); + + sleep(2); + + $item = $adapter1->getItem('key1'); + $this->assertFalse($item->isHit()); + } + + public function testMultipleCachesExpirationWhenCommonTtlIsSet() + { + if (isset($this->skippedTests[__FUNCTION__])) { + $this->markTestSkipped($this->skippedTests[__FUNCTION__]); + } + + $adapter1 = new ArrayAdapter(4); + $adapter2 = new ArrayAdapter(2); + + $cache = new ChainAdapter([$adapter1, $adapter2], 6); + + $cache->save($cache->getItem('key')->set('value')); + + $item = $adapter1->getItem('key'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value', $item->get()); + + $item = $adapter2->getItem('key'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value', $item->get()); + + sleep(2); + + $item = $adapter1->getItem('key'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value', $item->get()); + + $item = $adapter2->getItem('key'); + $this->assertFalse($item->isHit()); + + sleep(2); + + $item = $adapter1->getItem('key'); + $this->assertFalse($item->isHit()); + + $adapter2->save($adapter2->getItem('key1')->set('value1')); + + $item = $cache->getItem('key1'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value1', $item->get()); + + sleep(2); + + $item = $adapter1->getItem('key1'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value1', $item->get()); + + $item = $adapter2->getItem('key1'); + $this->assertFalse($item->isHit()); + + sleep(2); + + $item = $adapter1->getItem('key1'); + $this->assertTrue($item->isHit()); + $this->assertEquals('value1', $item->get()); + + sleep(2); + + $item = $adapter1->getItem('key1'); + $this->assertFalse($item->isHit()); + } + /** * @return MockObject|PruneableCacheInterface */ diff --git a/src/Symfony/Component/Cache/Tests/Fixtures/ExternalAdapter.php b/src/Symfony/Component/Cache/Tests/Fixtures/ExternalAdapter.php index 779a374ec7..be1f990121 100644 --- a/src/Symfony/Component/Cache/Tests/Fixtures/ExternalAdapter.php +++ b/src/Symfony/Component/Cache/Tests/Fixtures/ExternalAdapter.php @@ -24,9 +24,9 @@ class ExternalAdapter implements CacheItemPoolInterface { private $cache; - public function __construct() + public function __construct($defaultLifetime = 0) { - $this->cache = new ArrayAdapter(); + $this->cache = new ArrayAdapter($defaultLifetime); } public function getItem($key)