Skip to content

Commit 9ac837b

Browse files
premtsd-codepremtsd-code
committed
Clean up cached find semantics
1 parent 6f70767 commit 9ac837b

3 files changed

Lines changed: 92 additions & 55 deletions

File tree

‎src/Database/Database.php‎

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8569,11 +8569,13 @@ public function find(string $collection, array $queries = [], string $forPermiss
85698569
*
85708570
* The caller owns authorization context. Use Authorization::skip() around
85718571
* this method for internal reads that should bypass request-user roles.
8572+
* The caller owns invalidation and must purge cached find entries after
8573+
* writes that can change matching documents or returned document payloads.
85728574
*
85738575
* @param string $collection
85748576
* @param array<Query> $queries
85758577
* @param string|null $namespace
8576-
* @param array<string> $roles
8578+
* @param array<string> $cacheLabels
85778579
* @param string $forPermission
85788580
* @return array<Document>
85798581
* @throws DatabaseException
@@ -8585,7 +8587,7 @@ public function cachedFind(
85858587
string $collection,
85868588
array $queries = [],
85878589
?string $namespace = null,
8588-
array $roles = [],
8590+
array $cacheLabels = [],
85898591
string $forPermission = Database::PERMISSION_READ,
85908592
): array {
85918593
foreach ($queries as $query) {
@@ -8605,16 +8607,19 @@ public function cachedFind(
86058607

86068608
$payload = $this->withCache(
86078609
key: $this->getFindCacheKey($collectionDocument->getId(), $namespace),
8608-
callback: function () use ($collection, $queries, $forPermission, &$cacheMiss, &$documents): array {
8610+
callback: function () use ($collection, $collectionDocument, $queries, $forPermission, &$cacheMiss, &$documents): array {
86098611
$cacheMiss = true;
8610-
$documents = $this->find($collection, $queries, $forPermission);
8612+
$documents = $this->filterCachedFindDocuments(
8613+
$collectionDocument,
8614+
$this->find($collection, $queries, $forPermission),
8615+
);
86118616

86128617
return \array_map(
86138618
static fn (Document $document): array => $document->getArrayCopy(),
86148619
$documents,
86158620
);
86168621
},
8617-
hash: $this->getFindCacheField($collectionDocument, $queries, $roles, 'documents', $forPermission),
8622+
hash: $this->getFindCacheField($collectionDocument, $queries, $cacheLabels, 'documents', $forPermission),
86188623
);
86198624

86208625
if ($cacheMiss) {
@@ -8632,42 +8637,32 @@ public function cachedFind(
86328637
throw new AuthorizationException($this->authorization->getDescription());
86338638
}
86348639

8635-
$selects = Query::groupByType($queries)['selections'];
86368640
$documents = [];
8637-
8638-
// A cached list stores candidate IDs. Refresh each candidate so TTL,
8639-
// deletion, and permission changes are respected; callers still own
8640-
// purging when writes change which documents match the original query.
8641-
foreach ($payload as $payloadDocument) {
8642-
if (!\is_array($payloadDocument)) {
8641+
foreach ($payload as $document) {
8642+
if (!\is_array($document)) {
86438643
continue;
86448644
}
86458645

8646-
$cachedDocument = $this->createDocumentInstance($collection, $payloadDocument);
8647-
if ($cachedDocument->isEmpty()) {
8648-
continue;
8649-
}
8650-
8651-
$document = $this->silent(fn () => $this->getDocument($collection, $cachedDocument->getId(), $selects));
8652-
if ($document->isEmpty()) {
8653-
continue;
8654-
}
8655-
8656-
if (!$skipAuth && $collectionDocument->getId() !== self::METADATA) {
8657-
$permissions = [
8658-
...$collectionDocument->getPermissionsByType($forPermission),
8659-
...($documentSecurity ? $document->getPermissionsByType($forPermission) : []),
8660-
];
8661-
8662-
if (!$this->authorization->isValid(new Input($forPermission, $permissions))) {
8663-
continue;
8664-
}
8665-
}
8646+
$document = $this->createDocumentInstance($collection, $document);
8647+
$document = $this->casting($collectionDocument, $document);
86668648

86678649
$documents[] = $document;
86688650
}
86698651

8670-
return $documents;
8652+
return $this->filterCachedFindDocuments($collectionDocument, $documents);
8653+
}
8654+
8655+
/**
8656+
* @param Document $collection
8657+
* @param array<Document> $documents
8658+
* @return array<Document>
8659+
*/
8660+
private function filterCachedFindDocuments(Document $collection, array $documents): array
8661+
{
8662+
return \array_values(\array_filter(
8663+
$documents,
8664+
fn (Document $document): bool => !$this->isTtlExpired($collection, $document),
8665+
));
86718666
}
86728667

86738668
/**
@@ -9668,22 +9663,22 @@ public function getFindCacheKey(string $collectionId, ?string $namespace = null)
96689663
*
96699664
* @param Document|null $collection
96709665
* @param array<Query> $queries
9671-
* @param array<string> $roles
9666+
* @param array<string> $cacheLabels
96729667
* @param string $field
96739668
* @param string $forPermission
96749669
* @return string
96759670
*/
96769671
public function getFindCacheField(
96779672
?Document $collection = null,
96789673
array $queries = [],
9679-
array $roles = [],
9674+
array $cacheLabels = [],
96809675
string $field = 'documents',
96819676
string $forPermission = self::PERMISSION_READ,
96829677
): string {
96839678
$this->checkQueryTypes($queries);
96849679

9685-
$roles = \array_values(\array_unique($roles));
9686-
\sort($roles);
9680+
$cacheLabels = \array_values(\array_unique($cacheLabels));
9681+
\sort($cacheLabels);
96879682

96889683
$authorizationRoles = \array_values(\array_unique($this->authorization->getRoles()));
96899684
\sort($authorizationRoles);
@@ -9707,7 +9702,7 @@ public function getFindCacheField(
97079702
return \sprintf(
97089703
'%s:%s:%s:%s',
97099704
$this->getFindCacheSchemaHash($collection),
9710-
\md5(\json_encode($roles) ?: ''),
9705+
\md5(\json_encode($cacheLabels) ?: ''),
97119706
\md5(\json_encode($queryPayload) ?: ''),
97129707
$field,
97139708
);
@@ -9767,6 +9762,8 @@ private function getFindCacheSchemaHash(?Document $collection): string
97679762
return \md5(
97689763
\json_encode($collection->getAttribute('attributes', []))
97699764
. \json_encode($collection->getAttribute('indexes', []))
9765+
. \json_encode($collection->getAttribute('$permissions', []))
9766+
. \json_encode($collection->getAttribute('documentSecurity', false))
97709767
);
97719768
}
97729769

‎tests/unit/CacheKeyTest.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,8 @@ public function testFindCacheFieldUsesListCacheShape(): void
189189
$schemaHash = \md5(
190190
(\json_encode($collection->getAttribute('attributes', [])) ?: '')
191191
. (\json_encode($collection->getAttribute('indexes', [])) ?: '')
192+
. (\json_encode($collection->getAttribute('$permissions', [])) ?: '')
193+
. (\json_encode($collection->getAttribute('documentSecurity', false)) ?: '')
192194
);
193195
$field = $db->getFindCacheField($collection, $queries, ['waf']);
194196

@@ -207,7 +209,7 @@ public function testFindCacheFieldChangesWithInputs(): void
207209
'indexes' => [],
208210
]),
209211
[Query::limit(10)],
210-
['role-a'],
212+
['cache-label-a'],
211213
);
212214

213215
$this->assertNotSame(
@@ -218,13 +220,13 @@ public function testFindCacheFieldChangesWithInputs(): void
218220
'indexes' => [],
219221
]),
220222
[Query::limit(10)],
221-
['role-a'],
223+
['cache-label-a'],
222224
),
223225
);
224-
$this->assertNotSame($field, $db->getFindCacheField(null, [Query::limit(20)], ['role-a']));
225-
$this->assertNotSame($field, $db->getFindCacheField(null, [Query::limit(10)], ['role-b']));
226-
$this->assertNotSame($field, $db->getFindCacheField(null, [Query::limit(10)], ['role-a'], 'documents', Database::PERMISSION_UPDATE));
227-
$this->assertStringEndsWith(':total', $db->getFindCacheField(null, [Query::limit(10)], ['role-a'], 'total'));
226+
$this->assertNotSame($field, $db->getFindCacheField(null, [Query::limit(20)], ['cache-label-a']));
227+
$this->assertNotSame($field, $db->getFindCacheField(null, [Query::limit(10)], ['cache-label-b']));
228+
$this->assertNotSame($field, $db->getFindCacheField(null, [Query::limit(10)], ['cache-label-a'], 'documents', Database::PERMISSION_UPDATE));
229+
$this->assertStringEndsWith(':total', $db->getFindCacheField(null, [Query::limit(10)], ['cache-label-a'], 'total'));
228230
}
229231

230232
public function testFindCacheFieldChangesWithActiveAuthorizationContext(): void

‎tests/unit/ListCacheTest.php‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ public function testCachedFindUsesCacheUntilPurged(): void
243243
));
244244
}
245245

