Skip to content

Commit 0f2cfe9

Browse files
committed
fix(memory): address remaining review findings
- exists($database, $collection) now keys lookup by the $database argument and the filtered collection name, so callers can probe a database other than the adapter's currently-bound one. Storage map is now [database][collectionName] => storageKey. - updateDocument / updateDocuments / deleteDocument / deleteDocuments guard permission cleanup with the current tenant under shared tables, so deleting a row in tenant A no longer wipes another tenant's permissions for a document that happens to share the same $id. - updateDocuments validates every row (including unique-index checks) before any write lands, so a uniqueness violation on row N no longer leaves rows 0..N-1 partially committed. - TYPE_CONTAINS_ALL on object attributes now requires every candidate to be present, instead of returning true on the first match (was behaving like CONTAINS_ANY).
1 parent 5299d73 commit 0f2cfe9

1 file changed

Lines changed: 48 additions & 16 deletions

File tree

src/Database/Adapter/Memory.php

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class Memory extends Adapter
2525
/**
2626
* Map of database name to the set of collection storage keys it owns.
2727
*
28-
* @var array<string, array<string, true>>
28+
* @var array<string, array<string, string>>
2929
*/
3030
protected array $databases = [];
3131

@@ -179,7 +179,7 @@ public function exists(string $database, ?string $collection = null): bool
179179
return false;
180180
}
181181

182-
return isset($this->databases[$database][$this->key($collection)]);
182+
return isset($this->databases[$database][$this->filter($collection)]);
183183
}
184184

185185
public function list(): array
@@ -198,7 +198,7 @@ public function delete(string $name): bool
198198
return true;
199199
}
200200

