[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

Author: timacdonald

src/Illuminate/Cache/DatabaseStore.php

@@ -378,9 +378,10 @@ public function forgetIfExpired($key)
  */
  protected function forgetMany(array $keys)
  {
- $this->table()->whereIn('key', array_map(function ($key) {
- return $this->prefix.$key;
- }, $keys))->delete();
+ $this->table()->whereIn('key', collect($keys)->flatMap(fn ($key) => [
+ $this->prefix.$key,
+ "{$this->prefix}illuminate:cache:flexible:created:{$key}",
+ ])->all())->delete();
 
  return true;
  }
@@ -395,9 +396,13 @@ protected function forgetMany(array $keys)
  protected function forgetManyIfExpired(array $keys, bool $prefixed = false)
  {
  $this->table()
- ->whereIn('key', $prefixed ? $keys : array_map(function ($key) {
- return $this->prefix.$key;
- }, $keys))
+ ->whereIn('key', collect($keys)->flatMap(fn ($key) => $prefixed ? [
+ $key,
+ $this->prefix.'illuminate:cache:flexible:created:'.Str::chopStart($key, $this->prefix),
+ ] : [
+ "{$this->prefix}{$key}",
+ "{$this->prefix}illuminate:cache:flexible:created:{$key}",
+ ])->all())
  ->where('expiration', '<=', $this->getTime())
  ->delete();
 

src/Illuminate/Cache/FileStore.php

@@ -248,6 +248,8 @@ public function restoreLock($name, $owner)
  public function forget($key)
  {
  if ($this->files->exists($file = $this->path($key))) {
+ $this->files->delete($this->path("illuminate:cache:flexible:created:{$key}"));
+
  return $this->files->delete($file);
  }
 

src/Illuminate/Cache/Repository.php

@@ -488,13 +488,13 @@ public function flexible($key, $ttl, $callback, $lock = null)
  {
  [
  $key => $value,
- "{$key}:created" => $created,
- ] = $this->many([$key, "{$key}:created"]);
+ "illuminate:cache:flexible:created:{$key}" => $created,
+ ] = $this->many([$key, "illuminate:cache:flexible:created:{$key}"]);
 
- if ($created === null) {
+ if (in_array(null, [$value, $created], true)) {
  return tap(value($callback), fn ($value) => $this->putMany([
  $key => $value,
- "{$key}:created" => Carbon::now()->getTimestamp(),
+ "illuminate:cache:flexible:created:{$key}" => Carbon::now()->getTimestamp(),
  ], $ttl[1]));
  }
 
@@ -504,22 +504,22 @@ public function flexible($key, $ttl, $callback, $lock = null)
 
  $refresh = function () use ($key, $ttl, $callback, $lock, $created) {
  $this->store->lock(
- "illuminate:cache:refresh:lock:{$key}",
+ "illuminate:cache:flexible:lock:{$key}",
  $lock['seconds'] ?? 0,
  $lock['owner'] ?? null,
  )->get(function () use ($key, $callback, $created, $ttl) {
- if ($created !== $this->get("{$key}:created")) {
+ if ($created !== $this->get("illuminate:cache:flexible:created:{$key}")) {
  return;
  }
 
  $this->putMany([
  $key => value($callback),
- "{$key}:created" => Carbon::now()->getTimestamp(),
+ "illuminate:cache:flexible:created:{$key}" => Carbon::now()->getTimestamp(),
  ], $ttl[1]);
  });
  };
 
- defer($refresh, "illuminate:cache:refresh:{$key}");
+ defer($refresh, "illuminate:cache:flexible:{$key}");
 
  return $value;
  }

tests/Cache/CacheDatabaseStoreTest.php

@@ -37,7 +37,7 @@ public function testNullIsReturnedAndItemDeletedWhenItemIsExpired()
  $getQuery->shouldReceive('get')->once()->andReturn(collect([(object) ['key' => 'prefixfoo', 'expiration' => 1]]));
 
  $deleteQuery = m::mock(stdClass::class);
- $deleteQuery->shouldReceive('whereIn')->once()->with('key', ['prefixfoo'])->andReturn($deleteQuery);
+ $deleteQuery->shouldReceive('whereIn')->once()->with('key', ['prefixfoo', 'prefixilluminate:cache:flexible:created:foo'])->andReturn($deleteQuery);
  $deleteQuery->shouldReceive('where')->once()->with('expiration', '<=', m::any())->andReturn($deleteQuery);
  $deleteQuery->shouldReceive('delete')->once()->andReturnNull();
 
@@ -105,7 +105,7 @@ public function testItemsMayBeRemovedFromCache()
  $store = $this->getStore();
  $table = m::mock(stdClass::class);
  $store->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($table);
- $table->shouldReceive('whereIn')->once()->with('key', ['prefixfoo'])->andReturn($table);
+ $table->shouldReceive('whereIn')->once()->with('key', ['prefixfoo', 'prefixilluminate:cache:flexible:created:foo'])->andReturn($table);
  $table->shouldReceive('delete')->once();
 
  $store->forget('foo');

tests/Cache/CacheFileStoreTest.php

@@ -285,14 +285,15 @@ public function testRemoveDeletesFileDoesntExist()
 
  public function testRemoveDeletesFile()
  {
- $files = $this->mockFilesystem();
- $hash = sha1('foobar');
- $cache_dir = substr($hash, 0, 2).'/'.substr($hash, 2, 2);
+ $files = new Filesystem;
  $store = new FileStore($files, __DIR__);
  $store->put('foobar', 'Hello Baby', 10);
- $files->expects($this->once())->method('exists')->with($this->equalTo(__DIR__.'/'.$cache_dir.'/'.$hash))->willReturn(true);
- $files->expects($this->once())->method('delete')->with($this->equalTo(__DIR__.'/'.$cache_dir.'/'.$hash));
+
+ $this->assertFileExists($store->path('foobar'));
+
  $store->forget('foobar');
+
+ $this->assertFileDoesNotExist($store->path('foobar'));
  }
 
  public function testFlushCleansDirectory()

tests/Integration/Cache/RepositoryTest.php

@@ -2,12 +2,17 @@
 
 namespace Illuminate\Tests\Integration\Cache;
 
+use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
 use Illuminate\Support\Carbon;
 use Illuminate\Support\Facades\Cache;
+use Orchestra\Testbench\Attributes\WithMigration;
 use Orchestra\Testbench\TestCase;
 
+#[WithMigration('cache')]
 class RepositoryTest extends TestCase
 {
+ use LazilyRefreshDatabase;
+
  public function testStaleWhileRevalidate(): void
  {
  Carbon::setTestNow('2000-01-01 00:00:00');
@@ -22,7 +27,7 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(1, $value);
  $this->assertCount(0, defer());
  $this->assertSame(1, $cache->get('foo'));
- $this->assertSame(946684800, $cache->get('foo:created'));
+ $this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // Cache is fresh. The value should be retrieved from the cache and used...
  $value = $cache->flexible('foo', [10, 20], function () use (&$count) {
@@ -31,7 +36,7 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(1, $value);
  $this->assertCount(0, defer());
  $this->assertSame(1, $cache->get('foo'));
- $this->assertSame(946684800, $cache->get('foo:created'));
+ $this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
 
  Carbon::setTestNow(now()->addSeconds(11));
 
@@ -43,7 +48,7 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(1, $value);
  $this->assertCount(1, defer());
  $this->assertSame(1, $cache->get('foo'));
- $this->assertSame(946684800, $cache->get('foo:created'));
+ $this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // We will hit it again within the same request. This should not queue
  // up an additional deferred callback as only one can be registered at
@@ -54,14 +59,14 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(1, $value);
  $this->assertCount(1, defer());
  $this->assertSame(1, $cache->get('foo'));
- $this->assertSame(946684800, $cache->get('foo:created'));
+ $this->assertSame(946684800, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // We will now simulate the end of the request lifecycle by executing the
  // deferred callback. This should refresh the cache.
  defer()->invoke();
  $this->assertCount(0, defer());
  $this->assertSame(2, $cache->get('foo')); // this has been updated!
- $this->assertSame(946684811, $cache->get('foo:created')); // this has been updated!
+ $this->assertSame(946684811, $cache->get('illuminate:cache:flexible:created:foo')); // this has been updated!
 
  // Now the cache is fresh again...
  $value = $cache->flexible('foo', [10, 20], function () use (&$count) {
@@ -70,7 +75,7 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(2, $value);
  $this->assertCount(0, defer());
  $this->assertSame(2, $cache->get('foo'));
- $this->assertSame(946684811, $cache->get('foo:created'));
+ $this->assertSame(946684811, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // Let's now progress time beyond the stale TTL...
  Carbon::setTestNow(now()->addSeconds(21));
@@ -82,7 +87,7 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(3, $value);
  $this->assertCount(0, defer());
  $this->assertSame(3, $cache->get('foo'));
- $this->assertSame(946684832, $cache->get('foo:created'));
+ $this->assertSame(946684832, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // Now lets see what happens when another request, job, or command is
  // also trying to refresh the same key at the same time. Will push past
@@ -94,28 +99,28 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(3, $value);
  $this->assertCount(1, defer());
  $this->assertSame(3, $cache->get('foo'));
- $this->assertSame(946684832, $cache->get('foo:created'));
+ $this->assertSame(946684832, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // Now we will execute the deferred callback but we will first aquire
  // our own lock. This means that the value should not be refreshed by
  // deferred callback.
  /** @var Lock */
- $lock = $cache->lock('illuminate:cache:refresh:lock:foo');
+ $lock = $cache->lock('illuminate:cache:flexible:lock:foo');
 
  $this->assertTrue($lock->acquire());
  defer()->first()();
  $this->assertSame(3, $value);
  $this->assertCount(1, defer());
  $this->assertSame(3, $cache->get('foo'));
- $this->assertSame(946684832, $cache->get('foo:created'));
+ $this->assertSame(946684832, $cache->get('illuminate:cache:flexible:created:foo'));
  $this->assertTrue($lock->release());
 
  // Now we have cleared the lock we will, one last time, confirm that
  // the deferred callack does refresh the value when the lock is not active.
  defer()->invoke();
  $this->assertCount(0, defer());
  $this->assertSame(4, $cache->get('foo'));
- $this->assertSame(946684843, $cache->get('foo:created'));
+ $this->assertSame(946684843, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // The last thing is to check that we don't refresh the cache in the
  // deferred callback if another thread has already done the work for us.
@@ -127,13 +132,13 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(4, $value);
  $this->assertCount(1, defer());
  $this->assertSame(4, $cache->get('foo'));
- $this->assertSame(946684843, $cache->get('foo:created'));
+ $this->assertSame(946684843, $cache->get('illuminate:cache:flexible:created:foo'));
 
  // There is now a deferred callback ready to refresh the cache. We will
  // simulate another thread updating the value.
  $cache->putMany([
  'foo' => 99,
- 'foo:created' => 946684863,
+ 'illuminate:cache:flexible:created:foo' => 946684863,
  ]);
 
  // then we will run the refresh callback
@@ -144,6 +149,90 @@ public function testStaleWhileRevalidate(): void
  $this->assertSame(99, $value);
  $this->assertCount(0, defer());
  $this->assertSame(99, $cache->get('foo'));
- $this->assertSame(946684863, $cache->get('foo:created'));
+ $this->assertSame(946684863, $cache->get('illuminate:cache:flexible:created:foo'));
+ }
+
+ public function testItHandlesStrayTtlKeyAfterMainKeyIsForgotten()
+ {
+ $cache = Cache::driver('array');
+ $count = 0;
+
+ $value = $cache->flexible('count', [5, 10], function () use (&$count) {
+ $count = 1;
+
+ return $count;
+ });
+
+ $this->assertSame(1, $value);
+ $this->assertSame(1, $count);
+
+ $cache->forget('count');
+
+ $value = $cache->flexible('count', [5, 10], function () use (&$count) {
+ $count = 2;
+
+ return $count;
+ });
+ $this->assertSame(2, $value);
+ $this->assertSame(2, $count);
+ }
+
+ public function testItImplicitlyClearsTtlKeysFromDatabaseCache()
+ {
+ $this->freezeTime();
+ $cache = Cache::driver('database');
+
+ $cache->flexible('count', [5, 10], fn () => 1);
+
+ $this->assertTrue($cache->has('count'));
+ $this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
+
+ $cache->forget('count');
+
+ $this->assertEmpty($cache->getConnection()->table('cache')->get());
+ $this->assertTrue($cache->missing('count'));
+ $this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
+
+ $cache->flexible('count', [5, 10], fn () => 1);
+
+ $this->assertTrue($cache->has('count'));
+ $this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
+
+ $this->travel(20)->seconds();
+ $cache->forgetIfExpired('count');
+
+ $this->assertEmpty($cache->getConnection()->table('cache')->get());
+ $this->assertTrue($cache->missing('count'));
+ $this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
+ }
+
+ public function testItImplicitlyClearsTtlKeysFromFileDriver()
+ {
+ $this->freezeTime();
+ $cache = Cache::driver('file');
+
+ $cache->flexible('count', [5, 10], fn () => 1);
+
+ $this->assertTrue($cache->has('count'));
+ $this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
+
+ $cache->forget('count');
+
+ $this->assertFalse($cache->getFilesystem()->exists($cache->path('count')));
+ $this->assertFalse($cache->getFilesystem()->exists($cache->path('illuminate:cache:flexible:created:count')));
+ $this->assertTrue($cache->missing('count'));
+ $this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
+
+ $cache->flexible('count', [5, 10], fn () => 1);
+
+ $this->assertTrue($cache->has('count'));
+ $this->assertTrue($cache->has('illuminate:cache:flexible:created:count'));
+
+ $this->travel(20)->seconds();
+
+ $this->assertTrue($cache->missing('count'));
+ $this->assertFalse($cache->getFilesystem()->exists($cache->path('count')));
+ $this->assertFalse($cache->getFilesystem()->exists($cache->path('illuminate:cache:flexible:created:count')));
+ $this->assertTrue($cache->missing('illuminate:cache:flexible:created:count'));
  }
 }