246-
public function testCachedFindSeparatesEntriesByRoles(): void
246+
public function testCachedFindSeparatesEntriesByCacheLabels(): void
247247
{
248248
$cache = new HashMemoryCache();
249249
$database = $this->createDatabase($cache);
@@ -283,8 +283,8 @@ public function testCachedFindSeparatesEntriesByRoles(): void
283283
$cached = $database->cachedFind('wafRules', $queries, '_39', ['waf']);
284284
$this->assertCount(1, $cached);
285285

286-
$roleSeparated = $database->cachedFind('wafRules', $queries, '_39', ['manager']);
287-
$this->assertCount(2, $roleSeparated);
286+
$labelSeparated = $database->cachedFind('wafRules', $queries, '_39', ['manager']);
287+
$this->assertCount(2, $labelSeparated);
288288
}
289289

290290
public function testCachedFindSeparatesEntriesByPermissionMode(): void
@@ -453,7 +453,7 @@ public function testCachedFindBypassesCacheForRandomOrder(): void
453453
$this->assertCount(2, $second);
454454
}
455455

456-
public function testCachedFindRevalidatesDocumentPermissionsOnCacheHit(): void
456+
public function testCachedFindReliesOnPurgeForDocumentSecurityCollections(): void
457457
{
458458
$cache = new HashMemoryCache();
459459
$database = $this->createDatabase($cache);
@@ -504,7 +504,12 @@ public function testCachedFindRevalidatesDocumentPermissionsOnCacheHit(): void
504504

505505
$cached = $database->cachedFind('secureRules', $queries, '_39', ['waf']);
506506

507-
$this->assertSame([], $cached);
507+
$this->assertCount(1, $cached);
508+
509+
$this->assertTrue($database->purgeCachedFind('secureRules', '_39'));
510+
511+
$fresh = $database->cachedFind('secureRules', $queries, '_39', ['waf']);
512+
$this->assertSame([], $fresh);
508513
}
509514

