Skip to content

Commit 0dfae8c

Browse files
abnegateabnegate
authored
Merge pull request #905 from utopia-php/fix/leasable-withcache
fix(cache): gate the query cache write on a lease to reject stale lists
2 parents c71349d + e787492 commit 0dfae8c

2 files changed

Lines changed: 218 additions & 1 deletion

File tree

‎src/Database/Database.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8709,6 +8709,16 @@ public function withCache(
87098709
}
87108710
}
87118711

8712+
// Capture the generation before the callback runs its read: if a
8713+
// concurrent write purges this query key in between, saveWithLease()
8714+
// below rejects the now-stale list instead of re-poisoning the cache.
8715+
$generation = '0';
8716+
try {
8717+
$generation = $this->cache->getGeneration($key);
8718+
} catch (Throwable $e) {
8719+
Console::warning('Warning: Failed to get cache generation: ' . $e->getMessage());
8720+
}
8721+
87128722
$callbackValue = $callback();
87138723

87148724
if ($callbackValue !== false) {
@@ -8795,7 +8805,7 @@ public function withCache(
87958805
}
87968806

87978807
if ($encoded !== false) {
8798-
$this->cache->save($key, $encoded, $hash);
8808+
$this->cache->saveWithLease($key, $encoded, $hash, $generation);
87998809
}
88008810
} catch (Throwable $e) {
88018811
Console::warning('Warning: Failed to save cache value: ' . $e->getMessage());

‎tests/unit/WithCacheLeaseTest.php‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
<?php
2+
3+
namespace Tests\Unit;
4+
5+
use PHPUnit\Framework\TestCase;
6+
use Utopia\Cache\Adapter;
7+
use Utopia\Cache\Cache;
8+
use Utopia\Cache\Feature\Leasable;
9+
use Utopia\Database\Adapter\Memory as DatabaseMemory;
10+
use Utopia\Database\Database;
11+
use Utopia\Database\Document;
12+
use Utopia\Database\Helpers\Permission;
13+
use Utopia\Database\Helpers\Role;
14+
15+
class WithCacheLeaseTest extends TestCase
16+
{
17+
private LeasableMemoryCache $cacheAdapter;
18+
19+
private Database $database;
20+
21+
private string $key;
22+
23+
protected function setUp(): void
24+
{
25+
$this->cacheAdapter = new LeasableMemoryCache();
26+
$this->database = new Database(new DatabaseMemory(), new Cache($this->cacheAdapter));
27+
$this->database
28+
->setDatabase('utopiaTests')
29+
->setNamespace('with_cache_' . \uniqid());
30+
31+
$this->database->create();
32+
$this->database->createCollection('projects');
33+
$this->database->createAttribute('projects', 'name', Database::VAR_STRING, 255, false);
34+
$this->database->createDocument('projects', new Document([
35+
'$id' => 'project',
36+
'$permissions' => [
37+
Permission::read(Role::any()),
38+
],
39+
'name' => 'fresh',
40+
]));
41+
42+
$this->key = $this->database->getQueryCacheKey('projects');
43+
}
44+
45+
public function testStaleListWriteAfterConcurrentPurgeIsRejected(): void
46+
{
47+
$hash = 'list-hash';
48+
49+
$document = $this->database->getDocument('projects', 'project');
50+
51+
// The callback stands in for the database read of an older request: a
52+
// concurrent writer purges the query key after the read started but
53+
// before the result is cached. Without a lease the stale list below
54+
// would land in the cache after the purge.
55+
$result = $this->database->withCache($this->key, function () use ($hash, $document) {
56+
$this->cacheAdapter->purge($this->key, $hash);
57+
58+
return [$document];
59+
}, $hash);
60+
61+
$this->assertCount(1, $result);
62+
$this->assertFalse(
63+
$this->cacheAdapter->load($this->key, Database::TTL, $hash),
64+
'A list read whose query key was purged mid-flight must not be re-cached.'
65+
);
66+
}
67+
68+
public function testListWriteLandsWhenNoConcurrentPurge(): void
69+
{
70+
$hash = 'list-hash';
71+
72+
$document = $this->database->getDocument('projects', 'project');
73+
74+
$result = $this->database->withCache($this->key, fn () => [$document], $hash);
75+
76+
$this->assertCount(1, $result);
77+
$this->assertNotFalse(
78+
$this->cacheAdapter->load($this->key, Database::TTL, $hash),
79+
'A list read with no concurrent purge must populate the cache.'
80+
);
81+
}
82+
}
83+
84+
class LeasableMemoryCache implements Adapter, Leasable
85+
{
86+
private const string GENERATION_FIELD = '__utopia_gen__';
87+
88+
/**
89+
* @var array<string, array<string, mixed>>
90+
*/
91+
private array $store = [];
92+
93+
public function load(string $key, int $ttl, string $hash = ''): mixed
94+
{
95+
if ($hash === '') {
96+
$hash = $key;
97+
}
98+
99+
if (! isset($this->store[$key][$hash])) {
100+
return false;
101+
}
102+
103+
$saved = $this->store[$key][$hash];
104+
105+
return ($saved['time'] + $ttl > \time()) ? $saved['data'] : false;
106+
}
107+
108+
public function save(string $key, array|string $data, string $hash = ''): bool|string|array
109+
{
110+
if (empty($key) || empty($data)) {
111+
return false;
112+
}
113+
114+
if ($hash === '') {
115+
$hash = $key;
116+
}
117+
118+
if ($hash === self::GENERATION_FIELD) {
119+
return false;
120+
}
121+
122+
$this->store[$key][$hash] = ['time' => \time(), 'data' => $data];
123+
124+
return $data;
125+
}
126+
127+
public function getGeneration(string $key): string
128+
{
129+
return $this->store[$key][self::GENERATION_FIELD]['data'] ?? '0';
130+
}
131+
132+
public function saveWithLease(string $key, array|string $data, string $hash, string $generation): bool|string|array
133+
{
134+
if (empty($key) || empty($data)) {
135+
return false;
136+
}
137+
138+
if ($this->getGeneration($key) !== $generation) {
139+
return false;
140+
}
141+
142+
return $this->save($key, $data, $hash);
143+
}
144+
145+
public function touch(string $key, string $hash = ''): bool
146+
{
147+
if ($hash === '') {
148+
$hash = $key;
149+
}
150+
151+
if (! isset($this->store[$key][$hash])) {
152+
return false;
153+
}
154+
155+
$this->store[$key][$hash]['time'] = \time();
156+
157+
return true;
158+
}
159+
160+
/**
161+
* @return string[]
162+
*/
163+
public function list(string $key): array
164+
{
165+
return \array_values(\array_filter(
166+
\array_keys($this->store[$key] ?? []),
167+
fn (string $field): bool => $field !== self::GENERATION_FIELD
168+
));
169+
}
170+
171+
public function purge(string $key, string $hash = ''): bool
172+
{
173+
$generation = (string) (((int) $this->getGeneration($key)) + 1);
174+
175+
if ($hash !== '' && $hash !== self::GENERATION_FIELD) {
176+
unset($this->store[$key][$hash]);
177+
} else {
178+
$this->store[$key] = [];
179+
}
180+
181+
$this->store[$key][self::GENERATION_FIELD] = ['time' => \time(), 'data' => $generation];
182+
183+
return true;
184+
}
185+
186+
public function flush(): bool
187+
{
188+
$this->store = [];
189+
190+
return true;
191+
}
192+
193+
public function ping(): bool
194+
{
195+
return true;
196+
}
197+
198+
public function getSize(): int
199+
{
200+
return \count($this->store);
201+
}
202+
203+
public function getName(?string $key = null): string
204+
{
205+
return 'leasable-memory';
206+
}
207+
}

0 commit comments

Comments
 (0)