From 4de8d8ba7ff645ee873587feffe17d42602e08e3 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 22:36:57 +1300 Subject: [PATCH 1/9] feat(hook): add a queries hook that rewrites a read's queries Callers such as appwrite translate public join names into stored collection names in their routes before every read, so library reads they do not route themselves (relationship population, cursor pages, findOne, resolveCursor's paired read) never see the translation. A Hook\Queries registered on the handle runs once per getDocument, find, count, sum, aggregate, cursor and resolveCursor call, before validation, join resolution and authorization, so authorization runs on the rewritten target and a hook can refuse a read by throwing. It is a new hook rather than a generalised Transform: Transform is an adapter hook over raw SQL with a public signature, and changing it would break 8.1.2 in a patch release. Writes and the _metadata collection are out of scope; extending to writes later is additive. cursor() resolves the collection at entry only when a queries hook is registered, so without one a missing collection still throws when the generator is iterated, as in 8.1.2. With one, pages() hands the rewritten queries to nextPageCheck(), which resolves joins itself. A Mirror serves reads on its own pipeline, so a hook that is also Lifecycle is registered on the mirror in that branch too. Co-Authored-By: Claude Opus 5.5 --- src/Database/Database.php | 12 ++++++++ src/Database/Hook/Queries.php | 47 +++++++++++++++++++++++++++++++ src/Database/Mirror.php | 8 +++++- src/Database/Trait/Cursors.php | 2 ++ src/Database/Trait/Documents.php | 48 ++++++++++++++++++++++++++++++-- 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 src/Database/Hook/Queries.php diff --git a/src/Database/Database.php b/src/Database/Database.php index b2bb619a5..68020333e 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -239,6 +239,11 @@ class Database */ protected array $nestedHooks = []; + /** + * @var list + */ + protected array $queryHooks = []; + /** @var Value|null Whether every lifecycle hook is silenced. */ private ?Value $silenced = null; @@ -1364,6 +1369,7 @@ public function skipValidation(callable $callback): mixed * - {@see Hook\Decorator} — document transformation on read/write results * - {@see Hook\Relationships} — relationship resolution and mutation * - {@see Hook\Nested} — preparation of each nested related document before a write relates it, run in registration order + * - {@see Hook\Queries} — rewriting of a read's queries before it validates, resolves or authorizes them, run in registration order * - {@see Hook\Write} — row-level write interception (permissions, tenant) * - {@see Hook\Transform} — raw SQL transformation before execution * - {@see Invalidator} — the query cache invalidation, replacing the one setQueryCache() made @@ -1380,6 +1386,7 @@ public function addHook(\Utopia\Query\Hook $hook): static && ! $hook instanceof Hook\Decorator && ! $hook instanceof Relationships && ! $hook instanceof Hook\Nested + && ! $hook instanceof Hook\Queries && ! $hook instanceof Hook\Write && ! $hook instanceof Transform ) { @@ -1410,6 +1417,10 @@ public function addHook(\Utopia\Query\Hook $hook): static $this->nestedHooks[] = $hook; } + if ($hook instanceof Hook\Queries) { + $this->queryHooks[] = $hook; + } + if ($hook instanceof Hook\Write) { $this->adapter->addWriteHook($hook); } @@ -1439,6 +1450,7 @@ public function removeHook(\Utopia\Query\Hook|string $hook): static $this->lifecycleHooks = \array_values(\array_filter($this->lifecycleHooks, $kept)); $this->decorators = \array_values(\array_filter($this->decorators, $kept)); $this->nestedHooks = \array_values(\array_filter($this->nestedHooks, $kept)); + $this->queryHooks = \array_values(\array_filter($this->queryHooks, $kept)); if ($this->invalidator !== null && ! $kept($this->invalidator)) { $this->invalidator = null; diff --git a/src/Database/Hook/Queries.php b/src/Database/Hook/Queries.php new file mode 100644 index 000000000..156a39873 --- /dev/null +++ b/src/Database/Hook/Queries.php @@ -0,0 +1,47 @@ + $queries The queries the read was given, or the previous hook returned + * @return array The queries the read uses + */ + public function rewrite(Event $event, Collection $collection, array $queries): array; +} diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index 6359da817..30b95c29f 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -19,6 +19,7 @@ use Utopia\Database\Filter\Registry; use Utopia\Database\Hook\Lifecycle; use Utopia\Database\Hook\Nested; +use Utopia\Database\Hook\Queries; use Utopia\Database\Hook\Relationships; use Utopia\Database\Hook\Write; use Utopia\Database\Mirror\Failure; @@ -1836,7 +1837,8 @@ public function setAuthorization(Authorization $authorization): static * both sides relate documents exactly as it is configured to. A write hook also intercepts the destination's * writes. A nested hook runs on the mirror, for its pre-checks, and on the source, but never on the destination: * the destination replicates the related documents the source already prepared, so preparing them again would - * give it IDs the source does not have. + * give it IDs the source does not have. A queries hook runs on the mirror, which serves its reads, and is not + * forwarded to the destination. */ #[\Override] public function addHook(\Utopia\Query\Hook $hook): static @@ -1850,6 +1852,10 @@ public function addHook(\Utopia\Query\Hook $hook): static if ($hook instanceof Nested) { $this->nestedHooks[] = $hook; } + + if ($hook instanceof Queries) { + $this->queryHooks[] = $hook; + } } else { parent::addHook($hook); diff --git a/src/Database/Trait/Cursors.php b/src/Database/Trait/Cursors.php index b16a26ce4..d72359648 100644 --- a/src/Database/Trait/Cursors.php +++ b/src/Database/Trait/Cursors.php @@ -5,6 +5,7 @@ use Utopia\Database\Capability; use Utopia\Database\Collection; use Utopia\Database\Document; +use Utopia\Database\Event; use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\NotFound as NotFoundException; use Utopia\Database\Exception\Query as QueryException; @@ -47,6 +48,7 @@ trait Cursors public function resolveCursor(string $collection, string $id, CursorDirection $direction, array $queries = []): Document { $definition = $this->silent(fn (): Collection => $this->getCollection($collection)); + $queries = $this->rewriteQueries(Event::DocumentFind, $definition, $queries); $this->checkQueryTypes($queries); $queries = self::resolveOneToManyJoins($definition, \array_values(\array_filter( diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 22284eb8b..cc06b9b02 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -39,6 +39,7 @@ use Utopia\Database\Exception\Type as TypeException; use Utopia\Database\Hook\Lifecycle; use Utopia\Database\Hook\Preparation; +use Utopia\Database\Hook\Queries; use Utopia\Database\Id; use Utopia\Database\Operator; use Utopia\Database\OperatorType; @@ -518,7 +519,7 @@ public function getDocument(string $collection, string $id, array $queries = [], ? self::sharedDefinition() : $this->silent(fn () => $this->getCollection($collection)); - return $this->readDocument($definition, $id, $queries, $forUpdate); + return $this->readDocument($definition, $id, $this->rewriteQueries(Event::DocumentRead, $definition, $queries), $forUpdate); } /** @@ -3992,7 +3993,9 @@ private function containsDocument(mixed $value): bool */ public function find(string $collection, array $queries = [], PermissionType $forPermission = PermissionType::Read): array { - return $this->fetch($this->silent(fn () => $this->getCollection($collection)), $queries, $forPermission, false); + $definition = $this->silent(fn () => $this->getCollection($collection)); + + return $this->fetch($definition, $this->rewriteQueries(Event::DocumentFind, $definition, $queries), $forPermission, false); } /** @@ -4356,6 +4359,7 @@ public function count(string $collection, array $queries = [], ?int $max = null) $this->assertMax($max); $collection = $this->silent(fn () => $this->getCollection($collection)); + $queries = $this->rewriteQueries(Event::DocumentCount, $collection, $queries); $this->checkQueryTypes($queries); $queries = self::resolveOneToManyJoins($collection, $queries); @@ -4413,6 +4417,7 @@ public function sum(string $collection, string $attribute, array $queries = [], $this->assertMax($max); $collection = $this->silent(fn () => $this->getCollection($collection)); + $queries = $this->rewriteQueries(Event::DocumentSum, $collection, $queries); $this->checkQueryTypes($queries); $queries = self::resolveOneToManyJoins($collection, $queries); @@ -4631,11 +4636,15 @@ private function getSumValidator(Document $collection, array $attributes, bool $ * Yield each document matching the queries, read in batches of $batchSize. A limit in the queries caps the * iteration, and an offset or a cursorAfter in them positions the first batch only. * + * With a {@see Queries} hook registered, the collection is resolved and the queries rewritten when this is + * called, so a missing collection or a refusal throws then; without one, they throw when the generator is iterated. + * * @param array $queries * @return Generator * * @throws DatabaseException When the queries hold a cursorBefore * @throws LimitException When $batchSize is above BATCH_SIZE + * @throws NotFoundException When a {@see Queries} hook is registered and the collection does not exist */ public function cursor( string $collection, @@ -4644,6 +4653,11 @@ public function cursor( PermissionType $forPermission = PermissionType::Read, ): Generator { $batchSize = self::batchSize($batchSize); + + if ($this->queryHooks !== []) { + $queries = $this->rewriteQueries(Event::DocumentFind, $this->silent(fn () => $this->getCollection($collection)), $queries); + } + $parsed = Query::groupByType($queries); if ($parsed->cursor !== null && $parsed->cursorDirection === CursorDirection::Before) { @@ -4722,8 +4736,11 @@ private function pages( */ public function aggregate(string $collection, array $queries): array { + $definition = $this->silent(fn () => $this->getCollection($collection)); + $queries = self::aliasAggregates($this->rewriteQueries(Event::DocumentAggregate, $definition, $queries)); + $rows = []; - foreach ($this->fetch($this->silent(fn () => $this->getCollection($collection)), self::aliasAggregates($queries), PermissionType::Read, true) as $row) { + foreach ($this->fetch($definition, $queries, PermissionType::Read, true) as $row) { $rows[] = $row->getArrayCopy(); } @@ -5539,6 +5556,31 @@ private function checkQueryTypes(array $queries): void } } + /** + * The queries a read uses: the given ones as each {@see Queries} hook rewrote them, in registration order. + * + * @param array $queries + * @return array + * + * @throws QueryException When a query, given or rewritten, is not a {@see Query} + */ + private function rewriteQueries(Event $event, Collection $collection, array $queries): array + { + if ($this->queryHooks === [] || $collection->getId() === self::METADATA) { + return $queries; + } + + $this->checkQueryTypes($queries); + + foreach ($this->queryHooks as $hook) { + $queries = \array_values($hook->rewrite($event, $collection, $queries)); + } + + $this->checkQueryTypes($queries); + + return $queries; + } + private function castBefore(Document $collection, Document $document): Document { if (! $this->adapterHasFeature(Feature\Casting::class)) { From ba9101aecb266f22828d8d19722099cc1b62a266 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 22:36:57 +1300 Subject: [PATCH 2/9] docs: document hook\queries for 8.1.3 8.1.3 is additive, so the upgrade note says no code change is needed and points at how to implement the hook; the changelog states which reads it covers and that writes are not, and the README shows a hook that maps a public join name to a stored collection. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 22 ++++++++++++++++++++++ README.md | 24 ++++++++++++++++++++++++ UPGRADE.md | 9 +++++++++ 3 files changed, 55 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dbe6d0f9..df3245b04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog +## 8.1.3 + +8.1.3 adds a hook that rewrites the queries of a read. It changes nothing for a database without one; see +[Upgrading from 8.1.2 to 8.1.3](UPGRADE.md#upgrading-from-812-to-813). + +### Added + +- `Hook\Queries::rewrite(Event $event, Collection $collection, array $queries): array` rewrites the queries of a + read, registered with `addHook()` and removed with `removeHook()`. It is given the queries one read was given and + the definition of the collection being read, before the read checks, validates, parses, resolves joins or + authorizes anything, and the queries it returns replace the read's queries for the rest of the read, the query-cache + key included. It runs for `getDocument()` (`Event::DocumentRead`), `find()` and so `findOne()`, `cursor()` and + `resolveCursor()` (`Event::DocumentFind`), `count()` (`Event::DocumentCount`), `sum()` (`Event::DocumentSum`) and + `aggregate()` (`Event::DocumentAggregate`). Hooks run in registration order, each given the previous one's output. + Throwing refuses the read, with the exception reaching the caller unchanged before anything is read. It runs on the + reads the library makes itself, such as relationship population and each `cursor()` page, so it must return the same + queries when given its own output; it runs whether or not events are silenced. It never runs for reads of + `Database::METADATA`, for writes (`updateDocuments()` and `deleteDocuments()` included) or for the `query()`, + `mutate()` and `from()` builders. With such a hook registered, `cursor()` resolves the collection and rewrites its + queries when it is called, so a missing collection or a refusal throws then instead of when the generator is + iterated. A `Mirror` runs it on its own reads, also for a hook that is a `Hook\Lifecycle` too. + ## 8.1.2 8.1.2 removes public API that nothing outside the library needed, replaces the opt-in emulation of 7.4.1's fractional diff --git a/README.md b/README.md index 1dde33154..e68163f63 100644 --- a/README.md +++ b/README.md @@ -334,6 +334,30 @@ $database->silent(fn () => $database->ping(), ['audit-log']); A `Utopia\Database\Hook\Transform` rewrites SQL statements before they run, and a `Utopia\Database\Hook\Decorator` modifies the documents that reads and writes return. Both are registered with `addHook()` too, and `removeHook()` unregisters a hook. `Event\DispatcherHook` forwards events to listeners registered per event class and to a PSR-14 dispatcher. +A `Utopia\Database\Hook\Queries` rewrites the queries of `getDocument()`, `find()`, `findOne()`, `cursor()`, `resolveCursor()`, `count()`, `sum()` and `aggregate()` before the read validates them, resolves their joins or authorizes anything; throwing refuses the read. It also runs on the reads the library makes itself, so it must return the same queries when given its own output: + +```php +use Utopia\Database\Collection; +use Utopia\Database\Event; +use Utopia\Database\Hook\Queries; +use Utopia\Database\Query; + +final class PublicNames implements Queries +{ + public function rewrite(Event $event, Collection $collection, array $queries): array + { + return \array_map( + static fn (Query $query): Query => $query->getMethod()->isJoin() && $query->getAttribute() === 'public-movies' + ? (clone $query)->setAttribute('movies') + : $query, + $queries, + ); + } +} + +$database->addHook(new PublicNames()); +``` + ### Database Methods ```php diff --git a/UPGRADE.md b/UPGRADE.md index 4ce9516ee..399710b8c 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,12 @@ +# Upgrading from 8.1.2 to 8.1.3 + +8.1.3 is a patch release and needs no code change. It adds `Hook\Queries`, which rewrites the queries of a read +before the read validates them, resolves their joins or authorizes anything; a database without one reads as in 8.1.2. +To use it, implement `rewrite(Event $event, Collection $collection, array $queries): array` and register the hook with +`addHook()`. Return the queries the read should use, cloning any query you change, and return the same queries when +given your own output, as the hook also runs on the reads the library makes itself. Throw to refuse the read. See +[Hooks](README.md#hooks) for an example and [CHANGELOG.md](CHANGELOG.md#813) for which reads it covers. + # Upgrading from 8.1.1 to 8.1.2 8.1.2 is a patch release. It removes public API that nothing outside the library needed, replaces the opt-in emulation From 21718628b6db91eb307303a3a9f2f32813a2b280 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 22:37:27 +1300 Subject: [PATCH 3/9] test(hook): pin the contract of the queries hook before it exists The 8.1.3 queries hook rewrites a read's queries before the read validates, resolves joins or authorizes anything. These tests fail on 8.1.2 because the hook is missing, and each one guards a property the implementation could quietly lose: a join name only the hook resolves still pages through cursor(), a refusal wins over validation and authorization, writes and readDocument() never call the hook, _metadata reads are skipped, the hook runs inside silent() and findOne() calls it once, and a mirror rewrites only its own reads. A cursor() over a missing collection without hooks still throws only once iterated, so a patch release keeps 8.1.2's laziness. Co-Authored-By: Claude Opus 5.5 --- tests/unit/Hook/QueriesRecorder.php | 57 ++++ tests/unit/Hook/QueriesTest.php | 438 ++++++++++++++++++++++++++ tests/unit/Hook/RemoveHookTest.php | 19 ++ tests/unit/Mirror/QueriesHookTest.php | 82 +++++ 4 files changed, 596 insertions(+) create mode 100644 tests/unit/Hook/QueriesRecorder.php create mode 100644 tests/unit/Hook/QueriesTest.php create mode 100644 tests/unit/Mirror/QueriesHookTest.php diff --git a/tests/unit/Hook/QueriesRecorder.php b/tests/unit/Hook/QueriesRecorder.php new file mode 100644 index 000000000..15991c5bb --- /dev/null +++ b/tests/unit/Hook/QueriesRecorder.php @@ -0,0 +1,57 @@ + */ + public array $calls = []; + + /** @var Closure(array): array */ + private readonly Closure $rewriter; + + /** + * @param (Closure(array): array)|null $rewriter Returns the given queries when null + */ + public function __construct(?Closure $rewriter = null) + { + $this->rewriter = $rewriter ?? static fn (array $queries): array => $queries; + } + + /** + * A rewriter that adds $added to the queries unless they already hold an equal query, so it returns the same + * queries when given its own output. + * + * @return Closure(array): array + */ + public static function adding(Query $added): Closure + { + return static function (array $queries) use ($added): array { + foreach ($queries as $query) { + if ($query->toString() === $added->toString()) { + return $queries; + } + } + + return [...$queries, $added]; + }; + } + + #[\Override] + public function rewrite(Event $event, Collection $collection, array $queries): array + { + $this->calls[] = [$event, $collection->getId()]; + + /** @var array */ + return ($this->rewriter)($queries); + } +} diff --git a/tests/unit/Hook/QueriesTest.php b/tests/unit/Hook/QueriesTest.php new file mode 100644 index 000000000..9fabef2df --- /dev/null +++ b/tests/unit/Hook/QueriesTest.php @@ -0,0 +1,438 @@ +): mixed, mixed}> + */ + public static function joinedReads(): iterable + { + yield 'find' => [static fn (Database $database, array $joins): mixed => self::ids($database->find('tickets', [...$joins, Query::orderAsc('$id')])), ['k1', 'k2', 'k3']]; + yield 'count' => [static fn (Database $database, array $joins): mixed => $database->count('tickets', $joins), 3]; + yield 'sum' => [static fn (Database $database, array $joins): mixed => $database->sum('tickets', 'amount', $joins), 111]; + } + + /** + * @return iterable): mixed}> + */ + public static function reads(): iterable + { + yield 'getDocument' => [static fn (Database $database, string $collection, array $queries): mixed => $database->getDocument($collection, 'k1', $queries)]; + yield 'find' => [static fn (Database $database, string $collection, array $queries): mixed => $database->find($collection, $queries)]; + yield 'findOne' => [static fn (Database $database, string $collection, array $queries): mixed => $database->findOne($collection, $queries)]; + yield 'count' => [static fn (Database $database, string $collection, array $queries): mixed => $database->count($collection, $queries)]; + yield 'sum' => [static fn (Database $database, string $collection, array $queries): mixed => $database->sum($collection, 'amount', $queries)]; + yield 'aggregate' => [static fn (Database $database, string $collection, array $queries): mixed => $database->aggregate($collection, [...$queries, Query::count('*', 'total')])]; + yield 'cursor' => [static fn (Database $database, string $collection, array $queries): mixed => \iterator_to_array($database->cursor($collection, $queries), false)]; + yield 'resolveCursor' => [static fn (Database $database, string $collection, array $queries): mixed => $database->resolveCursor($collection, 'k1', CursorDirection::After, $queries)]; + } + + /** + * The reads that authorize the collections they join; resolveCursor() reads with authorization skipped. + * + * @return iterable): mixed}> + */ + public static function authorizingReads(): iterable + { + foreach (self::reads() as $name => $read) { + if ($name !== 'resolveCursor') { + yield $name => $read; + } + } + } + + /** + * Each read, the event and collection the hook is given on each call it makes, the query the hook adds, and the + * read's result with that query added. + * + * @return iterable, Query, mixed}> + */ + public static function rewrittenReads(): iterable + { + $open = Query::equal('status', ['open']); + $tickets = 'tickets'; + + yield 'getDocument' => [ + static fn (Database $database): mixed => \array_intersect_key($database->getDocument($tickets, 'k2')->getArrayCopy(), ['name' => true, 'amount' => true]), + [[Event::DocumentRead, $tickets]], + Query::select(['name']), + ['name' => 'second'], + ]; + yield 'find' => [ + static fn (Database $database): mixed => self::ids($database->find($tickets, [Query::orderAsc('$id')])), + [[Event::DocumentFind, $tickets]], + $open, + ['k1', 'k3'], + ]; + yield 'findOne' => [ + static fn (Database $database): mixed => $database->findOne($tickets, [Query::orderDesc('amount')])->getId(), + [[Event::DocumentFind, $tickets]], + $open, + 'k3', + ]; + yield 'count' => [ + static fn (Database $database): mixed => $database->count($tickets), + [[Event::DocumentCount, $tickets]], + $open, + 2, + ]; + yield 'sum' => [ + static fn (Database $database): mixed => $database->sum($tickets, 'amount'), + [[Event::DocumentSum, $tickets]], + $open, + 101, + ]; + yield 'aggregate' => [ + static fn (Database $database): mixed => \array_map(static fn (array $row): array => \array_map(\intval(...), $row), $database->aggregate($tickets, [Query::count('*', 'total')])), + [[Event::DocumentAggregate, $tickets]], + $open, + [['total' => 2]], + ]; + yield 'cursor' => [ + static fn (Database $database): mixed => self::ids(\iterator_to_array($database->cursor($tickets, [Query::orderAsc('$id')]), false)), + [[Event::DocumentFind, $tickets], [Event::DocumentFind, $tickets]], + $open, + ['k1', 'k3'], + ]; + yield 'resolveCursor' => [ + static fn (Database $database): mixed => (int) $database->resolveCursor($tickets, 'k2', CursorDirection::After)->getAttribute('amount'), + [[Event::DocumentFind, $tickets]], + Query::orderDesc('amount'), + 10, + ]; + } + + public function testAddingAQueriesHookReturnsTheDatabase(): void + { + $database = HookFixture::memory(); + $recorder = new QueriesRecorder(); + + $this->assertSame($database, $database->addHook($recorder)); + + $database->find(HookFixture::COLLECTION); + $this->assertSame([[Event::DocumentFind, HookFixture::COLLECTION]], $recorder->calls); + } + + /** + * @param Closure(Database, array): mixed $read + */ + #[DataProvider('joinedReads')] + public function testAHookMappingAJoinNameLetsTheReadJoin(Closure $read, mixed $expected): void + { + $database = $this->joined(); + + try { + $read($database, [$this->publicThemesJoin()]); + $this->fail('A join of an unknown collection was read'); + } catch (QueryException $error) { + $this->assertSame("Joined collection 'public-themes' not found", $error->getMessage()); + } + + $database->addHook(new QueriesRecorder(self::mapping(self::PUBLIC_THEMES, 'themes'))); + + $this->assertSame($expected, $read($database, [$this->publicThemesJoin()])); + } + + /** + * @param Closure(Database, string, array): mixed $read + */ + #[DataProvider('authorizingReads')] + public function testTheReadAuthorizesTheCollectionTheHookJoins(Closure $read): void + { + $database = $this->joined(); + $database->addHook(new QueriesRecorder(self::mapping(self::PUBLIC_THEMES, 'closed'))); + + try { + $read($database, 'tickets', [$this->publicThemesJoin()]); + $this->fail('A join of an unreadable collection was read'); + } catch (AuthorizationException $error) { + $this->assertSame("Unauthorized access to joined collection 'closed'", $error->getMessage()); + } + } + + /** + * @param Closure(Database, string, array): mixed $read + */ + #[DataProvider('reads')] + public function testARefusalReachesTheCallerBeforeTheReadChecksAnything(Closure $read): void + { + $database = $this->joined(); + $refusal = new DomainException('refused'); + $database->addHook(new QueriesRecorder(static fn (array $queries): array => throw $refusal)); + + try { + $read($database, 'sealed', [Query::join('unknown', 'u', [Query::on('theme', '$id')])]); + $this->fail('A refused read was read'); + } catch (DomainException $error) { + $this->assertSame($refusal, $error); + } + } + + /** + * @param Closure(Database): mixed $read + * @param list $calls + */ + #[DataProvider('rewrittenReads')] + public function testEveryReadUsesTheQueriesTheHookReturns(Closure $read, array $calls, Query $added, mixed $expected): void + { + $database = $this->joined(); + $recorder = new QueriesRecorder(QueriesRecorder::adding($added)); + $database->addHook($recorder); + + $this->assertSame($expected, $read($database)); + $this->assertSame($calls, $recorder->calls); + } + + public function testFindOneRunsTheHookOnce(): void + { + $database = $this->posts(); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::equal('title', ['second']))); + $database->addHook($recorder); + + $this->assertSame('second', $database->findOne(HookFixture::COLLECTION)->getId()); + $this->assertSame([[Event::DocumentFind, HookFixture::COLLECTION]], $recorder->calls); + } + + public function testACursorPagesPastItsFirstBatchOverAJoinOnlyTheHookResolves(): void + { + $database = $this->joined(); + $database->addHook(new QueriesRecorder(self::mapping(self::PUBLIC_THEMES, 'themes'))); + + $documents = $database->cursor('tickets', [$this->publicThemesJoin(), Query::orderAsc('$id')], batchSize: 1); + + $this->assertSame(['k1', 'k2', 'k3'], self::ids(\iterator_to_array($documents, false))); + } + + public function testHooksRunInRegistrationOrderEachGivenThePreviousOutput(): void + { + $database = $this->posts(); + $order = []; + $output = []; + $given = []; + $adding = QueriesRecorder::adding(Query::equal('title', ['first'])); + $first = new QueriesRecorder(static function (array $queries) use ($adding, &$order, &$output): array { + $order[] = 'first'; + $output = $adding($queries); + + return $output; + }); + $second = new QueriesRecorder(static function (array $queries) use (&$order, &$given): array { + $order[] = 'second'; + $given = $queries; + + return $queries; + }); + $database->addHook($first)->addHook($second); + + $this->assertSame(['first'], self::ids($database->find(HookFixture::COLLECTION))); + $this->assertSame(['first', 'second'], $order); + $this->assertNotSame([], $given); + $this->assertSame($output, $given); + } + + public function testTheHookIsNeverGivenAReadOfTheMetadataCollection(): void + { + $database = $this->posts(); + $recorder = new QueriesRecorder(); + $database->addHook($recorder); + + $database->getCollection(HookFixture::COLLECTION); + $database->listCollections(); + $database->getDocument(Database::METADATA, HookFixture::COLLECTION); + $database->find(Database::METADATA); + $database->count(Database::METADATA); + $database->getDocument(HookFixture::COLLECTION, 'first'); + + $this->assertSame([[Event::DocumentRead, HookFixture::COLLECTION]], $recorder->calls); + } + + public function testBulkWritesWithFiltersDoNotRunTheHook(): void + { + $database = $this->posts(); + $recorder = new QueriesRecorder(); + $database->addHook($recorder); + + $this->assertSame(1, $database->updateDocuments(HookFixture::COLLECTION, new Document(['views' => 9]), [Query::equal('title', ['first'])])); + $this->assertSame(1, $database->deleteDocuments(HookFixture::COLLECTION, [Query::equal('title', ['second'])])); + + $this->assertSame([], $recorder->calls); + } + + public function testAHookReturningSomethingOtherThanAQueryIsRefused(): void + { + $database = $this->posts(); + $database->addHook(new QueriesRecorder(static fn (array $queries): array => [...$queries, 'title'])); + + try { + $database->find(HookFixture::COLLECTION); + $this->fail('A query that is not a Query was read'); + } catch (QueryException $error) { + $this->assertSame('Invalid query type: "string". Expected instances of "'.Query::class.'"', $error->getMessage()); + } + } + + public function testTheHookRunsWhileEventsAreSilenced(): void + { + $database = $this->posts(); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::equal('title', ['second']))); + $database->addHook($recorder); + + $documents = $database->silent(static fn (): array => $database->find(HookFixture::COLLECTION)); + + $this->assertSame(['second'], self::ids($documents)); + $this->assertSame([[Event::DocumentFind, HookFixture::COLLECTION]], $recorder->calls); + } + + public function testWritesThatReadOnlyThroughTheirOwnDocumentReadDoNotRunTheHook(): void + { + $database = $this->posts(); + $recorder = new QueriesRecorder(); + $database->addHook($recorder); + + $increased = $database->increaseDocumentAttribute(HookFixture::COLLECTION, 'first', 'views', 5); + $deleted = $database->deleteDocument(HookFixture::COLLECTION, 'second'); + + $this->assertSame([], $recorder->calls); + $this->assertSame(6, $increased->getAttribute('views')); + $this->assertTrue($deleted); + } + + public function testACursorWithoutHooksOverAMissingCollectionThrowsOnlyOnceIterated(): void + { + $database = HookFixture::memory(); + + try { + $documents = $database->cursor('missing'); + } catch (NotFoundException) { + $this->fail('cursor() resolved its collection before it was iterated'); + } + + try { + \iterator_to_array($documents); + $this->fail('A cursor over a missing collection was iterated'); + } catch (NotFoundException $error) { + $this->assertSame('Collection not found', $error->getMessage()); + } + } + + /** + * @return Closure(array): array + */ + private static function mapping(string $from, string $to): Closure + { + return static fn (array $queries): array => \array_map( + static fn (Query $query): Query => $query->getMethod()->isJoin() && $query->getAttribute() === $from + ? (clone $query)->setAttribute($to) + : $query, + $queries, + ); + } + + /** + * @param array $documents + * @return list + */ + private static function ids(array $documents): array + { + return \array_values(\array_map(static fn (Document $document): string => $document->getId(), $documents)); + } + + private function publicThemesJoin(): Query + { + return Query::join(self::PUBLIC_THEMES, 'th', [Query::on('theme', '$id')]); + } + + private function posts(): Database + { + $database = HookFixture::memory(); + HookFixture::seed($database, ['first', 'second']); + + return $database; + } + + private function joined(): Database + { + $authorization = new Authorization(); + $authorization->addRole(Role::any()->toString()); + + $connection = new PDO('sqlite::memory:', null, null, [PDO::ATTR_PERSISTENT => false, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_EMULATE_PREPARES => true, PDO::ATTR_STRINGIFY_FETCHES => true]); + $database = new Database(new SQLite($connection), new Cache(new None())); + $database + ->setAuthorization($authorization) + ->setDatabase('queries_hook') + ->setNamespace('queries_hook_'.\uniqid()); + $database->addHook(new Permissions()); + $database->create(); + + $readable = [Permission::create(Role::any()), Permission::read(Role::any())]; + $database->createCollection(Collection::create( + id: 'themes', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: $readable, + documentSecurity: false, + )); + $database->createCollection(Collection::create( + id: 'closed', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [], + documentSecurity: false, + )); + $tickets = [ + Attribute::string(key: 'name', size: 64), + Attribute::string(key: 'theme', size: 64), + Attribute::string(key: 'status', size: 16), + Attribute::integer(key: 'amount'), + ]; + $database->createCollection(Collection::create(id: 'tickets', attributes: $tickets, permissions: $readable, documentSecurity: false)); + $database->createCollection(Collection::create(id: 'sealed', attributes: $tickets, permissions: [], documentSecurity: false)); + + foreach (['t1' => 'first theme', 't2' => 'second theme', 't3' => 'third theme'] as $id => $name) { + $database->createDocument('themes', new Document(['$id' => $id, 'name' => $name])); + } + + foreach ([ + ['k1', 'first', 't1', 'open', 1], + ['k2', 'second', 't2', 'closed', 10], + ['k3', 'third', 't3', 'open', 100], + ['k4', 'fourth', 'missing', 'closed', 1000], + ] as [$id, $name, $theme, $status, $amount]) { + $database->createDocument('tickets', new Document([ + '$id' => $id, + 'name' => $name, + 'theme' => $theme, + 'status' => $status, + 'amount' => $amount, + ])); + } + + return $database; + } +} diff --git a/tests/unit/Hook/RemoveHookTest.php b/tests/unit/Hook/RemoveHookTest.php index 9e915d61e..8001215f0 100644 --- a/tests/unit/Hook/RemoveHookTest.php +++ b/tests/unit/Hook/RemoveHookTest.php @@ -20,6 +20,7 @@ use Utopia\Database\Hook\Write; use Utopia\Database\Mirror; use Utopia\Database\Permission; +use Utopia\Database\Query; use Utopia\Database\Relationship; use Utopia\Database\Role; @@ -121,6 +122,24 @@ public function transform(Event $event, string $query): string $this->assertSame($transformed, $transform->transformed); } + public function testRemovingAQueriesHookStopsItRewritingReads(): void + { + $database = HookFixture::memory(); + HookFixture::seed($database, ['first', 'second']); + $removed = new QueriesRecorder(QueriesRecorder::adding(Query::equal('title', ['first']))); + $kept = new QueriesRecorder(); + $database->addHook($removed)->addHook($kept); + + $this->assertSame($database, $database->removeHook($removed)); + $this->assertSame(2, $database->count(HookFixture::COLLECTION)); + $this->assertSame([], $removed->calls); + $this->assertSame([[Event::DocumentCount, HookFixture::COLLECTION]], $kept->calls); + + $database->removeHook(QueriesRecorder::class); + $this->assertSame(2, $database->count(HookFixture::COLLECTION)); + $this->assertSame([[Event::DocumentCount, HookFixture::COLLECTION]], $kept->calls); + } + public function testRemovingTheRelationshipsHookStopsRelatingDocuments(): void { $database = $this->family(); diff --git a/tests/unit/Mirror/QueriesHookTest.php b/tests/unit/Mirror/QueriesHookTest.php new file mode 100644 index 000000000..6b2cab869 --- /dev/null +++ b/tests/unit/Mirror/QueriesHookTest.php @@ -0,0 +1,82 @@ + + */ + public static function lifecycles(): iterable + { + yield 'a queries hook' => [false]; + yield 'a queries hook that is also a lifecycle hook' => [true]; + } + + #[DataProvider('lifecycles')] + public function testTheMirrorRewritesItsOwnReadsOnce(bool $lifecycle): void + { + $hook = self::hook($lifecycle); + $source = HookFixture::memory(); + HookFixture::seed($source, ['first', 'second']); + $mirror = new Mirror($source, HookFixture::memory()); + + $mirror->addHook($hook); + + $this->assertSame(['first'], self::ids($mirror->find(HookFixture::COLLECTION))); + $this->assertSame([[Event::DocumentFind, HookFixture::COLLECTION]], $hook->calls); + } + + #[DataProvider('lifecycles')] + public function testNeitherSideRewritesItsReadsWithTheMirrorsHook(bool $lifecycle): void + { + $hook = self::hook($lifecycle); + $source = HookFixture::memory(); + $destination = HookFixture::memory(); + HookFixture::seed($source, ['first', 'second']); + HookFixture::seed($destination, ['first', 'second']); + $mirror = new Mirror($source, $destination); + + $mirror->addHook($hook); + + $this->assertSame(['first', 'second'], self::ids($mirror->getSource()->find(HookFixture::COLLECTION))); + $this->assertSame(['first', 'second'], self::ids($destination->find(HookFixture::COLLECTION))); + $this->assertSame([], $hook->calls); + } + + private static function hook(bool $lifecycle): QueriesRecorder + { + $adding = QueriesRecorder::adding(Query::equal('title', ['first'])); + + if (! $lifecycle) { + return new QueriesRecorder($adding); + } + + return new class ($adding) extends QueriesRecorder implements Lifecycle { + #[\Override] + public function handle(Domain $event): void + { + } + }; + } + + /** + * @param array $documents + * @return list + */ + private static function ids(array $documents): array + { + return \array_values(\array_map(static fn (Document $document): string => $document->getId(), $documents)); + } +} From 7634d954c63d1a9b92668a313ef983bb7fdd2469 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 22:38:38 +1300 Subject: [PATCH 4/9] test(hook): check only plain hooks stay off the mirror's source A hook that is both Lifecycle and Queries goes through Mirror's lifecycle branch, which already registers it on the source (Nested hooks work the same way). So the source can rewrite with it, and the mirror test now checks only that the mirror's own reads apply it once. Co-Authored-By: Claude Opus 5.5 --- tests/unit/Mirror/QueriesHookTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/Mirror/QueriesHookTest.php b/tests/unit/Mirror/QueriesHookTest.php index 6b2cab869..e2e651ab2 100644 --- a/tests/unit/Mirror/QueriesHookTest.php +++ b/tests/unit/Mirror/QueriesHookTest.php @@ -38,10 +38,9 @@ public function testTheMirrorRewritesItsOwnReadsOnce(bool $lifecycle): void $this->assertSame([[Event::DocumentFind, HookFixture::COLLECTION]], $hook->calls); } - #[DataProvider('lifecycles')] - public function testNeitherSideRewritesItsReadsWithTheMirrorsHook(bool $lifecycle): void + public function testNeitherSideRewritesItsReadsWithTheMirrorsHook(): void { - $hook = self::hook($lifecycle); + $hook = self::hook(false); $source = HookFixture::memory(); $destination = HookFixture::memory(); HookFixture::seed($source, ['first', 'second']); From 87f42dabcfab197fe42d967b99c28b87e2b8baa6 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 22:39:34 +1300 Subject: [PATCH 5/9] docs(mirror): state that a lifecycle queries hook also rewrites the source's reads Mirror registers every lifecycle hook on the source, so a hook that is both Lifecycle and Queries rewrites reads made on the source directly, while the mirror's own reads still apply it once. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 +++- src/Database/Mirror.php | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df3245b04..021a783c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,9 @@ `Database::METADATA`, for writes (`updateDocuments()` and `deleteDocuments()` included) or for the `query()`, `mutate()` and `from()` builders. With such a hook registered, `cursor()` resolves the collection and rewrites its queries when it is called, so a missing collection or a refusal throws then instead of when the generator is - iterated. A `Mirror` runs it on its own reads, also for a hook that is a `Hook\Lifecycle` too. + iterated. A `Mirror` runs it once on each of its own reads and never on its destination's. One that is also a + `Hook\Lifecycle` is registered on the source too, as every lifecycle hook is, so it also rewrites the reads made on + the source directly. ## 8.1.2 diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index 30b95c29f..ff8f4e257 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -1837,8 +1837,9 @@ public function setAuthorization(Authorization $authorization): static * both sides relate documents exactly as it is configured to. A write hook also intercepts the destination's * writes. A nested hook runs on the mirror, for its pre-checks, and on the source, but never on the destination: * the destination replicates the related documents the source already prepared, so preparing them again would - * give it IDs the source does not have. A queries hook runs on the mirror, which serves its reads, and is not - * forwarded to the destination. + * give it IDs the source does not have. A queries hook runs once on each read of the mirror, which serves its + * reads, and never on the destination. One that is also a lifecycle hook is registered on the source too, as every + * lifecycle hook is, so it also rewrites the reads made on the source directly. */ #[\Override] public function addHook(\Utopia\Query\Hook $hook): static From 96b531ea4d50ec6e6b7702c521550ba0ad4aa50c Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 22:50:52 +1300 Subject: [PATCH 6/9] test(hook): type the queries hook tests' rewriters as query lists PHPStan at level max infers no element type for an untyped array closure parameter unless the closure is passed straight to a typed parameter, so the returned rewriters and the data-provider reads failed analysis. A variadic Query parameter gives every closure a list, and integer() decodes the numeric attributes without casting mixed. Co-Authored-By: Claude Opus 5.5 --- tests/unit/Hook/QueriesRecorder.php | 12 +++--- tests/unit/Hook/QueriesTest.php | 65 ++++++++++++++++------------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/tests/unit/Hook/QueriesRecorder.php b/tests/unit/Hook/QueriesRecorder.php index 15991c5bb..8c0ac33d6 100644 --- a/tests/unit/Hook/QueriesRecorder.php +++ b/tests/unit/Hook/QueriesRecorder.php @@ -16,26 +16,26 @@ class QueriesRecorder implements Queries /** @var list */ public array $calls = []; - /** @var Closure(array): array */ + /** @var Closure(Query ...): array */ private readonly Closure $rewriter; /** - * @param (Closure(array): array)|null $rewriter Returns the given queries when null + * @param (Closure(Query ...): array)|null $rewriter Returns the given queries when null */ public function __construct(?Closure $rewriter = null) { - $this->rewriter = $rewriter ?? static fn (array $queries): array => $queries; + $this->rewriter = $rewriter ?? static fn (Query ...$queries): array => $queries; } /** * A rewriter that adds $added to the queries unless they already hold an equal query, so it returns the same * queries when given its own output. * - * @return Closure(array): array + * @return Closure(Query ...): array */ public static function adding(Query $added): Closure { - return static function (array $queries) use ($added): array { + return static function (Query ...$queries) use ($added): array { foreach ($queries as $query) { if ($query->toString() === $added->toString()) { return $queries; @@ -52,6 +52,6 @@ public function rewrite(Event $event, Collection $collection, array $queries): a $this->calls[] = [$event, $collection->getId()]; /** @var array */ - return ($this->rewriter)($queries); + return ($this->rewriter)(...$queries); } } diff --git a/tests/unit/Hook/QueriesTest.php b/tests/unit/Hook/QueriesTest.php index 9fabef2df..5b0a0a2d9 100644 --- a/tests/unit/Hook/QueriesTest.php +++ b/tests/unit/Hook/QueriesTest.php @@ -31,34 +31,34 @@ final class QueriesTest extends TestCase private const string PUBLIC_THEMES = 'public-themes'; /** - * @return iterable): mixed, mixed}> + * @return iterable */ public static function joinedReads(): iterable { - yield 'find' => [static fn (Database $database, array $joins): mixed => self::ids($database->find('tickets', [...$joins, Query::orderAsc('$id')])), ['k1', 'k2', 'k3']]; - yield 'count' => [static fn (Database $database, array $joins): mixed => $database->count('tickets', $joins), 3]; - yield 'sum' => [static fn (Database $database, array $joins): mixed => $database->sum('tickets', 'amount', $joins), 111]; + yield 'find' => [static fn (Database $database, Query ...$joins): mixed => self::ids($database->find('tickets', [...$joins, Query::orderAsc('$id')])), ['k1', 'k2', 'k3']]; + yield 'count' => [static fn (Database $database, Query ...$joins): mixed => $database->count('tickets', $joins), 3]; + yield 'sum' => [static fn (Database $database, Query ...$joins): mixed => $database->sum('tickets', 'amount', $joins), 111]; } /** - * @return iterable): mixed}> + * @return iterable */ public static function reads(): iterable { - yield 'getDocument' => [static fn (Database $database, string $collection, array $queries): mixed => $database->getDocument($collection, 'k1', $queries)]; - yield 'find' => [static fn (Database $database, string $collection, array $queries): mixed => $database->find($collection, $queries)]; - yield 'findOne' => [static fn (Database $database, string $collection, array $queries): mixed => $database->findOne($collection, $queries)]; - yield 'count' => [static fn (Database $database, string $collection, array $queries): mixed => $database->count($collection, $queries)]; - yield 'sum' => [static fn (Database $database, string $collection, array $queries): mixed => $database->sum($collection, 'amount', $queries)]; - yield 'aggregate' => [static fn (Database $database, string $collection, array $queries): mixed => $database->aggregate($collection, [...$queries, Query::count('*', 'total')])]; - yield 'cursor' => [static fn (Database $database, string $collection, array $queries): mixed => \iterator_to_array($database->cursor($collection, $queries), false)]; - yield 'resolveCursor' => [static fn (Database $database, string $collection, array $queries): mixed => $database->resolveCursor($collection, 'k1', CursorDirection::After, $queries)]; + yield 'getDocument' => [static fn (Database $database, string $collection, Query ...$queries): mixed => $database->getDocument($collection, 'k1', $queries)]; + yield 'find' => [static fn (Database $database, string $collection, Query ...$queries): mixed => $database->find($collection, $queries)]; + yield 'findOne' => [static fn (Database $database, string $collection, Query ...$queries): mixed => $database->findOne($collection, $queries)]; + yield 'count' => [static fn (Database $database, string $collection, Query ...$queries): mixed => $database->count($collection, $queries)]; + yield 'sum' => [static fn (Database $database, string $collection, Query ...$queries): mixed => $database->sum($collection, 'amount', $queries)]; + yield 'aggregate' => [static fn (Database $database, string $collection, Query ...$queries): mixed => $database->aggregate($collection, [...$queries, Query::count('*', 'total')])]; + yield 'cursor' => [static fn (Database $database, string $collection, Query ...$queries): mixed => \iterator_to_array($database->cursor($collection, $queries), false)]; + yield 'resolveCursor' => [static fn (Database $database, string $collection, Query ...$queries): mixed => $database->resolveCursor($collection, 'k1', CursorDirection::After, $queries)]; } /** * The reads that authorize the collections they join; resolveCursor() reads with authorization skipped. * - * @return iterable): mixed}> + * @return iterable */ public static function authorizingReads(): iterable { @@ -111,7 +111,7 @@ public static function rewrittenReads(): iterable 101, ]; yield 'aggregate' => [ - static fn (Database $database): mixed => \array_map(static fn (array $row): array => \array_map(\intval(...), $row), $database->aggregate($tickets, [Query::count('*', 'total')])), + static fn (Database $database): mixed => \array_map(static fn (array $row): array => \array_map(self::integer(...), $row), $database->aggregate($tickets, [Query::count('*', 'total')])), [[Event::DocumentAggregate, $tickets]], $open, [['total' => 2]], @@ -123,7 +123,7 @@ public static function rewrittenReads(): iterable ['k1', 'k3'], ]; yield 'resolveCursor' => [ - static fn (Database $database): mixed => (int) $database->resolveCursor($tickets, 'k2', CursorDirection::After)->getAttribute('amount'), + static fn (Database $database): mixed => self::integer($database->resolveCursor($tickets, 'k2', CursorDirection::After)->getAttribute('amount')), [[Event::DocumentFind, $tickets]], Query::orderDesc('amount'), 10, @@ -142,7 +142,7 @@ public function testAddingAQueriesHookReturnsTheDatabase(): void } /** - * @param Closure(Database, array): mixed $read + * @param Closure(Database, Query ...): mixed $read */ #[DataProvider('joinedReads')] public function testAHookMappingAJoinNameLetsTheReadJoin(Closure $read, mixed $expected): void @@ -150,7 +150,7 @@ public function testAHookMappingAJoinNameLetsTheReadJoin(Closure $read, mixed $e $database = $this->joined(); try { - $read($database, [$this->publicThemesJoin()]); + $read($database, $this->publicThemesJoin()); $this->fail('A join of an unknown collection was read'); } catch (QueryException $error) { $this->assertSame("Joined collection 'public-themes' not found", $error->getMessage()); @@ -158,11 +158,11 @@ public function testAHookMappingAJoinNameLetsTheReadJoin(Closure $read, mixed $e $database->addHook(new QueriesRecorder(self::mapping(self::PUBLIC_THEMES, 'themes'))); - $this->assertSame($expected, $read($database, [$this->publicThemesJoin()])); + $this->assertSame($expected, $read($database, $this->publicThemesJoin())); } /** - * @param Closure(Database, string, array): mixed $read + * @param Closure(Database, string, Query ...): mixed $read */ #[DataProvider('authorizingReads')] public function testTheReadAuthorizesTheCollectionTheHookJoins(Closure $read): void @@ -171,7 +171,7 @@ public function testTheReadAuthorizesTheCollectionTheHookJoins(Closure $read): v $database->addHook(new QueriesRecorder(self::mapping(self::PUBLIC_THEMES, 'closed'))); try { - $read($database, 'tickets', [$this->publicThemesJoin()]); + $read($database, 'tickets', $this->publicThemesJoin()); $this->fail('A join of an unreadable collection was read'); } catch (AuthorizationException $error) { $this->assertSame("Unauthorized access to joined collection 'closed'", $error->getMessage()); @@ -179,17 +179,17 @@ public function testTheReadAuthorizesTheCollectionTheHookJoins(Closure $read): v } /** - * @param Closure(Database, string, array): mixed $read + * @param Closure(Database, string, Query ...): mixed $read */ #[DataProvider('reads')] public function testARefusalReachesTheCallerBeforeTheReadChecksAnything(Closure $read): void { $database = $this->joined(); $refusal = new DomainException('refused'); - $database->addHook(new QueriesRecorder(static fn (array $queries): array => throw $refusal)); + $database->addHook(new QueriesRecorder(static fn (Query ...$queries): array => throw $refusal)); try { - $read($database, 'sealed', [Query::join('unknown', 'u', [Query::on('theme', '$id')])]); + $read($database, 'sealed', Query::join('unknown', 'u', [Query::on('theme', '$id')])); $this->fail('A refused read was read'); } catch (DomainException $error) { $this->assertSame($refusal, $error); @@ -238,13 +238,13 @@ public function testHooksRunInRegistrationOrderEachGivenThePreviousOutput(): voi $output = []; $given = []; $adding = QueriesRecorder::adding(Query::equal('title', ['first'])); - $first = new QueriesRecorder(static function (array $queries) use ($adding, &$order, &$output): array { + $first = new QueriesRecorder(static function (Query ...$queries) use ($adding, &$order, &$output): array { $order[] = 'first'; - $output = $adding($queries); + $output = $adding(...$queries); return $output; }); - $second = new QueriesRecorder(static function (array $queries) use (&$order, &$given): array { + $second = new QueriesRecorder(static function (Query ...$queries) use (&$order, &$given): array { $order[] = 'second'; $given = $queries; @@ -289,7 +289,7 @@ public function testBulkWritesWithFiltersDoNotRunTheHook(): void public function testAHookReturningSomethingOtherThanAQueryIsRefused(): void { $database = $this->posts(); - $database->addHook(new QueriesRecorder(static fn (array $queries): array => [...$queries, 'title'])); + $database->addHook(new QueriesRecorder(static fn (Query ...$queries): array => [...$queries, 'title'])); try { $database->find(HookFixture::COLLECTION); @@ -344,11 +344,11 @@ public function testACursorWithoutHooksOverAMissingCollectionThrowsOnlyOnceItera } /** - * @return Closure(array): array + * @return Closure(Query ...): array */ private static function mapping(string $from, string $to): Closure { - return static fn (array $queries): array => \array_map( + return static fn (Query ...$queries): array => \array_map( static fn (Query $query): Query => $query->getMethod()->isJoin() && $query->getAttribute() === $from ? (clone $query)->setAttribute($to) : $query, @@ -365,6 +365,11 @@ private static function ids(array $documents): array return \array_values(\array_map(static fn (Document $document): string => $document->getId(), $documents)); } + private static function integer(mixed $value): ?int + { + return \is_numeric($value) ? (int) $value : null; + } + private function publicThemesJoin(): Query { return Query::join(self::PUBLIC_THEMES, 'th', [Query::on('theme', '$id')]); From 1d7c7dea17e674965a808adc8d9095dc725b2927 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 23:29:05 +1300 Subject: [PATCH 7/9] fix(hook): keep writes and the reads they make out of queries hooks The queries hook is scoped to reads, but writes loaded stored rows, read back written rows and ran their relationship checks through find() and getDocument(), so a hook that filters rows changed what a write saw. An upsert of a row the hook hid was treated as a create and skipped the update authorization; updateDocument(s) and upserts with an operator handed onNext the raw Operator when the refetch came back empty; and a restrict delete passed when the hook hid the related row. Writes now make their own reads under a coroutine-scoped skipQueryHooks() scope: the stored-row loads and refetches of the write methods, the populations of the documents they return, the relationship hook's checks, links and cascades, and the mirror's upgrade-status read. The scope wraps those reads rather than whole writes, so a read the caller's own code makes during a write (onNext, a nested hook, a transaction resolver) still runs the hook. Reads keep calling find() and getDocument() instead of fetch(), so subclass overrides of find() still apply to them as in 8.1.2. The scope travels in State\Snapshot, so concurrent relationship reads started under it keep it. Co-Authored-By: Claude Opus 5.5 --- src/Database/Database.php | 31 +++- src/Database/Hook/Relationships.php | 88 +++++----- src/Database/Mirror.php | 4 +- src/Database/State/Snapshot.php | 2 + src/Database/Trait/Documents.php | 36 ++-- tests/unit/Hook/QueriesWritesTest.php | 240 ++++++++++++++++++++++++++ tests/unit/Mirror/QueriesHookTest.php | 20 +++ 7 files changed, 356 insertions(+), 65 deletions(-) create mode 100644 tests/unit/Hook/QueriesWritesTest.php diff --git a/src/Database/Database.php b/src/Database/Database.php index 68020333e..aac66d79f 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -280,6 +280,9 @@ class Database /** @var Value|null */ private ?Value $grantChecking = null; + /** @var Value|null Whether {@see Hook\Queries} hooks rewrite the queries of reads. */ + private ?Value $rewriting = null; + protected ?Relationships $relationshipHook = null; protected bool $dropUnknownAttributes = false; @@ -1360,6 +1363,22 @@ public function skipValidation(callable $callback): mixed return $this->withValidation(false, $callback); } + /** + * Run the callback with no {@see Hook\Queries} hook rewriting the reads it makes, for the calling coroutine and + * the coroutines it starts. A write makes its own reads under it, so it sees the documents as they are stored. + * + * @internal + * + * @template T + * + * @param callable(): T $callback + * @return T + */ + public function skipQueryHooks(callable $callback): mixed + { + return $this->rewriting()->with(false, $callback); + } + /** * Register a hook into the database pipeline. * @@ -1618,6 +1637,7 @@ public function snapshot(): Snapshot ignoreDuplicates: $this->ignoringDuplicates()->get(), requestTimestamp: $this->requestTimestamp()->get(), checkGrants: $this->grantChecking()->get(), + rewriting: $this->rewriting()->get(), ); } @@ -1658,7 +1678,8 @@ public function withSnapshot(Snapshot $snapshot, callable $callback): mixed */ private function withToggles(Snapshot $snapshot, callable $callback): mixed { - $checked = fn (): mixed => $this->grantChecking()->with($snapshot->checkGrants, $callback); + $rewritten = fn (): mixed => $this->rewriting()->with($snapshot->rewriting, $callback); + $checked = fn (): mixed => $this->grantChecking()->with($snapshot->checkGrants, $rewritten); $timestamped = fn (): mixed => $this->requestTimestamp()->with($snapshot->requestTimestamp, $checked); $deduplicated = fn (): mixed => $this->ignoringDuplicates()->with($snapshot->ignoreDuplicates, $timestamped); $sequenced = fn (): mixed => $this->sequencePreservation()->with($snapshot->preserveSequence, $deduplicated); @@ -1746,6 +1767,14 @@ private function grantChecking(): Value return $this->grantChecking ??= new Value(false); } + /** + * @return Value + */ + private function rewriting(): Value + { + return $this->rewriting ??= new Value(true); + } + protected function isIgnoringDuplicates(): bool { return $this->ignoringDuplicates()->get(); diff --git a/src/Database/Hook/Relationships.php b/src/Database/Hook/Relationships.php index 2d15ccc68..799b372ea 100644 --- a/src/Database/Hook/Relationships.php +++ b/src/Database/Hook/Relationships.php @@ -1082,7 +1082,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen } if (\is_string($value)) { - $related = $this->database->skipRelationships(fn () => $this->database->getDocument($relatedCollection->getId(), $value, [Query::select([Document::ID])])); + $related = $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $value, [Query::select([Document::ID])]))); if ($related->isEmpty()) { $this->refuseUnreadable($relatedCollection, $value, PermissionType::Read); $document->setAttribute($key, null); @@ -1118,7 +1118,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen } $related = $this->database->skipRelationships( - fn () => $this->database->getDocument($relatedCollection->getId(), $value, [Query::select([Document::ID])]) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $value, [Query::select([Document::ID])])) ); if ($related->isEmpty()) { @@ -1141,7 +1141,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen )); } } elseif ($value instanceof Document) { - $related = $this->database->skipRelationships(fn () => $this->database->getDocument($relatedCollection->getId(), $value->getId())); + $related = $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $value->getId()))); /** @var Document|null $oldValueDoc2 */ $oldValueDoc2 = $oldValue instanceof Document ? $oldValue : null; @@ -1175,7 +1175,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen $oldValueDocNull = $oldValue instanceof Document ? $oldValue : null; if ($oldValueDocNull?->getId() !== null) { $oldRelated = $this->database->skipRelationships( - fn () => $this->database->getDocument($relatedCollection->getId(), $oldValueDocNull->getId()) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $oldValueDocNull->getId())) ); $this->database->skipRelationships(fn () => $this->database->updateDocument( $relatedCollection->getId(), @@ -1242,12 +1242,12 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen $unlinkedIds = []; foreach (\array_chunk($stringRelations, $this->relationQueryChunkSize()) as $chunk) { $unlinked = $this->database->skipRelationships( - fn () => $this->database->find($relatedCollection->getId(), [ + fn () => $this->database->skipQueryHooks(fn () => $this->database->find($relatedCollection->getId(), [ Query::select([Document::ID]), Query::equal(Document::ID, $chunk), $this->notReferencing($twoWayKey, $document->getId()), Query::limit(\count($chunk)), - ]) + ])) ); foreach ($unlinked as $related) { $unlinkedIds[] = $related->getId(); @@ -1273,7 +1273,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen } $related = $this->database->skipRelationships( - fn () => $this->database->getDocument($relatedCollection->getId(), $relation->getId(), [Query::select([Document::ID])]) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $relation->getId(), [Query::select([Document::ID])])) ); if ($related->isEmpty()) { @@ -1305,7 +1305,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen } $related = $this->database->skipRelationships( - fn () => $this->database->getDocument($relatedCollection->getId(), $value, [Query::select([Document::ID])]) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $value, [Query::select([Document::ID])])) ); if ($related->isEmpty()) { @@ -1321,7 +1321,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen } $related = $this->database->skipRelationships( - fn () => $this->database->getDocument($relatedCollection->getId(), $value->getId(), [Query::select([Document::ID])]) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $value->getId(), [Query::select([Document::ID])])) ); if ($related->isEmpty()) { @@ -1390,12 +1390,12 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen // validator's maxQueryValues ceiling. $junctionIds = []; foreach (\array_chunk($removedDocuments, $this->relationQueryChunkSize()) as $chunk) { - $junctions = $this->database->find($junction, [ + $junctions = $this->database->skipQueryHooks(fn () => $this->database->find($junction, [ Query::select([Document::ID]), Query::equal($key, $chunk), Query::equal($twoWayKey, [$document->getId()]), Query::limit(PHP_INT_MAX), - ]); + ])); foreach ($junctions as $junctionDoc) { $junctionIds[] = $junctionDoc->getId(); } @@ -1438,7 +1438,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen continue; } - $related = $this->database->getDocument($relatedCollection->getId(), $relation, [Query::select([Document::ID])]); + $related = $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $relation, [Query::select([Document::ID])])); if ($related->isEmpty()) { $this->refuseUnreadable($relatedCollection, $relation, PermissionType::Read); @@ -1448,7 +1448,7 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen $this->authorizeLink($relatedCollection, $related); } elseif ($relation instanceof Document) { - $related = $this->database->getDocument($relatedCollection->getId(), $relation->getId(), [Query::select([Document::ID])]); + $related = $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $relation->getId(), [Query::select([Document::ID])])); if (! $related->isEmpty() && ! \in_array($relation->getId(), $oldIds)) { $this->authorizeLink($relatedCollection, $related); @@ -1650,11 +1650,11 @@ private function withoutRemoved(array $documents): array $ids = \array_values(\array_map(fn (Document $document): string => $document->getId(), $byId)); foreach (\array_chunk($ids, $this->relationQueryChunkSize()) as $chunk) { - $found = $this->database->getAuthorization()->skip(fn () => $this->database->find($collectionId, [ + $found = $this->database->getAuthorization()->skip(fn () => $this->database->skipQueryHooks(fn () => $this->database->find($collectionId, [ Query::equal(Document::ID, $chunk), Query::select([Document::ID]), Query::limit(\count($chunk)), - ])); + ]))); foreach ($found as $existing) { $remaining[$collectionId][$existing->getId()] = $byId[$existing->getId()]; @@ -2097,7 +2097,7 @@ private function relateDocuments( return $this->prepareRelated($prepared, $collection, $relatedCollection, $key, $document, $relation, $relationType, $twoWayKey, $side, $coroutine); } - $related = $this->database->getDocument($relatedCollection->getId(), $relation->getId()); + $related = $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $relation->getId())); if ($related->isEmpty()) { $this->refuseUnreadable($relatedCollection, $relation->getId(), PermissionType::Update); @@ -2201,7 +2201,7 @@ private function relateDocumentsById( $this->writePrepared($prepared); } - $related = $this->database->skipRelationships(fn () => $this->database->getDocument($relatedCollection->getId(), $relationId)); + $related = $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($relatedCollection->getId(), $relationId))); if ($related->isEmpty()) { $this->refuseUnreadable($relatedCollection, $relationId, PermissionType::Read); @@ -2267,11 +2267,11 @@ private function getJunctionCollection(Document $collection, Document $relatedCo private function isLinkedElsewhere(Document $collection, string $key, string $relatedId, Document $document): bool { - return ! $this->database->getAuthorization()->skip(fn () => $this->database->skipRelationships(fn () => $this->database->findOne($collection->getId(), [ + return ! $this->database->getAuthorization()->skip(fn () => $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->findOne($collection->getId(), [ Query::select([Document::ID]), Query::equal($key, [$relatedId]), Query::notEqual(Document::ID, $document->getId()), - ])))->isEmpty(); + ]))))->isEmpty(); } /** @@ -2722,10 +2722,10 @@ private function deleteRestrict( && ! $twoWay ) { $this->database->getAuthorization()->skip(function () use ($document, $relatedCollection, $twoWayKey) { - $related = $this->database->findOne($relatedCollection->getId(), [ + $related = $this->database->skipQueryHooks(fn () => $this->database->findOne($relatedCollection->getId(), [ Query::select([Document::ID]), Query::equal($twoWayKey, [$document->getId()]), - ]); + ])); if ($related->isEmpty()) { return; @@ -2745,10 +2745,10 @@ private function deleteRestrict( $relationType === RelationshipType::ManyToOne && $side === RelationshipSide::Child ) { - $related = $this->database->getAuthorization()->skip(fn () => $this->database->findOne($relatedCollection->getId(), [ + $related = $this->database->getAuthorization()->skip(fn () => $this->database->skipQueryHooks(fn () => $this->database->findOne($relatedCollection->getId(), [ Query::select([Document::ID]), Query::equal($twoWayKey, [$document->getId()]), - ])); + ]))); if (! $related->isEmpty()) { throw new RestrictedException('Cannot delete document because it has at least one related document.'); @@ -2761,19 +2761,19 @@ private function hasRelatedDocument(Document $collection, Document $relatedColle $authorization = $this->database->getAuthorization(); if ($relationType === RelationshipType::OneToMany) { - return ! $authorization->skip(fn () => $this->database->findOne($relatedCollection->getId(), [ + return ! $authorization->skip(fn () => $this->database->skipQueryHooks(fn () => $this->database->findOne($relatedCollection->getId(), [ Query::select([Document::ID]), Query::equal($twoWayKey, [$document->getId()]), - ]))->isEmpty(); + ])))->isEmpty(); } $relatedIds = $this->findRelatedIds($collection, $relatedCollection, $document, $key, $relationType, $twoWay, $twoWayKey, $side); foreach (\array_chunk($relatedIds, $this->relationQueryChunkSize()) as $chunk) { - $related = $authorization->skip(fn () => $this->database->findOne($relatedCollection->getId(), [ + $related = $authorization->skip(fn () => $this->database->skipQueryHooks(fn () => $this->database->findOne($relatedCollection->getId(), [ Query::select([Document::ID]), Query::equal(Document::ID, $chunk), - ])); + ]))); if (! $related->isEmpty()) { return true; @@ -2816,7 +2816,7 @@ private function findRelatedIds(Document $collection, Document $relatedCollectio private function findStoredRelatedIds(Document $collection, Document $document, string $key): array { $stored = $this->database->getAuthorization()->skip(fn () => $this->database->skipRelationships( - fn () => $this->database->getDocument($collection->getId(), $document->getId(), forUpdate: true) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($collection->getId(), $document->getId(), forUpdate: true)) )); $relatedId = $stored->getAttribute($key); @@ -2839,14 +2839,14 @@ private function findReferencingIds(Document $relatedCollection, Document $docum */ private function findJunctionRelatedIds(Document $collection, Document $relatedCollection, Document $document, string $key, string $twoWayKey, RelationshipSide $side): array { - $junctions = $this->database->getAuthorization()->skip(fn () => $this->database->skipRelationships(fn () => $this->database->find( + $junctions = $this->database->getAuthorization()->skip(fn () => $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->find( $this->getJunctionCollection($collection, $relatedCollection, $side), [ Query::select([$key]), Query::equal($twoWayKey, [$document->getId()]), Query::limit(PHP_INT_MAX), ], - ))); + )))); $relatedIds = []; foreach ($junctions as $junction) { @@ -2875,11 +2875,11 @@ private function findJunctionRelatedIds(Document $collection, Document $relatedC */ private function findReferencingDocuments(Document $relatedCollection, Document $document, string $twoWayKey): array { - return $this->database->getAuthorization()->skip(fn () => $this->database->find($relatedCollection->getId(), [ + return $this->database->getAuthorization()->skip(fn () => $this->database->skipQueryHooks(fn () => $this->database->find($relatedCollection->getId(), [ Query::select([Document::ID]), Query::equal($twoWayKey, [$document->getId()]), Query::limit(PHP_INT_MAX), - ])); + ]))); } /** @@ -2926,10 +2926,10 @@ private function deleteSetNull(Document $collection, Document $relatedCollection } $written = $this->database->getAuthorization()->skip(function () use ($document, $relatedCollection, $twoWayKey): ?Document { - $related = $this->database->findOne($relatedCollection->getId(), [ + $related = $this->database->skipQueryHooks(fn () => $this->database->findOne($relatedCollection->getId(), [ Query::select([Document::ID]), Query::equal($twoWayKey, [$document->getId()]), - ]); + ])); if ($related->isEmpty()) { return null; @@ -2963,11 +2963,11 @@ private function deleteSetNull(Document $collection, Document $relatedCollection case RelationshipType::ManyToMany: $junction = $this->getJunctionCollection($collection, $relatedCollection, $side); - $junctions = $this->database->find($junction, [ + $junctions = $this->database->skipQueryHooks(fn () => $this->database->find($junction, [ Query::select([Document::ID]), Query::equal($twoWayKey, [$document->getId()]), Query::limit(PHP_INT_MAX), - ]); + ])); $junctionIds = \array_map(fn (Document $junctionDocument) => $junctionDocument->getId(), $junctions); $this->database->skipRelationships(fn () => $this->deleteRelatedDocuments($junction, $junctionIds)); @@ -2993,11 +2993,11 @@ private function deleteCascade(Document $collection, Document $relatedCollection case RelationshipType::ManyToMany: $junction = $this->getJunctionCollection($collection, $relatedCollection, $side); - $junctions = $this->database->skipRelationships(fn () => $this->database->find($junction, [ + $junctions = $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->find($junction, [ Query::select([Document::ID, $key]), Query::equal($twoWayKey, [$document->getId()]), Query::limit(PHP_INT_MAX), - ])); + ]))); $junctionIds = []; $relatedIds = []; @@ -3106,12 +3106,12 @@ private function linkRelatedDocuments(Document $collection, string $twoWayKey, s continue; } - $unlinked = $this->database->getAuthorization()->skip(fn () => $this->database->skipRelationships(fn () => $this->database->find($collection->getId(), [ + $unlinked = $this->database->getAuthorization()->skip(fn () => $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->find($collection->getId(), [ Query::select([Document::ID]), Query::equal(Document::ID, $chunk), $this->notReferencing($twoWayKey, $documentId), Query::limit(\count($chunk)), - ]))); + ])))); foreach ($unlinked as $related) { $this->linkRelatedDocument($collection, $related->getId(), $twoWayKey, $documentId); @@ -3130,7 +3130,7 @@ private function linkRelatedDocument(Document $collection, string $id, string $t $authorization = $this->database->getAuthorization(); $related = $authorization->skip(fn () => $this->database->skipRelationships( - fn () => $this->database->getDocument($collection->getId(), $id, forUpdate: true) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($collection->getId(), $id, forUpdate: true)) )); if ($related->isEmpty() || $related->getAttribute($twoWayKey) === $documentId) { @@ -3304,7 +3304,7 @@ private function grantedView(Document $collection, string $id, Preparation $prep $staged = $preparation->resolve === null ? null : ($preparation->resolve)($collection->getId(), $id); return $staged ?? $this->database->getAuthorization()->skip(fn (): Document => $this->database->skipRelationships( - fn (): Document => $this->database->getDocument($collection->getId(), $id, [Query::select([Document::PERMISSIONS])], $preparation->lock) + fn (): Document => $this->database->skipQueryHooks(fn () => $this->database->getDocument($collection->getId(), $id, [Query::select([Document::PERMISSIONS])], $preparation->lock)) )); } @@ -3461,7 +3461,7 @@ private function viewRelated(Document $collection, string $id, PermissionType $c { $staged = $resolve === null ? null : $resolve($collection->getId(), $id); if ($staged === null) { - $stored = $this->database->skipRelationships(fn () => $this->database->getDocument($collection->getId(), $id, $queries)); + $stored = $this->database->skipRelationships(fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($collection->getId(), $id, $queries))); if ($stored->isEmpty()) { $this->refuseUnreadable($collection, $id, $check); } @@ -3543,7 +3543,7 @@ private function refuseUnreadable(Document $collection, string $id, PermissionTy } $stored = $authorization->skip(fn () => $this->database->skipRelationships( - fn () => $this->database->getDocument($collection->getId(), $id, [Query::select([Document::ID])]) + fn () => $this->database->skipQueryHooks(fn () => $this->database->getDocument($collection->getId(), $id, [Query::select([Document::ID])])) )); if ($stored->isEmpty()) { return; diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index ff8f4e257..5a180b90a 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -1636,7 +1636,7 @@ protected function getUpgradeStatus(string $collection): ?Document return $this->getSource()->getAuthorization()->skip(function () use ($collection) { try { - return $this->source->getDocument('upgrades', $collection); + return $this->source->skipQueryHooks(fn (): Document => $this->source->getDocument('upgrades', $collection)); } catch (Throwable) { return; } @@ -1839,7 +1839,7 @@ public function setAuthorization(Authorization $authorization): static * the destination replicates the related documents the source already prepared, so preparing them again would * give it IDs the source does not have. A queries hook runs once on each read of the mirror, which serves its * reads, and never on the destination. One that is also a lifecycle hook is registered on the source too, as every - * lifecycle hook is, so it also rewrites the reads made on the source directly. + * lifecycle hook is, so it also rewrites the reads made on the source directly, though never those a write makes. */ #[\Override] public function addHook(\Utopia\Query\Hook $hook): static diff --git a/src/Database/State/Snapshot.php b/src/Database/State/Snapshot.php index d477ee2ed..818fd3eb3 100644 --- a/src/Database/State/Snapshot.php +++ b/src/Database/State/Snapshot.php @@ -28,6 +28,7 @@ * @param bool $ignoreDuplicates Whether creating a document that exists is skipped instead of failing * @param DateTime|null $requestTimestamp The time an update conflicts after * @param bool $checkGrants Whether a related document may only be given permissions for roles the caller holds + * @param bool $rewriting Whether {@see \Utopia\Database\Hook\Queries} hooks rewrite the queries of reads */ public function __construct( public bool $authorization, @@ -46,6 +47,7 @@ public function __construct( public bool $ignoreDuplicates, public ?DateTime $requestTimestamp, public bool $checkGrants = false, + public bool $rewriting = true, ) { } } diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index cc06b9b02..93da4a57e 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -468,13 +468,13 @@ protected function refetchDocuments(Document $collection, array $documents, arra $refetchedMap = []; foreach (\array_chunk($sequences, \max(1, $this->maxQueryValues)) as $chunk) { $refetched = $this->getAuthorization()->skip(fn () => $this->silent( - fn () => $this->find( + fn () => $this->skipQueryHooks(fn () => $this->find( $collection->getId(), \array_merge([ Query::equal(Document::SEQUENCE, $chunk), Query::limit(\count($chunk)), ], $selections) - ) + )) )); foreach ($refetched as $document) { @@ -1247,7 +1247,7 @@ public function createDocument(string $collection, Document $document): Document $hook = $this->relationshipHook; if ($hook !== null && ! $hook->isInBatchPopulation() && $hook->isEnabled()) { $fetchDepth = $hook->getWriteStackCount(); - $documents = $this->silent(fn () => $hook->populateDocuments([$document], $collection, $fetchDepth)); + $documents = $this->silent(fn () => $this->skipQueryHooks(fn () => $hook->populateDocuments([$document], $collection, $fetchDepth))); $document = $documents[0]; } @@ -1609,7 +1609,7 @@ function () use ($collection, $chunk): array { $hook = $this->relationshipHook; if ($hook !== null && ! $hook->isInBatchPopulation() && $hook->isEnabled()) { - $batch = $this->silent(fn () => $hook->populateDocuments($batch, $collection, $hook->getFetchDepth())); + $batch = $this->silent(fn () => $this->skipQueryHooks(fn () => $hook->populateDocuments($batch, $collection, $hook->getFetchDepth()))); } /** @var array $batch */ @@ -1664,7 +1664,7 @@ public function updateDocument(string $collection, string $id, Document $documen : $collection->getId(); $document = $this->withMutation(Event::DocumentUpdate, $cacheTarget, function () use ($collection, $id, $document, $preparation, $newUpdatedAt, &$hasOperators) { $old = $this->authorization->skip(fn () => $this->silent( - fn () => $this->readDocument($collection, $id, forUpdate: true) + fn () => $this->skipQueryHooks(fn () => $this->readDocument($collection, $id, forUpdate: true)) )); if ($old->isEmpty()) { return new Document(); @@ -1922,7 +1922,7 @@ public function updateDocument(string $collection, string $id, Document $documen $hook = $this->relationshipHook; if ($hook !== null && ! $hook->isInBatchPopulation() && $hook->isEnabled()) { - $documents = $this->silent(fn () => $hook->populateDocuments([$document], $collection, $hook->getFetchDepth())); + $documents = $this->silent(fn () => $this->skipQueryHooks(fn () => $hook->populateDocuments([$document], $collection, $hook->getFetchDepth()))); $document = $documents[0]; } @@ -2089,12 +2089,12 @@ public function updateDocuments( $new[] = Query::cursorAfter($last); } - $batch = $this->silent(fn () => $this->fetch( + $batch = $this->silent(fn () => $this->skipQueryHooks(fn () => $this->fetch( $collection, array_merge($new, $queries), PermissionType::Update, false, - )); + ))); if (empty($batch)) { break; @@ -2243,7 +2243,7 @@ function (Document $upserted, ?Document $previous) use (&$result, &$created): vo ); if ($result === null) { - return $this->getDocument($collection, $document->getId()); + return $this->skipQueryHooks(fn (): Document => $this->getDocument($collection, $document->getId())); } $listeners = $this->listens(Event::DocumentUpsert); @@ -2590,7 +2590,7 @@ function () use ($collection, $increase, $chunk): array { $hook = $this->relationshipHook; if ($hook !== null && ! $hook->isInBatchPopulation() && $hook->isEnabled()) { - $batch = $this->silent(fn () => $hook->populateDocuments($batch, $collection, $hook->getFetchDepth())); + $batch = $this->silent(fn () => $this->skipQueryHooks(fn () => $hook->populateDocuments($batch, $collection, $hook->getFetchDepth()))); } if ($hasOperators && $onNext !== null) { @@ -2658,10 +2658,10 @@ private function findDocumentsToUpsert(string $collection, array $documents): ar $existing = []; foreach ($batches as $batch) { foreach (\array_chunk(\array_values(\array_unique($batch['ids'])), \max(1, $this->maxQueryValues)) as $chunk) { - $read = fn (): array => $this->authorization->skip(fn () => $this->silent(fn () => $this->find($collection, [ + $read = fn (): array => $this->authorization->skip(fn () => $this->silent(fn () => $this->skipQueryHooks(fn () => $this->find($collection, [ Query::equal(Document::ID, $chunk), Query::limit($this->maxQueryValues), - ], forPermission: PermissionType::Update))); + ], forPermission: PermissionType::Update)))); $found = $perTenant ? $this->withTenant($batch['tenant'], $read) @@ -2731,7 +2731,7 @@ public function increaseDocumentAttribute( : $collection->getId(); $document = $this->withMutation(Event::DocumentIncrease, $cacheTarget, function () use ($collection, $id, $attribute, $value, $max, $numericAttribute) { /** @var Document $document */ - $document = $this->authorization->skip(fn () => $this->silent(fn () => $this->readDocument($collection, $id, forUpdate: true))); // Skip ensures user does not need read permission for this + $document = $this->authorization->skip(fn () => $this->silent(fn () => $this->skipQueryHooks(fn () => $this->readDocument($collection, $id, forUpdate: true)))); // Skip ensures user does not need read permission for this if ($document->isEmpty()) { throw new NotFoundException('Document not found'); @@ -2862,7 +2862,7 @@ public function decreaseDocumentAttribute( : $collection->getId(); $document = $this->withMutation(Event::DocumentDecrease, $cacheTarget, function () use ($collection, $id, $attribute, $value, $min, $numericAttribute) { /** @var Document $document */ - $document = $this->authorization->skip(fn () => $this->silent(fn () => $this->readDocument($collection, $id, forUpdate: true))); // Skip ensures user does not need read permission for this + $document = $this->authorization->skip(fn () => $this->silent(fn () => $this->skipQueryHooks(fn () => $this->readDocument($collection, $id, forUpdate: true)))); // Skip ensures user does not need read permission for this if ($document->isEmpty()) { throw new NotFoundException('Document not found'); @@ -2967,7 +2967,7 @@ public function deleteDocument(string $collection, string $id): bool $deleted = $this->withMutation(Event::DocumentDelete, $cacheTarget, function () use ($collection, $id, $report, &$changed): ?Document { $changed = []; $document = $this->authorization->skip(fn () => $this->silent( - fn () => $this->readDocument($collection, $id, forUpdate: true) + fn () => $this->skipQueryHooks(fn () => $this->readDocument($collection, $id, forUpdate: true)) )); if ($document->isEmpty()) { @@ -3135,12 +3135,12 @@ public function deleteDocuments( /** * @var array $batch */ - $batch = $this->silent(fn () => $this->fetch( + $batch = $this->silent(fn () => $this->skipQueryHooks(fn () => $this->fetch( $collection, array_merge($new, $queries), PermissionType::Delete, false, - )); + ))); if (empty($batch)) { break; @@ -5566,7 +5566,7 @@ private function checkQueryTypes(array $queries): void */ private function rewriteQueries(Event $event, Collection $collection, array $queries): array { - if ($this->queryHooks === [] || $collection->getId() === self::METADATA) { + if ($this->queryHooks === [] || $collection->getId() === self::METADATA || ! $this->rewriting()->get()) { return $queries; } diff --git a/tests/unit/Hook/QueriesWritesTest.php b/tests/unit/Hook/QueriesWritesTest.php new file mode 100644 index 000000000..a5751bcd0 --- /dev/null +++ b/tests/unit/Hook/QueriesWritesTest.php @@ -0,0 +1,240 @@ +addRole(Role::any()->toString()); + $database = new Database(new SQLite(new PDO('sqlite::memory:')), new Cache(new None())); + $database->setAuthorization($authorization)->setDatabase('queries_writes')->setNamespace('queries_writes_'.\uniqid()); + $database->addHook(new Permissions()); + $database->create(); + $database->createCollection(Collection::create( + id: 'notes', + attributes: [Attribute::string(key: 'title', size: 64), Attribute::string(key: 'status', size: 16)], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + documentSecurity: true, + )); + $authorization->skip(static fn (): Document => $database->createDocument('notes', new Document([ + '$id' => 'n1', + 'title' => 'owner text', + 'status' => 'archived', + '$permissions' => [Permission::update(Role::user('owner'))], + ]))); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::notEqual('status', ['archived']))); + $database->addHook($recorder); + + try { + $database->upsertDocument('notes', new Document(['$id' => 'n1', 'title' => 'overwritten', 'status' => 'open'])); + $this->fail('An upsert overwrote a row its caller may not update'); + } catch (AuthorizationException) { + } + + $this->assertSame([], $recorder->calls); + $database->removeHook($recorder); + $this->assertSame('owner text', $authorization->skip(static fn (): Document => $database->getDocument('notes', 'n1'))->getAttribute('title')); + } + + public function testUpdateDocumentsGivesOnNextTheValueAnOperatorStoredWhileTheHookHidesTheRow(): void + { + [$database, $recorder] = $this->hidingPosts(); + $views = []; + + $modified = $database->updateDocuments( + HookFixture::COLLECTION, + new Document(['views' => Operator::increment(5)]), + [Query::equal('title', ['first'])], + onNext: static function (Document $document) use (&$views): void { + $views[] = $document->getAttribute('views'); + }, + ); + + $this->assertSame(1, $modified); + $this->assertSame([6], $views); + $this->assertSame([], $recorder->calls); + } + + public function testUpdateDocumentReturnsTheValueAnOperatorStoredWhileTheHookHidesTheRow(): void + { + [$database, $recorder] = $this->hidingPosts(); + + $updated = $database->updateDocument(HookFixture::COLLECTION, 'first', new Document(['views' => Operator::increment(5)])); + + $this->assertSame(6, $updated->getAttribute('views')); + $this->assertSame([], $recorder->calls); + } + + public function testACallerReadInsideAWriteCallbackStillRunsTheHook(): void + { + $database = HookFixture::memory(); + HookFixture::seed($database, ['first', 'second']); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::equal('title', ['second']))); + $database->addHook($recorder); + $counts = []; + + $database->updateDocuments( + HookFixture::COLLECTION, + new Document(['views' => 9]), + [Query::equal('title', ['first'])], + onNext: static function () use ($database, &$counts): void { + $counts[] = $database->count(HookFixture::COLLECTION); + }, + ); + + $this->assertSame([1], $counts); + $this->assertSame([[Event::DocumentCount, HookFixture::COLLECTION]], $recorder->calls); + } + + public function testARestrictedDeleteIsRefusedWhileTheHookHidesTheRelatedRow(): void + { + $database = $this->albums(RelationshipDeleteAction::Restrict); + $database->updateDocument('artists', 'a1', new Document(['albums' => ['b1']])); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::notEqual('status', ['archived']))); + $database->addHook($recorder); + + try { + $database->deleteDocument('artists', 'a1'); + $this->fail('An artist with a related album was deleted'); + } catch (RestrictedException $error) { + $this->assertSame('Cannot delete document because it has at least one related document.', $error->getMessage()); + } + + $this->assertSame([], $recorder->calls); + $database->removeHook($recorder); + $this->assertFalse($database->getDocument('artists', 'a1')->isEmpty()); + } + + public function testAnUpdateUnlinksARelatedRowTheHookHides(): void + { + $database = $this->albums(RelationshipDeleteAction::SetNull); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::notEqual('status', ['archived']))); + $database->addHook($recorder); + + $updated = $database->updateDocument('artists', 'a1', new Document(['albums' => ['b2']])); + + $this->assertSame(['b2'], $this->ids($updated->getAttribute('albums'))); + $this->assertSame([], $recorder->calls); + $database->removeHook($recorder); + $this->assertNull($database->getDocument('albums', 'b1')->getAttribute('artist')); + } + + public function testWritesOfRelatedDocumentsDoNotRunTheHook(): void + { + $database = $this->albums(RelationshipDeleteAction::Cascade); + $recorder = new QueriesRecorder(); + $database->addHook($recorder); + + $created = $database->createDocument('artists', new Document([ + '$id' => 'a2', + 'status' => 'open', + 'albums' => [new Document(['$id' => 'b3', 'status' => 'open'])], + ])); + $database->updateDocument('albums', 'b3', new Document(['status' => 'archived'])); + $deleted = $database->deleteDocument('artists', 'a2'); + + $this->assertSame(['b3'], $this->ids($created->getAttribute('albums'))); + $this->assertTrue($deleted); + $this->assertSame([], $recorder->calls); + } + + public function testASnapshotTakenWhileQueryHooksAreSkippedSkipsThem(): void + { + $database = HookFixture::memory(); + HookFixture::seed($database, ['first', 'second']); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::equal('title', ['first']))); + $database->addHook($recorder); + + $skipped = $database->skipQueryHooks(static fn () => $database->snapshot()); + + $this->assertSame(2, $database->withSnapshot($skipped, static fn (): int => $database->count(HookFixture::COLLECTION))); + $this->assertSame([], $recorder->calls); + $this->assertSame(1, $database->withSnapshot($database->snapshot(), static fn (): int => $database->count(HookFixture::COLLECTION))); + $this->assertSame([[Event::DocumentCount, HookFixture::COLLECTION]], $recorder->calls); + } + + /** + * @return array{Database, QueriesRecorder} + */ + private function hidingPosts(): array + { + $database = HookFixture::sqlite(); + HookFixture::seed($database, ['first', 'second']); + $recorder = new QueriesRecorder(QueriesRecorder::adding(Query::equal('title', ['nothing']))); + $database->addHook($recorder); + + return [$database, $recorder]; + } + + /** + * Artist a1 with album b1, which is archived, and album b2. + */ + private function albums(RelationshipDeleteAction $onDelete): Database + { + $authorization = new Authorization(); + $authorization->addRole(Role::any()->toString()); + $database = new Database(new Memory(), new Cache(new None())); + $database->setAuthorization($authorization)->setDatabase('queries_writes')->setNamespace('queries_writes_'.\uniqid()); + $database->create(); + $database->addHook(new Relationships()); + $database->addHook(new Permissions()); + + $permissions = [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ]; + foreach (['artists', 'albums'] as $collection) { + $database->createCollection(Collection::create( + id: $collection, + attributes: [Attribute::string(key: 'status', size: 16)], + permissions: $permissions, + documentSecurity: false, + )); + } + $database->createRelationship('artists', Relationship::oneToMany('albums', key: 'albums', twoWay: true, twoWayKey: 'artist', onDelete: $onDelete)); + $database->createDocument('albums', new Document(['$id' => 'b1', 'status' => 'archived'])); + $database->createDocument('albums', new Document(['$id' => 'b2', 'status' => 'open'])); + $database->createDocument('artists', new Document(['$id' => 'a1', 'status' => 'open', 'albums' => ['b1', 'b2']])); + + return $database; + } + + /** + * @return list + */ + private function ids(mixed $documents): array + { + $this->assertIsArray($documents); + $this->assertContainsOnlyInstancesOf(Document::class, $documents); + + return \array_values(\array_map(static fn (Document $document): string => $document->getId(), $documents)); + } +} diff --git a/tests/unit/Mirror/QueriesHookTest.php b/tests/unit/Mirror/QueriesHookTest.php index e2e651ab2..bccfc21f3 100644 --- a/tests/unit/Mirror/QueriesHookTest.php +++ b/tests/unit/Mirror/QueriesHookTest.php @@ -54,6 +54,26 @@ public function testNeitherSideRewritesItsReadsWithTheMirrorsHook(): void $this->assertSame([], $hook->calls); } + public function testTheMirrorReplicatesAWriteWhileALifecycleQueriesHookRewritesTheSourcesReads(): void + { + $hook = self::hook(true); + $source = HookFixture::memory(); + $destination = HookFixture::memory(); + $mirror = new Mirror($source, $destination); + $mirror->createUpgrades(); + $source->getAuthorization()->skip(static fn (): Document => $source->createDocument('upgrades', new Document([ + '$id' => HookFixture::COLLECTION, + 'collectionId' => HookFixture::COLLECTION, + 'status' => 'upgraded', + ]))); + + $mirror->addHook($hook); + $mirror->createDocument(HookFixture::COLLECTION, new Document(['$id' => 'third', 'title' => 'third', 'views' => 3])); + + $this->assertSame([], $hook->calls); + $this->assertSame('third', $destination->getDocument(HookFixture::COLLECTION, 'third')->getAttribute('title')); + } + private static function hook(bool $lifecycle): QueriesRecorder { $adding = QueriesRecorder::adding(Query::equal('title', ['first'])); From a41451f9aeb5e5461b93f3d5f612517a2cfdaefa Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 23:29:05 +1300 Subject: [PATCH 8/9] test(hook): pin the queries hook's type check, cache key, paired row and mirror removal These behaviours were documented but untested: a query that is not a Query is refused before any hook sees it; a select a hook adds to getDocument() neither reads nor fills the cached full document; resolveCursor()'s paired-row read of a joined collection runs the hook; and a lifecycle queries hook rewrites the mirror source's reads until the mirror removes it. Co-Authored-By: Claude Opus 5.5 --- tests/unit/Event/HookFixture.php | 4 +-- tests/unit/Hook/QueriesTest.php | 50 +++++++++++++++++++++++++++ tests/unit/Mirror/QueriesHookTest.php | 19 ++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/tests/unit/Event/HookFixture.php b/tests/unit/Event/HookFixture.php index 64aef088b..815a4e658 100644 --- a/tests/unit/Event/HookFixture.php +++ b/tests/unit/Event/HookFixture.php @@ -44,9 +44,9 @@ public static function seed(Database $database, array $ids): void } } - public static function database(Adapter $adapter): Database + public static function database(Adapter $adapter, Cache $cache = new Cache(new None())): Database { - $database = new Database($adapter, new Cache(new None())); + $database = new Database($adapter, $cache); $database ->setAuthorization(new Authorization()) ->setDatabase('hooks') diff --git a/tests/unit/Hook/QueriesTest.php b/tests/unit/Hook/QueriesTest.php index 5b0a0a2d9..4d215d634 100644 --- a/tests/unit/Hook/QueriesTest.php +++ b/tests/unit/Hook/QueriesTest.php @@ -8,8 +8,10 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Tests\Unit\Event\HookFixture; +use Utopia\Cache\Adapter\Memory as MemoryCache; use Utopia\Cache\Adapter\None; use Utopia\Cache\Cache; +use Utopia\Database\Adapter\Memory; use Utopia\Database\Adapter\SQLite; use Utopia\Database\Attribute; use Utopia\Database\Collection; @@ -299,6 +301,54 @@ public function testAHookReturningSomethingOtherThanAQueryIsRefused(): void } } + public function testAQueryThatIsNotAQueryIsRefusedBeforeTheHookIsGivenIt(): void + { + $database = $this->posts(); + $recorder = new QueriesRecorder(); + $database->addHook($recorder); + + try { + /** @phpstan-ignore argument.type */ + $database->find(HookFixture::COLLECTION, ['title']); + $this->fail('A query that is not a Query was read'); + } catch (QueryException $error) { + $this->assertSame('Invalid query type: "string". Expected instances of "'.Query::class.'"', $error->getMessage()); + } + + $this->assertSame([], $recorder->calls); + } + + public function testResolveCursorRunsTheHookOnThePairedRowItReadsOverAJoin(): void + { + $database = $this->joined(); + $recorder = new QueriesRecorder(self::mapping(self::PUBLIC_THEMES, 'themes')); + $database->addHook($recorder); + + $cursor = $database->resolveCursor('tickets', 'k2', CursorDirection::After, [$this->publicThemesJoin(), Query::orderAsc('th.name')]); + + $this->assertSame('second theme', $cursor->getAttribute('th.name')); + $this->assertSame([[Event::DocumentFind, 'tickets'], [Event::DocumentFind, 'themes']], $recorder->calls); + } + + public function testASelectTheHookAddsToGetDocumentNeitherReadsNorFillsTheCachedDocument(): void + { + $cache = new MemoryCache(); + $database = HookFixture::database(new Memory(), new Cache($cache)); + HookFixture::seed($database, ['first']); + $selecting = new QueriesRecorder(QueriesRecorder::adding(Query::select(['title']))); + + $this->assertSame(1, $database->getDocument(HookFixture::COLLECTION, 'first')->getAttribute('views')); + $this->assertNotSame([], $cache->store); + + $database->addHook($selecting); + $selected = $database->getDocument(HookFixture::COLLECTION, 'first'); + $database->removeHook($selecting); + + $this->assertSame('first', $selected->getAttribute('title')); + $this->assertFalse($selected->offsetExists('views')); + $this->assertSame(1, $database->getDocument(HookFixture::COLLECTION, 'first')->getAttribute('views')); + } + public function testTheHookRunsWhileEventsAreSilenced(): void { $database = $this->posts(); diff --git a/tests/unit/Mirror/QueriesHookTest.php b/tests/unit/Mirror/QueriesHookTest.php index bccfc21f3..6bca7b742 100644 --- a/tests/unit/Mirror/QueriesHookTest.php +++ b/tests/unit/Mirror/QueriesHookTest.php @@ -54,6 +54,25 @@ public function testNeitherSideRewritesItsReadsWithTheMirrorsHook(): void $this->assertSame([], $hook->calls); } + public function testTheSourceRewritesItsReadsWithALifecycleQueriesHookUntilTheMirrorRemovesIt(): void + { + $hook = self::hook(true); + $source = HookFixture::memory(); + HookFixture::seed($source, ['first', 'second']); + $mirror = new Mirror($source, HookFixture::memory()); + + $mirror->addHook($hook); + + $this->assertSame(['first'], self::ids($mirror->getSource()->find(HookFixture::COLLECTION))); + $this->assertSame([[Event::DocumentFind, HookFixture::COLLECTION]], $hook->calls); + + $mirror->removeHook($hook); + + $this->assertSame(['first', 'second'], self::ids($mirror->getSource()->find(HookFixture::COLLECTION))); + $this->assertSame(['first', 'second'], self::ids($mirror->find(HookFixture::COLLECTION))); + $this->assertSame([[Event::DocumentFind, HookFixture::COLLECTION]], $hook->calls); + } + public function testTheMirrorReplicatesAWriteWhileALifecycleQueriesHookRewritesTheSourcesReads(): void { $hook = self::hook(true); From 2400b5f8ace5220fb51330c2af762219be41f03e Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 14 Oct 2026 23:29:05 +1300 Subject: [PATCH 9/9] docs: state exactly which reads run the queries hook The contract said the hook ran before the read checked anything, but the queries are type-checked before it, and it said writes were not covered without saying their own reads were not either. Name the reads that run it, including the population and relationship-query reads a read makes, list the writes, and say that the reads a write makes itself never run it while a read from the caller's own code during a write does. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 37 ++++++++++++++++++++++------------- README.md | 2 +- UPGRADE.md | 11 ++++++----- src/Database/Hook/Queries.php | 28 +++++++++++++++----------- 4 files changed, 47 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 021a783c9..6820d0021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,20 +9,29 @@ - `Hook\Queries::rewrite(Event $event, Collection $collection, array $queries): array` rewrites the queries of a read, registered with `addHook()` and removed with `removeHook()`. It is given the queries one read was given and - the definition of the collection being read, before the read checks, validates, parses, resolves joins or - authorizes anything, and the queries it returns replace the read's queries for the rest of the read, the query-cache - key included. It runs for `getDocument()` (`Event::DocumentRead`), `find()` and so `findOne()`, `cursor()` and - `resolveCursor()` (`Event::DocumentFind`), `count()` (`Event::DocumentCount`), `sum()` (`Event::DocumentSum`) and - `aggregate()` (`Event::DocumentAggregate`). Hooks run in registration order, each given the previous one's output. - Throwing refuses the read, with the exception reaching the caller unchanged before anything is read. It runs on the - reads the library makes itself, such as relationship population and each `cursor()` page, so it must return the same - queries when given its own output; it runs whether or not events are silenced. It never runs for reads of - `Database::METADATA`, for writes (`updateDocuments()` and `deleteDocuments()` included) or for the `query()`, - `mutate()` and `from()` builders. With such a hook registered, `cursor()` resolves the collection and rewrites its - queries when it is called, so a missing collection or a refusal throws then instead of when the generator is - iterated. A `Mirror` runs it once on each of its own reads and never on its destination's. One that is also a - `Hook\Lifecycle` is registered on the source too, as every lifecycle hook is, so it also rewrites the reads made on - the source directly. + the definition of the collection being read, once each query is checked to be a `Query` and before the read checks + its queries any further: before it validates, parses or authorizes them or resolves their joins. The queries it + returns replace the read's queries for the rest of the read, the query-cache key included. It runs for + `getDocument()` (`Event::DocumentRead`), `find()` and so `findOne()`, `cursor()` and `resolveCursor()` + (`Event::DocumentFind`), `count()` (`Event::DocumentCount`), `sum()` (`Event::DocumentSum`) and `aggregate()` + (`Event::DocumentAggregate`). Hooks run in registration order, each given the previous one's output. Throwing + refuses the read, with the exception reaching the caller unchanged before anything is read. It also runs on the + reads such a read makes itself: relationship population, the reads that resolve queries on relationship attributes, + `findOne()` through `find()`, each `cursor()` page and `resolveCursor()`'s read of the joined row it pairs with the + document. So it must return the same queries when given its own output; it runs whether or not events are silenced. + It never runs for reads of `Database::METADATA`, for the `query()`, `mutate()` and `from()` builders, or for writes + and the reads they make themselves. The writes are `createDocument()`, `createDocuments()`, `updateDocument()`, + `updateDocuments()`, `upsertDocument()`, `upsertDocuments()`, `increaseDocumentAttribute()`, + `decreaseDocumentAttribute()`, `deleteDocument()` and `deleteDocuments()`, and the `authorizeCreate()`, + `authorizeUpdate()` and `authorizeGrants()` checks of a later write; their own reads load the documents they change, + read back the ones they wrote, populate the relationships of the documents they return, and check, link and cascade + related documents, so a hook that hides rows never hides them from a write's permission and relationship checks. A + read the caller's own code makes while a write runs, such as in an `onNext` callback, runs it. With such a hook + registered, `cursor()` resolves the collection and rewrites its queries when it is called, so a missing collection + or a refusal throws then instead of when the generator is iterated. A `Mirror` runs it once on each of its own reads + and never on its destination's. One that is also a `Hook\Lifecycle` is registered on the source too, as every + lifecycle hook is, so it also rewrites the reads made on the source directly, except the reads the mirror's writes + make there. ## 8.1.2 diff --git a/README.md b/README.md index e68163f63..1335b1436 100644 --- a/README.md +++ b/README.md @@ -334,7 +334,7 @@ $database->silent(fn () => $database->ping(), ['audit-log']); A `Utopia\Database\Hook\Transform` rewrites SQL statements before they run, and a `Utopia\Database\Hook\Decorator` modifies the documents that reads and writes return. Both are registered with `addHook()` too, and `removeHook()` unregisters a hook. `Event\DispatcherHook` forwards events to listeners registered per event class and to a PSR-14 dispatcher. -A `Utopia\Database\Hook\Queries` rewrites the queries of `getDocument()`, `find()`, `findOne()`, `cursor()`, `resolveCursor()`, `count()`, `sum()` and `aggregate()` before the read validates them, resolves their joins or authorizes anything; throwing refuses the read. It also runs on the reads the library makes itself, so it must return the same queries when given its own output: +A `Utopia\Database\Hook\Queries` rewrites the queries of `getDocument()`, `find()`, `findOne()`, `cursor()`, `resolveCursor()`, `count()`, `sum()` and `aggregate()` before the read checks its queries; throwing refuses the read. It also runs on the reads those reads make themselves, such as relationship population, so it must return the same queries when given its own output. Writes, and the reads they make themselves, never run it: ```php use Utopia\Database\Collection; diff --git a/UPGRADE.md b/UPGRADE.md index 399710b8c..fe5d03261 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,11 +1,12 @@ # Upgrading from 8.1.2 to 8.1.3 8.1.3 is a patch release and needs no code change. It adds `Hook\Queries`, which rewrites the queries of a read -before the read validates them, resolves their joins or authorizes anything; a database without one reads as in 8.1.2. -To use it, implement `rewrite(Event $event, Collection $collection, array $queries): array` and register the hook with -`addHook()`. Return the queries the read should use, cloning any query you change, and return the same queries when -given your own output, as the hook also runs on the reads the library makes itself. Throw to refuse the read. See -[Hooks](README.md#hooks) for an example and [CHANGELOG.md](CHANGELOG.md#813) for which reads it covers. +before the read checks its queries; a database without one reads as in 8.1.2. To use it, implement +`rewrite(Event $event, Collection $collection, array $queries): array` and register the hook with `addHook()`. Return +the queries the read should use, cloning any query you change, and return the same queries when given your own output, +as the hook also runs on the reads a read makes itself, such as relationship population. Throw to refuse the read. +Writes, and the reads they make themselves, never run it. See [Hooks](README.md#hooks) for an example and +[CHANGELOG.md](CHANGELOG.md#813) for which reads it covers. # Upgrading from 8.1.1 to 8.1.2 diff --git a/src/Database/Hook/Queries.php b/src/Database/Hook/Queries.php index 156a39873..4ac4c259c 100644 --- a/src/Database/Hook/Queries.php +++ b/src/Database/Hook/Queries.php @@ -12,23 +12,29 @@ * Hook for rewriting the queries of a read before the read uses them. * * It is given the queries one read was given and the definition of the collection being read. It runs once per read, - * before the read checks, validates, parses, resolves joins or authorizes anything, and the queries it returns replace - * the read's queries for the rest of the read, the query-cache key included. getDocument() runs it with - * {@see Event::DocumentRead}; find(), and so findOne(), cursor() and resolveCursor() with {@see Event::DocumentFind}; - * count() with {@see Event::DocumentCount}; sum() with {@see Event::DocumentSum}; and aggregate() with - * {@see Event::DocumentAggregate}. Hooks run in the order they were registered, each given the queries the previous one - * returned. + * after the collection is resolved and each query is checked to be a {@see Query}, and before the read validates, + * parses or authorizes its queries or resolves their joins. The queries it returns replace the read's queries for the + * rest of the read, the query-cache key included. getDocument() runs it with {@see Event::DocumentRead}; find(), and + * so findOne(), cursor() and resolveCursor() with {@see Event::DocumentFind}; count() with {@see Event::DocumentCount}; + * sum() with {@see Event::DocumentSum}; and aggregate() with {@see Event::DocumentAggregate}. Hooks run in the order + * they were registered, each given the queries the previous one returned. * * Throwing refuses the read: the exception reaches the caller unchanged, before anything is read. With a hook * registered, cursor() resolves the collection and rewrites its queries when it is called, so a missing collection or * a refusal throws then, not when the generator is iterated. * - * It runs on every call of those methods, the calls the database makes itself included: relationship population, - * findOne() through find(), each page of cursor(), and the paired-row read of resolveCursor(). So it must return the - * same queries when given its own output. + * It runs on every read through those methods, the reads a read makes itself included: relationship population and + * the reads that resolve queries on relationship attributes, findOne() through find(), each page of cursor(), and + * resolveCursor()'s read of the joined row it pairs with the document. So it must return the same queries when given + * its own output. It runs whether or not events are silenced. * - * It runs whether or not events are silenced. It never runs for reads of {@see Database::METADATA}, for - * updateDocuments() or deleteDocuments(), or for the query(), mutate() and from() builders. + * It never runs for reads of {@see Database::METADATA}, for the query(), mutate() and from() builders, or for writes + * and the reads they make themselves. The writes are createDocument(), createDocuments(), updateDocument(), + * updateDocuments(), upsertDocument(), upsertDocuments(), increaseDocumentAttribute(), decreaseDocumentAttribute(), + * deleteDocument() and deleteDocuments(), and the authorizeCreate(), authorizeUpdate() and authorizeGrants() checks of + * a later write. Their own reads load the documents they change, read back the ones they wrote, populate the + * relationships of the documents they return, and check, link and cascade related documents. A read the caller's own + * code makes while a write runs, as an onNext callback can, runs it. * * The given queries belong to the caller, so a hook that changes one returns a clone of it. It must not modify the * collection, which can be a cached definition. A hook that needs the database implements {@see Attachable}.