Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Cache::flexible improvements #52891

Merged
merged 10 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/Illuminate/Cache/DatabaseStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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();

Expand Down
2 changes: 2 additions & 0 deletions src/Illuminate/Cache/FileStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
16 changes: 8 additions & 8 deletions src/Illuminate/Cache/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
}

Expand All @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Cache/CacheDatabaseStoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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');
Expand Down
11 changes: 6 additions & 5 deletions tests/Cache/CacheFileStoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Comment on lines -288 to +296
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm modified this test so it really does check that the file is cleaned up. There was an issue with the mock because of the new key being deleted. Mockery just burnt my time trying to make it work.

This test is 1000% better without the mocks anyway.

}

public function testFlushCleansDirectory()
Expand Down
117 changes: 103 additions & 14 deletions tests/Integration/Cache/RepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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) {
Expand All @@ -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));

Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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));
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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'));
}
}