8000 bug #37562 [Cache] Use the default expiry when saving (not when creat… · symfony/cache@94b66c0 · GitHub
[go: up one dir, main page]
Skip to content

Commit 94b66c0

Browse files
nicolas-grekasnicolas-grekas
committed
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
2 parents 2a3c1d6 + 59bde16 commit 94b66c0

File tree

8 files changed

+141
-27
lines changed

8 files changed

+141
-27
lines changed

‎Adapter/AbstractAdapter.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,11 @@ protected function __construct($namespace = '', $defaultLifetime = 0)
4848
throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s").', $this->maxIdLength - 24, \strlen($namespace), $namespace));
4949
}
5050
$this->createCacheItem = \Closure::bind(
51-
static function ($key, $value, $isHit) use ($defaultLifetime) {
51+
static function ($key, $value, $isHit) {
5252
$item = new CacheItem();
5353
$item->key = $key;
5454
$item->value = $value;
5555
$item->isHit = $isHit;
56-
$item->defaultLifetime = $defaultLifetime;
5756

5857
return $item;
5958
},
@@ -62,14 +61,16 @@ static function ($key, $value, $isHit) use ($defaultLifetime) {
6261
);
6362
$getId = function ($key) { return $this->getId((string) $key); };
6463
$this->mergeByLifetime = \Closure::bind(
65-
static function ($deferred, $namespace, &$expiredIds) use ($getId) {
64+
static function ($deferred, $namespace, &$expiredIds) use ($getId, $defaultLifetime) {
6665
$byLifetime = [];
6766
$now = time();
6867
$expiredIds = [];
6968

7069
foreach ($deferred as $key => $item) {
7170
if (null === $item->expiry) {
72-
$byLifetime[0 < $item->defaultLifetime ? $item->defaultLifetime : 0][$getId($key)] = $item->value;
71+
$byLifetime[0 < $defaultLifetime ? $defaultLifetime : 0][$getId($key)] = $item->value;
72+
} elseif (0 === $item->expiry) {
73+
$byLifetime[0][$getId($key)] = $item->value;
7374
} elseif ($item->expiry > $now) {
7475
$byLifetime[$item->expiry - $now][$getId($key)] = $item->value;
7576
} else {

‎Adapter/ArrayAdapter.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,22 @@ class ArrayAdapter implements AdapterInterface, LoggerAwareInterface, Resettable
2525
use ArrayTrait;
2626

2727
private $createCacheItem;
28+
private $defaultLifetime;
2829

2930
/**
3031
* @param int $defaultLifetime
3132
* @param bool $storeSerialized Disabling serialization can lead to cache corruptions when storing mutable values but increases performance otherwise
3233
*/
3334
public function __construct($defaultLifetime = 0, $storeSerialized = true)
3435
{
36+
$this->defaultLifetime = $defaultLifetime;
3537
$this->storeSerialized = $storeSerialized;
3638
$this->createCacheItem = \Closure::bind(
37-
static function ($key, $value, $isHit) use ($defaultLifetime) {
39+
static function ($key, $value, $isHit) {
3840
$item = new CacheItem();
3941
$item->key = $key;
4042
$item->value = $value;
4143
$item->isHit = $isHit;
42-
$item->defaultLifetime = $defaultLifetime;
4344

4445
return $item;
4546
},
@@ -127,8 +128,8 @@ public function save(CacheItemInterface $item)
127128
return false;
128129
}
129130
}
130-
if (null === $expiry && 0 < $item["\0*\0defaultLifetime"]) {
131-
$expiry = time() + $item["\0*\0defaultLifetime"];
131+
if (null === $expiry && 0 < $this->defaultLifetime) {
132+
$expiry = time() + $this->defaultLifetime;
132133
}
133134

134135
$this->values[$key] = $value;

‎Adapter/ChainAdapter.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,10 @@ public function __construct(array $adapters, $defaultLifetime = 0)
6161
$this->syncItem = \Closure::bind(
6262
static function ($sourceItem, $item) use ($defaultLifetime) {
6363
$item->value = $sourceItem->value;
64-
$item->expiry = $sourceItem->expiry;
6564
$item->isHit = $sourceItem->isHit;
6665

67-
if (0 < $sourceItem->defaultLifetime && $sourceItem->defaultLifetime < $defaultLifetime) {
68-
$defaultLifetime = $sourceItem->defaultLifetime;
69-
}
70-
if (0 < $defaultLifetime && ($item->defaultLifetime <= 0 || $defaultLifetime < $item->defaultLifetime)) {
71-
$item->defaultLifetime = $defaultLifetime;
66+
if (0 < $defaultLifetime) {
67+
$item->expiresAfter($defaultLifetime);
7268
}
7369

7470
return $item;

‎Adapter/ProxyAdapter.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class ProxyAdapter implements AdapterInterface, PruneableInterface, ResettableIn
2929
private $namespaceLen;
3030
private $createCacheItem;
3131
private $poolHash;
32+
private $defaultLifetime;
3233

3334
/**
3435
* @param string $namespace
@@ -40,11 +41,11 @@ public function __construct(CacheItemPoolInterface $pool, $namespace = '', $defa
4041
$this->poolHash = $poolHash = spl_object_hash($pool);
4142
$this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace);
4243
$this->namespaceLen = \strlen($namespace);
44+
$this->defaultLifetime = $defaultLifetime;
4345
$this->createCacheItem = \Closure::bind(
44-
static function ($key, $innerItem) use ($defaultLifetime, $poolHash) {
46+
static function ($key, $innerItem) use ($poolHash) {
4547
$item = new CacheItem();
4648
$item->key = $key;
47-
$item->defaultLifetime = $defaultLifetime;
4849
$item->poolHash = $poolHash;
4950

5051
if (null !== $innerItem) {
@@ -155,8 +156,8 @@ private function doSave(CacheItemInterface $item, $method)
155156
}
156157
$item = (array) $item;
157158
$expiry = $item["\0*\0expiry"];
158-
if (null === $expiry && 0 < $item["\0*\0defaultLifetime"]) {
159-
$expiry = time() + $item["\0*\0defaultLifetime"];
159+
if (null === $expiry && 0 < $this->defaultLifetime) {
160+
$expiry = time() + $this->defaultLifetime;
160161
}
161162

162163
if ($item["\0*\0poolHash"] === $this->poolHash && $item["\0*\0innerItem"]) {

‎Adapter/TagAwareAdapter.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ static function ($key, $value, CacheItem $protoItem) {
4646
$item = new CacheItem();
4747
$item->key = $key;
4848
$item->value = $value;
49-
$item->defaultLifetime = $protoItem->defaultLifetime;
5049
$item->expiry = $protoItem->expiry;
5150
$item->poolHash = $protoItem->poolHash;
5251

@@ -90,8 +89,7 @@ static function ($deferred) {
9089
$this->invalidateTags = \Closure::bind(
9190
static function (AdapterInterface $tagsAdapter, array $tags) {
9291
foreach ($tags as $v) {
93-
$v->defaultLifetime = 0;
94-
$v->expiry = null;
92+
$v->expiry = 0;
9593
$tagsAdapter->saveDeferred($v);
9694
}
9795

‎CacheItem.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ final class CacheItem implements CacheItemInterface
2424
protected $value;
2525
protected $isHit = false;
2626
protected $expiry;
27-
protected $defaultLifetime;
2827
protected $tags = [];
2928
protected $prevTags = [];
3029
protected $innerItem;
@@ -74,7 +73,7 @@ public function set($value)
7473
public function expiresAt($expiration)
7574
{
7675
if (null === $expiration) {
77-
$this->expiry = $this->defaultLifetime > 0 ? time() + $this->defaultLifetime : null;
76+
$this->expiry = null;
7877
} elseif ($expiration instanceof \DateTimeInterface) {
7978
$this->expiry = (int) $expiration->format('U');
8079
} else {
@@ -92,7 +91,7 @@ public function expiresAt($expiration)
9291
public function expiresAfter($time)
9392
{
9493
if (null === $time) {
95-
$this->expiry = $this->defaultLifetime > 0 ? time() + $this->defaultLifetime : null;
94+
$this->expiry = null;
9695
} elseif ($time instanceof \DateInterval) {
9796
$this->expiry = (int) \DateTime::createFromFormat('U', time())->add($time)->format('U');
9897
} elseif (\is_int($time)) {

‎Tests/Adapter/ChainAdapterTest.php

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class ChainAdapterTest extends AdapterTestCase
2727
{
2828
public function createCachePool($defaultLifetime = 0)
2929
{
30-
return new ChainAdapter([new ArrayAdapter($defaultLifetime), new ExternalAdapter(), new FilesystemAdapter('', $defaultLifetime)], $defaultLifetime);
30+
return new ChainAdapter([new ArrayAdapter($defaultLifetime), new ExternalAdapter($defaultLifetime), new FilesystemAdapter('', $defaultLifetime)], $defaultLifetime);
3131
}
3232

3333
public function testEmptyAdaptersException()
@@ -65,6 +65,124 @@ public function testPrune()
6565
$this->assertFalse($cache->prune());
6666
}
6767

68+
public function testMultipleCachesExpirationWhenCommonTtlIsNotSet()
69+
{
70+
if (isset($this->skippedTests[__FUNCTION__])) {
71+
$this->markTestSkipped($this->skippedTests[__FUNCTION__]);
72+
}
73+
74+
$adapter1 = new ArrayAdapter(4);
75+
$adapter2 = new ArrayAdapter(2);
76+
77+
$cache = new ChainAdapter([$adapter1, $adapter2]);
78+
79+
$cache->save($cache->getItem('key')->set('value'));
80+
81+
$item = $adapter1->getItem('key');
82+
$this->assertTrue($item->isHit());
83+
$this->assertEquals('value', $item->get());
84+
85+
$item = $adapter2->getItem('key');
86+
$this->assertTrue($item->isHit());
87+
$this->assertEquals('value', $item->get());
88+
89+
sleep(2);
90+
91+
$item = $adapter1->getItem('key');
92+
$this->assertTrue($item->isHit());
93+
$this->assertEquals('value', $item->get());
94+
95+
$item = $adapter2->getItem('key');
96+
$this->assertFalse($item->isHit());
97+
98+
sleep(2);
99+
100+
$item = $adapter1->getItem('key');
101+
$this->assertFalse($item->isHit());
102+
103+
$adapter2->save($adapter2->getItem('key1')->set('value1'));
104+
105+
$item = $cache->getItem('key1');
106+
$this->assertTrue($item->isHit());
107+
$this->assertEquals('value1', $item->get());
108+
109+
sleep(2);
110+
111+
$item = $adapter1->getItem('key1');
112+
$this->assertTrue($item->isHit());
113+
$this->assertEquals('value1', $item->get());
114+
115+
$item = $adapter2->getItem('key1');
116+
$this->assertFalse($item->isHit());
117+
118+
sleep(2);
119+
120+
$item = $adapter1->getItem('key1');
121+
$this->assertFalse($item->isHit());
122+
}
123+
124+
public function testMultipleCachesExpirationWhenCommonTtlIsSet()
125+
{
126+
if (isset($this->skippedTests[__FUNCTION__])) {
127+
$this->markTestSkipped($this->skippedTests[__FUNCTION__]);
128+
}
129+
130+
$adapter1 = new ArrayAdapter(4);
131+
$adapter2 = new ArrayAdapter(2);
132+
133+
$cache = new ChainAdapter([$adapter1, $adapter2], 6);
134+
135+
$cache->save($cache->getItem('key')->set('value'));
136+
137+
$item = $adapter1->getItem('key');
138+
$this->assertTrue($item->isHit());
139+
$this->assertEquals('value', $item->get());
140+
141+
$item = $adapter2->getItem('key');
142+
$this->assertTrue($item->isHit());
143+
$this->assertEquals('value', $item->get());
144+
145+
sleep(2);
146+
147+
$item = $adapter1->getItem('key');
148+
$this->assertTrue($item->isHit());
149+
$this->assertEquals('value', $item->get());
150+
151+
$item = $adapter2->getItem('key');
152+
$this->assertFalse($item->isHit());
153+
154+
sleep(2);
155+
156+
$item = $adapter1->getItem('key');
157+
$this->assertFalse($item->isHit());
158+
159+
$adapter2->save($adapter2->getItem('key1')->set('value1'));
160+
161+
$item = $cache->getItem('key1');
162+
$this->assertTrue($item->isHit());
163+
$this->assertEquals('value1', $item->get());
164+
165+
sleep(2);
166+
167+
$item = $adapter1->getItem('key1');
168+
$this->assertTrue($item->isHit());
169+
$this->assertEquals('value1', $item->get());
170+
171+
$item = $adapter2->getItem('key1');
172+
$this->assertFalse($item->isHit());
173+
174+
sleep(2);
175+
176+
$item = $adapter1->getItem('key1');
177+
$this->assertTrue($item->isHit());
178+
$this->assertEquals('value1', $item->get());
179+
180+
sleep(2);
181+
182+
$item = $adapter1->getItem('key1');
183+
$this->assertFalse($item->isHit());
184+
}
185+
68186
/**
69187
* @return MockObject|PruneableCacheInterface
70188
*/

‎Tests/Fixtures/ExternalAdapter.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ class ExternalAdapter implements CacheItemPoolInterface
2424
{
2525
private $cache;
2626

27-
public function __construct()
27+
public function __construct($defaultLifetime = 0)
2828
{
29-
$this->cache = new ArrayAdapter();
29+
$this->cache = new ArrayAdapter($defaultLifetime);
3030
}
3131

3232
public function getItem($key)

0 commit comments

Comments
 (0)
0