bug #37562 [Cache] Use the default expiry when saving (not when creating) items (philipp-kolesnikov)

This PR was merged into the 3.4 branch.

Discussion
----------

[Cache] Use the default expiry when saving (not when creating) items

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #37263
| License       | MIT
| Doc PR        |

This PR is for align expiration handling in ChainAdapter with ChainCache as proposed in #37263.

Commits
-------

49e9f3229c [Cache] Use the default expiry when saving (not when creating) items

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 {

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"]) {
+        if (null === $expiry && 0 < $this->defaultLifetime) {
-            $expiry = time() + $item["\0*\0defaultLifetime"];
+            $expiry = time() + $this->defaultLifetime;
         }
 
         $this->values[$key] = $value;

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) {
+                if (0 < $defaultLifetime) {
-                    $defaultLifetime = $sourceItem->defaultLifetime;
+                    $item->expiresAfter($defaultLifetime);
-                }
-                if (0 < $defaultLifetime && ($item->defaultLifetime <= 0 || $defaultLifetime < $item->defaultLifetime)) {
-                    $item->defaultLifetime = $defaultLifetime;
                 }
 
                 return $item;

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"]) {
+        if (null === $expiry && 0 < $this->defaultLifetime) {
-            $expiry = time() + $item["\0*\0defaultLifetime"];
+            $expiry = time() + $this->defaultLifetime;
         }
 
         if ($item["\0*\0poolHash"] === $this->poolHash && $item["\0*\0innerItem"]) {

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 = 0;
-                    $v->expiry = null;
                     $tagsAdapter->saveDeferred($v);
                 }
 

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)) {

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
      */

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)