From 84c98e3373ac40a48ce1ccf6bd241bbc292a4c2e Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 26 Apr 2026 03:17:45 +0000 Subject: [PATCH 1/5] feat: extend utopia-php/query, add Query::cursor and count(max) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit Query class becomes a thin extension of `Utopia\Query\Query`, which gives audit consumers a single canonical query type that includes cursorAfter / cursorBefore. The custom `Query::in()` alias is removed in favour of the inherited `Query::contains()`. Audit's lenient single-value factory signatures for equal/lessThan/greaterThan/between are preserved by overriding the base factories so existing callers keep working with mixed values (including DateTime on the time column). The ClickHouse adapter gains keyset-pagination cursor support: cursor rows accept Log/ArrayObject or plain arrays, an `id` tiebreaker is auto-appended to ORDER BY for deterministic pagination on non-unique columns, and cursorBefore flips direction at SQL build time then reverses results. count() and the four countBy* helpers (plus their Adapter abstracts and Audit facade wrappers) accept an optional `?int $max` argument. When non-null, ClickHouse uses a bounded subquery so very large tables can short-circuit. The Database adapter forwards $max to utopia-php/database's existing $max parameter. The countBy* helpers also stop loading rows just to count them — they call count() directly now. Co-Authored-By: Claude Opus 4.7 (1M context) --- composer.json | 1 + composer.lock | 54 ++++- src/Audit/Adapter.php | 11 +- src/Audit/Adapter/ClickHouse.php | 279 ++++++++++++++++++++++-- src/Audit/Adapter/Database.php | 41 ++-- src/Audit/Audit.php | 16 +- src/Audit/Query.php | 280 +++---------------------- tests/Audit/Adapter/ClickHouseTest.php | 104 ++++++++- tests/Audit/AuditBase.php | 4 +- tests/Audit/QueryTest.php | 6 +- 10 files changed, 492 insertions(+), 304 deletions(-) diff --git a/composer.json b/composer.json index 30a93e2..f275a14 100644 --- a/composer.json +++ b/composer.json @@ -30,6 +30,7 @@ "php": ">=8.0", "utopia-php/database": "5.*", "utopia-php/fetch": "0.5.*", + "utopia-php/query": "0.1.*", "utopia-php/validators": "0.2.*" }, "require-dev": { diff --git a/composer.lock b/composer.lock index 1b240b0..61e4e8f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "5c1ac810bef3135bbe906ff710194adc", + "content-hash": "5efb93f2e03736ca5c10216daf1467e1", "packages": [ { "name": "brick/math", @@ -2381,6 +2381,52 @@ }, "time": "2026-01-28T13:12:36+00:00" }, + { + "name": "utopia-php/query", + "version": "0.1.1", + "source": { + "type": "git", + "url": "https://github.com/utopia-php/query.git", + "reference": "964a10ed3185490505f4c0062f2eb7b89287fb27" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/utopia-php/query/zipball/964a10ed3185490505f4c0062f2eb7b89287fb27", + "reference": "964a10ed3185490505f4c0062f2eb7b89287fb27", + "shasum": "" + }, + "require": { + "php": ">=8.4" + }, + "require-dev": { + "laravel/pint": "*", + "phpstan/phpstan": "*", + "phpunit/phpunit": "^12.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Utopia\\Query\\": "src/Query" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "A simple library providing a query abstraction for filtering, ordering, and pagination", + "keywords": [ + "framework", + "php", + "query", + "upf", + "utopia" + ], + "support": { + "issues": "https://github.com/utopia-php/query/issues", + "source": "https://github.com/utopia-php/query/tree/0.1.1" + }, + "time": "2026-03-03T09:05:14+00:00" + }, { "name": "utopia-php/telemetry", "version": "0.1.1", @@ -4397,12 +4443,12 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": {}, "prefer-stable": false, "prefer-lowest": false, "platform": { "php": ">=8.0" }, - "platform-dev": [], - "plugin-api-version": "2.6.0" + "platform-dev": {}, + "plugin-api-version": "2.9.0" } diff --git a/src/Audit/Adapter.php b/src/Audit/Adapter.php index 0b7b484..a6d5a53 100644 --- a/src/Audit/Adapter.php +++ b/src/Audit/Adapter.php @@ -100,6 +100,7 @@ abstract public function countByUser( string $userId, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int; /** @@ -131,6 +132,7 @@ abstract public function countByResource( string $resource, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int; /** @@ -166,6 +168,7 @@ abstract public function countByUserAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int; /** @@ -201,6 +204,7 @@ abstract public function countByResourceAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int; /** @@ -226,10 +230,15 @@ abstract public function find(array $queries = []): array; /** * Count logs using custom queries. * + * When $max is non-null the count is bounded at the database level so + * very large tables can short-circuit once the cap is reached. Pass + * `null` for an unbounded total. + * * @param array<\Utopia\Audit\Query> $queries + * @param int|null $max Optional upper bound (inclusive) for the count * @return int * * @throws \Exception */ - abstract public function count(array $queries = []): int; + abstract public function count(array $queries = [], ?int $max = null): int; } diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index 0d598d5..a219de5 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -823,20 +823,37 @@ public function find(array $queries = []): array // Build SELECT clause $selectColumns = $this->getSelectColumns(); + $filters = $parsed['filters']; + $params = $parsed['params']; + $orderAttributes = $parsed['orderAttributes'] ?? []; + $cursorDirection = $parsed['cursorDirection'] ?? null; + + if (isset($parsed['cursor'])) { + $orderAttributes = $this->resolveCursorOrder($orderAttributes); + $cursorWhere = $this->buildCursorWhere($orderAttributes, $parsed['cursor'], $cursorDirection ?? 'after', $params); + $filters[] = $cursorWhere['clause']; + $params = $cursorWhere['params']; + } + // Build WHERE clause $whereClause = ''; $tenantFilter = $this->getTenantFilter(); - if (!empty($parsed['filters']) || $tenantFilter) { - $conditions = $parsed['filters']; + if (!empty($filters) || $tenantFilter) { + $conditions = $filters; if ($tenantFilter) { $conditions[] = ltrim($tenantFilter, ' AND'); } $whereClause = ' WHERE ' . implode(' AND ', $conditions); } - // Build ORDER BY clause + // Build ORDER BY clause — when cursor is in play, rebuild from + // orderAttributes (including the auto-added id tiebreaker), flipping + // directions for `cursorBefore`. $orderClause = ''; - if (!empty($parsed['orderBy'])) { + if (isset($parsed['cursor']) && !empty($orderAttributes)) { + $orderSql = $this->buildOrderBySql($orderAttributes, flip: $cursorDirection === 'before'); + $orderClause = ' ORDER BY ' . implode(', ', $orderSql); + } elseif (!empty($parsed['orderBy'])) { $orderClause = ' ORDER BY ' . implode(', ', $parsed['orderBy']); } @@ -850,23 +867,34 @@ public function find(array $queries = []): array FORMAT JSON "; - $result = $this->query($sql, $parsed['params']); - return $this->parseJsonResults($result); + $result = $this->query($sql, $params); + $rows = $this->parseJsonResults($result); + + if ($cursorDirection === 'before') { + $rows = array_reverse($rows); + } + + return $rows; } /** * Count logs using Query objects. * + * When $max is non-null the count is bounded at the database level via a + * `LIMIT {max}` inside a subquery — ClickHouse stops scanning once the cap + * is reached, keeping large counts cheap (e.g. for "5000+" UI badges). + * * @param array $queries + * @param int|null $max Optional upper bound (inclusive) for the count * @return int * @throws Exception */ - public function count(array $queries = []): int + public function count(array $queries = [], ?int $max = null): int { $tableName = $this->getTableName(); $escapedTable = $this->escapeIdentifier($this->database) . '.' . $this->escapeIdentifier($tableName); - // Parse queries - we only need filters and params, not ordering/limit/offset + // Parse queries - we only need filters and params, not ordering/limit/offset/cursor $parsed = $this->parseQueries($queries); // Build WHERE clause @@ -884,11 +912,22 @@ public function count(array $queries = []): int $params = $parsed['params']; unset($params['limit'], $params['offset']); - $sql = " - SELECT COUNT(*) as count - FROM {$escapedTable}{$whereClause} - FORMAT TabSeparated - "; + if ($max !== null) { + $params['max'] = $max; + $sql = " + SELECT COUNT(*) as count + FROM ( + SELECT 1 FROM {$escapedTable}{$whereClause} LIMIT {max:UInt64} + ) sub + FORMAT TabSeparated + "; + } else { + $sql = " + SELECT COUNT(*) as count + FROM {$escapedTable}{$whereClause} + FORMAT TabSeparated + "; + } $result = $this->query($sql, $params); $trimmed = trim($result); @@ -900,7 +939,7 @@ public function count(array $queries = []): int * Parse Query objects into SQL components. * * @param array $queries - * @return array{filters: array, params: array, orderBy?: array, limit?: int, offset?: int} + * @return array{filters: array, params: array, orderBy?: array, orderAttributes?: array, limit?: int, offset?: int, cursor?: array, cursorDirection?: string} * @throws Exception */ private function parseQueries(array $queries): array @@ -908,8 +947,11 @@ private function parseQueries(array $queries): array $filters = []; $params = []; $orderBy = []; + $orderAttributes = []; $limit = null; $offset = null; + $cursor = null; + $cursorDirection = null; $paramCounter = 0; foreach ($queries as $query) { @@ -988,7 +1030,7 @@ private function parseQueries(array $queries): array } break; - case Query::TYPE_IN: + case Query::TYPE_CONTAINS: $this->validateAttributeName($attribute); $escapedAttr = $this->escapeIdentifier((string) $attribute); $inParams = []; @@ -1004,12 +1046,14 @@ private function parseQueries(array $queries): array $this->validateAttributeName($attribute); $escapedAttr = $this->escapeIdentifier($attribute); $orderBy[] = "{$escapedAttr} DESC"; + $orderAttributes[] = ['attribute' => $attribute, 'direction' => 'DESC']; break; case Query::TYPE_ORDER_ASC: $this->validateAttributeName($attribute); $escapedAttr = $this->escapeIdentifier($attribute); $orderBy[] = "{$escapedAttr} ASC"; + $orderAttributes[] = ['attribute' => $attribute, 'direction' => 'ASC']; break; case Query::TYPE_LIMIT: @@ -1027,6 +1071,20 @@ private function parseQueries(array $queries): array $offset = $values[0]; $params['offset'] = $offset; break; + + case Query::TYPE_CURSOR_AFTER: + case Query::TYPE_CURSOR_BEFORE: + if ($cursor !== null) { + // Keep the first cursor encountered (matches base groupByType semantics) + break; + } + $rawCursor = $values[0] ?? null; + if ($rawCursor === null) { + break; // no-op cursor + } + $cursor = $this->normalizeCursorRow($rawCursor); + $cursorDirection = $method === Query::TYPE_CURSOR_AFTER ? 'after' : 'before'; + break; } } @@ -1037,6 +1095,7 @@ private function parseQueries(array $queries): array if (!empty($orderBy)) { $result['orderBy'] = $orderBy; + $result['orderAttributes'] = $orderAttributes; } if ($limit !== null) { @@ -1047,9 +1106,179 @@ private function parseQueries(array $queries): array $result['offset'] = $offset; } + if ($cursor !== null && $cursorDirection !== null) { + $result['cursor'] = $cursor; + $result['cursorDirection'] = $cursorDirection; + } + return $result; } + /** + * Normalize a user-supplied cursor row into a column-keyed array. + * + * Accepts a `Log` (or any `ArrayObject`) or a plain associative array. + * `Log` stores its identifier under `$id` (Appwrite convention) while the + * underlying column is `id` — this remaps `$id` → `id` so cursor pagination + * can match the SQL column. + * + * @param mixed $rawCursor + * @return array + * @throws Exception + */ + private function normalizeCursorRow(mixed $rawCursor): array + { + if ($rawCursor instanceof \ArrayObject) { + /** @var array $row */ + $row = $rawCursor->getArrayCopy(); + } elseif (is_array($rawCursor)) { + /** @var array $rawCursor */ + $row = $rawCursor; + } else { + throw new Exception( + 'Invalid cursor value: expected ArrayObject (Log) or associative array, got ' + . get_debug_type($rawCursor) + ); + } + + if (!array_key_exists('id', $row) && array_key_exists('$id', $row)) { + $row['id'] = $row['$id']; + } + + return $row; + } + + /** + * Resolve the effective order attributes for cursor pagination. + * + * Auto-appends `id` as a tiebreaker when not already present so keyset + * pagination is deterministic on non-unique columns (e.g. time). + * + * @param array $orderAttributes + * @return array + */ + private function resolveCursorOrder(array $orderAttributes): array + { + foreach ($orderAttributes as $entry) { + if ($entry['attribute'] === 'id') { + return $orderAttributes; + } + } + + $defaultDirection = 'DESC'; + if (!empty($orderAttributes)) { + $last = $orderAttributes[count($orderAttributes) - 1]; + $defaultDirection = $last['direction']; + } + + $orderAttributes[] = ['attribute' => 'id', 'direction' => $defaultDirection]; + + return $orderAttributes; + } + + /** + * Build keyset-pagination WHERE fragments for cursor support. + * + * Produces a tuple-compare clause across the order attributes: + * (a > A) OR (a = A AND b > B) OR ... + * + * For cursor `before`, the comparison directions are flipped relative to + * the requested ORDER BY (the caller is responsible for also flipping the + * actual ORDER BY at SQL build time so the page comes back from the right + * side, then reversing the rows post-fetch). + * + * @param array $orderAttributes + * @param array $cursor + * @param string $cursorDirection 'after' or 'before' + * @param array $params Existing params (mutated by adding cursor binds) + * @return array{clause: string, params: array} + * @throws Exception + */ + private function buildCursorWhere(array $orderAttributes, array $cursor, string $cursorDirection, array $params): array + { + $tuples = []; + foreach ($orderAttributes as $i => $entry) { + $attr = $entry['attribute']; + $direction = $entry['direction']; + + if (!array_key_exists($attr, $cursor)) { + throw new Exception("Cursor is missing required attribute '{$attr}'"); + } + + if ($cursorDirection === 'before') { + $direction = $direction === 'DESC' ? 'ASC' : 'DESC'; + } + + $conditions = []; + + for ($j = 0; $j < $i; $j++) { + $prev = $orderAttributes[$j]; + $prevAttr = $prev['attribute']; + if (!array_key_exists($prevAttr, $cursor)) { + throw new Exception("Cursor is missing required attribute '{$prevAttr}'"); + } + $prevValue = $cursor[$prevAttr]; + $prevEscaped = $this->escapeIdentifier($prevAttr); + $paramName = "cursor_eq_{$i}_{$j}"; + + if ($prevAttr === 'time') { + /** @var \DateTime|string|null $timeValue */ + $timeValue = $prevValue; + $conditions[] = "{$prevEscaped} = {{$paramName}:DateTime64(3)}"; + $params[$paramName] = $this->formatDateTime($timeValue); + } else { + $conditions[] = "{$prevEscaped} = {{$paramName}:String}"; + $params[$paramName] = $this->formatParamValue($prevValue); + } + } + + $value = $cursor[$attr]; + $escaped = $this->escapeIdentifier($attr); + $operator = $direction === 'DESC' ? '<' : '>'; + $paramName = "cursor_cmp_{$i}"; + + if ($attr === 'time') { + /** @var \DateTime|string|null $timeValue */ + $timeValue = $value; + $conditions[] = "{$escaped} {$operator} {{$paramName}:DateTime64(3)}"; + $params[$paramName] = $this->formatDateTime($timeValue); + } else { + $conditions[] = "{$escaped} {$operator} {{$paramName}:String}"; + $params[$paramName] = $this->formatParamValue($value); + } + + $tuples[] = '(' . implode(' AND ', $conditions) . ')'; + } + + return [ + 'clause' => '(' . implode(' OR ', $tuples) . ')', + 'params' => $params, + ]; + } + + /** + * Build the ORDER BY SQL fragment list, optionally flipping all directions. + * + * Used when cursor direction is `before` — we run the query in reverse to + * grab the previous-page rows, then `array_reverse` the result. + * + * @param array $orderAttributes + * @param bool $flip Whether to flip ASC↔DESC + * @return array + */ + private function buildOrderBySql(array $orderAttributes, bool $flip = false): array + { + $sql = []; + foreach ($orderAttributes as $entry) { + $direction = $entry['direction']; + if ($flip) { + $direction = $direction === 'DESC' ? 'ASC' : 'DESC'; + } + $sql[] = $this->escapeIdentifier($entry['attribute']) . ' ' . $direction; + } + return $sql; + } + /** * Create multiple audit log entries in batch. * @@ -1352,6 +1581,7 @@ public function countByUser( string $userId, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $queries = [ Query::equal('userId', $userId), @@ -1365,7 +1595,7 @@ public function countByUser( $queries[] = Query::lessThan('time', $before); } - return count($this->find($queries)); + return $this->count($queries, $max); } /** @@ -1409,6 +1639,7 @@ public function countByResource( string $resource, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $queries = [ Query::equal('resource', $resource), @@ -1422,7 +1653,7 @@ public function countByResource( $queries[] = Query::lessThan('time', $before); } - return count($this->find($queries)); + return $this->count($queries, $max); } /** @@ -1441,7 +1672,7 @@ public function getByUserAndEvents( ): array { $queries = [ Query::equal('userId', $userId), - Query::in('event', $events), + Query::contains('event', $events), ]; if ($after !== null && $before !== null) { @@ -1469,10 +1700,11 @@ public function countByUserAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $queries = [ Query::equal('userId', $userId), - Query::in('event', $events), + Query::contains('event', $events), ]; if ($after !== null && $before !== null) { @@ -1483,7 +1715,7 @@ public function countByUserAndEvents( $queries[] = Query::lessThan('time', $before); } - return count($this->find($queries)); + return $this->count($queries, $max); } /** @@ -1502,7 +1734,7 @@ public function getByResourceAndEvents( ): array { $queries = [ Query::equal('resource', $resource), - Query::in('event', $events), + Query::contains('event', $events), ]; if ($after !== null && $before !== null) { @@ -1530,10 +1762,11 @@ public function countByResourceAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $queries = [ Query::equal('resource', $resource), - Query::in('event', $events), + Query::contains('event', $events), ]; if ($after !== null && $before !== null) { @@ -1544,7 +1777,7 @@ public function countByResourceAndEvents( $queries[] = Query::lessThan('time', $before); } - return count($this->find($queries)); + return $this->count($queries, $max); } /** diff --git a/src/Audit/Adapter/Database.php b/src/Audit/Adapter/Database.php index e9310ee..cda3383 100644 --- a/src/Audit/Adapter/Database.php +++ b/src/Audit/Adapter/Database.php @@ -194,15 +194,17 @@ public function countByUser( string $userId, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $timeQueries = $this->buildTimeQueries($after, $before); - return $this->db->getAuthorization()->skip(function () use ($userId, $timeQueries) { + return $this->db->getAuthorization()->skip(function () use ($userId, $timeQueries, $max) { return $this->db->count( collection: $this->getCollectionName(), queries: [ Query::equal('userId', [$userId]), ...$timeQueries, - ] + ], + max: $max, ); }); } @@ -252,15 +254,17 @@ public function countByResource( string $resource, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $timeQueries = $this->buildTimeQueries($after, $before); - return $this->db->getAuthorization()->skip(function () use ($resource, $timeQueries) { + return $this->db->getAuthorization()->skip(function () use ($resource, $timeQueries, $max) { return $this->db->count( collection: $this->getCollectionName(), queries: [ Query::equal('resource', [$resource]), ...$timeQueries, - ] + ], + max: $max, ); }); } @@ -315,16 +319,18 @@ public function countByUserAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $timeQueries = $this->buildTimeQueries($after, $before); - return $this->db->getAuthorization()->skip(function () use ($userId, $events, $timeQueries) { + return $this->db->getAuthorization()->skip(function () use ($userId, $events, $timeQueries, $max) { return $this->db->count( collection: $this->getCollectionName(), queries: [ Query::equal('userId', [$userId]), Query::equal('event', $events), ...$timeQueries, - ] + ], + max: $max, ); }); } @@ -379,16 +385,18 @@ public function countByResourceAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { $timeQueries = $this->buildTimeQueries($after, $before); - return $this->db->getAuthorization()->skip(function () use ($resource, $events, $timeQueries) { + return $this->db->getAuthorization()->skip(function () use ($resource, $events, $timeQueries, $max) { return $this->db->count( collection: $this->getCollectionName(), queries: [ Query::equal('resource', [$resource]), Query::equal('event', $events), ...$timeQueries, - ] + ], + max: $max, ); }); } @@ -493,13 +501,14 @@ public function find(array $queries = []): array * Count logs using custom queries. * * Translates Audit Query objects to Database Query objects. - * Ignores limit and offset queries as they don't apply to count. + * Ignores limit, offset, and cursor queries as they don't apply to count. * * @param array<\Utopia\Audit\Query> $queries + * @param int|null $max Optional upper bound (inclusive) for the count * @return int * @throws AuthorizationException|\Exception */ - public function count(array $queries = []): int + public function count(array $queries = [], ?int $max = null): int { $dbQueries = []; @@ -508,9 +517,14 @@ public function count(array $queries = []): int throw new \Exception('Invalid query type. Expected Utopia\\Audit\\Query'); } - // Skip limit and offset for count queries + // Skip limit, offset, and cursor queries — they don't apply to count $method = $query->getMethod(); - if ($method === \Utopia\Audit\Query::TYPE_LIMIT || $method === \Utopia\Audit\Query::TYPE_OFFSET) { + if ( + $method === \Utopia\Audit\Query::TYPE_LIMIT + || $method === \Utopia\Audit\Query::TYPE_OFFSET + || $method === \Utopia\Audit\Query::TYPE_CURSOR_AFTER + || $method === \Utopia\Audit\Query::TYPE_CURSOR_BEFORE + ) { continue; } @@ -520,10 +534,11 @@ public function count(array $queries = []): int $dbQueries[] = Query::parseQuery($queryArray); } - return $this->db->getAuthorization()->skip(function () use ($dbQueries) { + return $this->db->getAuthorization()->skip(function () use ($dbQueries, $max) { return $this->db->count( collection: $this->getCollectionName(), queries: $dbQueries, + max: $max, ); }); } diff --git a/src/Audit/Audit.php b/src/Audit/Audit.php index bb17f83..0e1deb4 100644 --- a/src/Audit/Audit.php +++ b/src/Audit/Audit.php @@ -130,8 +130,9 @@ public function countLogsByUser( string $userId, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { - return $this->adapter->countByUser($userId, $after, $before); + return $this->adapter->countByUser($userId, $after, $before, $max); } /** @@ -165,8 +166,9 @@ public function countLogsByResource( string $resource, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { - return $this->adapter->countByResource($resource, $after, $before); + return $this->adapter->countByResource($resource, $after, $before, $max); } /** @@ -204,8 +206,9 @@ public function countLogsByUserAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { - return $this->adapter->countByUserAndEvents($userId, $events, $after, $before); + return $this->adapter->countByUserAndEvents($userId, $events, $after, $before, $max); } /** @@ -243,8 +246,9 @@ public function countLogsByResourceAndEvents( array $events, ?\DateTime $after = null, ?\DateTime $before = null, + ?int $max = null, ): int { - return $this->adapter->countByResourceAndEvents($resource, $events, $after, $before); + return $this->adapter->countByResourceAndEvents($resource, $events, $after, $before, $max); } /** @@ -281,8 +285,8 @@ public function find(array $queries = []): array * * @throws \Exception */ - public function count(array $queries = []): int + public function count(array $queries = [], ?int $max = null): int { - return $this->adapter->count($queries); + return $this->adapter->count($queries, $max); } } diff --git a/src/Audit/Query.php b/src/Audit/Query.php index ebc7d7a..cc2fecd 100644 --- a/src/Audit/Query.php +++ b/src/Audit/Query.php @@ -2,287 +2,73 @@ namespace Utopia\Audit; +use Utopia\Query\Query as BaseQuery; + /** - * Query class for ClickHouse Audit adapter + * Audit Query * - * Provides a fluent interface for building ClickHouse queries. - * Contains only methods needed for audit log operations. + * Thin extension of `Utopia\Query\Query` so audit consumers share the + * canonical query types — including `cursorAfter` / `cursorBefore` and + * `parse()` validation — while keeping audit's lenient single-value factory + * signatures (the base requires arrays / scalars only; audit accepts mixed + * including `DateTime` for the `time` column). */ -class Query +class Query extends BaseQuery { - // Filter methods - public const TYPE_EQUAL = 'equal'; - public const TYPE_GREATER = 'greaterThan'; - public const TYPE_LESSER = 'lessThan'; - public const TYPE_BETWEEN = 'between'; - public const TYPE_IN = 'contains'; - - // Order methods - public const TYPE_ORDER_DESC = 'orderDesc'; - public const TYPE_ORDER_ASC = 'orderAsc'; - - // Pagination methods - public const TYPE_LIMIT = 'limit'; - public const TYPE_OFFSET = 'offset'; - - protected string $method = ''; - protected string $attribute = ''; - - /** - * @var array - */ - protected array $values = []; - /** - * Construct a new query object + * Filter by equal condition. * - * @param string $method - * @param string $attribute - * @param array $values - */ - public function __construct(string $method, string $attribute = '', array $values = []) - { - $this->method = $method; - $this->attribute = $attribute; - $this->values = $values; - } - - /** - * @return string - */ - public function getMethod(): string - { - return $this->method; - } - - /** - * @return string - */ - public function getAttribute(): string - { - return $this->attribute; - } - - /** - * @return array - */ - public function getValues(): array - { - return $this->values; - } - - /** - * @param mixed $default - * @return mixed - */ - public function getValue(mixed $default = null): mixed - { - return $this->values[0] ?? $default; - } - - /** - * Filter by equal condition + * Accepts a single scalar/object/array value and stores it as the values + * array. Matches the legacy audit signature. * * @param string $attribute - * @param mixed $value - * @return self + * @param mixed $value Single value or array of values + * @return static */ - public static function equal(string $attribute, mixed $value): self + public static function equal(string $attribute, mixed $value): static { - return new self(self::TYPE_EQUAL, $attribute, [$value]); + /** @var array $values */ + $values = is_array($value) ? $value : [$value]; + return new static(self::TYPE_EQUAL, $attribute, $values); } /** - * Filter by less than condition + * Filter by less than condition. + * + * Accepts mixed (including `DateTime` for the `time` column); the + * adapter handles type-specific formatting. * * @param string $attribute * @param mixed $value - * @return self + * @return static */ - public static function lessThan(string $attribute, mixed $value): self + public static function lessThan(string $attribute, mixed $value): static { - return new self(self::TYPE_LESSER, $attribute, [$value]); + return new static(self::TYPE_LESSER, $attribute, [$value]); } /** - * Filter by greater than condition + * Filter by greater than condition. * * @param string $attribute * @param mixed $value - * @return self + * @return static */ - public static function greaterThan(string $attribute, mixed $value): self + public static function greaterThan(string $attribute, mixed $value): static { - return new self(self::TYPE_GREATER, $attribute, [$value]); + return new static(self::TYPE_GREATER, $attribute, [$value]); } /** - * Filter by BETWEEN condition + * Filter by BETWEEN condition. * * @param string $attribute * @param mixed $start * @param mixed $end - * @return self - */ - public static function between(string $attribute, mixed $start, mixed $end): self - { - return new self(self::TYPE_BETWEEN, $attribute, [$start, $end]); - } - - /** - * Filter by IN condition - * - * @param string $attribute - * @param array $values - * @return self - */ - public static function in(string $attribute, array $values): self - { - return new self(self::TYPE_IN, $attribute, $values); - } - - /** - * Order by descending - * - * @param string $attribute - * @return self - */ - public static function orderDesc(string $attribute = 'time'): self - { - return new self(self::TYPE_ORDER_DESC, $attribute); - } - - /** - * Order by ascending - * - * @param string $attribute - * @return self - */ - public static function orderAsc(string $attribute = 'time'): self - { - return new self(self::TYPE_ORDER_ASC, $attribute); - } - - /** - * Limit number of results - * - * @param int $limit - * @return self - */ - public static function limit(int $limit): self - { - return new self(self::TYPE_LIMIT, '', [$limit]); - } - - /** - * Offset results - * - * @param int $offset - * @return self - */ - public static function offset(int $offset): self - { - return new self(self::TYPE_OFFSET, '', [$offset]); - } - - /** - * Parse query from JSON string - * - * @param string $query - * @return self - * @throws \Exception - */ - public static function parse(string $query): self - { - try { - $query = \json_decode($query, true, flags: JSON_THROW_ON_ERROR); - } catch (\JsonException $e) { - throw new \Exception('Invalid query: ' . $e->getMessage()); - } - - if (!\is_array($query)) { - throw new \Exception('Invalid query. Must be an array, got ' . \gettype($query)); - } - - return self::parseQuery($query); - } - - /** - * Parse an array of queries - * - * @param array $queries - * @return array - * @throws \Exception - */ - public static function parseQueries(array $queries): array - { - $parsed = []; - - foreach ($queries as $query) { - $parsed[] = self::parse($query); - } - - return $parsed; - } - - /** - * Parse query from array - * - * @param array $query - * @return self - * @throws \Exception - */ - protected static function parseQuery(array $query): self - { - $method = $query['method'] ?? ''; - $attribute = $query['attribute'] ?? ''; - $values = $query['values'] ?? []; - - if (!\is_string($method)) { - throw new \Exception('Invalid query method. Must be a string, got ' . \gettype($method)); - } - - if (!\is_string($attribute)) { - throw new \Exception('Invalid query attribute. Must be a string, got ' . \gettype($attribute)); - } - - if (!\is_array($values)) { - throw new \Exception('Invalid query values. Must be an array, got ' . \gettype($values)); - } - - return new self($method, $attribute, $values); - } - - /** - * Convert query to array - * - * @return array - */ - public function toArray(): array - { - $array = ['method' => $this->method]; - - if (!empty($this->attribute)) { - $array['attribute'] = $this->attribute; - } - - $array['values'] = $this->values; - - return $array; - } - - /** - * Convert query to JSON string - * - * @return string - * @throws \Exception + * @return static */ - public function toString(): string + public static function between(string $attribute, mixed $start, mixed $end): static { - try { - return \json_encode($this->toArray(), flags: JSON_THROW_ON_ERROR); - } catch (\JsonException $e) { - throw new \Exception('Invalid Json: ' . $e->getMessage()); - } + return new static(self::TYPE_BETWEEN, $attribute, [$start, $end]); } } diff --git a/tests/Audit/Adapter/ClickHouseTest.php b/tests/Audit/Adapter/ClickHouseTest.php index 1374c71..c351fa6 100644 --- a/tests/Audit/Adapter/ClickHouseTest.php +++ b/tests/Audit/Adapter/ClickHouseTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Utopia\Audit\Adapter\ClickHouse; use Utopia\Audit\Audit; +use Utopia\Audit\Query; use Utopia\Tests\Audit\AuditBase; /** @@ -20,14 +21,25 @@ class ClickHouseTest extends TestCase protected function initializeAudit(): void { + $host = getenv('CLICKHOUSE_HOST') ?: 'clickhouse'; + $username = getenv('CLICKHOUSE_USER') ?: 'default'; + $password = getenv('CLICKHOUSE_PASSWORD') ?: 'clickhouse'; + $port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123); + $secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false); + $clickHouse = new ClickHouse( - host: 'clickhouse', - username: 'default', - password: 'clickhouse', - port: 8123 + host: $host, + username: $username, + password: $password, + port: $port, + secure: $secure ); - $clickHouse->setDatabase('default'); + if ($database = getenv('CLICKHOUSE_DATABASE')) { + $clickHouse->setDatabase($database); + } else { + $clickHouse->setDatabase('default'); + } $this->audit = new Audit($clickHouse); $this->audit->setup(); @@ -471,4 +483,86 @@ public function testParseResourceMethod(): void $this->assertEquals('table', $parsed['resourceType']); $this->assertEquals('database/6978484940ff05762e1a', $parsed['resourceParent']); } + + public function testCursorAfterPaginatesLogs(): void + { + $page1 = $this->audit->find([ + Query::orderAsc('id'), + Query::limit(2), + ]); + + $this->assertCount(2, $page1); + + $page2 = $this->audit->find([ + Query::orderAsc('id'), + Query::limit(2), + Query::cursorAfter($page1[count($page1) - 1]), + ]); + + $this->assertGreaterThanOrEqual(1, count($page2)); + foreach ($page2 as $log) { + $this->assertNotEquals($page1[0]->getId(), $log->getId()); + $this->assertNotEquals($page1[1]->getId(), $log->getId()); + } + } + + public function testCursorBeforeReversesPagination(): void + { + $all = $this->audit->find([ + Query::orderAsc('id'), + Query::limit(50), + ]); + + $this->assertGreaterThanOrEqual(3, count($all)); + + $before = $this->audit->find([ + Query::orderAsc('id'), + Query::limit(2), + Query::cursorBefore($all[count($all) - 1]), + ]); + + $this->assertCount(2, $before); + $this->assertEquals($all[count($all) - 3]->getId(), $before[0]->getId()); + $this->assertEquals($all[count($all) - 2]->getId(), $before[1]->getId()); + } + + public function testCursorAcceptsAssociativeArray(): void + { + $all = $this->audit->find([ + Query::orderAsc('id'), + Query::limit(50), + ]); + + $this->assertGreaterThanOrEqual(2, count($all)); + + $page = $this->audit->find([ + Query::orderAsc('id'), + Query::limit(50), + Query::cursorAfter(['id' => $all[0]->getId()]), + ]); + + $this->assertEquals(count($all) - 1, count($page)); + $this->assertEquals($all[1]->getId(), $page[0]->getId()); + } + + public function testCountWithMaxBound(): void + { + $unbounded = $this->audit->count(); + $this->assertGreaterThanOrEqual(4, $unbounded); + + $bounded = $this->audit->count([], max: 2); + $this->assertEquals(2, $bounded); + + $boundedAboveTotal = $this->audit->count([], max: 10_000); + $this->assertEquals($unbounded, $boundedAboveTotal); + } + + public function testCountByUserWithMaxBound(): void + { + $unbounded = $this->audit->countLogsByUser('userId'); + $this->assertEquals(3, $unbounded); + + $bounded = $this->audit->countLogsByUser('userId', max: 1); + $this->assertEquals(1, $bounded); + } } diff --git a/tests/Audit/AuditBase.php b/tests/Audit/AuditBase.php index d3d1ab2..823f4a5 100644 --- a/tests/Audit/AuditBase.php +++ b/tests/Audit/AuditBase.php @@ -688,7 +688,7 @@ public function testFind(): void // Test 6: Find with IN filter $logs = $this->audit->find([ - \Utopia\Audit\Query::in('event', ['event_0', 'event_1']), + \Utopia\Audit\Query::contains('event', ['event_0', 'event_1']), ]); $this->assertGreaterThanOrEqual(2, \count($logs)); @@ -765,7 +765,7 @@ public function testCount(): void // Test 3: Count with IN filter $count = $this->audit->count([ - \Utopia\Audit\Query::in('event', ['event_0', 'event_1']), + \Utopia\Audit\Query::contains('event', ['event_0', 'event_1']), ]); $this->assertGreaterThanOrEqual(2, $count); diff --git a/tests/Audit/QueryTest.php b/tests/Audit/QueryTest.php index 2dc3fa1..d151039 100644 --- a/tests/Audit/QueryTest.php +++ b/tests/Audit/QueryTest.php @@ -36,9 +36,9 @@ public function testQueryStaticFactoryMethods(): void $this->assertEquals('time', $query->getAttribute()); $this->assertEquals(['2023-01-01', '2024-01-01'], $query->getValues()); - // Test in - $query = Query::in('event', ['create', 'update', 'delete']); - $this->assertEquals(Query::TYPE_IN, $query->getMethod()); + // Test contains + $query = Query::contains('event', ['create', 'update', 'delete']); + $this->assertEquals(Query::TYPE_CONTAINS, $query->getMethod()); $this->assertEquals('event', $query->getAttribute()); $this->assertEquals(['create', 'update', 'delete'], $query->getValues()); From 2ddb7674d51e8bad758bff0b981e992e8c41e9d3 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 26 Apr 2026 05:12:17 +0000 Subject: [PATCH 2/5] chore: bump min PHP to 8.4 utopia-php/query 0.1.x already requires PHP 8.4; making it explicit here matches the actual transitive requirement and aligns with appwrite/appwrite and cloud which already run on 8.4. Co-Authored-By: Claude Opus 4.7 (1M context) --- composer.json | 2 +- composer.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index f275a14..90dba56 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,7 @@ } }, "require": { - "php": ">=8.0", + "php": ">=8.4", "utopia-php/database": "5.*", "utopia-php/fetch": "0.5.*", "utopia-php/query": "0.1.*", diff --git a/composer.lock b/composer.lock index 61e4e8f..53981ac 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "5efb93f2e03736ca5c10216daf1467e1", + "content-hash": "98622ab8912612e2e13a91c7786708f3", "packages": [ { "name": "brick/math", @@ -4447,7 +4447,7 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": ">=8.0" + "php": ">=8.4" }, "platform-dev": {}, "plugin-api-version": "2.9.0" From 14e11bdc7706e28739728647ff8c25c12a612770 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 26 Apr 2026 06:06:59 +0000 Subject: [PATCH 3/5] fix: address P2 review comments on cursor pagination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small hardening fixes from the greptile review: - Drop the always-true `!empty($orderAttributes)` guard in the cursor branch — resolveCursorOrder() always appends an `id` tiebreaker, so the guard was dead code and could mislead future readers. - normalizeCursorRow now removes `$id` after copying it to `id` so the cursor state no longer carries both keys. - Throw an explicit Exception when a cursor value is null. The previous path silently routed null `time` cursors through formatDateTime(null) which returns the current timestamp, turning a misconfigured cursor into a silent correctness bug (filtering on `time < now()` and skipping rows). - Replace the hard-coded `:String` type binding for non-`time` cursor attributes with a schema-driven getCursorParamType() helper that maps audit's VAR_STRING / VAR_DATETIME types to their ClickHouse equivalents and throws on unsupported types. This guards against silently misordering pages if a numeric column is added to the schema later — binding such a value as String would produce lexicographic comparisons ("9" > "10"). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Audit/Adapter/ClickHouse.php | 70 +++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 11 deletions(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index a219de5..cc69e44 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -847,10 +847,10 @@ public function find(array $queries = []): array } // Build ORDER BY clause — when cursor is in play, rebuild from - // orderAttributes (including the auto-added id tiebreaker), flipping - // directions for `cursorBefore`. + // orderAttributes (always non-empty after resolveCursorOrder, which + // appends an id tiebreaker), flipping directions for `cursorBefore`. $orderClause = ''; - if (isset($parsed['cursor']) && !empty($orderAttributes)) { + if (isset($parsed['cursor'])) { $orderSql = $this->buildOrderBySql($orderAttributes, flip: $cursorDirection === 'before'); $orderClause = ' ORDER BY ' . implode(', ', $orderSql); } elseif (!empty($parsed['orderBy'])) { @@ -1143,11 +1143,53 @@ private function normalizeCursorRow(mixed $rawCursor): array if (!array_key_exists('id', $row) && array_key_exists('$id', $row)) { $row['id'] = $row['$id']; + unset($row['$id']); } return $row; } + /** + * Resolve the ClickHouse parameter type for a cursor attribute. + * + * Cursor keyset comparisons must bind values with the column's actual + * SQL type — binding a numeric column as `String` would compare values + * lexicographically ("9" > "10") and silently skip rows on page + * boundaries. Throws if the column is unknown or has an unsupported type + * so a misconfigured cursor fails loudly instead of producing incorrect + * pagination. + * + * @param string $attribute + * @return string ClickHouse parameter type (e.g. 'String', 'DateTime64(3)') + * @throws Exception + */ + private function getCursorParamType(string $attribute): string + { + if ($attribute === 'id') { + return 'String'; + } + + $meta = $this->getAttribute($attribute); + if ($meta === null) { + throw new Exception("Cursor cannot order by unknown attribute '{$attribute}'"); + } + + $type = $meta['type'] ?? null; + + if ($type === Database::VAR_DATETIME) { + return 'DateTime64(3)'; + } + + if ($type === Database::VAR_STRING) { + return 'String'; + } + + $observed = is_string($type) ? $type : get_debug_type($type); + throw new Exception( + "Cursor pagination does not support ordering by attribute '{$attribute}' of type '{$observed}'" + ); + } + /** * Resolve the effective order attributes for cursor pagination. * @@ -1218,32 +1260,38 @@ private function buildCursorWhere(array $orderAttributes, array $cursor, string throw new Exception("Cursor is missing required attribute '{$prevAttr}'"); } $prevValue = $cursor[$prevAttr]; + if ($prevValue === null) { + throw new Exception("Cursor value for '{$prevAttr}' cannot be null"); + } $prevEscaped = $this->escapeIdentifier($prevAttr); + $prevType = $this->getCursorParamType($prevAttr); $paramName = "cursor_eq_{$i}_{$j}"; - if ($prevAttr === 'time') { - /** @var \DateTime|string|null $timeValue */ + $conditions[] = "{$prevEscaped} = {{$paramName}:{$prevType}}"; + if ($prevType === 'DateTime64(3)') { + /** @var \DateTime|string $timeValue */ $timeValue = $prevValue; - $conditions[] = "{$prevEscaped} = {{$paramName}:DateTime64(3)}"; $params[$paramName] = $this->formatDateTime($timeValue); } else { - $conditions[] = "{$prevEscaped} = {{$paramName}:String}"; $params[$paramName] = $this->formatParamValue($prevValue); } } $value = $cursor[$attr]; + if ($value === null) { + throw new Exception("Cursor value for '{$attr}' cannot be null"); + } $escaped = $this->escapeIdentifier($attr); + $chType = $this->getCursorParamType($attr); $operator = $direction === 'DESC' ? '<' : '>'; $paramName = "cursor_cmp_{$i}"; - if ($attr === 'time') { - /** @var \DateTime|string|null $timeValue */ + $conditions[] = "{$escaped} {$operator} {{$paramName}:{$chType}}"; + if ($chType === 'DateTime64(3)') { + /** @var \DateTime|string $timeValue */ $timeValue = $value; - $conditions[] = "{$escaped} {$operator} {{$paramName}:DateTime64(3)}"; $params[$paramName] = $this->formatDateTime($timeValue); } else { - $conditions[] = "{$escaped} {$operator} {{$paramName}:String}"; $params[$paramName] = $this->formatParamValue($value); } From cd5112f704171252a7455180a3242c33ca23d8d4 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 27 Apr 2026 02:46:36 +0000 Subject: [PATCH 4/5] feat: add 9 query methods, align with usage adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds notEqual, lessThanEqual, greaterThanEqual, notContains, notBetween, isNull, isNotNull, startsWith, endsWith — bringing the audit ClickHouse adapter to parity with the usage adapter. startsWith / endsWith use ClickHouse's built-in functions of the same name; isNull / isNotNull emit `IS NULL` / `IS NOT NULL` without binding; the comparison helpers follow the existing param-bound pattern. Two structural changes make the shared parseQueries logic consistent between this adapter and utopia-php/usage's ClickHouse adapter: - getParamType() centralises the column → ClickHouse-type mapping (time → DateTime64(3), tenant → UInt64 when sharedTables, default → String). Previously TYPE_EQUAL and TYPE_CONTAINS hard-coded `:String` for every column — including `time` — so `Query::equal('time', ...)` bound a DateTime as String and would fail or produce wrong results. - formatTypedValue() routes DateTime-typed values through formatDateTime and everything else through formatParamValue, so each case body has one code path. - buildCursorWhere() uses the same dispatch and getCursorParamType is removed — getParamType is now strict-enough on its own (whitelist of known typed columns; everything else is String). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Audit/Adapter/ClickHouse.php | 255 ++++++++++++++++--------- tests/Audit/Adapter/ClickHouseTest.php | 94 +++++++++ 2 files changed, 257 insertions(+), 92 deletions(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index cc69e44..9ad5a4b 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -964,82 +964,160 @@ private function parseQueries(array $queries): array $method = $query->getMethod(); $attribute = $query->getAttribute(); /** @var string $attribute */ - - $values = $query->getValues(); $values = $query->getValues(); switch ($method) { case Query::TYPE_EQUAL: $this->validateAttributeName($attribute); - $escapedAttr = $this->escapeIdentifier((string) $attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); + + if (count($values) > 1) { + $inParams = []; + foreach ($values as $value) { + $paramName = 'param_' . $paramCounter++; + $inParams[] = "{{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $value); + } + $filters[] = "{$escapedAttr} IN (" . implode(', ', $inParams) . ")"; + } else { + $paramName = 'param_' . $paramCounter++; + $filters[] = "{$escapedAttr} = {{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $values[0] ?? null); + } + break; + + case Query::TYPE_NOT_EQUAL: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); $paramName = 'param_' . $paramCounter++; - $filters[] = "{$escapedAttr} = {{$paramName}:String}"; - $params[$paramName] = $this->formatParamValue($values[0]); + $filters[] = "{$escapedAttr} != {{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $values[0] ?? null); break; case Query::TYPE_LESSER: $this->validateAttributeName($attribute); - $escapedAttr = $this->escapeIdentifier((string) $attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); + $paramName = 'param_' . $paramCounter++; + $filters[] = "{$escapedAttr} < {{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $values[0] ?? null); + break; + + case Query::TYPE_LESSER_EQUAL: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); $paramName = 'param_' . $paramCounter++; - if ($attribute === 'time') { - $filters[] = "{$escapedAttr} < {{$paramName}:DateTime64(3)}"; - /** @var \DateTime|string|null $val */ - $val = $values[0]; - $params[$paramName] = $this->formatDateTime($val); - } else { - $filters[] = "{$escapedAttr} < {{$paramName}:String}"; - $params[$paramName] = $this->formatParamValue($values[0]); - } + $filters[] = "{$escapedAttr} <= {{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $values[0] ?? null); break; case Query::TYPE_GREATER: $this->validateAttributeName($attribute); - $escapedAttr = $this->escapeIdentifier((string) $attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); $paramName = 'param_' . $paramCounter++; - if ($attribute === 'time') { - $filters[] = "{$escapedAttr} > {{$paramName}:DateTime64(3)}"; - /** @var \DateTime|string|null $val */ - $val = $values[0]; - $params[$paramName] = $this->formatDateTime($val); - } else { - $filters[] = "{$escapedAttr} > {{$paramName}:String}"; - $params[$paramName] = $this->formatParamValue($values[0]); - } + $filters[] = "{$escapedAttr} > {{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $values[0] ?? null); + break; + + case Query::TYPE_GREATER_EQUAL: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); + $paramName = 'param_' . $paramCounter++; + $filters[] = "{$escapedAttr} >= {{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $values[0] ?? null); break; case Query::TYPE_BETWEEN: $this->validateAttributeName($attribute); - $escapedAttr = $this->escapeIdentifier((string) $attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); $paramName1 = 'param_' . $paramCounter++; $paramName2 = 'param_' . $paramCounter++; - // Use DateTime64 type for time column, String for others - // This prevents type mismatch when comparing DateTime64 with timezone-suffixed strings - if ($attribute === 'time') { - $paramType = 'DateTime64(3)'; - $filters[] = "{$escapedAttr} BETWEEN {{$paramName1}:{$paramType}} AND {{$paramName2}:{$paramType}}"; - /** @var \DateTime|string|null $val1 */ - $val1 = $values[0]; - /** @var \DateTime|string|null $val2 */ - $val2 = $values[1]; - $params[$paramName1] = $this->formatDateTime($val1); - $params[$paramName2] = $this->formatDateTime($val2); - } else { - $filters[] = "{$escapedAttr} BETWEEN {{$paramName1}:String} AND {{$paramName2}:String}"; - $params[$paramName1] = $this->formatParamValue($values[0]); - $params[$paramName2] = $this->formatParamValue($values[1]); - } + $filters[] = "{$escapedAttr} BETWEEN {{$paramName1}:{$chType}} AND {{$paramName2}:{$chType}}"; + $params[$paramName1] = $this->formatTypedValue($chType, $values[0] ?? null); + $params[$paramName2] = $this->formatTypedValue($chType, $values[1] ?? null); + break; + + case Query::TYPE_NOT_BETWEEN: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); + $paramName1 = 'param_' . $paramCounter++; + $paramName2 = 'param_' . $paramCounter++; + $filters[] = "{$escapedAttr} NOT BETWEEN {{$paramName1}:{$chType}} AND {{$paramName2}:{$chType}}"; + $params[$paramName1] = $this->formatTypedValue($chType, $values[0] ?? null); + $params[$paramName2] = $this->formatTypedValue($chType, $values[1] ?? null); break; case Query::TYPE_CONTAINS: $this->validateAttributeName($attribute); - $escapedAttr = $this->escapeIdentifier((string) $attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); + $inParams = []; + foreach ($values as $value) { + $paramName = 'param_' . $paramCounter++; + $inParams[] = "{{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $value); + } + if (!empty($inParams)) { + $filters[] = "{$escapedAttr} IN (" . implode(', ', $inParams) . ")"; + } + break; + + case Query::TYPE_NOT_CONTAINS: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $chType = $this->getParamType($attribute); $inParams = []; foreach ($values as $value) { $paramName = 'param_' . $paramCounter++; - $inParams[] = "{{$paramName}:String}"; - $params[$paramName] = $this->formatParamValue($value); + $inParams[] = "{{$paramName}:{$chType}}"; + $params[$paramName] = $this->formatTypedValue($chType, $value); + } + if (!empty($inParams)) { + $filters[] = "{$escapedAttr} NOT IN (" . implode(', ', $inParams) . ")"; + } + break; + + case Query::TYPE_IS_NULL: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $filters[] = "{$escapedAttr} IS NULL"; + break; + + case Query::TYPE_IS_NOT_NULL: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $filters[] = "{$escapedAttr} IS NOT NULL"; + break; + + case Query::TYPE_STARTS_WITH: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $needle = $values[0] ?? null; + if (!is_string($needle)) { + throw new Exception("startsWith needle must be a string for attribute '{$attribute}'"); } - $filters[] = "{$escapedAttr} IN (" . implode(', ', $inParams) . ")"; + $paramName = 'param_' . $paramCounter++; + $filters[] = "startsWith({$escapedAttr}, {{$paramName}:String})"; + $params[$paramName] = $needle; + break; + + case Query::TYPE_ENDS_WITH: + $this->validateAttributeName($attribute); + $escapedAttr = $this->escapeIdentifier($attribute); + $needle = $values[0] ?? null; + if (!is_string($needle)) { + throw new Exception("endsWith needle must be a string for attribute '{$attribute}'"); + } + $paramName = 'param_' . $paramCounter++; + $filters[] = "endsWith({$escapedAttr}, {{$paramName}:String})"; + $params[$paramName] = $needle; break; case Query::TYPE_ORDER_DESC: @@ -1150,44 +1228,49 @@ private function normalizeCursorRow(mixed $rawCursor): array } /** - * Resolve the ClickHouse parameter type for a cursor attribute. + * Resolve the ClickHouse parameter type for a column. * - * Cursor keyset comparisons must bind values with the column's actual - * SQL type — binding a numeric column as `String` would compare values - * lexicographically ("9" > "10") and silently skip rows on page - * boundaries. Throws if the column is unknown or has an unsupported type - * so a misconfigured cursor fails loudly instead of producing incorrect - * pagination. + * Used by both filter binding and cursor keyset comparison so values are + * bound with the column's actual SQL type — binding a numeric column as + * `String` would compare values lexicographically (`"9" > "10"`) and + * silently produce incorrect filter results or page boundaries. Add a + * branch here when introducing a new non-String column type. * * @param string $attribute - * @return string ClickHouse parameter type (e.g. 'String', 'DateTime64(3)') - * @throws Exception + * @return string ClickHouse parameter type (e.g. 'String', 'DateTime64(3)', 'UInt64') */ - private function getCursorParamType(string $attribute): string + private function getParamType(string $attribute): string { - if ($attribute === 'id') { - return 'String'; - } - - $meta = $this->getAttribute($attribute); - if ($meta === null) { - throw new Exception("Cursor cannot order by unknown attribute '{$attribute}'"); - } - - $type = $meta['type'] ?? null; - - if ($type === Database::VAR_DATETIME) { - return 'DateTime64(3)'; - } + return match (true) { + $attribute === 'time' => 'DateTime64(3)', + $attribute === 'tenant' && $this->sharedTables => 'UInt64', + default => 'String', + }; + } - if ($type === Database::VAR_STRING) { - return 'String'; + /** + * Format a value for the given ClickHouse parameter type. + * + * Routes DateTime-typed columns through formatDateTime() and everything + * else through formatParamValue(). Centralising this dispatch keeps + * parseQueries and buildCursorWhere consistent across libraries. + * + * @param string $chType ClickHouse parameter type as returned by getParamType() + * @param mixed $value + * @return string + * @throws Exception + */ + private function formatTypedValue(string $chType, mixed $value): string + { + if ($chType === 'DateTime64(3)') { + if ($value === null) { + throw new Exception('DateTime parameter value cannot be null'); + } + /** @var \DateTime|string $value */ + return $this->formatDateTime($value); } - $observed = is_string($type) ? $type : get_debug_type($type); - throw new Exception( - "Cursor pagination does not support ordering by attribute '{$attribute}' of type '{$observed}'" - ); + return $this->formatParamValue($value); } /** @@ -1264,17 +1347,11 @@ private function buildCursorWhere(array $orderAttributes, array $cursor, string throw new Exception("Cursor value for '{$prevAttr}' cannot be null"); } $prevEscaped = $this->escapeIdentifier($prevAttr); - $prevType = $this->getCursorParamType($prevAttr); + $prevType = $this->getParamType($prevAttr); $paramName = "cursor_eq_{$i}_{$j}"; $conditions[] = "{$prevEscaped} = {{$paramName}:{$prevType}}"; - if ($prevType === 'DateTime64(3)') { - /** @var \DateTime|string $timeValue */ - $timeValue = $prevValue; - $params[$paramName] = $this->formatDateTime($timeValue); - } else { - $params[$paramName] = $this->formatParamValue($prevValue); - } + $params[$paramName] = $this->formatTypedValue($prevType, $prevValue); } $value = $cursor[$attr]; @@ -1282,18 +1359,12 @@ private function buildCursorWhere(array $orderAttributes, array $cursor, string throw new Exception("Cursor value for '{$attr}' cannot be null"); } $escaped = $this->escapeIdentifier($attr); - $chType = $this->getCursorParamType($attr); + $chType = $this->getParamType($attr); $operator = $direction === 'DESC' ? '<' : '>'; $paramName = "cursor_cmp_{$i}"; $conditions[] = "{$escaped} {$operator} {{$paramName}:{$chType}}"; - if ($chType === 'DateTime64(3)') { - /** @var \DateTime|string $timeValue */ - $timeValue = $value; - $params[$paramName] = $this->formatDateTime($timeValue); - } else { - $params[$paramName] = $this->formatParamValue($value); - } + $params[$paramName] = $this->formatTypedValue($chType, $value); $tuples[] = '(' . implode(' AND ', $conditions) . ')'; } diff --git a/tests/Audit/Adapter/ClickHouseTest.php b/tests/Audit/Adapter/ClickHouseTest.php index c351fa6..605097e 100644 --- a/tests/Audit/Adapter/ClickHouseTest.php +++ b/tests/Audit/Adapter/ClickHouseTest.php @@ -565,4 +565,98 @@ public function testCountByUserWithMaxBound(): void $bounded = $this->audit->countLogsByUser('userId', max: 1); $this->assertEquals(1, $bounded); } + + public function testNotEqualQuery(): void + { + // Fixture: 3x event=update/delete for userId, plus 1x event=insert for null user + $logs = $this->audit->find([ + Query::notEqual('event', 'update'), + ]); + // 1 delete + 1 insert = 2 + $this->assertCount(2, $logs); + foreach ($logs as $log) { + $this->assertNotEquals('update', $log->getEvent()); + } + } + + public function testNotContainsQuery(): void + { + $logs = $this->audit->find([ + Query::notContains('event', ['update', 'delete']), + ]); + // Only the insert log + $this->assertCount(1, $logs); + $this->assertEquals('insert', $logs[0]->getEvent()); + } + + public function testLesserEqualAndGreaterEqualQueries(): void + { + $now = (new \DateTime())->modify('+1 minute'); + $past = (new \DateTime())->modify('-1 hour'); + + $allLe = $this->audit->find([ + Query::lessThanEqual('time', \Utopia\Database\DateTime::format($now)), + ]); + $this->assertGreaterThanOrEqual(4, count($allLe)); + + $noneLe = $this->audit->find([ + Query::lessThanEqual('time', \Utopia\Database\DateTime::format($past)), + ]); + $this->assertCount(0, $noneLe); + + $allGe = $this->audit->find([ + Query::greaterThanEqual('time', \Utopia\Database\DateTime::format($past)), + ]); + $this->assertGreaterThanOrEqual(4, count($allGe)); + } + + public function testNotBetweenQuery(): void + { + $past = (new \DateTime())->modify('-2 hour'); + $oldPast = (new \DateTime())->modify('-3 hour'); + + $logs = $this->audit->find([ + Query::notBetween( + 'time', + \Utopia\Database\DateTime::format($oldPast), + \Utopia\Database\DateTime::format($past), + ), + ]); + // All 4 fixture logs are outside the past window + $this->assertGreaterThanOrEqual(4, count($logs)); + } + + public function testIsNullAndIsNotNullQueries(): void + { + $nullUser = $this->audit->find([ + Query::isNull('userId'), + ]); + // Only the insert log has null userId + $this->assertCount(1, $nullUser); + $this->assertEquals('insert', $nullUser[0]->getEvent()); + + $notNullUser = $this->audit->find([ + Query::isNotNull('userId'), + ]); + $this->assertCount(3, $notNullUser); + } + + public function testStartsWithAndEndsWithQueries(): void + { + $resourcePrefix = $this->audit->find([ + Query::startsWith('resource', 'database/'), + ]); + // 3 logs are on database/document/* + $this->assertCount(3, $resourcePrefix); + foreach ($resourcePrefix as $log) { + $this->assertStringStartsWith('database/', $log->getResource()); + } + + $endsWithNull = $this->audit->find([ + Query::endsWith('resource', '/null'), + ]); + // 'user/null' is the only match + $this->assertCount(1, $endsWithNull); + $this->assertEquals('user/null', $endsWithNull[0]->getResource()); + } } From 08d4c274d5c16dfb8b5702a0a82273c7470ec7b5 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 4 May 2026 01:57:46 +0000 Subject: [PATCH 5/5] fix: reject empty values for filter methods that require them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the validator pattern in utopia-php/database (Validator/Query/Filter.php): contains/notContains/equal/etc. queries must have at least one value; an empty values array is rejected up front with ` queries require at least one value.` instead of silently producing a "no filter applied" WHERE clause. Without the guard, `Query::contains('event', [])` would skip the IN clause entirely and return all rows — exactly the opposite of the intended IN () semantics, which should match nothing. This was the P1 flagged in greptile's review of cd5112f. Defines a VALUE_REQUIRED_METHODS allow-list checked at the top of the parseQueries loop, before the per-method switch. The same guard is being applied to utopia-php/usage's ClickHouse adapter so both libraries reject the same empty-value cases consistently. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Audit/Adapter/ClickHouse.php | 31 ++++++++++++++++++++++++++ tests/Audit/Adapter/ClickHouseTest.php | 30 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index 9ad5a4b..a50fa41 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -23,6 +23,28 @@ class ClickHouse extends SQL private const DEFAULT_DATABASE = 'default'; + /** + * Filter methods that must be supplied at least one value. Empty `values` + * arrays for these methods are rejected up front so they can't silently + * compile into a "no filter applied" WHERE clause. + * + * @var list + */ + private const VALUE_REQUIRED_METHODS = [ + Query::TYPE_EQUAL, + Query::TYPE_NOT_EQUAL, + Query::TYPE_LESSER, + Query::TYPE_LESSER_EQUAL, + Query::TYPE_GREATER, + Query::TYPE_GREATER_EQUAL, + Query::TYPE_BETWEEN, + Query::TYPE_NOT_BETWEEN, + Query::TYPE_CONTAINS, + Query::TYPE_NOT_CONTAINS, + Query::TYPE_STARTS_WITH, + Query::TYPE_ENDS_WITH, + ]; + private string $host; private int $port; @@ -966,6 +988,15 @@ private function parseQueries(array $queries): array /** @var string $attribute */ $values = $query->getValues(); + // Reject empty values for filter methods that take values — mirrors + // the validator in utopia-php/database (Validator/Query/Filter.php) + // and prevents silently dropping the WHERE fragment, which would + // otherwise turn `Query::contains('attr', [])` into a full-table + // match instead of an empty result. + if (\in_array($method, self::VALUE_REQUIRED_METHODS, true) && empty($values)) { + throw new \Exception(\ucfirst($method) . ' queries require at least one value.'); + } + switch ($method) { case Query::TYPE_EQUAL: $this->validateAttributeName($attribute); diff --git a/tests/Audit/Adapter/ClickHouseTest.php b/tests/Audit/Adapter/ClickHouseTest.php index 605097e..78d8e73 100644 --- a/tests/Audit/Adapter/ClickHouseTest.php +++ b/tests/Audit/Adapter/ClickHouseTest.php @@ -659,4 +659,34 @@ public function testStartsWithAndEndsWithQueries(): void $this->assertCount(1, $endsWithNull); $this->assertEquals('user/null', $endsWithNull[0]->getResource()); } + + public function testContainsRejectsEmptyValues(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('Contains queries require at least one value.'); + + $this->audit->find([ + Query::contains('event', []), + ]); + } + + public function testNotContainsRejectsEmptyValues(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('NotContains queries require at least one value.'); + + $this->audit->find([ + Query::notContains('event', []), + ]); + } + + public function testEqualRejectsEmptyValues(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('Equal queries require at least one value.'); + + $this->audit->find([ + new Query(Query::TYPE_EQUAL, 'event', []), + ]); + } }