201-
foreach (\array_keys($this->databases[$name]) as $collectionKey) {
201+
foreach ($this->databases[$name] as $collectionKey) {
202202
unset($this->data[$collectionKey]);
203203
unset($this->permissions[$collectionKey]);
204204
}
@@ -227,7 +227,7 @@ public function createCollection(string $name, array $attributes = [], array $in
227227
if (! isset($this->databases[$database])) {
228228
$this->databases[$database] = [];
229229
}
230-
$this->databases[$database][$key] = true;
230+
$this->databases[$database][$this->filter($name)] = $key;
231231
}
232232

233233
foreach ($attributes as $attribute) {
@@ -259,9 +259,10 @@ public function deleteCollection(string $id): bool
259259
$key = $this->key($id);
260260
unset($this->data[$key]);
261261
unset($this->permissions[$key]);
262+
$filtered = $this->filter($id);
262263
foreach ($this->databases as $name => $collections) {
263-
if (isset($collections[$key])) {
264-
unset($this->databases[$name][$key]);
264+
if (isset($collections[$filtered]) && $collections[$filtered] === $key) {
265+
unset($this->databases[$name][$filtered]);
265266
}
266267
}
267268

@@ -912,10 +913,14 @@ public function updateDocument(Document $collection, string $id, Document $docum
912913
$this->data[$key]['documents'][$newKey] = $row;
913914

914915
if (! $skipPermissions) {
915-
// Remove any permissions keyed to the old uid and rewrite.
916+
// Remove any permissions keyed to the old uid (within the
917+
// current tenant only — other tenants may legitimately hold a
918+
// permission for the same $id under shared tables) and rewrite.
919+
$tenant = $this->getTenant();
916920
$this->permissions[$key] = \array_values(\array_filter(
917921
$this->permissions[$key],
918-
fn (array $p) => $p['document'] !== $id && $p['document'] !== $newId
922+
fn (array $p) => ($this->sharedTables && ($p['tenant'] ?? null) !== $tenant)
923+
|| ($p['document'] !== $id && $p['document'] !== $newId)
919924
));
920925
$this->writePermissions($key, $document);
921926
} elseif ($newId !== $id) {
@@ -949,7 +954,10 @@ public function updateDocuments(Document $collection, Document $updates, array $
949954
return 0;
950955
}
951956

952-
$count = 0;
957+
// Two-phase: validate every row (including unique-index checks)
958+
// before any write lands, so a uniqueness violation on document N
959+
// does not leave documents 0..N-1 partially committed.
960+
$prepared = [];
953961
foreach ($documents as $doc) {
954962
$uid = $doc->getId();
955963
$docKey = $this->documentKey($uid);
@@ -971,6 +979,15 @@ public function updateDocuments(Document $collection, Document $updates, array $
971979
$this->enforceUniqueIndexes($key, $merged, $uid);
972980
}
973981

982+
$prepared[] = ['uid' => $uid, 'docKey' => $docKey, 'attrs' => $resolvedAttrs];
983+
}
984+
985+
$tenant = $this->getTenant();
986+
foreach ($prepared as $entry) {
987+
$uid = $entry['uid'];
988+
$docKey = $entry['docKey'];
989+
$resolvedAttrs = $entry['attrs'];
990+
974991
$row = &$this->data[$key]['documents'][$docKey];
975992
foreach ($resolvedAttrs as $attribute => $value) {
976993
if (\is_array($value)) {
@@ -989,24 +1006,24 @@ public function updateDocuments(Document $collection, Document $updates, array $
9891006
$row['_permissions'] = \json_encode($updates->getPermissions());
9901007
$this->permissions[$key] = \array_values(\array_filter(
9911008
$this->permissions[$key],
992-
fn (array $p) => $p['document'] !== $uid
1009+
fn (array $p) => ($this->sharedTables && ($p['tenant'] ?? null) !== $tenant)
1010+
|| $p['document'] !== $uid
9931011
));
9941012
foreach (Database::PERMISSIONS as $type) {
9951013
foreach ($updates->getPermissionsByType($type) as $permission) {
9961014
$this->permissions[$key][] = [
9971015
'document' => $uid,
9981016
'type' => $type,
9991017
'permission' => \str_replace('"', '', $permission),
1000-
'tenant' => $this->getTenant(),
1018+
'tenant' => $tenant,
10011019
];
10021020
}
10031021
}
10041022
}
1005-
$count++;
10061023
unset($row);
10071024
}
10081025

1009-
return $count;
1026+
return \count($prepared);
10101027
}
10111028

10121029
public function upsertDocuments(Document $collection, string $attribute, array $changes): array
@@ -1052,9 +1069,11 @@ public function deleteDocument(string $collection, string $id): bool
10521069
}
10531070

10541071
unset($this->data[$key]['documents'][$docKey]);
1072+
$tenant = $this->getTenant();
10551073
$this->permissions[$key] = \array_values(\array_filter(
10561074
$this->permissions[$key] ?? [],
1057-
fn (array $p) => $p['document'] !== $id
1075+
fn (array $p) => ($this->sharedTables && ($p['tenant'] ?? null) !== $tenant)
1076+
|| $p['document'] !== $id
10581077
));
10591078

10601079
return true;
@@ -1093,9 +1112,11 @@ public function deleteDocuments(string $collection, array $sequences, array $per
10931112
: [];
10941113

10951114
if (! empty($deletedIds) || ! empty($permSet)) {
1115+
$tenant = $this->getTenant();
10961116
$this->permissions[$key] = \array_values(\array_filter(
10971117
$this->permissions[$key] ?? [],
1098-
fn (array $p) => ! isset($deletedIds[$p['document']]) && ! isset($permSet[$p['document']])
1118+
fn (array $p) => ($this->sharedTables && ($p['tenant'] ?? null) !== $tenant)
1119+
|| (! isset($deletedIds[$p['document']]) && ! isset($permSet[$p['document']]))
10991120
));
11001121
}
11011122

@@ -2183,7 +2204,6 @@ protected function matchesObject(mixed $value, Query $query): bool
21832204

21842205
case Query::TYPE_CONTAINS:
21852206
case Query::TYPE_CONTAINS_ANY:
2186-
case Query::TYPE_CONTAINS_ALL:
21872207
if ($haystack === null) {
21882208
return false;
21892209
}
@@ -2195,6 +2215,18 @@ protected function matchesObject(mixed $value, Query $query): bool
21952215

21962216
return false;
21972217

2218+
case Query::TYPE_CONTAINS_ALL:
2219+
if ($haystack === null) {
2220+
return false;
2221+
}
2222+
foreach ($values as $candidate) {
2223+
if (! $this->jsonContains($haystack, $this->wrapScalarObjectValue($candidate))) {
2224+
return false;
2225+
}
2226+
}
2227+
2228+
return true;
2229+
21982230
case Query::TYPE_NOT_CONTAINS:
21992231
if ($haystack === null) {
22002232
return true;

0 commit comments

Comments
 (0)