diff --git a/README.md b/README.md index 1335b1436..5f755e5ad 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,8 @@ What an adapter supports is reported by `$database->getAdapter()->supports(Capab `Utopia\Database\PDO` wraps PHP's PDO: it reconnects when a connection is lost outside a transaction and retries the call. Each SQL adapter also accepts a plain `PDO`. Pass the PDO attributes your application needs; the ones below are what the adapters expect. +The adapters send one statement per call, so disable MySQL multi-statements (on by default) with `\Pdo\Mysql::ATTR_MULTI_STATEMENTS => false` (PHP 8.4+) for MariaDB and MySQL; an injected `;` then cannot run a second statement. + #### MariaDB ```php @@ -167,6 +169,7 @@ $pdo = new PDO("mysql:host={$host};port={$port};charset=utf8mb4", 'root', 'passw \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION, \PDO::ATTR_EMULATE_PREPARES => true, \PDO::ATTR_STRINGIFY_FETCHES => true, + \Pdo\Mysql::ATTR_MULTI_STATEMENTS => false, ]); $cache = new Cache(new Memory()); // or use any cache adapter you wish @@ -194,6 +197,7 @@ $pdo = new PDO("mysql:host={$host};port={$port};charset=utf8mb4", 'root', 'passw \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION, \PDO::ATTR_EMULATE_PREPARES => true, \PDO::ATTR_STRINGIFY_FETCHES => true, + \Pdo\Mysql::ATTR_MULTI_STATEMENTS => false, ]); $cache = new Cache(new Memory()); // or use any cache adapter you wish diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index ab968f0f0..f8ea65360 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -958,12 +958,6 @@ protected function getInternalKeyForAttribute(string $attribute): string } /** - * Process-lifetime cache for {@see self::filter()}. Keys are referentially - * stable across the request lifetime and frequently re-queried per-row, so - * caching the regex result amortizes the preg_replace cost across all - * decode/encode/build passes. Bounded to avoid unbounded growth from - * unusual input. - * * @var array */ private static array $filteredKeyCache = []; @@ -971,11 +965,17 @@ protected function getInternalKeyForAttribute(string $attribute): string private const int FILTERED_KEY_CACHE_LIMIT = 4096; /** - * Filter Keys - * * @throws DatabaseException */ public function filter(string $value): string + { + return self::filterKey($value); + } + + /** + * @throws DatabaseException + */ + public static function filterKey(string $value): string { if (isset(self::$filteredKeyCache[$value])) { return self::$filteredKeyCache[$value]; @@ -995,10 +995,6 @@ public function filter(string $value): string } /** - * The column an attribute a caller names is written to. filter() keeps underscores, so a name such as `_tenant` - * or `_ten.ant` would reach a column the storage keeps for the document itself and move the document to another - * tenant, or rewrite its id or timestamps. - * * @throws NotFoundException When the attribute names a reserved column */ protected function writableColumn(string $attribute): string @@ -1012,6 +1008,18 @@ protected function writableColumn(string $attribute): string return $column; } + /** + * @return array + */ + protected function writableAttributes(Document $document): array + { + return \array_filter( + $document->getAttributes(), + fn (int|string $key): bool => ! Storage::isReserved($this->filter((string) $key)), + \ARRAY_FILTER_USE_KEY + ); + } + /** * The row as every write hook decorates a row written for the document. * diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 7b86d6d00..8c8e1802a 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -315,9 +315,9 @@ public function deleteCollection(string $collection): bool $permissions = $schema->table($this->getTableRaw(Storage::permissionsTable($id)))->dropIfExists(); try { - return $this->executeStatement($main->query.'; '.$permissions->query, Event::CollectionDelete); - } catch (PDOException $e) { - $error = $this->processException($e); + return $this->executeStatements([$main->query, $permissions->query], Event::CollectionDelete); + } catch (PDOException $exception) { + $error = $this->processException($exception); if ($error instanceof NotFoundException) { $this->executeStatement($permissions->query, Event::CollectionDelete); } @@ -623,7 +623,7 @@ public function createDocument(Document $collection, Document $document): Docume $spatialAttributes = $this->getSpatialAttributes($collection); $collection = $collection->getId(); - $attributes = $document->getAttributes(); + $attributes = $this->writableAttributes($document); $attributes[Storage::CREATED_AT] = $document->getCreatedAt(); $attributes[Storage::UPDATED_AT] = $document->getUpdatedAt(); $attributes[Storage::PERMISSIONS] = \json_encode($document->getPermissions()); @@ -671,27 +671,27 @@ public function createDocument(Document $collection, Document $document): Docume $context = $this->writeContext(); try { $this->runWriteHooks(fn ($hook) => $hook->afterDocumentCreate($name, [$document], $context)); - } catch (PDOException $e) { - $isOrphanedPermission = $e->getCode() === '23000' - && isset($e->errorInfo[1]) - && $e->errorInfo[1] === 1062 - && \str_contains($e->getMessage(), Storage::INDEX_1); + } catch (PDOException $exception) { + $isOrphanedPermission = $exception->getCode() === '23000' + && isset($exception->errorInfo[1]) + && $exception->errorInfo[1] === 1062 + && \str_contains($exception->getMessage(), Storage::INDEX_1); if (! $isOrphanedPermission) { - throw $e; + throw $exception; } // Clean up orphaned permissions from a previous failed delete, then retry $cleanupBuilder = $this->newBuilder(Storage::permissionsTable($name)); $cleanupBuilder->filter([BaseQuery::equal(Storage::PERMISSIONS_DOCUMENT, [$document->getId()])]); $cleanupResult = $cleanupBuilder->delete(); - $cleanupStmt = $this->executeResult($cleanupResult, Event::PermissionsDelete); - $this->execute($cleanupStmt); + $cleanupStatement = $this->executeResult($cleanupResult, Event::PermissionsDelete); + $this->execute($cleanupStatement); $this->runWriteHooks(fn ($hook) => $hook->afterDocumentCreate($name, [$document], $context)); } - } catch (PDOException $e) { - throw $this->processException($e); + } catch (PDOException $exception) { + throw $this->processException($exception); } return $document; @@ -711,7 +711,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $spatialAttributes = $this->getSpatialAttributes($collection); $collection = $collection->getId(); - $attributes = $document->getAttributes(); + $attributes = $this->writableAttributes($document); $attributes[Storage::CREATED_AT] = $document->getCreatedAt(); $attributes[Storage::UPDATED_AT] = $document->getUpdatedAt(); $attributes[Storage::PERMISSIONS] = json_encode($document->getPermissions()); @@ -1065,23 +1065,6 @@ protected function decodePolygon(string $wkb): array */ private int $appliedRound = 0; - /** - * MySQL and MariaDB run every statement of a multi-statement string, but report only the first one's error from - * execute(); a later statement's error surfaces when its result is reached. Each result is reached here, so a - * statement that fails after another succeeded fails the call instead of being lost. - */ - #[\Override] - protected function executeStatement(string $sql, Event $event): bool - { - $statement = $this->prepareStatement($sql, $event); - $executed = $this->execute($statement); - - while ($statement->nextRowset()) { - } - - return $executed; - } - /** * @param PDOStatement|DatabasePDOStatement|PDOStatementProxy $statement */ diff --git a/src/Database/Adapter/Memory.php b/src/Database/Adapter/Memory.php index 9ead7a952..1d51bada9 100644 --- a/src/Database/Adapter/Memory.php +++ b/src/Database/Adapter/Memory.php @@ -1391,10 +1391,7 @@ public function updateDocument(Document $collection, string $id, Document $docum } [$oldKey, $existing] = $located; - // Resolve any Operator-typed attributes against the existing row before - // computing the new payload so unique-index checks see the post-update - // values, matching MariaDB's atomic UPDATE semantics. - $resolved = $this->applyOperators($document->getAttributes(), $existing); + $resolved = $this->applyOperators($this->writableAttributes($document), $existing); foreach ($resolved as $attribute => $value) { $document->setAttribute($attribute, $value); } @@ -1535,7 +1532,7 @@ public function updateDocuments(Document $collection, Document $updates, array $ throw new NotFoundException('Collection not found'); } - $attributes = $updates->getAttributes(); + $attributes = $this->writableAttributes($updates); $hasCreatedAt = ! empty($updates->getCreatedAt()); $hasUpdatedAt = ! empty($updates->getUpdatedAt()); $hasPermissions = $updates->offsetExists(Document::PERMISSIONS); @@ -1547,14 +1544,14 @@ public function updateDocuments(Document $collection, Document $updates, array $ // before any write lands, so a uniqueness violation on document N // does not leave documents 0..N-1 partially committed. $prepared = []; - foreach ($documents as $doc) { - $uid = $doc->getId(); - $docKey = $this->documentKey($uid); - if (! isset($this->data[$key]['documents'][$docKey])) { + foreach ($documents as $document) { + $uid = $document->getId(); + $documentKey = $this->documentKey($uid); + if (! isset($this->data[$key]['documents'][$documentKey])) { continue; } - $existingRow = $this->data[$key]['documents'][$docKey]; + $existingRow = $this->data[$key]['documents'][$documentKey]; // Resolve operators per-row — each document's existing values feed // back into operator evaluation, so $attributes cannot be evaluated @@ -1574,8 +1571,8 @@ public function updateDocuments(Document $collection, Document $updates, array $ $prepared[] = [ 'uid' => $uid, - 'docKey' => $docKey, - 'attrs' => $resolved, + 'documentKey' => $documentKey, + 'attributes' => $resolved, 'newSignatures' => $newSignatures, 'oldSignatures' => $oldSignatures, ]; @@ -1583,20 +1580,20 @@ public function updateDocuments(Document $collection, Document $updates, array $ // Phase-1: hashed unique-index validation. For each pending row, probe // the hash table — but recognise that any hashed entry currently - // pointing at the row's own docKey is about to be replaced by the new + // pointing at the row's own document key is about to be replaced by the new // signature, so it is not a conflict. We also build a per-batch // pending map so two siblings collapsing to the same value collide. $pendingByIndex = []; foreach ($prepared as $entry) { - $docKey = $entry['docKey']; + $documentKey = $entry['documentKey']; foreach ($entry['newSignatures'] as $indexId => $hash) { $existing = $this->uniqueIndexHashes[$key][$indexId][$hash] ?? null; - if ($existing !== null && $existing !== $docKey) { + if ($existing !== null && $existing !== $documentKey) { // The entry's own pre-update hash binding does not count // as a conflict — it'll be removed when its row is rewritten. $existingIsSelf = false; foreach ($prepared as $other) { - if ($other['docKey'] === $existing && ($other['oldSignatures'][$indexId] ?? null) === $hash) { + if ($other['documentKey'] === $existing && ($other['oldSignatures'][$indexId] ?? null) === $hash) { $existingIsSelf = true; break; } @@ -1605,22 +1602,22 @@ public function updateDocuments(Document $collection, Document $updates, array $ throw new UniqueException(UniqueException::MESSAGE); } } - if (isset($pendingByIndex[$indexId][$hash]) && $pendingByIndex[$indexId][$hash] !== $docKey) { + if (isset($pendingByIndex[$indexId][$hash]) && $pendingByIndex[$indexId][$hash] !== $documentKey) { throw new UniqueException(UniqueException::MESSAGE); } - $pendingByIndex[$indexId][$hash] = $docKey; + $pendingByIndex[$indexId][$hash] = $documentKey; } } $tenant = $this->getTenant(); foreach ($prepared as $entry) { $uid = $entry['uid']; - $docKey = $entry['docKey']; - $resolved = $entry['attrs']; + $documentKey = $entry['documentKey']; + $resolved = $entry['attributes']; - $previousRow = $this->data[$key]['documents'][$docKey]; + $previousRow = $this->data[$key]['documents'][$documentKey]; - $row = &$this->data[$key]['documents'][$docKey]; + $row = &$this->data[$key]['documents'][$documentKey]; foreach ($resolved as $attribute => $value) { $row[$this->filter($attribute)] = $value; } @@ -1636,8 +1633,8 @@ public function updateDocuments(Document $collection, Document $updates, array $ } unset($row); - $this->journal(function () use ($key, $docKey, $previousRow): void { - $this->data[$key]['documents'][$docKey] = $previousRow; + $this->journal(function () use ($key, $documentKey, $previousRow): void { + $this->data[$key]['documents'][$documentKey] = $previousRow; }); if ($hasPermissions) { @@ -1656,7 +1653,7 @@ public function updateDocuments(Document $collection, Document $updates, array $ $indexId, $entry['newSignatures'][$indexId] ?? null, $entry['oldSignatures'][$indexId] ?? null, - $docKey, + $documentKey, ); } } @@ -1899,15 +1896,15 @@ public function sum(Document $collection, string $attribute, array $queries = [] #[\Override] public function increaseDocumentAttribute(Document $collection, string $id, string $attribute, int|float|string $value, string $updatedAt, int|float|string|null $min = null, int|float|string|null $max = null): bool { + $column = $this->writableColumn($attribute); $key = $this->key($collection->getId()); - $docKey = $this->documentKey($id); - if (! isset($this->data[$key]['documents'][$docKey])) { + $documentKey = $this->documentKey($id); + if (! isset($this->data[$key]['documents'][$documentKey])) { throw new NotFoundException('Document not found'); } - $column = $this->filter($attribute); - $previousValue = $this->data[$key]['documents'][$docKey][$column] ?? null; - $previousUpdatedAt = $this->data[$key]['documents'][$docKey][Storage::UPDATED_AT] ?? null; + $previousValue = $this->data[$key]['documents'][$documentKey][$column] ?? null; + $previousUpdatedAt = $this->data[$key]['documents'][$documentKey][Storage::UPDATED_AT] ?? null; $current = $previousValue ?? 0; $exact = (\is_int($current) || (\is_string($current) && BigInt::isIntegerString($current))) && (\is_int($value) || (\is_string($value) && BigInt::isIntegerString($value))); @@ -1934,14 +1931,14 @@ public function increaseDocumentAttribute(Document $collection, string $id, stri $result = $current + $value; } - $this->data[$key]['documents'][$docKey][$column] = $result; - $this->data[$key]['documents'][$docKey][Storage::UPDATED_AT] = $updatedAt; + $this->data[$key]['documents'][$documentKey][$column] = $result; + $this->data[$key]['documents'][$documentKey][Storage::UPDATED_AT] = $updatedAt; - $this->journal(function () use ($key, $docKey, $column, $previousValue, $previousUpdatedAt): void { - if (! isset($this->data[$key]['documents'][$docKey])) { + $this->journal(function () use ($key, $documentKey, $column, $previousValue, $previousUpdatedAt): void { + if (! isset($this->data[$key]['documents'][$documentKey])) { return; } - $row = &$this->data[$key]['documents'][$docKey]; + $row = &$this->data[$key]['documents'][$documentKey]; if ($previousValue === null) { unset($row[$column]); } else { @@ -2026,10 +2023,7 @@ public function getAttributeWidth(Document $collection): int protected function documentToRow(Document $document): array { $row = []; - foreach ($document->getAttributes() as $attribute => $value) { - // Store native PHP values directly — no JSON encoding for arrays. - // The Database casting layer accepts already-decoded arrays - // (decodeArrayValue / decodeObjectValue both pass through arrays). + foreach ($this->writableAttributes($document) as $attribute => $value) { $row[$this->filter($attribute)] = $value; } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index aad6142d6..4cfecf7e9 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -209,13 +209,19 @@ public function capabilities(): array #[\Override] public function setTimeout(int $milliseconds, Event $event = Event::All): void { - $this->timeout = $milliseconds; + if ($milliseconds <= 0) { + $this->clearTimeout($event); + + return; + } + + $this->setTimeoutState($milliseconds, $event); } #[\Override] public function clearTimeout(Event $event = Event::All): void { - $this->timeout = 0; + $this->clearTimeoutState($event); } #[\Override] @@ -2414,8 +2420,9 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25 $options['skip'] = $offset; } - if ($this->timeout) { - $options['maxTimeMS'] = $this->timeout; + $timeout = $this->getTimeout(Event::DocumentFind); + if ($timeout > 0) { + $options['maxTimeMS'] = $timeout; } $selections = $this->getAttributeSelections($queries); @@ -2601,17 +2608,6 @@ public function count(Document $collection, array $queries = [], ?int $max = nul $this->escapeQueryAttributes($collection, $queries); - $filters = []; - $options = []; - - if (! \is_null($max) && $max > 0) { - $options['limit'] = $max; - } - - if ($this->timeout) { - $options['maxTimeMS'] = $this->timeout; - } - /** @var array $filters */ $filters = $this->buildFilters($queries); $filters = $this->applyReadFilters($filters, $collection->getId(), PermissionType::Read); @@ -2626,8 +2622,9 @@ public function count(Document $collection, array $queries = [], ?int $max = nul **/ $options = $this->getTransactionOptions(); - if ($this->timeout) { - $options['maxTimeMS'] = $this->timeout; + $timeout = $this->getTimeout(Event::DocumentCount); + if ($timeout > 0) { + $options['maxTimeMS'] = $timeout; } $pipeline = []; @@ -2719,8 +2716,9 @@ public function sum(Document $collection, string $attribute, array $queries = [] $options = $this->getTransactionOptions(); - if ($this->timeout) { - $options['maxTimeMS'] = $this->timeout; + $timeout = $this->getTimeout(Event::DocumentSum); + if ($timeout > 0) { + $options['maxTimeMS'] = $timeout; } try { diff --git a/src/Database/Adapter/Pool.php b/src/Database/Adapter/Pool.php index 2654813bf..97e110797 100644 --- a/src/Database/Adapter/Pool.php +++ b/src/Database/Adapter/Pool.php @@ -556,34 +556,28 @@ protected function syncTimeouts(Adapter $adapter): void } /** @var Adapter&Feature\Timeouts $adapter */ - if (empty($this->timeouts)) { + $global = $this->timeouts[Event::All->value] ?? null; + if ($global === null) { $adapter->clearTimeout(); - - return; } - if (count($this->timeouts) === 1 && isset($this->timeouts[Event::All->value])) { - $adapter->setTimeout($this->timeouts[Event::All->value]); + foreach (Event::cases() as $event) { + if ($event === Event::All) { + continue; + } - return; - } + $milliseconds = $this->timeouts[$event->value] ?? null; + if ($milliseconds === null) { + $adapter->clearTimeout($event); - // The concrete adapters keep one timeout scalar, which Postgres writes - // into SET statement_timeout and Mongo into maxTimeMS for every - // statement, so the last value applied is the one every statement runs - // under. Apply the per-event entries first and the global one last, or a - // per-event timeout set after the global one bounds everything. - $adapter->clearTimeout(); - foreach ($this->timeouts as $event => $milliseconds) { - if ($event === Event::All->value) { continue; } - $adapter->setTimeout($milliseconds, Event::from($event)); + $adapter->setTimeout($milliseconds, $event); } - if (isset($this->timeouts[Event::All->value])) { - $adapter->setTimeout($this->timeouts[Event::All->value]); + if ($global !== null) { + $adapter->setTimeout($global); } } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 162cfdb1f..9b22d47bc 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -62,7 +62,7 @@ class Postgres extends SQL implements Feature\Spatial, Feature\Timeouts private const string QUOTED_IDENTIFIER = '/["\x{AB}\x{BB}\x{201C}\x{201D}\x{201E}\x{300C}\x{300D}][\s\x{A0}\x{202F}]*([^"\x{AB}\x{BB}\x{201C}\x{201D}\x{201E}\x{300C}\x{300D}]+?)[\s\x{A0}\x{202F}]*["\x{AB}\x{BB}\x{201C}\x{201D}\x{201E}\x{300C}\x{300D}]/u'; - private const string HASHED_IDENTIFIER = '/^[0-9a-f]{32}(?:_[A-Za-z0-9_-]+)?$/'; + private const string HASHED_IDENTIFIER = '/^[0-9a-f]{32}(?:_[A-Za-z0-9_-]+)?$/D'; /** * The catalog's format_type() spellings mapped onto getSqlType()'s. @@ -270,8 +270,6 @@ public function createCollection(string $collection, array $attributes = [], arr $indexStatements[] = $schema->createIndex($tableRaw, $permissionsIndex, [Storage::PERMISSIONS], method: 'gin')->query; } - $collectionSql = $collectionResult->query.'; '.implode('; ', $indexStatements); - $permissionsTable = $schema->table($permissionsTableRaw); $permissionsTable->id(Storage::SEQUENCE); $permissionsTable->integer(Storage::TENANT)->nullable()->default(null); @@ -294,14 +292,19 @@ public function createCollection(string $collection, array $attributes = [], arr $permissionsIndexStatements[] = $schema->createIndex($permissionsTableRaw, $permissionIndex, [Storage::PERMISSIONS_PERMISSION, Storage::PERMISSIONS_TYPE], method: 'btree')->query; } - $permissionsSql = $permissionsResult->query.'; '.implode('; ', $permissionsIndexStatements); - $created = false; + $declaringIndexes = false; try { - $this->executeStatement($collectionSql, Event::CollectionCreate); + $this->executeStatement($collectionResult->query, Event::CollectionCreate); $created = true; - $this->executeStatement($permissionsSql, Event::CollectionCreate); + $this->executeStatements([ + ...$indexStatements, + $permissionsResult->query, + ...$permissionsIndexStatements, + ], Event::CollectionCreate); + + $declaringIndexes = true; foreach ($indexes as $index) { $indexAttributesWithType = []; @@ -328,7 +331,7 @@ public function createCollection(string $collection, array $attributes = [], arr $error = $this->processException($error); } - if ($created && ! ($error instanceof DuplicateException)) { + if ($created && ! ($declaringIndexes && $error instanceof DuplicateException)) { $this->discardCreatedCollection($id); } @@ -353,7 +356,7 @@ public function analyzeCollection(string $collection): bool $permissions = $schema->analyzeTable($this->getTableRaw(Storage::permissionsTable($name))); try { - return $this->executeStatement($main->query.'; '.$permissions->query, Event::CollectionUpdate); + return $this->executeStatements([$main->query, $permissions->query], Event::CollectionUpdate); } catch (PDOException $e) { throw $this->processException($e); } @@ -1106,7 +1109,7 @@ public function createDocument(Document $collection, Document $document): Docume $spatialAttributes = $this->getSpatialAttributes($collection); $collection = $collection->getId(); - $attributes = $document->getAttributes(); + $attributes = $this->writableAttributes($document); $attributes[Storage::CREATED_AT] = $document->getCreatedAt(); $attributes[Storage::UPDATED_AT] = $document->getUpdatedAt(); $attributes[Storage::PERMISSIONS] = \json_encode($document->getPermissions()); @@ -1171,7 +1174,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $spatialAttributes = $this->getSpatialAttributes($collection); $collection = $collection->getId(); - $attributes = $document->getAttributes(); + $attributes = $this->writableAttributes($document); $attributes[Storage::CREATED_AT] = $document->getCreatedAt(); $attributes[Storage::UPDATED_AT] = $document->getUpdatedAt(); $attributes[Storage::PERMISSIONS] = \json_encode($document->getPermissions()); @@ -1459,7 +1462,7 @@ protected function decodePolygon(string $wkb): array }, $rings); } - if (preg_match('/^[0-9a-fA-F]+$/', $wkb)) { + if (\preg_match('/^[0-9a-fA-F]+$/D', $wkb) === 1) { $wkb = hex2bin($wkb); if ($wkb === false) { throw new DatabaseException('Invalid hex WKB'); diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index a80614295..df407ea6d 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -908,7 +908,7 @@ private function buildDocumentsInsert(string $name, array $documents, array $spa } foreach ($documents as $document) { - foreach ($document->getAttributes() as $key => $value) { + foreach ($this->writableAttributes($document) as $key => $value) { $attributeKeySet[$key] = true; } } @@ -1134,7 +1134,7 @@ public function updateDocuments(Document $collection, Document $updates, array $ $spatialAttributes = $this->getSpatialAttributes($collection); $collection = $collection->getId(); - $attributes = $updates->getAttributes(); + $attributes = $this->writableAttributes($updates); if (! empty($updates->getUpdatedAt())) { $attributes[Storage::UPDATED_AT] = $updates->getUpdatedAt(); @@ -1156,9 +1156,6 @@ public function updateDocuments(Document $collection, Document $updates, array $ $builder = $this->newBuilder($name); - // Single pass over update attributes, bucketing into regular / spatial / - // operator and applying JSON / boolean conversions inline. Hoisted - // guards keep the hot path branch-light. $spatialMap = \array_fill_keys($spatialAttributes, true); $intBools = $this->supports(Capability::IntegerBooleans); @@ -1262,33 +1259,25 @@ public function upsertDocuments(Document $collection, array $changes, ?string $i $collection = $collection->getId(); $name = $this->filter($collection); + $extractions = \array_map( + fn (Change $change): array => Operator::extractOperators($this->writableAttributes($change->new)), + $changes + ); $hasOperators = false; - $firstChange = $changes[0]; - $firstDoc = $firstChange->new; - $firstExtracted = Operator::extractOperators($firstDoc->getAttributes()); - - if (! empty($firstExtracted['operators'])) { - $hasOperators = true; - } else { - foreach ($changes as $change) { - $doc = $change->new; - $extracted = Operator::extractOperators($doc->getAttributes()); - if (! empty($extracted['operators'])) { - $hasOperators = true; - break; - } + foreach ($extractions as $extracted) { + if (! empty($extracted['operators'])) { + $hasOperators = true; + break; } } if (! $hasOperators) { - $this->executeUpsertBatch($name, $changes, $spatialAttributes, $increase ?? '', [], $attributeDefaults, false); + $this->executeUpsertBatch($name, $changes, $extractions, $spatialAttributes, $increase ?? '', [], $attributeDefaults, false); } else { $groups = []; - foreach ($changes as $change) { - $document = $change->new; - $extracted = Operator::extractOperators($document->getAttributes()); - $operators = $extracted['operators']; + foreach ($changes as $index => $change) { + $operators = $extractions[$index]['operators']; if (empty($operators)) { $signature = 'no_ops'; @@ -1303,23 +1292,25 @@ public function upsertDocuments(Document $collection, array $changes, ?string $i if (! isset($groups[$signature])) { $groups[$signature] = [ - 'documents' => [], + 'changes' => [], + 'extractions' => [], 'operators' => $operators, ]; } - $groups[$signature]['documents'][] = $change; + $groups[$signature]['changes'][$index] = $change; + $groups[$signature]['extractions'][$index] = $extractions[$index]; } foreach ($groups as $group) { - $this->executeUpsertBatch($name, $group['documents'], $spatialAttributes, '', $group['operators'], $attributeDefaults, true); + $this->executeUpsertBatch($name, $group['changes'], $group['extractions'], $spatialAttributes, '', $group['operators'], $attributeDefaults, true); } } $context = $this->writeContext(); $this->runWriteHooks(fn ($hook) => $hook->afterDocumentUpsert($name, $changes, $context)); - } catch (PDOException $e) { - throw $this->processException($e); + } catch (PDOException $exception) { + throw $this->processException($exception); } return \array_map(static fn (Change $change): Document => $change->new, $changes); @@ -3089,16 +3080,7 @@ public function deleteCollection(string $collection): bool $main = $schema->table($this->getTableRaw($id))->drop(); $permissions = $schema->table($this->getTableRaw(Storage::permissionsTable($id)))->dropIfExists(); - try { - return $this->executeStatement($main->query.'; '.$permissions->query, Event::CollectionDelete); - } catch (PDOException $e) { - $error = $this->processException($e); - if ($error instanceof NotFoundException && $this->inTransaction === 0) { - $this->executeStatement($permissions->query, Event::CollectionDelete); - } - - throw $error; - } + return $this->executeConvergingStatements([$main->query, $permissions->query], Event::CollectionDelete, NotFoundException::class); } /** @@ -3120,7 +3102,7 @@ protected function dropCreatedCollection(string $id): void $main = $schema->table($this->getTableRaw($id))->dropIfExists(); $permissions = $schema->table($this->getTableRaw(Storage::permissionsTable($id)))->dropIfExists(); - $this->executeStatement($main->query.'; '.$permissions->query, Event::CollectionCreate); + $this->executeStatements([$main->query, $permissions->query], Event::CollectionCreate); } /** @@ -3145,18 +3127,16 @@ public function createRelationship(string $collection, Relationship $relationshi return $result->query; }; - $sql = match ($relationship->type) { - RelationshipType::OneToOne => $addColumn($name, $key) . ';' . ($relationship->twoWay ? $addColumn($relatedName, $twoWayKey) . ';' : ''), - RelationshipType::OneToMany => $addColumn($relatedName, $twoWayKey) . ';', - RelationshipType::ManyToOne => $addColumn($name, $key) . ';', - RelationshipType::ManyToMany => null, + $statements = match ($relationship->type) { + RelationshipType::OneToOne => $relationship->twoWay + ? [$addColumn($name, $key), $addColumn($relatedName, $twoWayKey)] + : [$addColumn($name, $key)], + RelationshipType::OneToMany => [$addColumn($relatedName, $twoWayKey)], + RelationshipType::ManyToOne => [$addColumn($name, $key)], + RelationshipType::ManyToMany => [], }; - if ($sql === null) { - return true; - } - - return $this->executeStatement($sql, Event::AttributeCreate); + return $this->executeConvergingStatements($statements, Event::AttributeCreate, DuplicateException::class); } /** @@ -3184,52 +3164,48 @@ public function updateRelationship(string $collection, Relationship $relationshi return $result->query; }; - $sql = ''; + $statements = []; switch ($relationship->type) { case RelationshipType::OneToOne: if (($twoWay || $side === RelationshipSide::Parent) && $newKey !== null && $key !== $newKey) { - $sql = $renameColumn($name, $key, $newKey) . ';'; + $statements[] = $renameColumn($name, $key, $newKey); } if (($twoWay || $side === RelationshipSide::Child) && $newTwoWayKey !== null && $twoWayKey !== $newTwoWayKey) { - $sql .= $renameColumn($relatedName, $twoWayKey, $newTwoWayKey) . ';'; + $statements[] = $renameColumn($relatedName, $twoWayKey, $newTwoWayKey); } break; case RelationshipType::OneToMany: if ($side === RelationshipSide::Parent) { if ($newTwoWayKey !== null && $twoWayKey !== $newTwoWayKey) { - $sql = $renameColumn($relatedName, $twoWayKey, $newTwoWayKey) . ';'; + $statements[] = $renameColumn($relatedName, $twoWayKey, $newTwoWayKey); } } elseif ($newKey !== null && $key !== $newKey) { - $sql = $renameColumn($name, $key, $newKey) . ';'; + $statements[] = $renameColumn($name, $key, $newKey); } break; case RelationshipType::ManyToOne: if ($side === RelationshipSide::Child) { if ($newTwoWayKey !== null && $twoWayKey !== $newTwoWayKey) { - $sql = $renameColumn($relatedName, $twoWayKey, $newTwoWayKey) . ';'; + $statements[] = $renameColumn($relatedName, $twoWayKey, $newTwoWayKey); } } elseif ($newKey !== null && $key !== $newKey) { - $sql = $renameColumn($name, $key, $newKey) . ';'; + $statements[] = $renameColumn($name, $key, $newKey); } break; case RelationshipType::ManyToMany: $junctionName = $this->getJunctionName($collection, $relationship->relatedCollection, $side); if ($newKey !== null && $key !== $newKey) { - $sql = $renameColumn($junctionName, $key, $newKey) . ';'; + $statements[] = $renameColumn($junctionName, $key, $newKey); } if ($newTwoWayKey !== null && $twoWayKey !== $newTwoWayKey) { - $sql .= $renameColumn($junctionName, $twoWayKey, $newTwoWayKey) . ';'; + $statements[] = $renameColumn($junctionName, $twoWayKey, $newTwoWayKey); } break; } - if ($sql === '') { - return true; - } - - return $this->executeStatement($sql, Event::AttributeUpdate); + return $this->executeStatements($statements, Event::AttributeUpdate); } /** @@ -3255,31 +3231,31 @@ public function deleteRelationship(string $collection, Relationship $relationshi return $result->query; }; - $sql = ''; + $statements = []; switch ($relationship->type) { case RelationshipType::OneToOne: if ($side === RelationshipSide::Parent) { - $sql = $dropColumn($name, $key) . ';'; + $statements[] = $dropColumn($name, $key); if ($twoWay) { - $sql .= $dropColumn($relatedName, $twoWayKey) . ';'; + $statements[] = $dropColumn($relatedName, $twoWayKey); } } else { - $sql = $dropColumn($relatedName, $twoWayKey) . ';'; + $statements[] = $dropColumn($relatedName, $twoWayKey); if ($twoWay) { - $sql .= $dropColumn($name, $key) . ';'; + $statements[] = $dropColumn($name, $key); } } break; case RelationshipType::OneToMany: - $sql = $side === RelationshipSide::Parent - ? $dropColumn($relatedName, $twoWayKey) . ';' - : $dropColumn($name, $key) . ';'; + $statements[] = $side === RelationshipSide::Parent + ? $dropColumn($relatedName, $twoWayKey) + : $dropColumn($name, $key); break; case RelationshipType::ManyToOne: - $sql = $side === RelationshipSide::Parent - ? $dropColumn($name, $key) . ';' - : $dropColumn($relatedName, $twoWayKey) . ';'; + $statements[] = $side === RelationshipSide::Parent + ? $dropColumn($name, $key) + : $dropColumn($relatedName, $twoWayKey); break; case RelationshipType::ManyToMany: $junctionName = $this->getJunctionName($collection, $relationship->relatedCollection, $side); @@ -3287,11 +3263,12 @@ public function deleteRelationship(string $collection, Relationship $relationshi $junctionResult = $schema->table($this->getTableRaw($junctionName))->drop(); $permissionsResult = $schema->table($this->getTableRaw(Storage::permissionsTable($junctionName)))->drop(); - $sql = $junctionResult->query . '; ' . $permissionsResult->query; + $statements[] = $junctionResult->query; + $statements[] = $permissionsResult->query; break; } - return $this->executeStatement($sql, Event::AttributeDelete); + return $this->executeConvergingStatements($statements, Event::AttributeDelete, NotFoundException::class); } /** @@ -3884,6 +3861,55 @@ protected function executeStatement(string $sql, Event $event): bool return $this->execute($this->prepareStatement($sql, $event)); } + /** + * @param list $statements + */ + protected function executeStatements(array $statements, Event $event): bool + { + foreach ($statements as $statement) { + if (! $this->executeStatement($statement, $event)) { + return false; + } + } + + return true; + } + + /** + * Inside a transaction the first refusal is thrown at once, since PostgreSQL refuses every later statement of an + * aborted transaction. + * + * @param list $statements + * @param class-string $applied + * + * @throws DatabaseException + */ + protected function executeConvergingStatements(array $statements, Event $event, string $applied): bool + { + $refusal = null; + + foreach ($statements as $statement) { + try { + if (! $this->executeStatement($statement, $event)) { + return false; + } + } catch (PDOException $exception) { + $error = $this->processException($exception); + if (! $error instanceof $applied || $this->inTransaction > 0) { + throw $error; + } + + $refusal ??= $error; + } + } + + if ($refusal !== null) { + throw $refusal; + } + + return true; + } + private function transformQuery(Event $event, string $sql): string { foreach ($this->transforms as $transform) { @@ -3900,6 +3926,7 @@ private function transformQuery(Event $event, string $sql): string * * @param string $name The filtered collection name * @param array $changes The changes to upsert + * @param array, updates: array}> $extractions Each change's writable attributes split into operators and updates, keyed as $changes * @param list $spatialAttributes Spatial column names * @param string $attribute Increment attribute name (empty if none) * @param array $operators Operator map keyed by attribute name @@ -3910,6 +3937,7 @@ private function transformQuery(Event $event, string $sql): string protected function executeUpsertBatch( string $name, array $changes, + array $extractions, array $spatialAttributes, string $attribute, array $operators, @@ -3932,13 +3960,12 @@ protected function executeUpsertBatch( $allColumnNames = []; $documentsData = []; - foreach ($changes as $change) { + foreach ($changes as $index => $change) { $document = $change->new; + $currentRegularAttributes = $extractions[$index]['updates']; if ($hasOperators) { - $extracted = Operator::extractOperators($document->getAttributes()); - $currentRegularAttributes = $extracted['updates']; - $extractedOperators = $extracted['operators']; + $extractedOperators = $extractions[$index]['operators']; if ($change->old->isEmpty() && ! empty($extractedOperators)) { foreach ($extractedOperators as $operatorKey => $operator) { @@ -3951,7 +3978,6 @@ protected function executeUpsertBatch( $currentRegularAttributes[Storage::CREATED_AT] = $document->getCreatedAt() ? $document->getCreatedAt() : null; $currentRegularAttributes[Storage::UPDATED_AT] = $document->getUpdatedAt() ? $document->getUpdatedAt() : null; } else { - $currentRegularAttributes = $document->getAttributes(); $currentRegularAttributes[Storage::UID] = $document->getId(); $currentRegularAttributes[Storage::CREATED_AT] = $document->getCreatedAt() ? DateTime::setTimezone($document->getCreatedAt()) : null; $currentRegularAttributes[Storage::UPDATED_AT] = $document->getUpdatedAt() ? DateTime::setTimezone($document->getUpdatedAt()) : null; @@ -6535,7 +6561,7 @@ private function remapRow(array &$row): void */ protected function buildDocumentRow(Document $document, array $attributeKeys, array $spatialMap = [], ?bool $intBools = null): array { - $attributes = $document->getAttributes(); + $attributes = $this->writableAttributes($document); $row = [ Storage::UID => $document->getId(), Storage::CREATED_AT => $document->getCreatedAt(), diff --git a/src/Database/Adapter/SQL/Hook/Column/AllowNull.php b/src/Database/Adapter/SQL/Hook/Column/AllowNull.php index 8e8339fd0..ce1d14aa0 100644 --- a/src/Database/Adapter/SQL/Hook/Column/AllowNull.php +++ b/src/Database/Adapter/SQL/Hook/Column/AllowNull.php @@ -9,7 +9,7 @@ final readonly class AllowNull implements Filter { - private const string IDENTIFIER_PATTERN = '/^[a-zA-Z0-9_\-][a-zA-Z0-9_.\-]*$/'; + private const string IDENTIFIER_PATTERN = '/^[a-zA-Z0-9_\-][a-zA-Z0-9_.\-]*$/D'; public function __construct( private Filter $filter, diff --git a/src/Database/Adapter/SQL/Hook/Permission/Filter.php b/src/Database/Adapter/SQL/Hook/Permission/Filter.php index 8e8bbaff1..b0e2bcc9d 100644 --- a/src/Database/Adapter/SQL/Hook/Permission/Filter.php +++ b/src/Database/Adapter/SQL/Hook/Permission/Filter.php @@ -21,13 +21,13 @@ */ readonly class Filter implements FilterHook, JoinFilter { - private const string IDENTIFIER_PATTERN = '/^[a-zA-Z_][a-zA-Z0-9_.\-]*$/'; + private const string IDENTIFIER_PATTERN = '/^[a-zA-Z_][a-zA-Z0-9_.\-]*$/D'; - private const string QUOTED_IDENTIFIER_PATTERN = '/^[a-zA-Z0-9_\-][a-zA-Z0-9_.\-]*$/'; + private const string QUOTED_IDENTIFIER_PATTERN = '/^[a-zA-Z0-9_\-][a-zA-Z0-9_.\-]*$/D'; private const string NO_SEMIJOIN = '/*+ NO_SEMIJOIN() */ '; - private const string COLLATION_PATTERN = '/^[a-zA-Z_][a-zA-Z0-9_]*$/'; + private const string COLLATION_PATTERN = '/^[a-zA-Z_][a-zA-Z0-9_]*$/D'; protected string $documentCollation; diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 13c453b1f..cb76fd27f 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -670,19 +670,17 @@ public function deleteCollection(string $collection): bool { $id = $this->filter($collection); + $statements = []; + // FTS5 shadow tables don't drop with the parent. foreach ($this->findFulltextTables($id) as $ftsTable) { - $sql = 'DROP TABLE IF EXISTS '.$this->quote($ftsTable); - $this->execute($this->prepare($sql, event: Event::CollectionDelete)); + $statements[] = 'DROP TABLE IF EXISTS '.$this->quote($ftsTable); } - $sql = "DROP TABLE IF EXISTS {$this->getTable($id)}"; - - $this->execute($this->prepare($sql, event: Event::CollectionDelete)); + $statements[] = "DROP TABLE IF EXISTS {$this->getTable($id)}"; + $statements[] = "DROP TABLE IF EXISTS {$this->getTable(Storage::permissionsTable($id))}"; - $sql = "DROP TABLE IF EXISTS {$this->getTable(Storage::permissionsTable($id))}"; - - $this->execute($this->prepare($sql, event: Event::CollectionDelete)); + $this->executeStatements($statements, Event::CollectionDelete); unset($this->ftsTableCache[$this->getFulltextTablePrefix($id)]); @@ -1213,7 +1211,7 @@ public function createDocument(Document $collection, Document $document): Docume $this->syncWriteHooks(); $collection = $collection->getId(); - $attributes = $document->getAttributes(); + $attributes = $this->writableAttributes($document); $attributes[Storage::CREATED_AT] = $document->getCreatedAt(); $attributes[Storage::UPDATED_AT] = $document->getUpdatedAt(); $attributes[Storage::PERMISSIONS] = json_encode($document->getPermissions()); @@ -1272,7 +1270,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $spatialAttributes = $this->getSpatialAttributes($collection); $collection = $collection->getId(); - $attributes = $document->getAttributes(); + $attributes = $this->writableAttributes($document); $attributes[Storage::CREATED_AT] = $document->getCreatedAt(); $attributes[Storage::UPDATED_AT] = $document->getUpdatedAt(); $attributes[Storage::PERMISSIONS] = json_encode($document->getPermissions()); @@ -2249,6 +2247,7 @@ protected function getConflictTenantIncrementExpression(string $column): string * * @param string $name The filtered collection name * @param array $changes The changes to upsert + * @param array, updates: array}> $extractions Each change's writable attributes split into operators and updates, keyed as $changes * @param array $spatialAttributes Spatial column names * @param string $attribute Increment attribute name (empty if none) * @param array $operators Operator map keyed by attribute name @@ -2260,6 +2259,7 @@ protected function getConflictTenantIncrementExpression(string $column): string protected function executeUpsertBatch( string $name, array $changes, + array $extractions, array $spatialAttributes, string $attribute, array $operators, @@ -2272,13 +2272,12 @@ protected function executeUpsertBatch( $allColumnNames = []; $documentsData = []; - foreach ($changes as $change) { + foreach ($changes as $index => $change) { $document = $change->new; + $currentRegularAttributes = $extractions[$index]['updates']; if ($hasOperators) { - $extracted = Operator::extractOperators($document->getAttributes()); - $currentRegularAttributes = $extracted['updates']; - $extractedOperators = $extracted['operators']; + $extractedOperators = $extractions[$index]['operators']; if ($change->old->isEmpty() && ! empty($extractedOperators)) { foreach ($extractedOperators as $operatorKey => $operator) { @@ -2295,7 +2294,6 @@ protected function executeUpsertBatch( $currentRegularAttributes[Storage::CREATED_AT] = $document->getCreatedAt() ? $document->getCreatedAt() : null; $currentRegularAttributes[Storage::UPDATED_AT] = $document->getUpdatedAt() ? $document->getUpdatedAt() : null; } else { - $currentRegularAttributes = $document->getAttributes(); $currentRegularAttributes[Storage::UID] = $document->getId(); $currentRegularAttributes[Storage::CREATED_AT] = $document->getCreatedAt() ? DatabaseDateTime::setTimezone($document->getCreatedAt()) : null; $currentRegularAttributes[Storage::UPDATED_AT] = $document->getUpdatedAt() ? DatabaseDateTime::setTimezone($document->getUpdatedAt()) : null; @@ -2309,15 +2307,15 @@ protected function executeUpsertBatch( $currentRegularAttributes = $this->decorateRow($currentRegularAttributes, $document); - foreach (\array_keys($currentRegularAttributes) as $colName) { - $allColumnNames[$colName] = true; + foreach (\array_keys($currentRegularAttributes) as $column) { + $allColumnNames[$column] = true; } $documentsData[] = ['regularAttributes' => $currentRegularAttributes]; } - foreach (\array_keys($operators) as $colName) { - $allColumnNames[$colName] = true; + foreach (\array_keys($operators) as $column) { + $allColumnNames[$column] = true; } $allColumnNames = \array_keys($allColumnNames); @@ -2358,8 +2356,8 @@ protected function executeUpsertBatch( } $regularAttributes = []; - foreach ($allColumnNames as $colName) { - $regularAttributes[$colName] = null; + foreach ($allColumnNames as $column) { + $regularAttributes[$column] = null; } foreach ($documentsData[0]['regularAttributes'] as $key => $value) { $regularAttributes[$key] = $value; @@ -2468,13 +2466,6 @@ public function createAttributes(string $collection, array $attributes): bool return true; } - /** - * SQL::createRelationship concatenates multiple ALTER TABLE statements - * with `;` and runs them through a single prepare/execute, which only - * works because MySQL accepts multi-statement queries. SQLite's PDO - * driver runs the first statement and silently drops the rest, so - * re-implement the dispatch with one statement per call. - */ #[\Override] public function createRelationship(string $collection, Relationship $relationship): bool { @@ -2496,11 +2487,7 @@ public function createRelationship(string $collection, Relationship $relationshi RelationshipType::ManyToMany => [], }; - foreach ($statements as $statement) { - $this->execute($this->prepare($statement, event: Event::AttributeCreate)); - } - - return true; + return $this->executeConvergingStatements($statements, Event::AttributeCreate, DuplicateException::class); } #[\Override] @@ -2555,11 +2542,7 @@ public function updateRelationship(string $collection, Relationship $relationshi break; } - foreach ($statements as $statement) { - $this->execute($this->prepare($statement, event: Event::AttributeUpdate)); - } - - return true; + return $this->executeStatements($statements, Event::AttributeUpdate); } #[\Override] @@ -2605,11 +2588,7 @@ public function deleteRelationship(string $collection, Relationship $relationshi break; } - foreach ($statements as $statement) { - $this->execute($this->prepare($statement, event: Event::AttributeDelete)); - } - - return true; + return $this->executeConvergingStatements($statements, Event::AttributeDelete, NotFoundException::class); } /** diff --git a/src/Database/Validator/BigInt.php b/src/Database/Validator/BigInt.php index 3f95dfd36..94928406a 100644 --- a/src/Database/Validator/BigInt.php +++ b/src/Database/Validator/BigInt.php @@ -64,7 +64,7 @@ public function isValid(mixed $value): bool public static function isIntegerString(string $value, bool $signed = true): bool { - return \preg_match($signed ? '/^-?\d+$/' : '/^\d+$/', $value) === 1; + return \preg_match($signed ? '/^-?\d+$/D' : '/^\d+$/D', $value) === 1; } /** @@ -390,7 +390,7 @@ public static function integralValue(int|float|string $value): int|string|null } if (\is_string($value)) { - return \preg_match('/^(-?\d+)\.0+$/', $value, $matches) === 1 ? self::toNative($matches[1]) : null; + return \preg_match('/^(-?\d+)\.0+$/D', $value, $matches) === 1 ? self::toNative($matches[1]) : null; } if (! \is_float($value) || ! \is_finite($value) || \floor($value) !== $value) { diff --git a/src/Database/Validator/Query/Aggregate.php b/src/Database/Validator/Query/Aggregate.php index cf197460c..02ef52f7c 100644 --- a/src/Database/Validator/Query/Aggregate.php +++ b/src/Database/Validator/Query/Aggregate.php @@ -20,7 +20,7 @@ class Aggregate extends Base public const int MAX_ALIAS_LENGTH = 63; - private const string ALIAS_PATTERN = '/^[A-Za-z_][A-Za-z0-9_]*$/'; + private const string ALIAS_PATTERN = '/^[A-Za-z_][A-Za-z0-9_]*$/D'; private const array NUMERIC_METHODS = [ Method::Sum, diff --git a/src/Database/Validator/Query/Base.php b/src/Database/Validator/Query/Base.php index ffbe0e665..dfafd8c68 100644 --- a/src/Database/Validator/Query/Base.php +++ b/src/Database/Validator/Query/Base.php @@ -90,7 +90,7 @@ protected function isValidQuery(Query $query): bool */ protected function isAllowedJoinColumn(string $column): bool { - return $column !== '' && \preg_match('/^[A-Za-z_$][A-Za-z0-9_$]*$/', $column) === 1; + return $column !== '' && \preg_match('/^[A-Za-z_$][A-Za-z0-9_$]*$/D', $column) === 1; } abstract public function getMethodType(): string; diff --git a/src/Database/Validator/Query/Join.php b/src/Database/Validator/Query/Join.php index b3b229e92..b8bafccd7 100644 --- a/src/Database/Validator/Query/Join.php +++ b/src/Database/Validator/Query/Join.php @@ -18,7 +18,7 @@ class Join extends Base /** * The characters of an Appwrite ID but `.`, which separates an alias from its column. */ - public const string ALIAS_PATTERN = '/^[A-Za-z0-9_][A-Za-z0-9_-]*$/'; + public const string ALIAS_PATTERN = '/^[A-Za-z0-9_][A-Za-z0-9_-]*$/D'; /** * PostgreSQL folds an identifier past 63 bytes, so two longer aliases could read as one. diff --git a/src/Database/Validator/Sequence.php b/src/Database/Validator/Sequence.php index 59153747a..88ac6f865 100644 --- a/src/Database/Validator/Sequence.php +++ b/src/Database/Validator/Sequence.php @@ -65,7 +65,7 @@ public function isValid(mixed $value): bool $idType = ColumnType::tryFrom($this->idAttributeType); return match ($idType) { - ColumnType::Uuid7 => \is_string($value) && preg_match('/^[a-f0-9]{8}-[a-f0-9]{4}-7[a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$/i', $value) === 1, + ColumnType::Uuid7 => \is_string($value) && \preg_match('/^[a-f0-9]{8}-[a-f0-9]{4}-7[a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$/Di', $value) === 1, ColumnType::Integer => (new Range(1, Database::MAX_BIG_INT, ColumnType::Integer->value))->isValid($value), default => false, }; diff --git a/src/Database/Validator/Structure.php b/src/Database/Validator/Structure.php index 261c22f53..67661ce72 100644 --- a/src/Database/Validator/Structure.php +++ b/src/Database/Validator/Structure.php @@ -4,6 +4,7 @@ use Closure; use Exception; +use Utopia\Database\Adapter; use Utopia\Database\Adapter\Profile; use Utopia\Database\Attribute; use Utopia\Database\Capability; @@ -237,10 +238,8 @@ private function isStoredAsNull(string $key): bool */ protected function checkForUnknownAttributes(array $structure, array $definitions): bool { - // A key named like a column the storage keeps for the document itself, even one declared as an attribute, - // would be written over the document's own $sequence, $tenant or $permissions. foreach (\array_keys($structure) as $key) { - if (Storage::isReserved((string) $key)) { + if (Storage::isReserved(Adapter::filterKey((string) $key))) { $this->message = 'Reserved attribute: "'.$key.'"'; return false; diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 7f7a1080f..0cb953b1c 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -3,6 +3,7 @@ namespace Tests\E2E\Adapter; use PDO; +use Pdo\Mysql as MySQLPDO; use PHPUnit\Framework\TestCase; use Tests\E2E\Adapter\Scopes\AggregationTests; use Tests\E2E\Adapter\Scopes\AttributeTests; @@ -88,6 +89,13 @@ abstract class Base extends TestCase PDO::ATTR_STRINGIFY_FETCHES => true, ]; + /** + * @var array + */ + protected const array MYSQL_PDO_ATTRIBUTES = [ + MySQLPDO::ATTR_MULTI_STATEMENTS => false, + ] + self::PDO_ATTRIBUTES; + protected static string $namespace; protected static ?Authorization $authorization = null; diff --git a/tests/e2e/Adapter/MariaDBTest.php b/tests/e2e/Adapter/MariaDBTest.php index dbea51b47..498e89d46 100644 --- a/tests/e2e/Adapter/MariaDBTest.php +++ b/tests/e2e/Adapter/MariaDBTest.php @@ -44,7 +44,7 @@ public function getDatabase(bool $fresh = false): Database $dbUser = 'root'; $dbPass = 'password'; - $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::PDO_ATTRIBUTES); + $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::MYSQL_PDO_ATTRIBUTES); $redis = new Redis(); $redis->connect('redis', 6379); @@ -157,7 +157,7 @@ private function connect(): Database 'mysql:host=mariadb;port=3306;charset=utf8mb4', 'root', 'password', - [PhpPDO::ATTR_PERSISTENT => false] + self::PDO_ATTRIBUTES, + [PhpPDO::ATTR_PERSISTENT => false] + self::MYSQL_PDO_ATTRIBUTES, ); $main = $this->getDatabase(); diff --git a/tests/e2e/Adapter/MirrorTest.php b/tests/e2e/Adapter/MirrorTest.php index 2d3d0725a..11a7485ea 100644 --- a/tests/e2e/Adapter/MirrorTest.php +++ b/tests/e2e/Adapter/MirrorTest.php @@ -53,7 +53,7 @@ protected function getDatabase(bool $fresh = false): Mirror $dbUser = 'root'; $dbPass = 'password'; - $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::PDO_ATTRIBUTES); + $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::MYSQL_PDO_ATTRIBUTES); $redis = new Redis(); $redis->connect('redis'); @@ -68,7 +68,7 @@ protected function getDatabase(bool $fresh = false): Mirror $mirrorUser = 'root'; $mirrorPass = 'password'; - $mirrorPdo = new PDO("mysql:host={$mirrorHost};port={$mirrorPort};charset=utf8mb4", $mirrorUser, $mirrorPass, self::PDO_ATTRIBUTES); + $mirrorPdo = new PDO("mysql:host={$mirrorHost};port={$mirrorPort};charset=utf8mb4", $mirrorUser, $mirrorPass, self::MYSQL_PDO_ATTRIBUTES); $mirrorRedis = new Redis(); $mirrorRedis->connect('redis-mirror'); diff --git a/tests/e2e/Adapter/MySQLTest.php b/tests/e2e/Adapter/MySQLTest.php index 5eaf3d034..1d6f4bebb 100644 --- a/tests/e2e/Adapter/MySQLTest.php +++ b/tests/e2e/Adapter/MySQLTest.php @@ -41,7 +41,7 @@ public function getDatabase(): Database $dbUser = 'root'; $dbPass = 'password'; - $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::PDO_ATTRIBUTES); + $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::MYSQL_PDO_ATTRIBUTES); $redis = new Redis(); $redis->connect('redis', 6379); diff --git a/tests/e2e/Adapter/PoolTest.php b/tests/e2e/Adapter/PoolTest.php index 99a6de45e..2484e6151 100644 --- a/tests/e2e/Adapter/PoolTest.php +++ b/tests/e2e/Adapter/PoolTest.php @@ -60,7 +60,7 @@ public function getDatabase(): Database dsn: "mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", username: $dbUser, password: $dbPass, - config: self::PDO_ATTRIBUTES, + config: self::MYSQL_PDO_ATTRIBUTES, )); }, timeout: 0.0); diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 6f619144b..c081da149 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -6462,6 +6462,106 @@ public function testIncreaseOfAStorageColumnIsRefused(): void $database->deleteCollection($collection); } + private function createStorageColumnKeyCollection(Database $database, string $collection): void + { + $database->createCollection(Collection::create( + id: $collection, + attributes: $database->getAdapter()->supports(Capability::DefinedAttributes) ? [Attribute::string(key: 'name', size: 64, required: false)] : [], + permissions: [Permission::create(Role::any()), Permission::read(Role::any()), Permission::update(Role::any())], + documentSecurity: true, + )); + } + + public function testStorageColumnKeyIsNotWrittenOnCreate(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $collection = 'storageKeyCreate'; + $this->createStorageColumnKeyCollection($database, $collection); + + $database->skipValidation(function () use ($database, $collection): void { + $database->createDocument($collection, new Document(['$id' => 'plain', Storage::UID => 'hijacked', 'name' => 'plain'])); + $database->createDocument($collection, new Document(['$id' => 'mangled', '_u!id' => 'hijacked', 'name' => 'mangled'])); + $database->createDocument($collection, new Document(['$id' => 'sequenced', Storage::SEQUENCE => 777777, 'name' => 'sequenced'])); + $database->createDocuments($collection, [ + new Document(['$id' => 'batched', '_u!id' => 'hijacked', 'name' => 'batched']), + new Document(['$id' => 'batchedSequence', Storage::SEQUENCE => 888888, 'name' => 'batchedSequence']), + ]); + }); + + foreach (['plain', 'mangled', 'sequenced', 'batched', 'batchedSequence'] as $id) { + $document = $database->getDocument($collection, $id); + $this->assertSame($id, $document->getId(), "A storage-column key must not rewrite the id of {$id}"); + $this->assertSame($id, $document->getAttribute('name')); + } + $this->assertTrue($database->getDocument($collection, 'hijacked')->isEmpty()); + $this->assertNotSame('777777', (string) $database->getDocument($collection, 'sequenced')->getSequence()); + $this->assertNotSame('888888', (string) $database->getDocument($collection, 'batchedSequence')->getSequence()); + + $database->deleteCollection($collection); + } + + public function testStorageColumnKeyIsNotWrittenOnUpdate(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $collection = 'storageKeyUpdate'; + $this->createStorageColumnKeyCollection($database, $collection); + $permissions = [Permission::read(Role::any()), Permission::update(Role::any())]; + $database->createDocument($collection, new Document(['$id' => 'target', '$permissions' => $permissions, 'name' => 'before'])); + + $database->skipValidation(fn (): Document => $database->updateDocument( + $collection, + 'target', + new Document(['$id' => 'target', '_u!id' => 'hijacked', 'name' => 'after']), + )); + + $this->assertTrue($database->getDocument($collection, 'hijacked')->isEmpty()); + $document = $database->getDocument($collection, 'target'); + $this->assertSame('target', $document->getId()); + $this->assertSame('after', $document->getAttribute('name')); + + $database->skipValidation(fn (): int => $database->updateDocuments( + $collection, + new Document([Storage::PERMISSIONS => \json_encode([Permission::read(Role::user('hijacker'))]), 'name' => 'bulk']), + )); + + $document = $database->getDocument($collection, 'target'); + $this->assertSame('bulk', $document->getAttribute('name')); + $this->assertSame($permissions, $document->getPermissions()); + + $database->deleteCollection($collection); + } + + public function testStorageColumnKeyIsNotWrittenOnUpsert(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (! $database->getAdapter()->hasFeature(Feature\Upserts::class)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $collection = 'storageKeyUpsert'; + $this->createStorageColumnKeyCollection($database, $collection); + + $upserted = ['$id' => 'upserted', '$permissions' => [Permission::read(Role::any())], Storage::SEQUENCE => 999999, 'name' => 'upserted']; + if ($database->hasSharedTables()) { + $upserted['_ten!ant'] = (int) $database->getTenant() + 1; + } + + $database->skipValidation(fn (): int => $database->upsertDocuments($collection, [new Document($upserted)])); + + $document = $database->getDocument($collection, 'upserted'); + $this->assertSame('upserted', $document->getId(), 'A storage-column key must not move the document to another tenant'); + $this->assertSame('upserted', $document->getAttribute('name')); + $this->assertNotSame('999999', (string) $document->getSequence()); + + $database->deleteCollection($collection); + } + public function testIncreaseDecreasePreserveDates(): void { $document = $this->initIncreaseDecreaseFixture(); diff --git a/tests/e2e/Adapter/SharedTables/MariaDBTest.php b/tests/e2e/Adapter/SharedTables/MariaDBTest.php index 25278ca71..07ec6a47a 100644 --- a/tests/e2e/Adapter/SharedTables/MariaDBTest.php +++ b/tests/e2e/Adapter/SharedTables/MariaDBTest.php @@ -40,7 +40,7 @@ public function getDatabase(bool $fresh = false): Database $dbUser = 'root'; $dbPass = 'password'; - $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::PDO_ATTRIBUTES); + $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::MYSQL_PDO_ATTRIBUTES); $redis = new Redis(); $redis->connect('redis', 6379); $redis->select(7); diff --git a/tests/e2e/Adapter/SharedTables/MySQLTest.php b/tests/e2e/Adapter/SharedTables/MySQLTest.php index d910b5928..616c18524 100644 --- a/tests/e2e/Adapter/SharedTables/MySQLTest.php +++ b/tests/e2e/Adapter/SharedTables/MySQLTest.php @@ -43,7 +43,7 @@ public function getDatabase(): Database $dbUser = 'root'; $dbPass = 'password'; - $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::PDO_ATTRIBUTES); + $pdo = new PDO("mysql:host={$dbHost};port={$dbPort};charset=utf8mb4", $dbUser, $dbPass, self::MYSQL_PDO_ATTRIBUTES); $redis = new Redis(); $redis->connect('redis', 6379); diff --git a/tests/unit/Adapter/MariaDBCreateCollectionCleanupTest.php b/tests/unit/Adapter/MariaDBCreateCollectionCleanupTest.php index 02a80890e..44328df03 100644 --- a/tests/unit/Adapter/MariaDBCreateCollectionCleanupTest.php +++ b/tests/unit/Adapter/MariaDBCreateCollectionCleanupTest.php @@ -48,11 +48,9 @@ public function testAPermissionsTableThatFailsDropsTheCollectionTable(string $cl $this->assertSame('Query timed out', $error->getMessage()); } - $this->assertCount(3, $this->statements); - $this->assertSame( - 'DROP TABLE IF EXISTS `database`.`namespace_books`; DROP TABLE IF EXISTS `database`.`namespace_books_perms`', - $this->statements[2], - ); + $this->assertCount(4, $this->statements); + $this->assertSame('DROP TABLE IF EXISTS `database`.`namespace_books`', $this->statements[2]); + $this->assertSame('DROP TABLE IF EXISTS `database`.`namespace_books_perms`', $this->statements[3]); } /** diff --git a/tests/unit/Adapter/MariaDBDeleteCollectionTest.php b/tests/unit/Adapter/MariaDBDeleteCollectionTest.php index 28e6fc3cb..605077201 100644 --- a/tests/unit/Adapter/MariaDBDeleteCollectionTest.php +++ b/tests/unit/Adapter/MariaDBDeleteCollectionTest.php @@ -35,12 +35,12 @@ public static function adapters(): array * @param class-string $adapterClass */ #[DataProvider('adapters')] - public function testDropsBothTablesInOneStatement(string $adapterClass): void + public function testDropsTheMainTableThenThePermissionsTable(string $adapterClass): void { $adapter = $this->createAdapter($adapterClass, mainTableExists: true); $this->assertTrue($adapter->deleteCollection('places')); - $this->assertSame([self::MAIN.'; '.self::PERMISSIONS], $this->statements); + $this->assertSame([self::MAIN, self::PERMISSIONS], $this->statements); } /** @@ -58,7 +58,7 @@ public function testDropsThePermissionsTableWhenTheMainTableIsGone(string $adapt $this->assertSame('Collection not found', $e->getMessage()); } - $this->assertSame([self::MAIN.'; '.self::PERMISSIONS, self::PERMISSIONS], $this->statements); + $this->assertSame([self::MAIN, self::PERMISSIONS], $this->statements); } /** @@ -71,7 +71,7 @@ private function createAdapter(string $adapterClass, bool $mainTableExists): Mar $this->statements[] = $query; $statement = $this->createStub(PDOStatement::class); - if (! $mainTableExists && \str_starts_with($query, self::MAIN.';')) { + if (! $mainTableExists && $query === self::MAIN) { $statement->method('execute')->willThrowException($this->unknownTable()); } else { $statement->method('execute')->willReturn(true); diff --git a/tests/unit/Adapter/MemoryWritePathsTest.php b/tests/unit/Adapter/MemoryWritePathsTest.php index 2b8908b75..0ddb0ebd4 100644 --- a/tests/unit/Adapter/MemoryWritePathsTest.php +++ b/tests/unit/Adapter/MemoryWritePathsTest.php @@ -12,12 +12,14 @@ use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Duplicate as DuplicateException; +use Utopia\Database\Exception\NotFound as NotFoundException; use Utopia\Database\Exception\Unique as UniqueException; use Utopia\Database\Index; use Utopia\Database\Operator; use Utopia\Database\Permission; use Utopia\Database\Query; use Utopia\Database\Role; +use Utopia\Database\Storage; use Utopia\Database\Validator\Authorization; use Utopia\Query\CursorDirection; @@ -156,6 +158,27 @@ public function testRollbackOfAnIncrementRemovesTheValueAndTimestampItAdded(): v $this->assertNull($after->getUpdatedAt()); } + public function testIncreaseRefusesAStorageColumn(): void + { + $adapter = $this->sharedAdapter(); + $this->storeAddress($adapter, 'home', 'x', 1); + $before = $adapter->getDocument($this->collection(), 'home'); + + foreach ([Storage::TENANT, '_ten.ant', Storage::SEQUENCE, Storage::UPDATED_AT] as $attribute) { + try { + $adapter->increaseDocumentAttribute($this->collection(), 'home', $attribute, 5, self::UPDATED_AT); + $this->fail('Incrementing '.$attribute.' must be refused'); + } catch (NotFoundException $exception) { + $this->assertSame('Attribute not found', $exception->getMessage()); + } + } + + $after = $adapter->getDocument($this->collection(), 'home'); + $this->assertSame($before->getTenant(), $after->getTenant()); + $this->assertSame($before->getSequence(), $after->getSequence()); + $this->assertSame($before->getUpdatedAt(), $after->getUpdatedAt()); + } + public function testADivisionOrModuloByZeroThatSkippedValidationKeepsTheValue(): void { $adapter = $this->adapter(); diff --git a/tests/unit/Adapter/PoolSyncTest.php b/tests/unit/Adapter/PoolSyncTest.php index 5eed527e8..e37c76bda 100644 --- a/tests/unit/Adapter/PoolSyncTest.php +++ b/tests/unit/Adapter/PoolSyncTest.php @@ -225,7 +225,7 @@ public static function changes(): iterable yield 'timeout per event' => [ static fn (Pool $pool) => $pool->setTimeout(self::EVENT_TIMEOUT, Event::DocumentFind), $timeouts, - [Event::DocumentFind->value => self::EVENT_TIMEOUT, Event::All->value => self::TIMEOUT], + [Event::All->value => self::TIMEOUT, Event::DocumentFind->value => self::EVENT_TIMEOUT], ]; yield 'timeout cleared' => [ static fn (Pool $pool) => $pool->clearTimeout(), diff --git a/tests/unit/Adapter/PostgresCreateCollectionCleanupTest.php b/tests/unit/Adapter/PostgresCreateCollectionCleanupTest.php index 8a4aee71b..366d3ee92 100644 --- a/tests/unit/Adapter/PostgresCreateCollectionCleanupTest.php +++ b/tests/unit/Adapter/PostgresCreateCollectionCleanupTest.php @@ -20,6 +20,10 @@ final class PostgresCreateCollectionCleanupTest extends TestCase { private const string DECLARED_INDEX = 'CREATE INDEX "namespace__books_title_index"'; + private const string UID_INDEX = 'CREATE UNIQUE INDEX "namespace_books_uid"'; + + private const string PERMISSIONS_TABLE = 'CREATE TABLE "database"."namespace_books_perms"'; + /** @var list */ private array $statements = []; @@ -39,12 +43,10 @@ public function testADeclaredIndexThatFailsDropsBothTablesAndReachesTheCaller(): $this->assertSame('Attribute not found', $error->getMessage()); } - $this->assertCount(4, $this->statements); - $this->assertStringStartsWith(self::DECLARED_INDEX, $this->statements[2]); - $this->assertSame( - 'DROP TABLE IF EXISTS "database"."namespace_books"; DROP TABLE IF EXISTS "database"."namespace_books_perms"', - $this->statements[3], - ); + $this->assertCount(11, $this->statements); + $this->assertStringStartsWith(self::DECLARED_INDEX, $this->statements[8]); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_books"', $this->statements[9]); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_books_perms"', $this->statements[10]); } public function testACleanupThatFailsKeepsTheOriginalErrorAndLogsTheCleanupFailure(): void @@ -65,7 +67,7 @@ public function testACleanupThatFailsKeepsTheOriginalErrorAndLogsTheCleanupFailu $this->assertInstanceOf(NotFoundException::class, $error, 'the index failure reaches the caller, not the failed drop'); $this->assertSame('Attribute not found', $error->getMessage()); - $this->assertStringStartsWith('DROP TABLE IF EXISTS', $this->statements[3]); + $this->assertStringStartsWith('DROP TABLE IF EXISTS', $this->statements[9]); $this->assertStringContainsString("Failed to rollback collection 'books': SQLSTATE[25P02]", $log, 'the failed cleanup is logged'); } @@ -82,12 +84,46 @@ public function testADeclaredIndexThatAlreadyExistsKeepsBothTables(): void $this->assertInstanceOf(PDOException::class, $error->getPrevious()); } - $this->assertCount(3, $this->statements); + $this->assertCount(9, $this->statements); foreach ($this->statements as $statement) { $this->assertStringNotContainsString('DROP TABLE', $statement); } } + public function testAnInternalIndexThatAlreadyExistsDropsBothTables(): void + { + $this->failures[self::UID_INDEX] = $this->engineError('42P07', 'relation "namespace_books_uid" already exists'); + + try { + $this->adapter()->createCollection('books', [Attribute::string('title', size: 64)]); + $this->fail('An internal index that already exists must fail the collection'); + } catch (DuplicateException $error) { + $this->assertInstanceOf(PDOException::class, $error->getPrevious()); + } + + $this->assertCount(4, $this->statements); + $this->assertStringStartsWith(self::UID_INDEX, $this->statements[1]); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_books"', $this->statements[2]); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_books_perms"', $this->statements[3]); + } + + public function testAPermissionsTableThatAlreadyExistsDropsBothTables(): void + { + $this->failures[self::PERMISSIONS_TABLE] = $this->engineError('42P07', 'relation "namespace_books_perms" already exists'); + + try { + $this->adapter()->createCollection('books', [Attribute::string('title', size: 64)]); + $this->fail('A permissions table that already exists must fail the collection'); + } catch (DuplicateException $error) { + $this->assertInstanceOf(PDOException::class, $error->getPrevious()); + } + + $this->assertCount(8, $this->statements); + $this->assertStringStartsWith(self::PERMISSIONS_TABLE, $this->statements[5]); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_books"', $this->statements[6]); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_books_perms"', $this->statements[7]); + } + public function testASpatialIndexWithOrdersDropsBothTablesAndReachesTheCaller(): void { try { @@ -99,11 +135,9 @@ public function testASpatialIndexWithOrdersDropsBothTablesAndReachesTheCaller(): $this->assertSame('Spatial indexes with explicit orders are not supported. Remove the orders to create this index.', $error->getMessage()); } - $this->assertCount(3, $this->statements); - $this->assertSame( - 'DROP TABLE IF EXISTS "database"."namespace_places"; DROP TABLE IF EXISTS "database"."namespace_places_perms"', - $this->statements[2], - ); + $this->assertCount(10, $this->statements); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_places"', $this->statements[8]); + $this->assertSame('DROP TABLE IF EXISTS "database"."namespace_places_perms"', $this->statements[9]); } public function testATableThatFailsToBeCreatedDropsNothing(): void diff --git a/tests/unit/Adapter/PostgresPartialDDLTest.php b/tests/unit/Adapter/PostgresPartialDDLTest.php new file mode 100644 index 000000000..eb6c9356c --- /dev/null +++ b/tests/unit/Adapter/PostgresPartialDDLTest.php @@ -0,0 +1,172 @@ + */ + private array $statements = []; + + /** @var array */ + private array $failures = []; + + public function testADeleteWhosePermissionsDropFailedDropsThePermissionsTableOnRetry(): void + { + $adapter = $this->adapter(); + $this->failures[self::DROP_BOOKS_PERMISSIONS] = $this->engineError('57014', 'canceling statement due to statement timeout'); + + try { + $adapter->deleteCollection('books'); + $this->fail('A permissions table that fails to drop must fail the delete'); + } catch (TimeoutException $error) { + $this->assertSame('Query timed out', $error->getMessage()); + } + + $this->statements = []; + $this->failures = [self::DROP_BOOKS => $this->engineError('42P01', 'table "namespace_books" does not exist')]; + + try { + $adapter->deleteCollection('books'); + $this->fail('A retry whose collection table is gone must report it as not found'); + } catch (NotFoundException $error) { + $this->assertSame('Collection not found', $error->getMessage()); + } + + $this->assertSame([self::DROP_BOOKS, self::DROP_BOOKS_PERMISSIONS], $this->statements); + } + + public function testACreateRetriedAfterItsFirstColumnWasAddedAddsTheRest(): void + { + $this->failures[self::ADD_AUTHOR] = $this->engineError('42701', 'column "author" of relation "namespace_books" already exists'); + + try { + $this->adapter()->createRelationship('books', self::oneToOne()); + $this->fail('A relationship column that already exists must be reported as a duplicate'); + } catch (DuplicateException $error) { + $this->assertSame('Attribute already exists', $error->getMessage()); + } + + $this->assertCount(2, $this->statements); + $this->assertStringStartsWith(self::ADD_AUTHOR, $this->statements[0]); + $this->assertStringStartsWith(self::ADD_BOOK, $this->statements[1]); + } + + public function testADeleteRetriedAfterItsFirstColumnWasDroppedDropsTheRest(): void + { + $this->failures[self::DROP_AUTHOR] = $this->engineError('42703', 'column "author" of relation "namespace_books" does not exist'); + + try { + $this->adapter()->deleteRelationship('books', self::oneToOne(), RelationshipSide::Parent); + $this->fail('A relationship column that is already gone must be reported as not found'); + } catch (NotFoundException $error) { + $this->assertSame('Attribute not found', $error->getMessage()); + } + + $this->assertCount(2, $this->statements); + $this->assertStringStartsWith(self::DROP_AUTHOR, $this->statements[0]); + $this->assertStringStartsWith(self::DROP_BOOK, $this->statements[1]); + } + + public function testAColumnThatFailsForAnotherReasonStopsTheCreate(): void + { + $this->failures[self::ADD_AUTHOR] = $this->engineError('57014', 'canceling statement due to statement timeout'); + + try { + $this->adapter()->createRelationship('books', self::oneToOne()); + $this->fail('A relationship column that times out must fail the create'); + } catch (TimeoutException $error) { + $this->assertSame('Query timed out', $error->getMessage()); + } + + $this->assertCount(1, $this->statements); + } + + public function testAnAppliedColumnInsideATransactionStopsTheCreate(): void + { + $adapter = $this->adapter(); + $adapter->startTransaction(); + $started = \count($this->statements); + $this->failures[self::ADD_AUTHOR] = $this->engineError('42701', 'column "author" of relation "namespace_books" already exists'); + + try { + $adapter->createRelationship('books', self::oneToOne()); + $this->fail('A relationship column that already exists must be reported as a duplicate'); + } catch (DuplicateException $error) { + $this->assertSame('Attribute already exists', $error->getMessage()); + } + + $this->assertCount(1, \array_slice($this->statements, $started), 'An aborted transaction refuses every later statement'); + } + + private static function oneToOne(): Relationship + { + return Relationship::oneToOne(relatedCollection: 'authors', key: 'author', twoWay: true, twoWayKey: 'book'); + } + + private function engineError(string $state, string $message): PDOException + { + $error = new class ('SQLSTATE['.$state.']: '.$message, $state) extends PDOException { + public function __construct(string $message, string $state) + { + parent::__construct($message); + $this->code = $state; + } + }; + $error->errorInfo = [$state, 7, $message]; + + return $error; + } + + private function adapter(): Postgres + { + $pdo = $this->createStub(PDO::class); + $pdo->method('beginTransaction')->willReturn(true); + $pdo->method('prepare')->willReturnCallback(function (string $query): PDOStatement { + $this->statements[] = $query; + $failure = null; + foreach ($this->failures as $prefix => $error) { + if (\str_starts_with($query, $prefix)) { + $failure = $error; + } + } + + $statement = $this->createStub(PDOStatement::class); + if ($failure === null) { + $statement->method('execute')->willReturn(true); + } else { + $statement->method('execute')->willThrowException($failure); + } + + return $statement; + }); + + $adapter = new Postgres($pdo); + $adapter->setDatabase('database'); + $adapter->setNamespace('namespace'); + + return $adapter; + } +} diff --git a/tests/unit/Adapter/ReadWritePoolTest.php b/tests/unit/Adapter/ReadWritePoolTest.php index 34d72ec08..6ef06877e 100644 --- a/tests/unit/Adapter/ReadWritePoolTest.php +++ b/tests/unit/Adapter/ReadWritePoolTest.php @@ -20,6 +20,7 @@ use Utopia\Database\Collection; use Utopia\Database\Database; use Utopia\Database\Document; +use Utopia\Database\Event; use Utopia\Database\Hook\Permissions; use Utopia\Database\Hook\Write; use Utopia\Database\Index; @@ -346,8 +347,12 @@ public function testReadAdapterClearsStaleTimeout(): void $readAdapter->method('hasFeature')->willReturnCallback( static fn (string $feature): bool => $feature === Feature\Timeouts::class, ); - $readAdapter->expects($this->once()) - ->method('clearTimeout'); + $cleared = []; + $readAdapter->method('clearTimeout')->willReturnCallback( + static function (Event $event = Event::All) use (&$cleared): void { + $cleared[] = $event; + }, + ); $readAdapter->expects($this->once()) ->method('ping') ->willReturn(true); @@ -361,6 +366,7 @@ public function testReadAdapterClearsStaleTimeout(): void $pool->setAuthorization(new Authorization()); $this->assertTrue($pool->delegate('ping', [])); + $this->assertSame(Event::cases(), $cleared); } public function testNonReadNonStandardMethodGoesToWritePool(): void diff --git a/tests/unit/Adapter/SQLChildSideRelationshipRenameTest.php b/tests/unit/Adapter/SQLChildSideRelationshipRenameTest.php index 6d2fe9f6b..77db44d1b 100644 --- a/tests/unit/Adapter/SQLChildSideRelationshipRenameTest.php +++ b/tests/unit/Adapter/SQLChildSideRelationshipRenameTest.php @@ -25,8 +25,8 @@ final class SQLChildSideRelationshipRenameTest extends TestCase public static function renames(): iterable { $engines = [ - 'MariaDB' => [MariaDB::class, 'ALTER TABLE `database`.`namespace_%s` RENAME COLUMN `%s` TO `%s`;'], - 'Postgres' => [Postgres::class, 'ALTER TABLE "database"."namespace_%s" RENAME COLUMN "%s" TO "%s";'], + 'MariaDB' => [MariaDB::class, 'ALTER TABLE `database`.`namespace_%s` RENAME COLUMN `%s` TO `%s`'], + 'Postgres' => [Postgres::class, 'ALTER TABLE "database"."namespace_%s" RENAME COLUMN "%s" TO "%s"'], ]; foreach ($engines as $engine => [$class, $statement]) { diff --git a/tests/unit/Adapter/SingleStatementDDLTest.php b/tests/unit/Adapter/SingleStatementDDLTest.php new file mode 100644 index 000000000..ca80ab8ed --- /dev/null +++ b/tests/unit/Adapter/SingleStatementDDLTest.php @@ -0,0 +1,157 @@ + + */ + private array $prepared = []; + + private ?string $refused = null; + + /** + * @return iterable + */ + public static function adapters(): iterable + { + yield 'MariaDB' => [static fn (PDO $pdo): SQL => new MariaDB($pdo)]; + yield 'MySQL' => [static fn (PDO $pdo): SQL => new MySQL($pdo)]; + yield 'Postgres' => [static fn (PDO $pdo): SQL => new Postgres($pdo)]; + yield 'SQLite' => [static fn (PDO $pdo): SQL => new SQLite($pdo)]; + } + + /** + * @return iterable + */ + public static function operations(): iterable + { + $operations = [ + 'create collection' => static fn (SQL $adapter): bool => $adapter->createCollection('books', [Attribute::string(key: 'title', size: 64)]), + 'delete collection' => static fn (SQL $adapter): bool => $adapter->deleteCollection('books'), + 'analyze collection' => static fn (SQL $adapter): bool => $adapter->analyzeCollection('books'), + 'create one-to-one relationship' => static fn (SQL $adapter): bool => $adapter->createRelationship('books', self::oneToOne()), + 'update one-to-one relationship' => static fn (SQL $adapter): bool => $adapter->updateRelationship('books', self::oneToOne(), RelationshipSide::Parent, new Update(key: 'writer', twoWayKey: 'novel')), + 'delete one-to-one relationship' => static fn (SQL $adapter): bool => $adapter->deleteRelationship('books', self::oneToOne(), RelationshipSide::Parent), + 'update many-to-many relationship' => static fn (SQL $adapter): bool => $adapter->updateRelationship('books', self::manyToMany(), RelationshipSide::Parent, new Update(key: 'writers', twoWayKey: 'novels')), + 'delete many-to-many relationship' => static fn (SQL $adapter): bool => $adapter->deleteRelationship('books', self::manyToMany(), RelationshipSide::Parent), + ]; + + foreach (self::adapters() as $adapterName => [$factory]) { + foreach ($operations as $operationName => $operation) { + yield "{$adapterName} {$operationName}" => [$factory, $operation]; + } + } + } + + /** + * @param Closure(PDO): SQL $factory + * @param Closure(SQL): bool $operation + */ + #[DataProvider('operations')] + public function testEveryPrepareHoldsOneStatement(Closure $factory, Closure $operation): void + { + $adapter = $this->adapter($factory); + + $this->assertTrue($operation($adapter)); + $this->assertNotEmpty($this->prepared, 'The operation must prepare its DDL'); + + foreach ($this->prepared as $query) { + $this->assertSame(1, self::countStatements($query), "Prepared more than one statement: {$query}"); + } + } + + /** + * @param Closure(PDO): SQL $factory + */ + #[DataProvider('adapters')] + public function testAFailedCreateDropsItsTablesOneStatementAtATime(Closure $factory): void + { + $adapter = $this->adapter($factory); + $this->refused = 'namespace_books_perms'; + + try { + $adapter->createCollection('books'); + $this->fail('A create whose permissions table is refused must fail'); + } catch (Throwable $error) { + $this->assertNotInstanceOf(AssertionFailedError::class, $error); + } + + $drops = \array_values(\array_filter($this->prepared, static fn (string $query): bool => \str_contains($query, 'DROP TABLE'))); + $this->assertCount(2, $drops, 'The rollback must drop the collection and its permissions table'); + + foreach ($this->prepared as $query) { + $this->assertSame(1, self::countStatements($query), "Prepared more than one statement: {$query}"); + } + } + + private static function oneToOne(): Relationship + { + return Relationship::oneToOne(relatedCollection: 'authors', key: 'author', twoWay: true, twoWayKey: 'book'); + } + + private static function manyToMany(): Relationship + { + return Relationship::manyToMany(relatedCollection: 'authors', key: 'authors', twoWay: true, twoWayKey: 'books'); + } + + private static function countStatements(string $query): int + { + $code = \preg_replace(['#/\*.*?\*/#s', "/'(?:[^']|'')*'/", '/"(?:[^"]|"")*"/', '/`[^`]*`/'], '', $query) ?? $query; + $statements = \array_filter(\array_map(\trim(...), \explode(';', $code)), static fn (string $statement): bool => $statement !== ''); + + return \count($statements); + } + + /** + * @param Closure(PDO): SQL $factory + */ + private function adapter(Closure $factory): SQL + { + $adapter = $factory($this->recorder()); + $adapter->setDatabase('library'); + $adapter->setNamespace('namespace'); + + return $adapter; + } + + private function recorder(): PDO + { + $pdo = $this->createStub(PDO::class); + $pdo->method('prepare')->willReturnCallback(function (string $query): PDOStatement { + $this->prepared[] = $query; + + $refused = $this->refused !== null && \str_starts_with(\ltrim($query), 'CREATE TABLE') && \str_contains($query, $this->refused); + + $statement = $this->createStub(PDOStatement::class); + $statement->method('execute')->willReturnCallback(static fn (): bool => $refused ? throw new PDOException('SQLSTATE[HY000]: General error: refused') : true); + $statement->method('fetchAll')->willReturn([]); + $statement->method('fetch')->willReturn(false); + $statement->method('closeCursor')->willReturn(true); + $statement->method('rowCount')->willReturn(0); + + return $statement; + }); + + return $pdo; + } +} diff --git a/tests/unit/Collections/AnalyzeCollectionTest.php b/tests/unit/Collections/AnalyzeCollectionTest.php index a2beb5c50..354f17234 100644 --- a/tests/unit/Collections/AnalyzeCollectionTest.php +++ b/tests/unit/Collections/AnalyzeCollectionTest.php @@ -66,6 +66,6 @@ public function testPostgresAnalyzesTheTableAndItsPermissions(): void $adapter->setNamespace('namespace'); $this->assertTrue($adapter->analyzeCollection('places')); - $this->assertSame(['ANALYZE "database"."namespace_places"; ANALYZE "database"."namespace_places_perms"'], $this->statements); + $this->assertSame(['ANALYZE "database"."namespace_places"', 'ANALYZE "database"."namespace_places_perms"'], $this->statements); } } diff --git a/tests/unit/EngineErrorMappingTest.php b/tests/unit/EngineErrorMappingTest.php index 79f95cdb2..5c20cc1dd 100644 --- a/tests/unit/EngineErrorMappingTest.php +++ b/tests/unit/EngineErrorMappingTest.php @@ -364,7 +364,7 @@ public function testPostgresDeleteCollectionWithoutItsTableIsNotFoundAndStillDro $this->assertInstanceOf(NotFoundException::class, $error); $this->assertSame('Collection not found', $error->getMessage()); $this->assertSame([ - 'DROP TABLE "utopiaTests"."engine_orders"; DROP TABLE IF EXISTS "utopiaTests"."engine_orders_perms"', + 'DROP TABLE "utopiaTests"."engine_orders"', 'DROP TABLE IF EXISTS "utopiaTests"."engine_orders_perms"', ], $statements); } @@ -387,14 +387,15 @@ public function testPostgresUniqueIndexOverDuplicateObjectPathValuesIsUnique(): $this->assertSame($duplicates, $error->getPrevious()); } - public function testPostgresDeleteCollectionDropsBothTablesInOneStatement(): void + public function testPostgresDeleteCollectionDropsTheMainTableThenThePermissionsTable(): void { $statements = []; $adapter = $this->postgresRecording($statements); $this->assertTrue($adapter->deleteCollection('orders')); $this->assertSame([ - 'DROP TABLE "utopiaTests"."engine_orders"; DROP TABLE IF EXISTS "utopiaTests"."engine_orders_perms"', + 'DROP TABLE "utopiaTests"."engine_orders"', + 'DROP TABLE IF EXISTS "utopiaTests"."engine_orders_perms"', ], $statements); } diff --git a/tests/unit/Hook/ColumnNameTest.php b/tests/unit/Hook/ColumnNameTest.php index 83effb93e..3422bd663 100644 --- a/tests/unit/Hook/ColumnNameTest.php +++ b/tests/unit/Hook/ColumnNameTest.php @@ -39,6 +39,7 @@ public static function invalidColumns(): iterable yield 'a statement separator' => ['x;y']; yield 'a quote character' => ['x`y']; yield 'an empty name' => ['']; + yield 'a trailing newline' => ["_uid\n"]; } #[DataProvider('invalidColumns')] @@ -110,6 +111,26 @@ public function testPermissionFilterRejectsAPermissionsTableOutsideTheIdentifier $filter->filter('posts'); } + public function testPermissionFilterRejectsAPermissionsTableWithATrailingNewline(): void + { + $filter = new Filter(['any'], static fn (string $table): string => $table."_perms\n"); + + $this->expectException(DatabaseException::class); + $this->expectExceptionMessage("Invalid permissions table name: posts_perms\n"); + + $filter->filter('posts'); + } + + public function testPermissionFilterRejectsACollationWithATrailingNewline(): void + { + $filter = new Filter(['any'], static fn (string $table): string => $table.'_perms'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid collation name: utf8mb4_bin\n"); + + $filter->collate("utf8mb4_bin\n"); + } + public function testPermissionFilterWithNoColumnsMatchesOnlyCollectionWidePermissions(): void { $filter = new Filter(['any'], static fn (string $table): string => $table.'_perms', columns: []); diff --git a/tests/unit/MongoTimeoutTest.php b/tests/unit/MongoTimeoutTest.php new file mode 100644 index 000000000..71a95d737 --- /dev/null +++ b/tests/unit/MongoTimeoutTest.php @@ -0,0 +1,203 @@ +> + */ + private array $options = []; + + private Mongo $adapter; + + #[\Override] + protected function setUp(): void + { + $this->options = []; + $this->adapter = $this->createAdapter(); + } + + public function testAnEventTimeoutBoundsOnlyThatEvent(): void + { + $this->adapter->setTimeout(500, Event::DocumentCount); + + $this->assertArrayNotHasKey(self::MAX_TIME, $this->findOptions(), 'a count timeout must not bound find()'); + $this->assertSame(500, $this->countOptions()[self::MAX_TIME] ?? null, 'count() must carry its own timeout'); + $this->assertArrayNotHasKey(self::MAX_TIME, $this->sumOptions(), 'a count timeout must not bound sum()'); + } + + public function testEachReadCarriesItsOwnEventTimeout(): void + { + $this->adapter->setTimeout(100, Event::DocumentFind); + $this->adapter->setTimeout(200, Event::DocumentCount); + $this->adapter->setTimeout(300, Event::DocumentSum); + + $this->assertSame(100, $this->findOptions()[self::MAX_TIME] ?? null); + $this->assertSame(200, $this->countOptions()[self::MAX_TIME] ?? null); + $this->assertSame(300, $this->sumOptions()[self::MAX_TIME] ?? null); + } + + public function testAnEventTimeoutOverridesTheGlobalTimeout(): void + { + $this->adapter->setTimeout(1000); + $this->adapter->setTimeout(50, Event::DocumentFind); + + $this->assertSame(50, $this->findOptions()[self::MAX_TIME] ?? null); + $this->assertSame(1000, $this->countOptions()[self::MAX_TIME] ?? null); + } + + public function testClearingOneEventKeepsTheGlobalTimeout(): void + { + $this->adapter->setTimeout(1000); + $this->adapter->setTimeout(50, Event::DocumentFind); + $this->adapter->clearTimeout(Event::DocumentFind); + + $this->assertSame(1000, $this->findOptions()[self::MAX_TIME] ?? null, 'clearing find must fall back to the global timeout'); + $this->assertSame(1000, $this->countOptions()[self::MAX_TIME] ?? null, 'clearing find must leave count bounded'); + } + + public function testClearingAnEventWithoutItsOwnTimeoutKeepsTheGlobalTimeout(): void + { + $this->adapter->setTimeout(1000); + $this->adapter->clearTimeout(Event::DocumentCount); + + $this->assertSame(1000, $this->countOptions()[self::MAX_TIME] ?? null); + $this->assertSame(1000, $this->findOptions()[self::MAX_TIME] ?? null); + } + + public function testAZeroTimeoutClearsThatEvent(): void + { + $this->adapter->setTimeout(1000); + $this->adapter->setTimeout(50, Event::DocumentCount); + $this->adapter->setTimeout(0, Event::DocumentCount); + + $this->assertSame(1000, $this->countOptions()[self::MAX_TIME] ?? null, 'a zero count timeout must fall back to the global timeout'); + $this->assertSame(1000, $this->findOptions()[self::MAX_TIME] ?? null, 'a zero count timeout must not touch find()'); + $this->assertSame(1000, $this->adapter->getTimeout(Event::DocumentCount)); + } + + public function testClearingTheGlobalTimeoutClearsEveryEvent(): void + { + $this->adapter->setTimeout(1000); + $this->adapter->setTimeout(50, Event::DocumentFind); + $this->adapter->clearTimeout(); + + $this->assertArrayNotHasKey(self::MAX_TIME, $this->findOptions()); + $this->assertArrayNotHasKey(self::MAX_TIME, $this->countOptions()); + $this->assertArrayNotHasKey(self::MAX_TIME, $this->sumOptions()); + } + + /** + * @return array + */ + private function findOptions(): array + { + $this->adapter->find(new Document(['$id' => self::COLLECTION])); + + return $this->lastOptions(); + } + + /** + * @return array + */ + private function countOptions(): array + { + $this->adapter->count(new Document(['$id' => self::COLLECTION])); + + return $this->lastOptions(); + } + + /** + * @return array + */ + private function sumOptions(): array + { + $this->adapter->sum(new Document(['$id' => self::COLLECTION]), 'price'); + + return $this->lastOptions(); + } + + /** + * @return array + */ + private function lastOptions(): array + { + $this->assertNotEmpty($this->options, 'the read must reach the client'); + + return $this->options[\array_key_last($this->options)]; + } + + private function createAdapter(): Mongo + { + $record = function (array $options): void { + $this->options[] = $options; + }; + + $client = new class ($record) extends Client { + /** + * @param Closure(array): void $record + */ + public function __construct(private readonly Closure $record) + { + } + + #[\Override] + public function connect(): self + { + return $this; + } + + #[\Override] + public function close(): void + { + } + + /** + * @param array $filters + * @param array $options + */ + #[\Override] + public function find(string $collection, array $filters = [], array $options = []): stdClass + { + ($this->record)($options); + + return (object) ['cursor' => (object) ['firstBatch' => [], 'id' => 0]]; + } + + /** + * @param array $pipeline + * @param array $options + */ + #[\Override] + public function aggregate(string $collection, array $pipeline, array $options = []): stdClass + { + ($this->record)($options); + + return (object) ['cursor' => (object) ['firstBatch' => []]]; + } + }; + + $authorization = new Authorization(); + $authorization->disable(); + + $adapter = new Mongo($client); + $adapter->setAuthorization($authorization); + $adapter->setNamespace('timeouts'); + + return $adapter; + } +} diff --git a/tests/unit/PoolTimeoutTest.php b/tests/unit/PoolTimeoutTest.php index 4bbaafd2b..ddf3752fd 100644 --- a/tests/unit/PoolTimeoutTest.php +++ b/tests/unit/PoolTimeoutTest.php @@ -135,6 +135,25 @@ public function testHandleWithNoTimeoutResetsTheConnectionItIsGiven(): void $this->assertSame(0, $connection->getTimeout()); } + public function testGlobalOnlyHandleDropsThePerEventTimeoutTheLastHolderLeft(): void + { + $connection = new TimeoutRecordingMemory(); + $pool = new UtopiaPool(new Stack(), 'memory', 1, fn () => $connection, timeout: 0.0); + + $perEvent = new Pool($pool); + $perEvent->setAuthorization(new Authorization()); + $perEvent->setTimeout(50, Event::DocumentFind); + $perEvent->getDriver(); + + $global = new Pool($pool); + $global->setAuthorization(new Authorization()); + $global->setTimeout(1000); + $global->getDriver(); + + $this->assertSame([Event::All->value => 1000], $connection->timeouts, 'A handle with only a global timeout must not inherit the last holder\'s per-event one'); + $this->assertSame(1000, $connection->getTimeout(Event::DocumentFind)); + } + /** * The pool's own map is what a checkout replays, so clearing every timeout * has to empty it. The inherited implementation walks the events it finds diff --git a/tests/unit/PostgresIdentifierTest.php b/tests/unit/PostgresIdentifierTest.php index 22e298b48..93a0518fd 100644 --- a/tests/unit/PostgresIdentifierTest.php +++ b/tests/unit/PostgresIdentifierTest.php @@ -38,7 +38,7 @@ public function testCreateCollectionSqlUsesHashedTableName(): void $this->assertNotSame(self::NAMESPACE.'_'.self::COLLECTION, $physical); $this->assertStringContainsString('"'.$physical.'"', $queries[0]); $this->assertStringNotContainsString(self::COLLECTION, $queries[0]); - $this->assertStringContainsString('"'.$permissions.'"', $queries[1]); + $this->assertStringContainsString('"'.$permissions.'"', $queries[5]); } public function testExistsBindsHashedTableName(): void @@ -59,7 +59,7 @@ private function createCollectionQueries(string $collection): array $statement = $this->getMockBuilder(PDOStatement::class) ->disableOriginalConstructor() ->getMock(); - $statement->expects($this->exactly(2)) + $statement->expects($this->exactly(8)) ->method('execute') ->willReturn(true); @@ -67,7 +67,7 @@ private function createCollectionQueries(string $collection): array $pdo = $this->getMockBuilder(PDO::class) ->disableOriginalConstructor() ->getMock(); - $pdo->expects($this->exactly(2)) + $pdo->expects($this->exactly(8)) ->method('prepare') ->willReturnCallback(function (string $sql) use (&$queries, $statement): PDOStatement { $queries[] = $sql; @@ -83,7 +83,7 @@ private function createCollectionQueries(string $collection): array $adapter->setAuthorization($authorization); $this->assertTrue($adapter->createCollection($collection)); - $this->assertCount(2, $queries); + $this->assertCount(8, $queries); return $queries; } diff --git a/tests/unit/PostgresQueryBehaviorTest.php b/tests/unit/PostgresQueryBehaviorTest.php index 479f77176..fe2a835d5 100644 --- a/tests/unit/PostgresQueryBehaviorTest.php +++ b/tests/unit/PostgresQueryBehaviorTest.php @@ -40,7 +40,7 @@ public function testCreateCollectionStoresJsonbPermissionsWithGinIndex(): void $statement = $this->getMockBuilder(\PDOStatement::class) ->disableOriginalConstructor() ->getMock(); - $statement->expects($this->exactly(2)) + $statement->expects($this->exactly(8)) ->method('execute') ->willReturn(true); @@ -48,7 +48,7 @@ public function testCreateCollectionStoresJsonbPermissionsWithGinIndex(): void $pdo = $this->getMockBuilder(\PDO::class) ->disableOriginalConstructor() ->getMock(); - $pdo->expects($this->exactly(2)) + $pdo->expects($this->exactly(8)) ->method('prepare') ->willReturnCallback(function (string $sql) use (&$queries, $statement): \PDOStatement { $queries[] = $sql; @@ -65,7 +65,7 @@ public function testCreateCollectionStoresJsonbPermissionsWithGinIndex(): void $this->assertTrue($adapter->createCollection('movies')); $this->assertStringContainsString('"'.Storage::PERMISSIONS.'" JSONB', $queries[0]); - $this->assertStringContainsString('USING GIN ("'.Storage::PERMISSIONS.'")', $queries[0]); + $this->assertStringContainsString('USING GIN ("'.Storage::PERMISSIONS.'")', $queries[4]); } public function testVectorDistanceIsProjectedHydratedAndOrderedBeforeTieBreaker(): void diff --git a/tests/unit/SQLTimeoutScopeTest.php b/tests/unit/SQLTimeoutScopeTest.php index b8d2f5017..2c1fc9a2f 100644 --- a/tests/unit/SQLTimeoutScopeTest.php +++ b/tests/unit/SQLTimeoutScopeTest.php @@ -41,10 +41,10 @@ public function testPublicReadTimeoutDoesNotAffectCollectionDelete(): void $statement = $this->getMockBuilder(\PDOStatement::class) ->disableOriginalConstructor() ->getMock(); - $statement->expects($this->exactly(2))->method('execute')->willReturn(true); + $statement->expects($this->exactly(3))->method('execute')->willReturn(true); $statement->expects($this->once())->method('fetchAll')->willReturn([]); $statement->expects($this->once())->method('closeCursor')->willReturn(true); - $pdo->expects($this->exactly(2))->method('prepare')->willReturn($statement); + $pdo->expects($this->exactly(3))->method('prepare')->willReturn($statement); $adapter = new MySQL($pdo); $adapter->setDatabase('database'); diff --git a/tests/unit/Validator/BigIntTest.php b/tests/unit/Validator/BigIntTest.php index 31f4ae659..584f4599a 100644 --- a/tests/unit/Validator/BigIntTest.php +++ b/tests/unit/Validator/BigIntTest.php @@ -27,6 +27,7 @@ public static function wholeNumbers(): iterable yield 'negative decimal with a zero fraction' => ['-7.0', '-7']; yield 'decimal with an empty fraction' => ['7.', '7']; yield 'decimal with a fraction' => ['7.0500', null]; + yield 'integer with a trailing newline' => ["7\n", null]; yield 'exponent' => ['1.8446744073709552e19', null]; yield 'infinity' => ['INF', null]; yield 'empty' => ['', null]; @@ -103,6 +104,7 @@ public static function valuesThatAreNotIntegers(): iterable yield 'null' => [null]; yield 'a word' => ['abc']; yield 'a decimal string' => ['1.5']; + yield 'an integer string with a trailing newline' => ["5\n"]; } public function testPowerRefusesANegativeExponent(): void @@ -180,4 +182,17 @@ public function testNativeIntegersNormalizeToTheirDigits(): void $this->assertSame(\PHP_INT_MIN, BigInt::toNative(\PHP_INT_MIN)); $this->assertSame(12, BigInt::toNative(12)); } + + public function testAnIntegerStringWithATrailingNewlineIsNotAnInteger(): void + { + $this->assertFalse(BigInt::isIntegerString("5\n")); + $this->assertFalse(BigInt::isIntegerString("5\n", false)); + $this->assertFalse((new BigInt(true))->isValid("5\n")); + } + + public function testADecimalWithATrailingNewlineHasNoIntegralValue(): void + { + $this->assertSame(5, BigInt::integralValue('5.0')); + $this->assertNull(BigInt::integralValue("5.0\n")); + } } diff --git a/tests/unit/Validator/Query/AggregateTest.php b/tests/unit/Validator/Query/AggregateTest.php index 7c6a05f2a..550da0cd3 100644 --- a/tests/unit/Validator/Query/AggregateTest.php +++ b/tests/unit/Validator/Query/AggregateTest.php @@ -53,6 +53,12 @@ public function testInvalidAliasRejected(): void $this->assertSame('Invalid aggregate alias', $this->validator->getDescription()); } + public function testAliasWithATrailingNewlineRejected(): void + { + $this->assertFalse($this->validator->isValid(Query::sum('price', "total\n"))); + $this->assertSame('Invalid aggregate alias', $this->validator->getDescription()); + } + public function testUnknownAttributeAllowedWithoutAttributeSupport(): void { $validator = new Aggregate(attributes: [], supportForAttributes: false); diff --git a/tests/unit/Validator/Query/BaseTest.php b/tests/unit/Validator/Query/BaseTest.php new file mode 100644 index 000000000..f92508cd1 --- /dev/null +++ b/tests/unit/Validator/Query/BaseTest.php @@ -0,0 +1,41 @@ +isAllowedJoinColumn($column); + } + + #[\Override] + public function getMethodType(): string + { + return self::METHOD_TYPE_FILTER; + } + }; + + $this->assertSame($allowed, $validator->allows($column)); + } + + /** + * @return iterable + */ + public static function joinColumns(): iterable + { + yield 'an identifier' => ['title', true]; + yield 'an internal attribute' => ['$id', true]; + yield 'an empty name' => ['', false]; + yield 'a dotted path' => ['a.b', false]; + yield 'a trailing newline' => ["title\n", false]; + } +} diff --git a/tests/unit/Validator/Query/JoinAliasTest.php b/tests/unit/Validator/Query/JoinAliasTest.php index a34848567..df010d713 100644 --- a/tests/unit/Validator/Query/JoinAliasTest.php +++ b/tests/unit/Validator/Query/JoinAliasTest.php @@ -75,6 +75,7 @@ public static function rejectedJoins(): iterable yield 'past 63 characters' => [Query::join('orders', \str_repeat('a', 64), [Query::on('$id', 'customerId')]), 'Join alias must be at most 63 characters']; yield 'a dot' => [Query::join('orders', 'a.b', [Query::on('$id', 'customerId')]), $invalid]; yield 'a quote' => [Query::crossJoin('orders', 'x`y'), $invalid]; + yield 'a trailing newline' => [Query::join('orders', "ord\n", [Query::on('$id', 'customerId')]), $invalid]; } #[DataProvider('rejectedTableIds')] @@ -99,6 +100,7 @@ public static function rejectedTableIds(): iterable yield 'a dot' => [Query::leftJoin('orders.v2', '', [Query::on('$id', 'customerId')]), $pattern('orders.v2')]; yield 'a leading hyphen' => [Query::rightJoin('-orders', '', [Query::on('$id', 'customerId')]), $pattern('-orders')]; + yield 'a trailing newline' => [Query::crossJoin("orders\n", ''), $pattern("orders\n")]; yield 'a dot on a cross join' => [Query::crossJoin('orders.v2', ''), $pattern('orders.v2')]; yield 'a dot on a join built without an alias' => [new Query(Method::FullOuterJoin, 'orders.v2', [Query::on('$id', 'customerId')]), $pattern('orders.v2')]; yield 'a dot on a parsed join' => [Query::parseQuery(['method' => 'join', 'attribute' => 'orders.v2', 'values' => ['$id', '=', 'customerId']]), $pattern('orders.v2')]; diff --git a/tests/unit/Validator/SequenceTest.php b/tests/unit/Validator/SequenceTest.php new file mode 100644 index 000000000..d70f85cec --- /dev/null +++ b/tests/unit/Validator/SequenceTest.php @@ -0,0 +1,22 @@ +assertTrue((new Sequence(ColumnType::Uuid7->value, true))->isValid(self::UUID7)); + } + + public function testRejectsAUuid7WithATrailingNewline(): void + { + $this->assertFalse((new Sequence(ColumnType::Uuid7->value, true))->isValid(self::UUID7."\n")); + } +} diff --git a/tests/unit/Validator/StructureTest.php b/tests/unit/Validator/StructureTest.php index cc87963d4..a8fcdef34 100644 --- a/tests/unit/Validator/StructureTest.php +++ b/tests/unit/Validator/StructureTest.php @@ -365,6 +365,32 @@ public function test_reserved_keys_without_defined_attributes(): void } } + public function test_reserved_keys_after_filtering(): void + { + $document = [ + '$collection' => Id::custom('posts'), + 'title' => 'Demo Title', + 'description' => 'Demo description', + 'rating' => 5, + 'price' => 1.99, + 'published' => true, + 'tags' => ['dog', 'cat', 'mouse'], + 'feedback' => 'team@appwrite.io', + '$createdAt' => '2000-04-01T12:00:00.000+00:00', + '$updatedAt' => '2000-04-01T12:00:00.000+00:00', + ]; + + foreach ([[Capability::UnsignedBigInt], [Capability::DefinedAttributes, Capability::UnsignedBigInt]] as $capabilities) { + $validator = new Structure(new Document($this->collection), Profiles::of(capabilities: $capabilities)); + $this->assertTrue($validator->isValid(new Document($document)), $validator->getDescription()); + + foreach (['_u!id', '_ten.ant', '_perm$issions', '_i d'] as $key) { + $this->assertFalse($validator->isValid(new Document([...$document, $key => 'stored over the document\'s own field'])), $key); + $this->assertSame('Invalid document structure: Reserved attribute: "'.$key.'"', $validator->getDescription()); + } + } + } + public function test_reserved_keys_with_defined_attributes(): void { $collection = $this->collection; diff --git a/tests/unit/WritableAttributesTest.php b/tests/unit/WritableAttributesTest.php new file mode 100644 index 000000000..9ab4937e7 --- /dev/null +++ b/tests/unit/WritableAttributesTest.php @@ -0,0 +1,41 @@ + + */ + public function writable(Document $document): array + { + return $this->writableAttributes($document); + } + }; + + $document = new Document([ + '$id' => 'kept', + '$permissions' => [], + 'name' => 'kept', + '_private' => 'kept', + Storage::UID => 'dropped', + '_u!id' => 'dropped', + Storage::SEQUENCE => 1, + Storage::TENANT => 2, + '_ten.ant' => 2, + Storage::PERMISSIONS => '[]', + Storage::CREATED_AT => 'dropped', + Storage::UPDATED_AT => 'dropped', + ]); + + $this->assertSame(['name' => 'kept', '_private' => 'kept'], $adapter->writable($document)); + } +}