Skip to content

Commit b05e6f8

Browse files
timacdonaldtimacdonald
authored
[11.x] Cache::flexible improvements (#52891)
* Handle stray flexible TTL keys * Ensure forgetting keys cleans up flexible TTL keys * Remove redundant `has` check * Ensure lazy database drivers clean up ttl keys * Rename ttl key * Fix tests * Fix tests * fix test * Rename cache lock key * Rename defer key
1 parent 47ba027 commit b05e6f8

File tree

6 files changed

+132
-35
lines changed

6 files changed

+132
-35
lines changed

‎src/Illuminate/Cache/DatabaseStore.php‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,10 @@ public function forgetIfExpired($key)
378378
*/
379379
protected function forgetMany(array $keys)
380380
{
381-
$this->table()->whereIn('key', array_map(function ($key) {
382-
return $this->prefix.$key;
383-
}, $keys))->delete();
381+
$this->table()->whereIn('key', collect($keys)->flatMap(fn ($key) => [
382+
$this->prefix.$key,
383+
"{$this->prefix}illuminate:cache:flexible:created:{$key}",
384+
])->all())->delete();
384385

385386
return true;
386387
}
@@ -395,9 +396,13 @@ protected function forgetMany(array $keys)
395396
protected function forgetManyIfExpired(array $keys, bool $prefixed = false)
396397
{
397398
$this->table()
398-
->whereIn('key', $prefixed ? $keys : array_map(function ($key) {
399-
return $this->prefix.$key;
400-
}, $keys))
399+
->whereIn('key', collect($keys)->flatMap(fn ($key) => $prefixed ? [
400+
$key,
401+
$this->prefix.'illuminate:cache:flexible:created:'.Str::chopStart($key, $this->prefix),
402+
] : [
403+
"{$this->prefix}{$key}",
404+
"{$this->prefix}illuminate:cache:flexible:created:{$key}",
405+
])->all())
401406
->where('expiration', '<=', $this->getTime())
402407
->delete();
403408

‎src/Illuminate/Cache/FileStore.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,8 @@ public function restoreLock($name, $owner)
248248
public function forget($key)
249249
{
250250
if ($this->files->exists($file = $this->path($key))) {
251+
$this->files->delete($this->path("illuminate:cache:flexible:created:{$key}"));
252+
251253
return $this->files->delete($file);
252254
}
253255

‎src/Illuminate/Cache/Repository.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -488,13 +488,13 @@ public function flexible($key, $ttl, $callback, $lock = null)
488488
{
489489
[
490490
$key => $value,
491-
"{$key}:created" => $created,
492-
] = $this->many([$key, "{$key}:created"]);
491+
"illuminate:cache:flexible:created:{$key}" => $created,
492+
] = $this->many([$key, "illuminate:cache:flexible:created:{$key}"]);
493493

494-
if ($created === null) {
494+
if (in_array(null, [$value, $created], true)) {
495495
return tap(value($callback), fn ($value) => $this->putMany([
496496
$key => $value,
497-
"{$key}:created" => Carbon::now()->getTimestamp(),
497+
"illuminate:cache:flexible:created:{$key}" => Carbon::now()->getTimestamp(),
498498
], $ttl[1]));
499499
}
500500

@@ -504,22 +504,22 @@ public function flexible($key, $ttl, $callback, $lock = null)
504504

505505
$refresh = function () use ($key, $ttl, $callback, $lock, $created) {
506506
$this->store->lock(
507-
"illuminate:cache:refresh:lock:{$key}",
507+
"illuminate:cache:flexible:lock:{$key}",
508508
$lock['seconds'] ?? 0,
509509
$lock['owner'] ?? null,
510510
)->get(function () use ($key, $callback, $created, $ttl) {
511-
if ($created !== $this->get("{$key}:created")) {
511+
if ($created !== $this->get("illuminate:cache:flexible:created:{$key}")) {
512512
return;
513513
}
514514

515515
$this->putMany([
516516
$key => value($callback),
517-
"{$key}:created" => Carbon::now()->getTimestamp(),
517+
"illuminate:cache:flexible:created:{$key}" => Carbon::now()->getTimestamp(),
518518
], $ttl[1]);
519519
});
520520
};
521521

522-
defer($refresh, "illuminate:cache:refresh:{$key}");
522+
defer($refresh, "illuminate:cache:flexible:{$key}");
523523

524524
return $value;
525525
}

‎tests/Cache/CacheDatabaseStoreTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function testNullIsReturnedAndItemDeletedWhenItemIsExpired()
3737
$getQuery->shouldReceive('get')->once()->andReturn(collect([(object) ['key' => 'prefixfoo', 'expiration' => 1]]));
3838

3939
$deleteQuery = m::mock(stdClass::class);
40-
$deleteQuery->shouldReceive('whereIn')->once()->with('key', ['prefixfoo'])->andReturn($deleteQuery);
40+
$deleteQuery->shouldReceive('whereIn')->once()->with('key', ['prefixfoo', 'prefixilluminate:cache:flexible:created:foo'])->andReturn($deleteQuery);
4141
$deleteQuery->shouldReceive('where')->once()->with('expiration', '<=', m::any())->andReturn($deleteQuery);
4242
$deleteQuery->shouldReceive('delete')->once()->andReturnNull();
4343

@@ -105,7 +105,7 @@ public function testItemsMayBeRemovedFromCache()
105105
$store = $this->getStore();
106106
$table = m::mock(stdClass::class);
107107
$store->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($table);
108-
$table->shouldReceive('whereIn')->once()->with('key', ['prefixfoo'])->andReturn($table);
108+
$table->shouldReceive('whereIn')->once()->with('key', ['prefixfoo', 'prefixilluminate:cache:flexible:created:foo'])->andReturn($table);
109109
$table->shouldReceive('delete')->once();
110110

111111
$store->forget('foo');

‎tests/Cache/CacheFileStoreTest.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,14 +285,15 @@ public function testRemoveDeletesFileDoesntExist()
285285

286286
public function testRemoveDeletesFile()
287287
{
288-
$files = $this->mockFilesystem();
289-
$hash = sha1('foobar');
290-
$cache_dir = substr($hash, 0, 2).'/'.substr($hash, 2, 2);
288+
$files = new Filesystem;
291289
$store = new FileStore($files, __DIR__);
292290
$store->put('foobar', 'Hello Baby', 10);
293-
$files->expects($this->once())->method('exists')->with($this->equalTo(__DIR__.'/'.$cache_dir.'/'.$hash))->willReturn(true);
294-
$files->expects($this->once())->method('delete')->with($this->equalTo(__DIR__.'/'.$cache_dir.'/'.$hash));
291+
292+
$this->assertFileExists($store->path('foobar'));
293+
295294
$store->forget('foobar');
295+
296+
$this->assertFileDoesNotExist($store->path('foobar'));
296297
}
297298

298299
public function testFlushCleansDirectory()

‎tests/Integration/Cache/RepositoryTest.php‎

Lines changed: 103 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@
22

33
namespace Illuminate\Tests\Integration\Cache;
44

5+
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
56
use Illuminate\Support\Carbon;
67
use Illuminate\Support\Facades\Cache;
8+
use Orchestra\Testbench\Attributes\WithMigration;
79
use Orchestra\Testbench\TestCase;
810

11+
#[WithMigration('cache')]
912
class RepositoryTest extends TestCase
1013
{
14+
use LazilyRefreshDatabase;
15+
1116
public function testStaleWhileRevalidate(): void
1217
{
1318
Carbon::setTestNow('2000-01-01 00:00:00');
@@ -22,7 +27,7 @@ public function testStaleWhileRevalidate(): void
2227
$this->assertSame(1, $value);
2328
$this->assertCount(0, defer());
2429
$this->assertSame(1, $cache->get('foo'));
25-
$this->assertSame(946684800, $cache->get('foo:created'));
30+
$this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
2631

2732
// Cache is fresh. The value should be retrieved from the cache and used...
2833
$value = $cache->flexible('foo', [10, 20], function () use (&$count) {
@@ -31,7 +36,7 @@ public function testStaleWhileRevalidate(): void
3136
$this->assertSame(1, $value);
3237
$this->assertCount(0, defer());
3338
$this->assertSame(1, $cache->get('foo'));
34-
$this->assertSame(946684800, $cache->get('foo:created'));
39+
$this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
3540

3641
Carbon::setTestNow(now()->addSeconds(11));
3742

@@ -43,7 +48,7 @@ public function testStaleWhileRevalidate(): void
4348
$this->assertSame(1, $value);
4449
$this->assertCount(1, defer());
4550
$this->assertSame(1, $cache->get('foo'));
46-
$this->assertSame(946684800, $cache->get('foo:created'));
51+
$this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
4752

4853
// We will hit it again within the same request. This should not queue
4954
// up an additional deferred callback as only one can be registered at
@@ -54,14 +59,14 @@ public function testStaleWhileRevalidate(): void
5459
$this->assertSame(1, $value);
5560
$this->assertCount(1, defer());
5661
$this->assertSame(1, $cache->get('foo'));
57-
$this->assertSame(946684800, $cache->get('foo:created'));
62+
$this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
5863

5964
// We will now simulate the end of the request lifecycle by executing the
6065
// deferred callback. This should refresh the cache.
6166
defer()->invoke();
6267
$this->assertCount(0, defer());
6368
$this->assertSame(2, $cache->get('foo')); // this has been updated!
64-
$this->assertSame(946684811, $cache->get('foo:created')); // this has been updated!
69+
$this->assertSame(946684811, $cache->get('illuminate:cache:flexible:created:foo')); // this has been updated!
6570

6671
// Now the cache is fresh again...
6772
$value = $cache->flexible('foo', [10, 20], function () use (&$count) {
@@ -70,7 +75,7 @@ public function testStaleWhileRevalidate(): void
7075
$this->assertSame(2, $value);
7176
$this->assertCount(0, defer());
7277
$this->assertSame(2, $cache->get('foo'));
73-
$this->assertSame(946684811, $cache->get('foo:created'));
78+
$this->assertSame(946684811, $cache->get('illuminate:cache:flexible:created:foo'));
7479

7580
// Let's now progress time beyond the stale TTL...
7681
Carbon::setTestNow(now()->addSeconds(21));
@@ -82,7 +87,7 @@ public function testStaleWhileRevalidate(): void
8287
$this->assertSame(3, $value);
8388
$this->assertCount(0, defer());
8489
$this->assertSame(3, $cache->get('foo'));
85-
$this->assertSame(946684832, $cache->get('foo:created'));
90+
$this->assertSame(946684832, $cache->get('illuminate:cache:flexible:created:foo'));
8691

8792
// Now lets see what happens when another request, job, or command is
8893
// also trying to refresh the same key at the same time. Will push past
@@ -94,28 +99,28 @@ public function testStaleWhileRevalidate(): void
9499
$this->assertSame(3, $value);
95100
$this->assertCount(1, defer());
96101
$this->assertSame(3, $cache->get('foo'));
97-
$this->assertSame(946684832, $cache->get('foo:created'));
102+
$this->assertSame(946684832, $cache->get('illuminate:cache:flexible:created:foo'));
98103

99104
// Now we will execute the deferred callback but we will first aquire
100105
// our own lock. This means that the value should not be refreshed by
101106
// deferred callback.
102107
/** @var Lock */
103-
$lock = $cache->lock('illuminate:cache:refresh:lock:foo');
108+
$lock = $cache->lock('illuminate:cache:flexible:lock:foo');
104109

105110
$this->assertTrue($lock->acquire());
106111
defer()->first()();
107112
$this->assertSame(3, $value);
108113
$this->assertCount(1, defer());
109114
$this->assertSame(3, $cache->get('foo'));
110-
$this->assertSame(946684832, $cache->get('foo:created'));
115+
$this->assertSame(946684832, $cache->get('illuminate:cache:flexible:created:foo'));
111116
$this->assertTrue($lock->release());
112117

113118
// Now we have cleared the lock we will, one last time, confirm that
114119
// the deferred callack does refresh the value when the lock is not active.
115120
defer()->invoke();
116121
$this->assertCount(0, defer());
117122
$this->assertSame(4, $cache->get('foo'));
118-
$this->assertSame(946684843, $cache->get('foo:created'));
123+
$this->assertSame(946684843, $cache->get('illuminate:cache:flexible:created:foo'));
119124

120125
// The last thing is to check that we don't refresh the cache in the
121126
// deferred callback if another thread has already done the work for us.
@@ -127,13 +132,13 @@ public function testStaleWhileRevalidate(): void
127132
$this->assertSame(4, $value);
128133
$this->assertCount(1, defer());
129134
$this->assertSame(4, $cache->get('foo'));
130-
$this->assertSame(946684843, $cache->get('foo:created'));
135+
$this->assertSame(946684843, $cache->get('illuminate:cache:flexible:created:foo'));
131136

132137
// There is now a deferred callback ready to refresh the cache. We will
133138
// simulate another thread updating the value.
134139
$cache->putMany([
135140
'foo' => 99,
136-
'foo:created' => 946684863,
141+
'illuminate:cache:flexible:created:foo' => 946684863,
137142
]);
138143

139144
// then we will run the refresh callback
@@ -144,6 +149,90 @@ public function testStaleWhileRevalidate(): void
144149
$this->assertSame(99, $value);
145150
$this->assertCount(0, defer());
146151
$this->assertSame(99, $cache->get('foo'));
147-
$this->assertSame(946684863, $cache->get('foo:created'));
152+
$this->assertSame(946684863, $cache->get('illuminate:cache:flexible:created:foo'));
153+
}
154+
155+
public function testItHandlesStrayTtlKeyAfterMainKeyIsForgotten()
156+
{
157+
$cache = Cache::driver('array');
158+
$count = 0;
159+
160+
$value = $cache->flexible('count', [5, 10], function () use (&$count) {
161+
$count = 1;
162+
163+
return $count;
164+
});
165+
166+
$this->assertSame(1, $value);
167+
$this->assertSame(1, $count);
168+
169+
$cache->forget('count');
170+
171+
$value = $cache->flexible('count', [5, 10], function () use (&$count) {
172+
$count = 2;
173+
174+
return $count;
175+
});
176+
$this->assertSame(2, $value);
177+
$this->assertSame(2, $count);
178+
}
179+
180+
public function testItImplicitlyClearsTtlKeysFromDatabaseCache()
181+
{
182+
$this->freezeTime();
183+
$cache = Cache::driver('database');
184+
185+
$cache->flexible('count', [5, 10], fn () => 1);
186+
187+
$this->assertTrue($cache->has('count'));
188+
$this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
189+
190+
$cache->forget('count');
191+
192+
$this->assertEmpty($cache->getConnection()->table('cache')->get());
193+
$this->assertTrue($cache->missing('count'));
194+
$this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
195+
196+
$cache->flexible('count', [5, 10], fn () => 1);
197+
198+
$this->assertTrue($cache->has('count'));
199+
$this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
200+
201+
$this->travel(20)->seconds();
202+
$cache->forgetIfExpired('count');
203+
204+
$this->assertEmpty($cache->getConnection()->table('cache')->get());
205+
$this->assertTrue($cache->missing('count'));
206+
$this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
207+
}
208+
209+
public function testItImplicitlyClearsTtlKeysFromFileDriver()
210+
{
211+
$this->freezeTime();
212+
$cache = Cache::driver('file');
213+
214+
$cache->flexible('count', [5, 10], fn () => 1);
215+
216+
$this->assertTrue($cache->has('count'));
217+
$this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
218+
219+
$cache->forget('count');
220+
221+
$this->assertFalse($cache->getFilesystem()->exists($cache->path('count')));
222+
$this->assertFalse($cache->getFilesystem()->exists($cache->path('illuminate:cache:flexible:created:count')));
223+
$this->assertTrue($cache->missing('count'));
224+
$this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
225+
226+
$cache->flexible('count', [5, 10], fn () => 1);
227+
228+
$this->assertTrue($cache->has('count'));
229+
$this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
230+
231+
$this->travel(20)->seconds();
232+
233+
$this->assertTrue($cache->missing('count'));
234+
$this->assertFalse($cache->getFilesystem()->exists($cache->path('count')));
235+
$this->assertFalse($cache->getFilesystem()->exists($cache->path('illuminate:cache:flexible:created:count')));
236+
$this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
148237
}
149238
}

0 commit comments

Comments
 (0)