510515
public function testCachedFindFiltersTtlExpiredDocumentsOnCacheHit(): void
@@ -542,20 +547,53 @@ public function testCachedFindFiltersTtlExpiredDocumentsOnCacheHit(): void
542547

543548
$database->createDocument('ttlRules', new Document([
544549
'$id' => 'rule-a',
545-
'expiresAt' => \date('c', \time() + 3600),
550+
'expiresAt' => \date('c', \time() - 10),
546551
]));
547552

548553
$first = $database->cachedFind('ttlRules', $queries, '_39', ['waf']);
549-
$this->assertCount(1, $first);
550-
551-
$database->updateDocument('ttlRules', 'rule-a', new Document([
552-
'expiresAt' => \date('c', \time() - 10),
553-
]));
554+
$this->assertSame([], $first);
554555

555556
$cached = $database->cachedFind('ttlRules', $queries, '_39', ['waf']);
556557

557558
$this->assertSame([], $cached);
558559
}
560+
561+
public function testCachedFindRehydratesNestedDocumentPayloads(): void
562+
{
563+
$cache = new HashMemoryCache();
564+
$database = $this->createDatabase($cache);
565+
$database->createCollection('parents', permissions: [
566+
Permission::read(Role::any()),
567+
]);
568+
569+
$queries = [
570+
Query::limit(25),
571+
];
572+
573+
$collection = $database->getCollection('parents');
574+
$cache->save(
575+
$database->getFindCacheKey($collection->getId(), '_39'),
576+
[
577+
'value' => [
578+
[
579+
'$id' => 'parent-a',
580+
'child' => [
581+
'$id' => 'child-a',
582+
'$collection' => 'children',
583+
'name' => 'Child A',
584+
],
585+
],
586+
],
587+
],
588+
$database->getFindCacheField($collection, $queries, ['waf']),
589+
);
590+
591+
$parents = $database->cachedFind('parents', $queries, '_39', ['waf']);
592+
593+
$this->assertCount(1, $parents);
594+
$this->assertInstanceOf(Document::class, $parents[0]->getAttribute('child'));
595+
$this->assertSame('child-a', $parents[0]->getAttribute('child')->getId());
596+
}
559597
}
560598

561599
class HashMemoryCache implements Adapter

0 commit comments

Comments
 (0)