From 4fd55874069b0e2b3545de41037a2c0f8404ecc4 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 17 Sep 2024 18:02:59 +0900 Subject: [PATCH 01/34] Make findOne return empty document instead of false when no matches are found --- src/Database/Database.php | 24 ++++++++++++--------- tests/e2e/Adapter/Base.php | 44 +++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 587a9bc49..008aedc97 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4035,10 +4035,10 @@ private function updateDocumentRelationships(Document $collection, Document $old } if ( $oldValue?->getId() !== $value - && $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ + && !($this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ Query::select(['$id']), Query::equal($twoWayKey, [$value]), - ])) + ]))->isEmpty()) ) { // Have to do this here because otherwise relations would be updated before the database can throw the unique violation throw new DuplicateException('Document already has a related document'); @@ -4056,10 +4056,10 @@ private function updateDocumentRelationships(Document $collection, Document $old if ( $oldValue?->getId() !== $value->getId() - && $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ + && !($this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ Query::select(['$id']), Query::equal($twoWayKey, [$value->getId()]), - ])) + ]))->isEmpty()) ) { // Have to do this here because otherwise relations would be updated before the database can throw the unique violation throw new DuplicateException('Document already has a related document'); @@ -4726,7 +4726,7 @@ private function deleteRestrict( Query::equal($twoWayKey, [$document->getId()]) ]); - if (!$related instanceof Document) { + if ($related->isEmpty()) { return; } @@ -4749,7 +4749,7 @@ private function deleteRestrict( Query::equal($twoWayKey, [$document->getId()]) ])); - if ($related) { + if (!$related->isEmpty()) { throw new RestrictedException('Cannot delete document because it has at least one related document.'); } } @@ -4793,7 +4793,7 @@ private function deleteSetNull(Document $collection, Document $relatedCollection $related = $this->getDocument($relatedCollection->getId(), $value->getId(), [Query::select(['$id'])]); } - if (!$related instanceof Document) { + if ($related->isEmpty()) { return; } @@ -5172,10 +5172,10 @@ public function find(string $collection, array $queries = []): array /** * @param string $collection * @param array $queries - * @return false|Document + * @return Document * @throws DatabaseException */ - public function findOne(string $collection, array $queries = []): false|Document + public function findOne(string $collection, array $queries = []): Document { $results = $this->silent(fn () => $this->find($collection, \array_merge([ Query::limit(1) @@ -5185,7 +5185,11 @@ public function findOne(string $collection, array $queries = []): false|Document $this->trigger(self::EVENT_DOCUMENT_FIND, $found); - return $found; + if (!$found) { + return new Document(); + } else { + return $found; + } } /** diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 8b1dcec2a..487061511 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -4280,13 +4280,13 @@ public function testFindOne(): void Query::orderAsc('name') ]); - $this->assertTrue($document instanceof Document); + $this->assertFalse($document->isEmpty()); $this->assertEquals('Frozen', $document->getAttribute('name')); $document = static::getDatabase()->findOne('movies', [ Query::offset(10) ]); - $this->assertEquals(false, $document); + $this->assertTrue($document->isEmpty()); } public function testFindNull(): void @@ -5524,7 +5524,7 @@ public function testRenameAttribute(): void // Document should be there if adapter migrated properly $document = $database->findOne('colors'); - $this->assertTrue($document instanceof Document); + $this->assertFalse($document->isEmpty()); $this->assertEquals('black', $document->getAttribute('verbose')); $this->assertEquals('#000000', $document->getAttribute('hex')); $this->assertEquals(null, $document->getAttribute('name')); @@ -6568,7 +6568,7 @@ public function testOneToOneOneWayRelationship(): void Query::select(['*', 'library.name']) ]); - if (!$person instanceof Document) { + if ($person->isEmpty()) { throw new Exception('Person not found'); } @@ -7044,7 +7044,7 @@ public function testOneToOneTwoWayRelationship(): void Query::select(['*', 'city.name']) ]); - if (!$country instanceof Document) { + if ($country->isEmpty()) { throw new Exception('Country not found'); } @@ -7618,7 +7618,7 @@ public function testOneToManyOneWayRelationship(): void Query::select(['*', 'albums.name']) ]); - if (!$artist instanceof Document) { + if ($artist->isEmpty()) { $this->fail('Artist not found'); } @@ -8051,7 +8051,7 @@ public function testOneToManyTwoWayRelationship(): void Query::select(['*', 'accounts.name']) ]); - if (!$customer instanceof Document) { + if ($customer->isEmpty()) { throw new Exception('Customer not found'); } @@ -8463,7 +8463,7 @@ public function testManyToOneOneWayRelationship(): void Query::select(['*', 'movie.name']) ]); - if (!$review instanceof Document) { + if ($review->isEmpty()) { throw new Exception('Review not found'); } @@ -8840,7 +8840,7 @@ public function testManyToOneTwoWayRelationship(): void Query::select(['*', 'store.name']) ]); - if (!$product instanceof Document) { + if ($product->isEmpty()) { throw new Exception('Product not found'); } @@ -9222,7 +9222,7 @@ public function testManyToManyOneWayRelationship(): void Query::select(['*', 'songs.name']) ]); - if (!$playlist instanceof Document) { + if ($playlist->isEmpty()) { throw new Exception('Playlist not found'); } @@ -9604,7 +9604,7 @@ public function testManyToManyTwoWayRelationship(): void Query::select(['*', 'classes.name']) ]); - if (!$student instanceof Document) { + if ($student->isEmpty()) { throw new Exception('Student not found'); } @@ -9908,7 +9908,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name', 'models.name']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -9930,7 +9930,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name', '$id']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -9945,7 +9945,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name', '$internalId']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -9960,7 +9960,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name', '$collection']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -9975,7 +9975,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name', '$createdAt']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -9990,7 +9990,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name', '$updatedAt']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -10005,7 +10005,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name', '$permissions']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -10021,7 +10021,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['*', 'models.year']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -10037,7 +10037,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['*', 'models.*']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -10054,7 +10054,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['models.*']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } @@ -10070,7 +10070,7 @@ public function testSelectRelationshipAttributes(): void Query::select(['name']), ]); - if (!$make instanceof Document) { + if ($make->isEmpty()) { throw new Exception('Make not found'); } From 7fc54021d22e80a3e29105b77cdfc4d21ff9336e Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 7 Oct 2024 11:06:43 +0900 Subject: [PATCH 02/34] Update src/Database/Database.php Co-authored-by: Jake Barnby --- src/Database/Database.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 008aedc97..b496f1e16 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5187,9 +5187,9 @@ public function findOne(string $collection, array $queries = []): Document if (!$found) { return new Document(); - } else { - return $found; } + + return $found; } /** From 923260ac25780b3e60c255a5529da4cc440ae174 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 7 Oct 2024 11:09:46 +0900 Subject: [PATCH 03/34] Run Linter --- src/Database/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index b496f1e16..11e8eab01 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5188,7 +5188,7 @@ public function findOne(string $collection, array $queries = []): Document if (!$found) { return new Document(); } - + return $found; } From 9d0d9504e30593ce9717dcea4bfd9cb11fcb7dad Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 17 Oct 2024 21:14:43 +1300 Subject: [PATCH 04/34] Add not found exception --- src/Database/Adapter/MariaDB.php | 3 +- src/Database/Adapter/Postgres.php | 23 +++++++------ src/Database/Adapter/SQLite.php | 5 +-- src/Database/Database.php | 53 +++++++++++++++-------------- src/Database/Exception/NotFound.php | 9 +++++ 5 files changed, 53 insertions(+), 40 deletions(-) create mode 100644 src/Database/Exception/NotFound.php diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 246079eed..1a2b2348c 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -9,6 +9,7 @@ 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\Timeout as TimeoutException; use Utopia\Database\Exception\Truncate as TruncateException; use Utopia\Database\Query; @@ -709,7 +710,7 @@ public function createIndex(string $collection, string $id, string $type, array $collection = $this->getDocument(Database::METADATA, $collection); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $collectionAttributes = \json_decode($collection->getAttribute('attributes', []), true); diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 2b9007000..ae2f34619 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -9,8 +9,9 @@ use Utopia\Database\Database; use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; -use Utopia\Database\Exception\Duplicate; -use Utopia\Database\Exception\Timeout; +use Utopia\Database\Exception\Duplicate as DuplicateException; +use Utopia\Database\Exception\NotFound as NotFoundException; +use Utopia\Database\Exception\Timeout as TimeoutException; use Utopia\Database\Exception\Truncate as TruncateException; use Utopia\Database\Query; use Utopia\Database\Validator\Authorization; @@ -869,7 +870,7 @@ public function createDocument(string $collection, Document $document): Document } catch (Throwable $e) { switch ($e->getCode()) { case 23505: - throw new Duplicate('Duplicated document: ' . $e->getMessage()); + throw new DuplicateException('Duplicated document: ' . $e->getMessage()); default: throw $e; } @@ -972,7 +973,7 @@ public function createDocuments(string $collection, array $documents, int $batch } catch (PDOException $e) { throw match ($e->getCode()) { - 1062, 23000 => new Duplicate('Duplicated document: ' . $e->getMessage()), + 1062, 23000 => new DuplicateException('Duplicated document: ' . $e->getMessage()), default => $e, }; } @@ -1196,7 +1197,7 @@ public function updateDocument(string $collection, string $id, Document $documen switch ($e->getCode()) { case 1062: case 23505: - throw new Duplicate('Duplicated document: ' . $e->getMessage()); + throw new DuplicateException('Duplicated document: ' . $e->getMessage()); default: throw $e; } @@ -1448,7 +1449,7 @@ public function updateDocuments(string $collection, array $documents, int $batch } catch (PDOException $e) { throw match ($e->getCode()) { - 1062, 23000 => new Duplicate('Duplicated document: ' . $e->getMessage()), + 1062, 23000 => new DuplicateException('Duplicated document: ' . $e->getMessage()), default => $e, }; } @@ -1587,7 +1588,7 @@ public function deleteDocument(string $collection, string $id): bool * @return array * @throws Exception * @throws PDOException - * @throws Timeout + * @throws TimeoutException */ public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER): array { @@ -2212,8 +2213,8 @@ public function setTimeout(int $milliseconds, string $event = Database::EVENT_AL /** * @param PDOException $e - * @throws Timeout - * @throws Duplicate + * @throws TimeoutException + * @throws DuplicateException */ protected function processException(PDOException $e): void { @@ -2222,11 +2223,11 @@ protected function processException(PDOException $e): void */ if ($e->getCode() === '57014' && isset($e->errorInfo[1]) && $e->errorInfo[1] === 7) { - throw new Timeout($e->getMessage(), $e->getCode(), $e); + throw new TimeoutException($e->getMessage(), $e->getCode(), $e); } if ($e->getCode() === '42701' && isset($e->errorInfo[1]) && $e->errorInfo[1] === 7) { - throw new Duplicate($e->getMessage(), $e->getCode(), $e); + throw new DuplicateException($e->getMessage(), $e->getCode(), $e); } // Data is too big for column resize diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index bbe2bdfde..d18963c4f 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -10,6 +10,7 @@ use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Duplicate; use Utopia\Database\Exception\Duplicate as DuplicateException; +use Utopia\Database\Exception\NotFound as NotFoundException; use Utopia\Database\Exception\Timeout as TimeoutException; use Utopia\Database\Helpers\ID; @@ -307,7 +308,7 @@ public function deleteAttribute(string $collection, string $id, bool $array = fa $collection = $this->getDocument(Database::METADATA, $name); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $indexes = \json_decode($collection->getAttribute('indexes', []), true); @@ -349,7 +350,7 @@ public function renameIndex(string $collection, string $old, string $new): bool $collection = $this->getDocument(Database::METADATA, $collection); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $old = $this->filter($old); diff --git a/src/Database/Database.php b/src/Database/Database.php index 84bc5c616..621092dfd 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -9,6 +9,7 @@ use Utopia\Database\Exception\Conflict as ConflictException; use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Exception\Limit as LimitException; +use Utopia\Database\Exception\NotFound as NotFoundException; use Utopia\Database\Exception\Query as QueryException; use Utopia\Database\Exception\Relationship as RelationshipException; use Utopia\Database\Exception\Restricted as RestrictedException; @@ -1155,14 +1156,14 @@ public function updateCollection(string $id, array $permissions, bool $documentS $collection = $this->silent(fn () => $this->getCollection($id)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } if ( $this->adapter->getSharedTables() && $collection->getAttribute('$tenant') != $this->adapter->getTenant() ) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $collection @@ -1252,11 +1253,11 @@ public function getSizeOfCollection(string $collection): int $collection = $this->silent(fn () => $this->getCollection($collection)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } if ($this->adapter->getSharedTables() && $collection->getAttribute('$tenant') != $this->adapter->getTenant()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } return $this->adapter->getSizeOfCollection($collection->getId()); @@ -1278,11 +1279,11 @@ public function getSizeOfCollectionOnDisk(string $collection): int $collection = $this->silent(fn () => $this->getCollection($collection)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } if ($this->adapter->getSharedTables() && $collection->getAttribute('$tenant') != $this->adapter->getTenant()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } return $this->adapter->getSizeOfCollectionOnDisk($collection->getId()); @@ -1305,11 +1306,11 @@ public function deleteCollection(string $id): bool $collection = $this->silent(fn () => $this->getDocument(self::METADATA, $id)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } if ($this->adapter->getSharedTables() && $collection->getAttribute('$tenant') != $this->adapter->getTenant()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $relationships = \array_filter( @@ -1369,11 +1370,11 @@ public function createAttribute(string $collection, string $id, string $type, in $collection = $this->silent(fn () => $this->getCollection($collection)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } if ($this->adapter->getSharedTables() && $collection->getAttribute('$tenant') != $this->adapter->getTenant()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } // Attribute IDs are case insensitive @@ -1569,7 +1570,7 @@ protected function updateIndexMeta(string $collection, string $id, callable $upd $index = \array_search($id, \array_map(fn ($index) => $index['$id'], $indexes)); if ($index === false) { - throw new DatabaseException('Index not found'); + throw new NotFoundException('Index not found'); } // Execute update from callback @@ -1612,7 +1613,7 @@ protected function updateAttributeMeta(string $collection, string $id, callable $index = \array_search($id, \array_map(fn ($attribute) => $attribute['$id'], $attributes)); if ($index === false) { - throw new DatabaseException('Attribute not found'); + throw new NotFoundException('Attribute not found'); } // Execute update from callback @@ -1930,7 +1931,7 @@ public function deleteAttribute(string $collection, string $id): bool } if (\is_null($attribute)) { - throw new DatabaseException('Attribute not found'); + throw new NotFoundException('Attribute not found'); } if ($attribute['type'] === self::VAR_RELATIONSHIP) { @@ -1996,7 +1997,7 @@ public function renameAttribute(string $collection, string $old, string $new): b $attribute = \in_array($old, \array_map(fn ($attribute) => $attribute['$id'], $attributes)); if ($attribute === false) { - throw new DatabaseException('Attribute not found'); + throw new NotFoundException('Attribute not found'); } $attributeNew = \in_array($new, \array_map(fn ($attribute) => $attribute['$id'], $attributes)); @@ -2070,13 +2071,13 @@ public function createRelationship( $collection = $this->silent(fn () => $this->getCollection($collection)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $relatedCollection = $this->silent(fn () => $this->getCollection($relatedCollection)); if ($relatedCollection->isEmpty()) { - throw new DatabaseException('Related collection not found'); + throw new NotFoundException('Related collection not found'); } $id ??= $relatedCollection->getId(); @@ -2281,7 +2282,7 @@ public function updateRelationship( $attributeIndex = array_search($id, array_map(fn ($attribute) => $attribute['$id'], $attributes)); if ($attributeIndex === false) { - throw new DatabaseException('Attribute not found'); + throw new NotFoundException('Attribute not found'); } $attribute = $attributes[$attributeIndex]; @@ -2472,7 +2473,7 @@ public function deleteRelationship(string $collection, string $id): bool } if (\is_null($relationship)) { - throw new DatabaseException('Attribute not found'); + throw new NotFoundException('Attribute not found'); } $collection->setAttribute('attributes', \array_values($attributes)); @@ -2594,7 +2595,7 @@ public function renameIndex(string $collection, string $old, string $new): bool $index = \in_array($old, \array_map(fn ($index) => $index['$id'], $indexes)); if ($index === false) { - throw new DatabaseException('Index not found'); + throw new NotFoundException('Index not found'); } $indexNew = \in_array($new, \array_map(fn ($index) => $index['$id'], $indexes)); @@ -2819,7 +2820,7 @@ public function getDocument(string $collection, string $id, array $queries = [], } if (empty($collection)) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } if (empty($id)) { @@ -2829,7 +2830,7 @@ public function getDocument(string $collection, string $id, array $queries = [], $collection = $this->silent(fn () => $this->getCollection($collection)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $attributes = $collection->getAttribute('attributes', []); @@ -4425,7 +4426,7 @@ public function increaseDocumentAttribute(string $collection, string $id, string }); if (empty($attr)) { - throw new DatabaseException('Attribute not found'); + throw new NotFoundException('Attribute not found'); } $whiteList = [self::VAR_INTEGER, self::VAR_FLOAT]; @@ -4520,7 +4521,7 @@ public function decreaseDocumentAttribute(string $collection, string $id, string }); if (empty($attr)) { - throw new DatabaseException('Attribute not found'); + throw new NotFoundException('Attribute not found'); } $whiteList = [self::VAR_INTEGER, self::VAR_FLOAT]; @@ -5076,7 +5077,7 @@ public function find(string $collection, array $queries = []): array $collection = $this->silent(fn () => $this->getCollection($collection)); if ($collection->isEmpty()) { - throw new DatabaseException('Collection not found'); + throw new NotFoundException('Collection not found'); } $attributes = $collection->getAttribute('attributes', []); @@ -5548,7 +5549,7 @@ public function casting(Document $collection, Document $document): Document protected function encodeAttribute(string $name, mixed $value, Document $document): mixed { if (!array_key_exists($name, self::$filters) && !array_key_exists($name, $this->instanceFilters)) { - throw new DatabaseException("Filter: {$name} not found"); + throw new NotFoundException("Filter: {$name} not found"); } try { @@ -5584,7 +5585,7 @@ protected function decodeAttribute(string $name, mixed $value, Document $documen } if (!array_key_exists($name, self::$filters) && !array_key_exists($name, $this->instanceFilters)) { - throw new DatabaseException('Filter not found'); + throw new NotFoundException('Filter not found'); } if (array_key_exists($name, $this->instanceFilters)) { diff --git a/src/Database/Exception/NotFound.php b/src/Database/Exception/NotFound.php new file mode 100644 index 000000000..a7e7168f6 --- /dev/null +++ b/src/Database/Exception/NotFound.php @@ -0,0 +1,9 @@ + Date: Thu, 17 Oct 2024 21:26:34 +1300 Subject: [PATCH 05/34] Fix stan --- composer.json | 2 +- src/Database/Adapter/Postgres.php | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index d0097c5d5..a99e03468 100755 --- a/composer.json +++ b/composer.json @@ -29,7 +29,7 @@ ], "lint": "./vendor/bin/pint --test", "format": "./vendor/bin/pint", - "check": "./vendor/bin/phpstan analyse --level 7 src tests --memory-limit 512M", + "check": "./vendor/bin/phpstan analyse --level 7 src tests --memory-limit 2G", "coverage": "./vendor/bin/coverage-check ./tmp/clover.xml 90" }, "require": { diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index ae2f34619..f59371cd8 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -10,7 +10,6 @@ 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\Timeout as TimeoutException; use Utopia\Database\Exception\Truncate as TruncateException; use Utopia\Database\Query; @@ -888,7 +887,7 @@ public function createDocument(string $collection, Document $document): Document * * @return array * - * @throws Duplicate + * @throws DuplicateException */ public function createDocuments(string $collection, array $documents, int $batchSize = Database::INSERT_BATCH_SIZE): array { @@ -1215,7 +1214,7 @@ public function updateDocument(string $collection, string $id, Document $documen * * @return array * - * @throws Duplicate + * @throws DuplicateException */ public function updateDocuments(string $collection, array $documents, int $batchSize = Database::INSERT_BATCH_SIZE): array { From 5b86dcfabfdcc9a6f7a0ac008ff42eb3ca482896 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 1 Nov 2024 20:21:14 +1300 Subject: [PATCH 06/34] Merge pull request #462 from utopia-php/feat-batch-update Feat bulk update # Conflicts: # src/Database/Adapter/MariaDB.php # src/Database/Adapter/Mongo.php # src/Database/Adapter/Postgres.php # src/Database/Database.php # tests/e2e/Adapter/Base.php --- composer.json | 2 +- composer.lock | 4 +- src/Database/Adapter.php | 20 +- src/Database/Adapter/MariaDB.php | 388 ++++++++-------- src/Database/Adapter/Mongo.php | 69 ++- src/Database/Adapter/Postgres.php | 381 ++++++++-------- src/Database/Adapter/SQL.php | 18 +- src/Database/Adapter/SQLite.php | 256 +---------- src/Database/Database.php | 138 +++--- src/Database/Validator/PartialStructure.php | 50 +++ src/Database/Validator/Structure.php | 54 +++ tests/e2e/Adapter/Base.php | 465 +++++++++++++++++--- 12 files changed, 1078 insertions(+), 767 deletions(-) create mode 100644 src/Database/Validator/PartialStructure.php diff --git a/composer.json b/composer.json index d0097c5d5..a99e03468 100755 --- a/composer.json +++ b/composer.json @@ -29,7 +29,7 @@ ], "lint": "./vendor/bin/pint --test", "format": "./vendor/bin/pint", - "check": "./vendor/bin/phpstan analyse --level 7 src tests --memory-limit 512M", + "check": "./vendor/bin/phpstan analyse --level 7 src tests --memory-limit 2G", "coverage": "./vendor/bin/coverage-check ./tmp/clover.xml 90" }, "require": { diff --git a/composer.lock b/composer.lock index b01ed7d49..ee4eb4242 100644 --- a/composer.lock +++ b/composer.lock @@ -2585,7 +2585,7 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": {}, "prefer-stable": false, "prefer-lowest": false, "platform": { @@ -2593,6 +2593,6 @@ "ext-mbstring": "*", "php": ">=8.0" }, - "platform-dev": [], + "platform-dev": {}, "plugin-api-version": "2.6.0" } diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index c0f45fb8a..d174a8759 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -565,17 +565,19 @@ abstract public function createDocuments(string $collection, array $documents, i abstract public function updateDocument(string $collection, string $id, Document $document): Document; /** - * Update Documents in batches + * Update documents + * + * Updates all documents which match the given query. * * @param string $collection + * @param Document $updates * @param array $documents - * @param int $batchSize * - * @return array + * @return int * * @throws DatabaseException */ - abstract public function updateDocuments(string $collection, array $documents, int $batchSize): array; + abstract public function updateDocuments(string $collection, Document $updates, array $documents): int; /** * Delete Document @@ -600,10 +602,11 @@ abstract public function deleteDocument(string $collection, string $id): bool; * @param array $orderTypes * @param array $cursor * @param string $cursorDirection + * @param string $forPermission * * @return array */ - abstract public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER): array; + abstract public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER, string $forPermission = Database::PERMISSION_READ): array; /** * Sum an attribute @@ -747,6 +750,13 @@ abstract public function getSupportForRelationships(): bool; abstract public function getSupportForUpdateLock(): bool; + /** + * Are batch operations supported? + * + * @return bool + */ + abstract public function getSupportForBatchOperations(): bool; + /** * Is attribute resizing supported? * diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 246079eed..3df5cdd2d 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1306,260 +1306,259 @@ public function updateDocument(string $collection, string $id, Document $documen } /** - * Update Documents in batches + * Update documents + * + * Updates all documents which match the given query. * * @param string $collection + * @param Document $updates * @param array $documents - * @param int $batchSize * - * @return array + * @return int * - * @throws DuplicateException - * @throws \Throwable + * @throws DatabaseException */ - public function updateDocuments(string $collection, array $documents, int $batchSize = Database::INSERT_BATCH_SIZE): array + public function updateDocuments(string $collection, Document $updates, array $documents): int { - if (empty($documents)) { - return $documents; + $attributes = $updates->getAttributes(); + + if (!empty($updates->getUpdatedAt())) { + $attributes['_updatedAt'] = $updates->getUpdatedAt(); } - try { - $name = $this->filter($collection); - $batches = \array_chunk($documents, max(1, $batchSize)); + if (!empty($updates->getPermissions())) { + $attributes['_permissions'] = json_encode($updates->getPermissions()); + } - foreach ($batches as $batch) { - $bindIndex = 0; - $batchKeys = []; - $bindValues = []; + if (empty($attributes)) { + return 0; + } - $removeQuery = ''; - $removeBindValues = []; + $name = $this->filter($collection); - $addQuery = ''; - $addBindValues = []; - /* @var $document Document */ - foreach ($batch as $index => $document) { - $attributes = $document->getAttributes(); - $attributes['_uid'] = $document->getId(); - $attributes['_createdAt'] = $document->getCreatedAt(); - $attributes['_updatedAt'] = $document->getUpdatedAt(); - $attributes['_permissions'] = json_encode($document->getPermissions()); + $columns = ''; - if ($this->sharedTables) { - $attributes['_tenant'] = $this->tenant; - } + $where = []; - $columns = \array_map(function ($attribute) { - return "`" . $this->filter($attribute) . "`"; - }, \array_keys($attributes)); + $ids = \array_map(fn ($document) => $document->getId(), $documents); + $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($index) => ":_id_{$index}", \array_keys($ids))) . ")"; - $bindKeys = []; + if ($this->sharedTables) { + $where[] = "_tenant = :_tenant"; + } - foreach ($attributes as $value) { - if (\is_array($value)) { - $value = json_encode($value); - } - $value = (is_bool($value)) ? (int)$value : $value; - $bindKey = 'key_' . $bindIndex; - $bindKeys[] = ':' . $bindKey; - $bindValues[$bindKey] = $value; - $bindIndex++; - } + $sqlWhere = 'WHERE ' . implode(' AND ', $where); - $batchKeys[] = '(' . implode(', ', $bindKeys) . ')'; + $bindIndex = 0; + foreach ($attributes as $attribute => $value) { + $column = $this->filter($attribute); + $bindKey = 'key_' . $bindIndex; + $columns .= "`{$column}`" . '=:' . $bindKey; - // Permissions logic - $sql = " - SELECT _type, _permission - FROM {$this->getSQLTable($name . '_perms')} - WHERE _document = :_uid - "; + if ($attribute !== \array_key_last($attributes)) { + $columns .= ','; + } - if ($this->sharedTables) { - $sql .= ' AND _tenant = :_tenant'; - } + $bindIndex++; + } - $sql = $this->trigger(Database::EVENT_PERMISSIONS_READ, $sql); + $sql = " + UPDATE {$this->getSQLTable($name)} + SET {$columns} + {$sqlWhere} + "; - $permissionsStmt = $this->getPDO()->prepare($sql); - $permissionsStmt->bindValue(':_uid', $document->getId()); + $sql = $this->trigger(Database::EVENT_DOCUMENTS_UPDATE, $sql); + $stmt = $this->getPDO()->prepare($sql); - if ($this->sharedTables) { - $permissionsStmt->bindValue(':_tenant', $this->tenant); - } + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } - $permissionsStmt->execute(); - $permissions = $permissionsStmt->fetchAll(); + foreach ($ids as $id => $value) { + $stmt->bindValue(":_id_{$id}", $value); + } - $initial = []; - foreach (Database::PERMISSIONS as $type) { - $initial[$type] = []; - } + $attributeIndex = 0; + foreach ($attributes as $attribute => $value) { + if (is_array($value)) { + $value = json_encode($value); + } - $permissions = \array_reduce($permissions, function (array $carry, array $item) { - $carry[$item['_type']][] = $item['_permission']; - return $carry; - }, $initial); + $bindKey = 'key_' . $attributeIndex; + $value = (is_bool($value)) ? (int)$value : $value; + $stmt->bindValue(':' . $bindKey, $value, $this->getPDOType($value)); + $attributeIndex++; + } - // Get removed Permissions - $removals = []; - foreach (Database::PERMISSIONS as $type) { - $diff = array_diff($permissions[$type], $document->getPermissionsByType($type)); - if (!empty($diff)) { - $removals[$type] = $diff; - } - } + $stmt->execute(); + $affected = $stmt->rowCount(); - // Build inner query to remove permissions - if (!empty($removals)) { - foreach ($removals as $type => $permissionsToRemove) { - $bindKey = 'uid_' . $index; - $removeBindKeys[] = ':uid_' . $index; - $removeBindValues[$bindKey] = $document->getId(); + // Permissions logic + if (!empty($updates->getPermissions())) { + $removeQueries = []; + $removeBindValues = []; - $tenantQuery = ''; - if ($this->sharedTables) { - $tenantQuery = ' AND _tenant = :_tenant'; - } + $addQuery = ''; + $addBindValues = []; - $removeQuery .= "( - _document = :uid_{$index} - {$tenantQuery} - AND _type = '{$type}' - AND _permission IN (" . \implode(', ', \array_map(function (string $i) use ($permissionsToRemove, $index, $type, &$removeBindKeys, &$removeBindValues) { - $bindKey = 'remove_' . $type . '_' . $index . '_' . $i; - $removeBindKeys[] = ':' . $bindKey; - $removeBindValues[$bindKey] = $permissionsToRemove[$i]; - - return ':' . $bindKey; - }, \array_keys($permissionsToRemove))) . - ") - )"; - - if ($type !== \array_key_last($removals)) { - $removeQuery .= ' OR '; - } - } + /* @var $document Document */ + foreach ($documents as $index => $document) { + // Permissions logic + $sql = " + SELECT _type, _permission + FROM {$this->getSQLTable($name . '_perms')} + WHERE _document = :_uid + "; - if ($index !== \array_key_last($batch)) { - $removeQuery .= ' OR '; - } - } + if ($this->sharedTables) { + $sql .= ' AND _tenant = :_tenant'; + } - // Get added Permissions - $additions = []; - foreach (Database::PERMISSIONS as $type) { - $diff = \array_diff($document->getPermissionsByType($type), $permissions[$type]); - if (!empty($diff)) { - $additions[$type] = $diff; - } - } + $sql = $this->trigger(Database::EVENT_PERMISSIONS_READ, $sql); - // Build inner query to add permissions - if (!empty($additions)) { - foreach ($additions as $type => $permissionsToAdd) { - foreach ($permissionsToAdd as $i => $permission) { - $bindKey = 'uid_' . $index; - $addBindValues[$bindKey] = $document->getId(); + $permissionsStmt = $this->getPDO()->prepare($sql); + $permissionsStmt->bindValue(':_uid', $document->getId()); - $bindKey = 'add_' . $type . '_' . $index . '_' . $i; - $addBindValues[$bindKey] = $permission; + if ($this->sharedTables) { + $permissionsStmt->bindValue(':_tenant', $this->tenant); + } - $addQuery .= "(:uid_{$index}, '{$type}', :{$bindKey}"; + $permissionsStmt->execute(); + $permissions = $permissionsStmt->fetchAll(); - if ($this->sharedTables) { - $addQuery .= ", :_tenant)"; - } else { - $addQuery .= ")"; - } + $initial = []; + foreach (Database::PERMISSIONS as $type) { + $initial[$type] = []; + } - if ($i !== \array_key_last($permissionsToAdd) || $type !== \array_key_last($additions)) { - $addQuery .= ', '; - } - } - } - if ($index !== \array_key_last($batch)) { - $addQuery .= ', '; + $permissions = \array_reduce($permissions, function (array $carry, array $item) { + $carry[$item['_type']][] = $item['_permission']; + return $carry; + }, $initial); + + // Get removed Permissions + $removals = []; + foreach (Database::PERMISSIONS as $type) { + $diff = array_diff($permissions[$type], $updates->getPermissionsByType($type)); + if (!empty($diff)) { + $removals[$type] = $diff; + } + } + + // Build inner query to remove permissions + if (!empty($removals)) { + foreach ($removals as $type => $permissionsToRemove) { + $bindKey = 'uid_' . $index; + $removeBindKeys[] = ':uid_' . $index; + $removeBindValues[$bindKey] = $document->getId(); + + $tenantQuery = ''; + if ($this->sharedTables) { + $tenantQuery = ' AND _tenant = :_tenant'; } + + $removeQueries[] = "( + _document = :uid_{$index} + {$tenantQuery} + AND _type = '{$type}' + AND _permission IN (" . \implode(', ', \array_map(function (string $i) use ($permissionsToRemove, $index, $type, &$removeBindKeys, &$removeBindValues) { + $bindKey = 'remove_' . $type . '_' . $index . '_' . $i; + $removeBindKeys[] = ':' . $bindKey; + $removeBindValues[$bindKey] = $permissionsToRemove[$i]; + + return ':' . $bindKey; + }, \array_keys($permissionsToRemove))) . + ") + )"; } } - $updateClause = ''; - for ($i = 0; $i < \count($columns); $i++) { - $column = $columns[$i]; - if (!empty($updateClause)) { - $updateClause .= ', '; + // Get added Permissions + $additions = []; + foreach (Database::PERMISSIONS as $type) { + $diff = \array_diff($updates->getPermissionsByType($type), $permissions[$type]); + if (!empty($diff)) { + $additions[$type] = $diff; } - $updateClause .= "{$column} = VALUES({$column})"; } - $stmt = $this->getPDO()->prepare(" - INSERT INTO {$this->getSQLTable($name)} (" . \implode(", ", $columns) . ") - VALUES " . \implode(', ', $batchKeys) . " - ON DUPLICATE KEY UPDATE $updateClause - "); + // Build inner query to add permissions + if (!empty($additions)) { + foreach ($additions as $type => $permissionsToAdd) { + foreach ($permissionsToAdd as $i => $permission) { + $bindKey = 'uid_' . $index; + $addBindValues[$bindKey] = $document->getId(); - foreach ($bindValues as $key => $value) { - $stmt->bindValue($key, $value, $this->getPDOType($value)); - } + $bindKey = 'add_' . $type . '_' . $index . '_' . $i; + $addBindValues[$bindKey] = $permission; - $stmt->execute(); + $addQuery .= "(:uid_{$index}, '{$type}', :{$bindKey}"; - if (!empty($removeQuery)) { - $stmtRemovePermissions = $this->getPDO()->prepare(" - DELETE - FROM {$this->getSQLTable($name . '_perms')} - WHERE ({$removeQuery}) - "); + if ($this->sharedTables) { + $addQuery .= ", :_tenant)"; + } else { + $addQuery .= ")"; + } - foreach ($removeBindValues as $key => $value) { - $stmtRemovePermissions->bindValue($key, $value, $this->getPDOType($value)); + if ($i !== \array_key_last($permissionsToAdd) || $type !== \array_key_last($additions)) { + $addQuery .= ', '; + } + } } - if ($this->sharedTables) { - $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); + if ($index !== \array_key_last($documents)) { + $addQuery .= ', '; } - $stmtRemovePermissions->execute(); } + } - if (!empty($addQuery)) { - $sqlAddPermissions = " - INSERT INTO {$this->getSQLTable($name . '_perms')} (`_document`, `_type`, `_permission` - "; + if (!empty($removeQueries)) { + $removeQuery = \implode(' OR ', $removeQueries); - if ($this->sharedTables) { - $sqlAddPermissions .= ', `_tenant`)'; - } else { - $sqlAddPermissions .= ')'; - } + $stmtRemovePermissions = $this->getPDO()->prepare(" + DELETE + FROM {$this->getSQLTable($name . '_perms')} + WHERE ({$removeQuery}) + "); - $sqlAddPermissions .= " VALUES {$addQuery}"; + foreach ($removeBindValues as $key => $value) { + $stmtRemovePermissions->bindValue($key, $value, $this->getPDOType($value)); + } - $stmtAddPermissions = $this->getPDO()->prepare($sqlAddPermissions); + if ($this->sharedTables) { + $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); + } + $stmtRemovePermissions->execute(); + } - foreach ($addBindValues as $key => $value) { - $stmtAddPermissions->bindValue($key, $value, $this->getPDOType($value)); - } + if (!empty($addQuery)) { + $sqlAddPermissions = " + INSERT INTO {$this->getSQLTable($name . '_perms')} (`_document`, `_type`, `_permission` + "; - if ($this->sharedTables) { - $stmtAddPermissions->bindValue(':_tenant', $this->tenant); - } + if ($this->sharedTables) { + $sqlAddPermissions .= ', `_tenant`)'; + } else { + $sqlAddPermissions .= ')'; + } - $stmtAddPermissions->execute(); + $sqlAddPermissions .= " VALUES {$addQuery}"; + + $stmtAddPermissions = $this->getPDO()->prepare($sqlAddPermissions); + + foreach ($addBindValues as $key => $value) { + $stmtAddPermissions->bindValue($key, $value, $this->getPDOType($value)); } - } - } catch (\Throwable $e) { - if ($e instanceof PDOException) { - switch ($e->getCode()) { - case 1062: - case 23000: - throw new DuplicateException('Duplicated document: ' . $e->getMessage(), previous: $e); + + if ($this->sharedTables) { + $stmtAddPermissions->bindValue(':_tenant', $this->tenant); } - } - throw $e; + $stmtAddPermissions->execute(); + } } - return $documents; + return $affected; } /** @@ -1690,11 +1689,12 @@ public function deleteDocument(string $collection, string $id): bool * @param array $orderTypes * @param array $cursor * @param string $cursorDirection + * @param string $forPermission * @return array * @throws DatabaseException * @throws TimeoutException */ - public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER): array + public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER, string $forPermission = Database::PERMISSION_READ): array { $name = $this->filter($collection); $roles = Authorization::getRoles(); @@ -1784,7 +1784,7 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, } if (Authorization::$status) { - $where[] = $this->getSQLPermissionsCondition($name, $roles); + $where[] = $this->getSQLPermissionsCondition($name, $roles, $forPermission); } if ($this->sharedTables) { diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index f17cec26b..61c0d7bb7 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -821,35 +821,46 @@ public function updateDocument(string $collection, string $id, Document $documen } /** - * Update Documents in batches + * Update documents + * + * Updates all documents which match the given query. * * @param string $collection + * @param Document $updates * @param array $documents - * @param int $batchSize * - * @return array + * @return int * - * @throws Duplicate + * @throws DatabaseException */ - public function updateDocuments(string $collection, array $documents, int $batchSize): array + public function updateDocuments(string $collection, Document $updates, array $documents): int { $name = $this->getNamespace() . '_' . $this->filter($collection); - foreach ($documents as $index => $document) { - $document = $document->getArrayCopy(); - $document = $this->replaceChars('$', '_', $document); - $document = $this->timeToMongo($document); + $queries = [ + Query::equal('$id', array_map(fn ($document) => $document->getId(), $documents)) + ]; - $filters = []; - $filters['_uid'] = $document['_uid']; - if ($this->sharedTables) { - $filters['_tenant'] = (string)$this->getTenant(); - } + $filters = $this->buildFilters($queries); + if ($this->sharedTables) { + $filters['_tenant'] = (string)$this->getTenant(); + } + + $record = $updates->getArrayCopy(); + $record = $this->replaceChars('$', '_', $record); + $record = $this->timeToMongo($record); - $this->client->update($name, $filters, $document); + $updateQuery = [ + '$set' => $record, + ]; + + try { + $this->client->update($name, $filters, $updateQuery, multi: true); + } catch (MongoException $e) { + throw new Duplicate($e->getMessage()); } - return $documents; + return 1; } /** @@ -954,12 +965,13 @@ public function updateAttribute(string $collection, string $id, string $type, in * @param array $orderTypes * @param array $cursor * @param string $cursorDirection + * @param string $forPermission * * @return array * @throws Exception * @throws Timeout */ - public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER): array + public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER, string $forPermission = Database::PERMISSION_READ): array { $name = $this->getNamespace() . '_' . $this->filter($collection); $queries = array_map(fn ($query) => clone $query, $queries); @@ -971,9 +983,9 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, } // permissions - if (Authorization::$status) { // skip if authorization is disabled + if (Authorization::$status) { $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; + $filters['_permissions']['$in'] = [new Regex("{$forPermission}\(\".*(?:{$roles}).*\"\)", 'i')]; } $options = []; @@ -1164,8 +1176,13 @@ private function timeToDocument(array $record): array */ private function timeToMongo(array $record): array { - $record['_createdAt'] = $this->toMongoDatetime($record['_createdAt']); - $record['_updatedAt'] = $this->toMongoDatetime($record['_updatedAt']); + if (isset($record['_createdAt'])) { + $record['_createdAt'] = $this->toMongoDatetime($record['_createdAt']); + } + + if (isset($record['_updatedAt'])) { + $record['_updatedAt'] = $this->toMongoDatetime($record['_updatedAt']); + } return $record; } @@ -1681,6 +1698,16 @@ public function getSupportForAttributeResizing(): bool return false; } + /** + * Are batch operations supported? + * + * @return bool + */ + public function getSupportForBatchOperations(): bool + { + return false; + } + /** * Get current attribute count from collection document * diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 2b9007000..0082f4cde 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1206,252 +1206,258 @@ public function updateDocument(string $collection, string $id, Document $documen } /** - * Update Documents in batches + * Update documents + * + * Updates all documents which match the given query. * * @param string $collection + * @param Document $updates * @param array $documents - * @param int $batchSize * - * @return array + * @return int * - * @throws Duplicate + * @throws DatabaseException */ - public function updateDocuments(string $collection, array $documents, int $batchSize = Database::INSERT_BATCH_SIZE): array + public function updateDocuments(string $collection, Document $updates, array $documents): int { - if (empty($documents)) { - return $documents; - } + $attributes = $updates->getAttributes(); - try { - $name = $this->filter($collection); - $batches = \array_chunk($documents, max(1, $batchSize)); + if (!empty($updates->getUpdatedAt())) { + $attributes['_updatedAt'] = $updates->getUpdatedAt(); + } - foreach ($batches as $batch) { - $bindIndex = 0; - $batchKeys = []; - $bindValues = []; + if (!empty($updates->getPermissions())) { + $attributes['_permissions'] = json_encode($updates->getPermissions()); + } - $removeQuery = ''; - $removeBindValues = []; + if (empty($attributes)) { + return 0; + } - $addQuery = ''; - $addBindValues = []; + $name = $this->filter($collection); - foreach ($batch as $index => $document) { - $attributes = $document->getAttributes(); - $attributes['_uid'] = $document->getId(); - $attributes['_createdAt'] = $document->getCreatedAt(); - $attributes['_updatedAt'] = $document->getUpdatedAt(); - $attributes['_permissions'] = json_encode($document->getPermissions()); + $columns = ''; - if ($this->sharedTables) { - $attributes['_tenant'] = $this->tenant; - } + $where = []; - $columns = \array_map(function ($attribute) { - return '"' . $this->filter($attribute) . '"'; - }, \array_keys($attributes)); + $ids = \array_map(fn ($document) => $document->getId(), $documents); + $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($index) => ":_id_{$index}", \array_keys($ids))) . ")"; - $bindKeys = []; + if ($this->sharedTables) { + $where[] = "_tenant = :_tenant"; + } - foreach ($attributes as $value) { - if (\is_array($value)) { - $value = json_encode($value); - } - $value = (is_bool($value)) ? (int)$value : $value; - $bindKey = 'key_' . $bindIndex; - $bindKeys[] = ':' . $bindKey; - $bindValues[$bindKey] = $value; - $bindIndex++; - } + $sqlWhere = 'WHERE ' . implode(' AND ', $where); - $batchKeys[] = '(' . implode(', ', $bindKeys) . ')'; + $bindIndex = 0; + foreach ($attributes as $attribute => $value) { + $column = $this->filter($attribute); + $bindKey = 'key_' . $bindIndex; + $columns .= "\"{$column}\"" . '=:' . $bindKey; - // Permissions logic - $sql = " - SELECT _type, _permission - FROM {$this->getSQLTable($name . '_perms')} - WHERE _document = :_uid - "; + if ($attribute !== \array_key_last($attributes)) { + $columns .= ','; + } - if ($this->sharedTables) { - $sql .= ' AND _tenant = :_tenant'; - } + $bindIndex++; + } - $sql = $this->trigger(Database::EVENT_PERMISSIONS_READ, $sql); + $sql = " + UPDATE {$this->getSQLTable($name)} + SET {$columns} + {$sqlWhere} + "; - $permissionsStmt = $this->getPDO()->prepare($sql); - $permissionsStmt->bindValue(':_uid', $document->getId()); + $sql = $this->trigger(Database::EVENT_DOCUMENTS_UPDATE, $sql); + $stmt = $this->getPDO()->prepare($sql); - if ($this->sharedTables) { - $permissionsStmt->bindValue(':_tenant', $this->tenant); - } + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } - $permissionsStmt->execute(); - $permissions = $permissionsStmt->fetchAll(); + foreach ($ids as $id => $value) { + $stmt->bindValue(":_id_{$id}", $value); + } - $initial = []; - foreach (Database::PERMISSIONS as $type) { - $initial[$type] = []; - } + $attributeIndex = 0; + foreach ($attributes as $attribute => $value) { + if (is_array($value)) { + $value = json_encode($value); + } - $permissions = \array_reduce($permissions, function (array $carry, array $item) { - $carry[$item['_type']][] = $item['_permission']; - return $carry; - }, $initial); + $bindKey = 'key_' . $attributeIndex; + $value = (is_bool($value)) ? (int)$value : $value; + $stmt->bindValue(':' . $bindKey, $value, $this->getPDOType($value)); + $attributeIndex++; + } - // Get removed Permissions - $removals = []; - foreach (Database::PERMISSIONS as $type) { - $diff = array_diff($permissions[$type], $document->getPermissionsByType($type)); - if (!empty($diff)) { - $removals[$type] = $diff; - } - } + $stmt->execute(); + $affected = $stmt->rowCount(); - // Build inner query to remove permissions - if (!empty($removals)) { - foreach ($removals as $type => $permissionsToRemove) { - $bindKey = 'uid_' . $index; - $removeBindKeys[] = ':uid_' . $index; - $removeBindValues[$bindKey] = $document->getId(); + // Permissions logic + if (!empty($updates->getPermissions())) { + $removeQueries = []; + $removeBindValues = []; - $tenantQuery = ''; - if ($this->sharedTables) { - $tenantQuery = ' AND _tenant = :_tenant'; - } + $addQuery = ''; + $addBindValues = []; - $removeQuery .= "( - _document = :uid_{$index} - {$tenantQuery} - AND _type = '{$type}' - AND _permission IN (" . implode(', ', \array_map(function (string $i) use ($permissionsToRemove, $index, $type, &$removeBindKeys, &$removeBindValues) { - $bindKey = 'remove_' . $type . '_' . $index . '_' . $i; - $removeBindKeys[] = ':' . $bindKey; - $removeBindValues[$bindKey] = $permissionsToRemove[$i]; - - return ':' . $bindKey; - }, \array_keys($permissionsToRemove))) . - ") - )"; - - if ($type !== \array_key_last($removals)) { - $removeQuery .= ' OR '; - } - } + /* @var $document Document */ + foreach ($documents as $index => $document) { + // Permissions logic + $sql = " + SELECT _type, _permission + FROM {$this->getSQLTable($name . '_perms')} + WHERE _document = :_uid + "; - if ($index !== \array_key_last($batch)) { - $removeQuery .= ' OR '; - } - } + if ($this->sharedTables) { + $sql .= ' AND _tenant = :_tenant'; + } - // Get added Permissions - $additions = []; - foreach (Database::PERMISSIONS as $type) { - $diff = \array_diff($document->getPermissionsByType($type), $permissions[$type]); - if (!empty($diff)) { - $additions[$type] = $diff; - } - } + $sql = $this->trigger(Database::EVENT_PERMISSIONS_READ, $sql); - // Build inner query to add permissions - if (!empty($additions)) { - foreach ($additions as $type => $permissionsToAdd) { - foreach ($permissionsToAdd as $i => $permission) { - $bindKey = 'uid_' . $index; - $addBindValues[$bindKey] = $document->getId(); + $permissionsStmt = $this->getPDO()->prepare($sql); + $permissionsStmt->bindValue(':_uid', $document->getId()); - $bindKey = 'add_' . $type . '_' . $index . '_' . $i; - $addBindValues[$bindKey] = $permission; + if ($this->sharedTables) { + $permissionsStmt->bindValue(':_tenant', $this->tenant); + } - $tenantQuery = $this->sharedTables ? ', :_tenant' : ''; + $permissionsStmt->execute(); + $permissions = $permissionsStmt->fetchAll(); - $addQuery .= "(:uid_{$index}, '{$type}', :{$bindKey} {$tenantQuery})"; + $initial = []; + foreach (Database::PERMISSIONS as $type) { + $initial[$type] = []; + } - if ($i !== \array_key_last($permissionsToAdd) || $type !== \array_key_last($additions)) { - $addQuery .= ', '; - } - } - } - if ($index !== \array_key_last($batch)) { - $addQuery .= ', '; - } + $permissions = \array_reduce($permissions, function (array $carry, array $item) { + $carry[$item['_type']][] = $item['_permission']; + return $carry; + }, $initial); + + // Get removed Permissions + $removals = []; + foreach (Database::PERMISSIONS as $type) { + $diff = array_diff($permissions[$type], $updates->getPermissionsByType($type)); + if (!empty($diff)) { + $removals[$type] = $diff; } } - $updateClause = ''; - for ($i = 0; $i < \count($columns); $i++) { - $column = $columns[$i]; - if (!empty($updateClause)) { - $updateClause .= ', '; + // Build inner query to remove permissions + if (!empty($removals)) { + foreach ($removals as $type => $permissionsToRemove) { + $bindKey = 'uid_' . $index; + $removeBindKeys[] = ':uid_' . $index; + $removeBindValues[$bindKey] = $document->getId(); + + $tenantQuery = ''; + if ($this->sharedTables) { + $tenantQuery = ' AND _tenant = :_tenant'; + } + + $removeQueries[] = "( + _document = :uid_{$index} + {$tenantQuery} + AND _type = '{$type}' + AND _permission IN (" . \implode(', ', \array_map(function (string $i) use ($permissionsToRemove, $index, $type, &$removeBindKeys, &$removeBindValues) { + $bindKey = 'remove_' . $type . '_' . $index . '_' . $i; + $removeBindKeys[] = ':' . $bindKey; + $removeBindValues[$bindKey] = $permissionsToRemove[$i]; + + return ':' . $bindKey; + }, \array_keys($permissionsToRemove))) . + ") + )"; } - $updateClause .= "{$column} = excluded.{$column}"; } - $sql = " - INSERT INTO {$this->getSQLTable($name)} (" . \implode(", ", $columns) . ") - VALUES " . \implode(', ', $batchKeys) . " - "; - - if ($this->sharedTables) { - $sql .= "ON CONFLICT (_tenant, LOWER(_uid)) DO UPDATE SET $updateClause"; - } else { - $sql .= "ON CONFLICT (LOWER(_uid)) DO UPDATE SET $updateClause"; + // Get added Permissions + $additions = []; + foreach (Database::PERMISSIONS as $type) { + $diff = \array_diff($updates->getPermissionsByType($type), $permissions[$type]); + if (!empty($diff)) { + $additions[$type] = $diff; + } } - $stmt = $this->getPDO()->prepare($sql); + // Build inner query to add permissions + if (!empty($additions)) { + foreach ($additions as $type => $permissionsToAdd) { + foreach ($permissionsToAdd as $i => $permission) { + $bindKey = 'uid_' . $index; + $addBindValues[$bindKey] = $document->getId(); - foreach ($bindValues as $key => $value) { - $stmt->bindValue($key, $value, $this->getPDOType($value)); - } + $bindKey = 'add_' . $type . '_' . $index . '_' . $i; + $addBindValues[$bindKey] = $permission; - $stmt->execute(); + $addQuery .= "(:uid_{$index}, '{$type}', :{$bindKey}"; - if (!empty($removeQuery)) { - $stmtRemovePermissions = $this->getPDO()->prepare(" - DELETE - FROM {$this->getSQLTable($name . '_perms')} - WHERE ({$removeQuery}) - "); + if ($this->sharedTables) { + $addQuery .= ", :_tenant)"; + } else { + $addQuery .= ")"; + } - foreach ($removeBindValues as $key => $value) { - $stmtRemovePermissions->bindValue($key, $value, $this->getPDOType($value)); + if ($i !== \array_key_last($permissionsToAdd) || $type !== \array_key_last($additions)) { + $addQuery .= ', '; + } + } } - if ($this->sharedTables) { - $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); + if ($index !== \array_key_last($documents)) { + $addQuery .= ', '; } + } + } - $stmtRemovePermissions->execute(); + if (!empty($removeQueries)) { + $removeQuery = \implode(' OR ', $removeQueries); + + $stmtRemovePermissions = $this->getPDO()->prepare(" + DELETE + FROM {$this->getSQLTable($name . '_perms')} + WHERE ({$removeQuery}) + "); + + foreach ($removeBindValues as $key => $value) { + $stmtRemovePermissions->bindValue($key, $value, $this->getPDOType($value)); } + if ($this->sharedTables) { + $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); + } + $stmtRemovePermissions->execute(); + } - if (!empty($addQuery)) { - $tenantQuery = $this->sharedTables ? ', _tenant' : ''; + if (!empty($addQuery)) { + $sqlAddPermissions = " + INSERT INTO {$this->getSQLTable($name . '_perms')} (\"_document\", \"_type\", \"_permission\" + "; - $stmtAddPermissions = $this->getPDO()->prepare(" - INSERT INTO {$this->getSQLTable($name . '_perms')} (_document, _type, _permission {$tenantQuery}) - VALUES {$addQuery} - "); + if ($this->sharedTables) { + $sqlAddPermissions .= ', "_tenant")'; + } else { + $sqlAddPermissions .= ')'; + } - foreach ($addBindValues as $key => $value) { - $stmtAddPermissions->bindValue($key, $value, $this->getPDOType($value)); - } + $sqlAddPermissions .= " VALUES {$addQuery}"; - if ($this->sharedTables) { - $stmtAddPermissions->bindValue(':_tenant', $this->tenant); - } + $stmtAddPermissions = $this->getPDO()->prepare($sqlAddPermissions); - $stmtAddPermissions->execute(); + foreach ($addBindValues as $key => $value) { + $stmtAddPermissions->bindValue($key, $value, $this->getPDOType($value)); } - } - return $documents; - } catch (PDOException $e) { + if ($this->sharedTables) { + $stmtAddPermissions->bindValue(':_tenant', $this->tenant); + } - throw match ($e->getCode()) { - 1062, 23000 => new Duplicate('Duplicated document: ' . $e->getMessage()), - default => $e, - }; + $stmtAddPermissions->execute(); + } } + + return $affected; } /** @@ -1583,13 +1589,14 @@ public function deleteDocument(string $collection, string $id): bool * @param array $orderTypes * @param array $cursor * @param string $cursorDirection + * @param string $forPermission * * @return array * @throws Exception * @throws PDOException * @throws Timeout */ - public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER): array + public function find(string $collection, array $queries = [], ?int $limit = 25, ?int $offset = null, array $orderAttributes = [], array $orderTypes = [], array $cursor = [], string $cursorDirection = Database::CURSOR_AFTER, string $forPermission = Database::PERMISSION_READ): array { $name = $this->filter($collection); $roles = Authorization::getRoles(); @@ -1677,7 +1684,7 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, } if (Authorization::$status) { - $where[] = $this->getSQLPermissionsCondition($name, $roles); + $where[] = $this->getSQLPermissionsCondition($name, $roles, $forPermission); } $sqlWhere = !empty($where) ? 'WHERE ' . implode(' AND ', $where) : ''; diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index e73515a57..b06492f3f 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -361,6 +361,16 @@ public function getSupportForAttributeResizing(): bool return true; } + /** + * Are batch operations supported? + * + * @return bool + */ + public function getSupportForBatchOperations(): bool + { + return true; + } + /** * Get current attribute count from collection document * @@ -968,8 +978,12 @@ protected function getSQLIndexType(string $type): string * @return string * @throws Exception */ - protected function getSQLPermissionsCondition(string $collection, array $roles): string + protected function getSQLPermissionsCondition(string $collection, array $roles, string $type = Database::PERMISSION_READ): string { + if (!in_array($type, Database::PERMISSIONS)) { + throw new DatabaseException('Unknown permission type: ' . $type); + } + $roles = array_map(fn (string $role) => $this->getPDO()->quote($role), $roles); $tenantQuery = ''; @@ -981,7 +995,7 @@ protected function getSQLPermissionsCondition(string $collection, array $roles): SELECT _document FROM {$this->getSQLTable($collection . '_perms')} WHERE _permission IN (" . implode(', ', $roles) . ") - AND _type = 'read' + AND _type = '{$type}' {$tenantQuery} )"; } diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index bbe2bdfde..d88c21595 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -787,259 +787,7 @@ public function updateDocument(string $collection, string $id, Document $documen return $document; } - /** - * Update Documents in batches - * - * @param string $collection - * @param array $documents - * @param int $batchSize - * - * @return array - * - * @throws Duplicate - */ - public function updateDocuments(string $collection, array $documents, int $batchSize = Database::INSERT_BATCH_SIZE): array - { - if (empty($documents)) { - return $documents; - } - - try { - $name = $this->filter($collection); - $batches = \array_chunk($documents, max(1, $batchSize)); - - foreach ($batches as $batch) { - $bindIndex = 0; - $batchKeys = []; - $bindValues = []; - - $removeQuery = ''; - $removeBindValues = []; - - $addQuery = ''; - $addBindValues = []; - - foreach ($batch as $index => $document) { - $attributes = $document->getAttributes(); - $attributes['_uid'] = $document->getId(); - $attributes['_createdAt'] = $document->getCreatedAt(); - $attributes['_updatedAt'] = $document->getUpdatedAt(); - $attributes['_permissions'] = json_encode($document->getPermissions()); - - if ($this->sharedTables) { - $attributes['_tenant'] = $this->tenant; - } - - $columns = \array_map(function ($attribute) { - return "`" . $this->filter($attribute) . "`"; - }, \array_keys($attributes)); - - $bindKeys = []; - - foreach ($attributes as $value) { - if (\is_array($value)) { - $value = json_encode($value); - } - $value = (is_bool($value)) ? (int)$value : $value; - $bindKey = 'key_' . $bindIndex; - $bindKeys[] = ':' . $bindKey; - $bindValues[$bindKey] = $value; - $bindIndex++; - } - - $batchKeys[] = '(' . implode(', ', $bindKeys) . ')'; - - // Permissions logic - $sql = " - SELECT _type, _permission - FROM {$this->getSQLTable($name . '_perms')} - WHERE _document = :_uid - "; - - if ($this->sharedTables) { - $sql .= " AND _tenant = :_tenant"; - } - - $sql = $this->trigger(Database::EVENT_PERMISSIONS_READ, $sql); - - /** - * Get current permissions from the database - */ - $permissionsStmt = $this->getPDO()->prepare($sql); - - $permissionsStmt->bindValue(':_uid', $document->getId()); - - if ($this->sharedTables) { - $permissionsStmt->bindValue(':_tenant', $this->tenant); - } - - $permissionsStmt->execute(); - $permissions = $permissionsStmt->fetchAll(); - - $initial = []; - foreach (Database::PERMISSIONS as $type) { - $initial[$type] = []; - } - - $permissions = \array_reduce($permissions, function (array $carry, array $item) { - $carry[$item['_type']][] = $item['_permission']; - return $carry; - }, $initial); - - // Get removed Permissions - $removals = []; - foreach (Database::PERMISSIONS as $type) { - $diff = array_diff($permissions[$type], $document->getPermissionsByType($type)); - if (!empty($diff)) { - $removals[$type] = $diff; - } - } - - // Build inner query to remove permissions - if (!empty($removals)) { - foreach ($removals as $type => $permissionsToRemove) { - $bindKey = 'uid_' . $index; - $removeBindKeys[] = ':uid_' . $index; - $removeBindValues[$bindKey] = $document->getId(); - - $tenantQuery = ''; - if ($this->sharedTables) { - $tenantQuery = ' AND _tenant = :_tenant'; - } - - $removeQuery .= "( - _document = :uid_{$index} - {$tenantQuery} - AND _type = '{$type}' - AND _permission IN (" . implode(', ', \array_map(function (string $i) use ($permissionsToRemove, $index, $type, &$removeBindKeys, &$removeBindValues) { - $bindKey = 'remove_' . $type . '_' . $index . '_' . $i; - $removeBindKeys[] = ':' . $bindKey; - $removeBindValues[$bindKey] = $permissionsToRemove[$i]; - - return ':' . $bindKey; - }, \array_keys($permissionsToRemove))) . - ") - )"; - - if ($type !== \array_key_last($removals)) { - $removeQuery .= ' OR '; - } - } - - if ($index !== \array_key_last($batch)) { - $removeQuery .= ' OR '; - } - } - - // Get added Permissions - $additions = []; - foreach (Database::PERMISSIONS as $type) { - $diff = \array_diff($document->getPermissionsByType($type), $permissions[$type]); - if (!empty($diff)) { - $additions[$type] = $diff; - } - } - - // Build inner query to add permissions - if (!empty($additions)) { - foreach ($additions as $type => $permissionsToAdd) { - foreach ($permissionsToAdd as $i => $permission) { - $bindKey = 'uid_' . $index; - $addBindValues[$bindKey] = $document->getId(); - - $bindKey = 'add_' . $type . '_' . $index . '_' . $i; - $addBindValues[$bindKey] = $permission; - - $tenantQuery = $this->sharedTables ? ', :_tenant' : ''; - - $addQuery .= "(:uid_{$index}, '{$type}', :{$bindKey} {$tenantQuery})"; - - if ($i !== \array_key_last($permissionsToAdd) || $type !== \array_key_last($additions)) { - $addQuery .= ', '; - } - } - } - if ($index !== \array_key_last($batch)) { - $addQuery .= ', '; - } - } - } - - $updateClause = ''; - for ($i = 0; $i < \count($columns); $i++) { - $column = $columns[$i]; - if (!empty($updateClause)) { - $updateClause .= ', '; - } - $updateClause .= "{$column} = excluded.{$column}"; - } - - $sql = " - INSERT INTO {$this->getSQLTable($name)} (" . \implode(", ", $columns) . ") - VALUES " . \implode(', ', $batchKeys) . " - - "; - - if ($this->sharedTables) { - $sql .= "ON CONFLICT (_tenant, _uid) DO UPDATE SET $updateClause"; - } else { - $sql .= "ON CONFLICT (_uid) DO UPDATE SET $updateClause"; - } - - $stmt = $this->getPDO()->prepare($sql); - - foreach ($bindValues as $key => $value) { - $stmt->bindValue($key, $value, $this->getPDOType($value)); - } - - $stmt->execute(); - - if (!empty($removeQuery)) { - $stmtRemovePermissions = $this->getPDO()->prepare(" - DELETE - FROM {$this->getSQLTable($name . '_perms')} - WHERE ({$removeQuery}) - "); - - foreach ($removeBindValues as $key => $value) { - $stmtRemovePermissions->bindValue($key, $value, $this->getPDOType($value)); - } - - if ($this->sharedTables) { - $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); - } - - $stmtRemovePermissions->execute(); - } - - if (!empty($addQuery)) { - $tenantQuery = $this->sharedTables ? ', _tenant' : ''; - - $stmtAddPermissions = $this->getPDO()->prepare(" - INSERT INTO {$this->getSQLTable($name . '_perms')} (_document, _type, _permission {$tenantQuery}) - VALUES {$addQuery} - "); - - foreach ($addBindValues as $key => $value) { - $stmtAddPermissions->bindValue($key, $value, $this->getPDOType($value)); - } - if ($this->sharedTables) { - $stmtAddPermissions->bindValue(':_tenant', $this->tenant); - } - - $stmtAddPermissions->execute(); - } - } - - return $documents; - } catch (PDOException $e) { - throw match ($e->getCode()) { - 1062, 23000 => new Duplicate('Duplicated document: ' . $e->getMessage()), - default => $e, - }; - } - } /** * Is schemas supported? @@ -1187,14 +935,14 @@ protected function getSQLIndex(string $collection, string $id, string $type, arr * @return string * @throws Exception */ - protected function getSQLPermissionsCondition(string $collection, array $roles): string + protected function getSQLPermissionsCondition(string $collection, array $roles, string $type = Database::PERMISSION_READ): string { $roles = array_map(fn (string $role) => $this->getPDO()->quote($role), $roles); return "table_main._uid IN ( SELECT distinct(_document) FROM `{$this->getNamespace()}_{$collection}_perms` WHERE _permission IN (" . implode(', ', $roles) . ") - AND _type = 'read' + AND _type = '{$type}' )"; } diff --git a/src/Database/Database.php b/src/Database/Database.php index c032bd499..cbd099e61 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -19,6 +19,7 @@ use Utopia\Database\Helpers\Role; use Utopia\Database\Validator\Authorization; use Utopia\Database\Validator\Index as IndexValidator; +use Utopia\Database\Validator\PartialStructure; use Utopia\Database\Validator\Permissions; use Utopia\Database\Validator\Queries\Document as DocumentValidator; use Utopia\Database\Validator\Queries\Documents as DocumentsValidator; @@ -3900,84 +3901,117 @@ public function updateDocument(string $collection, string $id, Document $documen } /** - * Update Documents in a batch + * Update documents + * + * Updates all documents which match the given query. * * @param string $collection - * @param array $documents + * @param Document $updates + * @param array $queries * @param int $batchSize * - * @return array + * @return int * * @throws AuthorizationException - * @throws Exception - * @throws StructureException + * @throws DatabaseException */ - public function updateDocuments(string $collection, array $documents, int $batchSize = self::INSERT_BATCH_SIZE): array + public function updateDocuments(string $collection, Document $updates, array $queries = [], int $batchSize = self::INSERT_BATCH_SIZE): int { if ($this->adapter->getSharedTables() && empty($this->adapter->getTenant())) { throw new DatabaseException('Missing tenant. Tenant must be set when table sharing is enabled.'); } - if (empty($documents)) { - return []; + if ($updates->isEmpty()) { + return 0; } + $queries = Query::groupByType($queries)['filters']; $collection = $this->silent(fn () => $this->getCollection($collection)); - $documents = $this->withTransaction(function () use ($collection, $documents, $batchSize) { - $time = DateTime::now(); + unset($updates['$id']); + unset($updates['$createdAt']); + unset($updates['$tenant']); - foreach ($documents as $key => $document) { - if (!$document->getId()) { - throw new DatabaseException('Must define $id attribute for each document'); - } + if (!$this->preserveDates) { + $updates['$updatedAt'] = DateTime::now(); + } - $updatedAt = $document->getUpdatedAt(); - $document->setAttribute('$updatedAt', empty($updatedAt) || !$this->preserveDates ? $time : $updatedAt); - $document = $this->encode($collection, $document); - - $old = Authorization::skip(fn () => $this->silent( - fn () => $this->getDocument( - $collection->getId(), - $document->getId(), - forUpdate: true - ) - )); + $updates = $this->encode($collection, $updates); - $validator = new Authorization(self::PERMISSION_UPDATE); - if ( - $collection->getId() !== self::METADATA - && !$validator->isValid($old->getUpdate()) - ) { - throw new AuthorizationException($validator->getDescription()); - } + // Check new document structure + $validator = new PartialStructure($collection); + if (!$validator->isValid($updates)) { + throw new StructureException($validator->getDescription()); + } + + $affected = $this->withTransaction(function () use ($collection, $queries, $batchSize, $updates) { + $lastDocument = null; + $totalModified = 0; + $affectedDocumentIds = []; + + $documentSecurity = $collection->getAttribute('documentSecurity', false); - $validator = new Structure($collection); - if (!$validator->isValid($document)) { - throw new StructureException($validator->getDescription()); + $authorization = new Authorization(self::PERMISSION_UPDATE); + $skipAuth = $authorization->isValid($collection->getUpdate()); + + if (!$skipAuth && !$documentSecurity && $collection->getId() !== self::METADATA) { + throw new AuthorizationException($authorization->getDescription()); + } + + // Resolve and update relationships + while (true) { + $affectedDocuments = $this->find($collection->getId(), array_merge( + empty($lastDocument) ? [ + Query::limit($batchSize), + ] : [ + Query::limit($batchSize), + Query::cursorAfter($lastDocument), + ], + $queries, + ), forPermission: Database::PERMISSION_UPDATE); + + if (empty($affectedDocuments)) { + break; } - if ($this->resolveRelationships) { - $documents[$key] = $this->silent(fn () => $this->updateDocumentRelationships($collection, $old, $document)); + foreach ($affectedDocuments as $document) { + if ($this->resolveRelationships) { + $newDocument = array_merge($document->getArrayCopy(), $updates->getArrayCopy()); + + $this->silent(fn () => $this->updateDocumentRelationships($collection, $document, new Document($newDocument))); + } + + $affectedDocumentIds[] = $document->getId(); } - } - return $this->adapter->updateDocuments($collection->getId(), $documents, $batchSize); - }); + $getResults = fn () => $this->adapter->updateDocuments( + $collection->getId(), + $updates, + $affectedDocuments + ); - foreach ($documents as $key => $document) { - if ($this->resolveRelationships) { - $document = $this->silent(fn () => $this->populateDocumentRelationships($collection, $document)); + $result = $skipAuth ? $authorization->skip($getResults) : $getResults(); + + $totalModified += $result; + + if (count($affectedDocuments) < $batchSize) { + break; + } else { + $lastDocument = end($affectedDocuments); + } } - $documents[$key] = $this->decode($collection, $document); + $this->trigger(self::EVENT_DOCUMENTS_UPDATE, $affectedDocumentIds); - $this->purgeCachedDocument($collection->getId(), $document->getId()); - } + foreach ($affectedDocumentIds as $id) { + $this->purgeRelatedDocuments($collection, $id); + $this->purgeCachedDocument($collection->getId(), $id); + } - $this->trigger(self::EVENT_DOCUMENTS_UPDATE, $documents); + return $totalModified; + }); - return $documents; + return $affected; } /** @@ -5061,13 +5095,14 @@ public function purgeCachedDocument(string $collectionId, string $id): bool * * @param string $collection * @param array $queries + * @param string $forPermission * * @return array * @throws DatabaseException * @throws QueryException * @throws TimeoutException */ - public function find(string $collection, array $queries = []): array + public function find(string $collection, array $queries = [], string $forPermission = Database::PERMISSION_READ): array { if ($this->adapter->getSharedTables() && empty($this->adapter->getTenant())) { throw new DatabaseException('Missing tenant. Tenant must be set when table sharing is enabled.'); @@ -5091,7 +5126,7 @@ public function find(string $collection, array $queries = []): array $authorization = new Authorization(self::PERMISSION_READ); $documentSecurity = $collection->getAttribute('documentSecurity', false); - $skipAuth = $authorization->isValid($collection->getRead()); + $skipAuth = $authorization->isValid($collection->getPermissionsByType($forPermission)); if (!$skipAuth && !$documentSecurity && $collection->getId() !== self::METADATA) { throw new AuthorizationException($authorization->getDescription()); @@ -5178,7 +5213,8 @@ public function find(string $collection, array $queries = []): array $orderAttributes, $orderTypes, $cursor, - $cursorDirection ?? Database::CURSOR_AFTER + $cursorDirection ?? Database::CURSOR_AFTER, + $forPermission ); $results = $skipAuth ? Authorization::skip($getResults) : $getResults(); diff --git a/src/Database/Validator/PartialStructure.php b/src/Database/Validator/PartialStructure.php new file mode 100644 index 000000000..1f52a451c --- /dev/null +++ b/src/Database/Validator/PartialStructure.php @@ -0,0 +1,50 @@ +message = 'Value must be an instance of Document'; + return false; + } + + if (empty($this->collection->getId()) || Database::METADATA !== $this->collection->getCollection()) { + $this->message = 'Collection not found'; + return false; + } + + $keys = []; + $structure = $document->getArrayCopy(); + $attributes = \array_merge($this->attributes, $this->collection->getAttribute('attributes', [])); + + foreach ($attributes as $attribute) { + $name = $attribute['$id'] ?? ''; + $keys[$name] = $attribute; + } + + if (!$this->checkForUnknownAttributes($structure, $keys)) { + return false; + } + + if (!$this->checkForInvalidAttributeValues($structure, $keys)) { + return false; + } + + return true; + } +} diff --git a/src/Database/Validator/Structure.php b/src/Database/Validator/Structure.php index 991a32240..2e4cfde97 100644 --- a/src/Database/Validator/Structure.php +++ b/src/Database/Validator/Structure.php @@ -227,6 +227,32 @@ public function isValid($document): bool $structure = $document->getArrayCopy(); $attributes = \array_merge($this->attributes, $this->collection->getAttribute('attributes', [])); + if (!$this->checkForAllRequiredValues($structure, $attributes, $keys)) { + return false; + } + + if (!$this->checkForUnknownAttributes($structure, $keys)) { + return false; + } + + if (!$this->checkForInvalidAttributeValues($structure, $keys)) { + return false; + } + + return true; + } + + /** + * Check for all required values + * + * @param array $structure + * @param array $attributes + * @param array $keys + * + * @return bool + */ + protected function checkForAllRequiredValues(array $structure, array $attributes, array &$keys): bool + { foreach ($attributes as $key => $attribute) { // Check all required attributes are set $name = $attribute['$id'] ?? ''; $required = $attribute['required'] ?? false; @@ -239,12 +265,40 @@ public function isValid($document): bool } } + return true; + } + + /** + * Check for Unknown Attributes + * + * @param array $structure + * @param array $keys + * + * @return bool + */ + protected function checkForUnknownAttributes(array $structure, array $keys): bool + { foreach ($structure as $key => $value) { if (!array_key_exists($key, $keys)) { // Check no unknown attributes are set $this->message = 'Unknown attribute: "'.$key.'"'; return false; } + } + return true; + } + + /** + * Check for invalid attribute values + * + * @param array $structure + * @param array $keys + * + * @return bool + */ + protected function checkForInvalidAttributeValues(array $structure, array $keys): bool + { + foreach ($structure as $key => $value) { $attribute = $keys[$key] ?? []; $type = $attribute['type'] ?? ''; $array = $attribute['array'] ?? false; diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 297ec9d7a..b0c62fce2 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -751,9 +751,18 @@ public function testPreserveDatesUpdate(): void $doc1 = static::getDatabase()->getDocument('preserve_update_dates', 'doc1'); $this->assertEquals($newDate, $doc1->getAttribute('$updatedAt')); - $doc2->setAttribute('$updatedAt', $newDate); - $doc3->setAttribute('$updatedAt', $newDate); - static::getDatabase()->updateDocuments('preserve_update_dates', [$doc2, $doc3], 2); + $this->getDatabase()->updateDocuments( + 'preserve_update_dates', + new Document([ + '$updatedAt' => $newDate + ]), + [ + Query::equal('$id', [ + $doc2->getId(), + $doc3->getId() + ]) + ] + ); $doc2 = static::getDatabase()->getDocument('preserve_update_dates', 'doc2'); $doc3 = static::getDatabase()->getDocument('preserve_update_dates', 'doc3'); @@ -2455,53 +2464,6 @@ public function testUpdateDocument(Document $document): Document return $document; } - /** - * @depends testCreateDocuments - * @param array $documents - */ - public function testUpdateDocuments(array $documents): void - { - $collection = 'testCreateDocuments'; - - foreach ($documents as $document) { - $document - ->setAttribute('string', 'text📝 updated') - ->setAttribute('integer', 6) - ->setAttribute('$permissions', [ - Permission::read(Role::users()), - Permission::create(Role::users()), - Permission::update(Role::users()), - Permission::delete(Role::users()), - ]); - } - - $documents = static::getDatabase()->updateDocuments( - $collection, - $documents, - \count($documents) - ); - - foreach ($documents as $document) { - $this->assertEquals('text📝 updated', $document->getAttribute('string')); - $this->assertEquals(6, $document->getAttribute('integer')); - } - - $documents = static::getDatabase()->find($collection, [ - Query::limit(\count($documents)) - ]); - - foreach ($documents as $document) { - $this->assertEquals('text📝 updated', $document->getAttribute('string')); - $this->assertEquals(6, $document->getAttribute('integer')); - $this->assertEquals([ - Permission::read(Role::users()), - Permission::create(Role::users()), - Permission::update(Role::users()), - Permission::delete(Role::users()), - ], $document->getAttribute('$permissions')); - } - } - /** * @depends testUpdateDocument */ @@ -15673,6 +15635,409 @@ public function testTransformations(): void $this->assertTrue($result->isEmpty()); } + public function testUpdateDocuments(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForBatchOperations()) { + $this->expectNotToPerformAssertions(); + return; + } + + $collection = 'testUpdateDocuments'; + Authorization::cleanRoles(); + Authorization::setRole(Role::any()->toString()); + + static::getDatabase()->createCollection($collection, attributes: [ + new Document([ + '$id' => ID::custom('string'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => 100, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], + ]), + new Document([ + '$id' => ID::custom('integer'), + 'type' => Database::VAR_INTEGER, + 'format' => '', + 'size' => 10000, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], + ]), + ], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()) + ], documentSecurity: false); + + for ($i = 0; $i < 10; $i++) { + static::getDatabase()->createDocument($collection, new Document([ + '$id' => 'doc' . $i, + 'string' => 'text📝 ' . $i, + 'integer' => $i + ])); + } + + // Test Update half of the documents + $affected = static::getDatabase()->updateDocuments($collection, new Document([ + 'string' => 'text📝 updated', + ]), [ + Query::greaterThanEqual('integer', 5), + ]); + + $this->assertEquals($affected, 5); + + $updatedDocuments = static::getDatabase()->find($collection, [ + Query::greaterThanEqual('integer', 5), + ]); + + $this->assertEquals(count($updatedDocuments), 5); + + foreach ($updatedDocuments as $document) { + $this->assertEquals('text📝 updated', $document->getAttribute('string')); + } + + $controlDocuments = static::getDatabase()->find($collection, [ + Query::lessThan('integer', 5), + ]); + + $this->assertEquals(count($controlDocuments), 5); + + foreach ($controlDocuments as $document) { + $this->assertNotEquals('text📝 updated', $document->getAttribute('string')); + } + + // Test Update all documents + $affected = static::getDatabase()->updateDocuments($collection, new Document([ + 'string' => 'text📝 updated all', + ])); + + $this->assertEquals(10, $affected); + + $updatedDocuments = static::getDatabase()->find($collection); + + $this->assertEquals(count($updatedDocuments), 10); + + foreach ($updatedDocuments as $document) { + $this->assertEquals('text📝 updated all', $document->getAttribute('string')); + } + + // Check collection level permissions + static::getDatabase()->updateCollection($collection, permissions: [ + Permission::read(Role::user('asd')), + Permission::create(Role::user('asd')), + Permission::update(Role::user('asd')), + Permission::delete(Role::user('asd')), + ], documentSecurity: false); + + try { + static::getDatabase()->updateDocuments($collection, new Document([ + 'string' => 'text📝 updated all', + ])); + $this->fail('Failed to throw exception'); + } catch (AuthorizationException $e) { + $this->assertStringStartsWith('Missing "update" permission for role "user:asd".', $e->getMessage()); + } + + // Check document level permissions + static::getDatabase()->updateCollection($collection, permissions: [], documentSecurity: true); + + Authorization::skip(function () use ($collection) { + static::getDatabase()->updateDocument($collection, 'doc0', new Document([ + 'string' => 'text📝 updated all', + '$permissions' => [ + Permission::read(Role::user('asd')), + Permission::create(Role::user('asd')), + Permission::update(Role::user('asd')), + Permission::delete(Role::user('asd')), + ], + ])); + }); + + Authorization::setRole(Role::user('asd')->toString()); + + static::getDatabase()->updateDocuments($collection, new Document([ + 'string' => 'permission text', + ])); + + $documents = static::getDatabase()->find($collection, [ + Query::equal('string', ['permission text']), + ]); + + $this->assertCount(1, $documents); + + Authorization::skip(function () use ($collection) { + $unmodifiedDocuments = static::getDatabase()->find($collection, [ + Query::equal('string', ['text📝 updated all']), + ]); + + $this->assertCount(9, $unmodifiedDocuments); + }); + + Authorization::skip(function () use ($collection) { + static::getDatabase()->updateDocuments($collection, new Document([ + '$permissions' => [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + ])); + }); + + // Test we can update more documents than batchSize + $affected = static::getDatabase()->updateDocuments($collection, new Document([ + 'string' => 'batchSize Test' + ]), batchSize: 2); + + $documents = static::getDatabase()->find($collection); + + $this->assertEquals(10, $affected); + + foreach ($documents as $document) { + $this->assertEquals('batchSize Test', $document->getAttribute('string')); + } + + Authorization::cleanRoles(); + Authorization::setRole(Role::any()->toString()); + } + + public function testUpdateDocumentsPermissions(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForBatchOperations()) { + $this->expectNotToPerformAssertions(); + return; + } + + $collection = 'testUpdateDocumentsPerms'; + + static::getDatabase()->createCollection($collection, attributes: [ + new Document([ + '$id' => ID::custom('string'), + 'type' => Database::VAR_STRING, + 'size' => 767, + 'required' => true, + ]) + ], permissions: [], documentSecurity: true); + + // Test we can bulk update permissions we have access to + Authorization::skip(function () use ($collection) { + for ($i = 0; $i < 10; $i++) { + static::getDatabase()->createDocument($collection, new Document([ + '$id' => 'doc' . $i, + 'string' => 'text📝 ' . $i, + '$permissions' => [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()) + ], + ])); + } + + static::getDatabase()->createDocument($collection, new Document([ + '$id' => 'doc' . $i, + 'string' => 'text📝 ' . $i, + '$permissions' => [ + Permission::read(Role::user('user1')), + Permission::create(Role::user('user1')), + Permission::update(Role::user('user1')), + Permission::delete(Role::user('user1')) + ], + ])); + }); + + $affected = static::getDatabase()->updateDocuments($collection, new Document([ + '$permissions' => [ + Permission::read(Role::user('user2')), + Permission::create(Role::user('user2')), + Permission::update(Role::user('user2')), + Permission::delete(Role::user('user2')) + ], + ])); + + $documents = Authorization::skip(function () use ($collection) { + return static::getDatabase()->find($collection); + }); + + $this->assertEquals(10, $affected); + $this->assertCount(11, $documents); + + $modifiedDocuments = array_filter($documents, function (Document $document) { + return $document->getAttribute('$permissions') == [ + Permission::read(Role::user('user2')), + Permission::create(Role::user('user2')), + Permission::update(Role::user('user2')), + Permission::delete(Role::user('user2')) + ]; + }); + + $this->assertCount(10, $modifiedDocuments); + + $unmodifiedDocuments = array_filter($documents, function (Document $document) { + return $document->getAttribute('$permissions') == [ + Permission::read(Role::user('user1')), + Permission::create(Role::user('user1')), + Permission::update(Role::user('user1')), + Permission::delete(Role::user('user1')) + ]; + }); + + $this->assertCount(1, $unmodifiedDocuments); + + Authorization::setRole(Role::user('user2')->toString()); + + // Test Bulk permission update with data + $affected = static::getDatabase()->updateDocuments($collection, new Document([ + '$permissions' => [ + Permission::read(Role::user('user3')), + Permission::create(Role::user('user3')), + Permission::update(Role::user('user3')), + Permission::delete(Role::user('user3')) + ], + 'string' => 'text📝 updated', + ])); + + $this->assertEquals(10, $affected); + + $documents = Authorization::skip(function () use ($collection) { + return $this->getDatabase()->find($collection); + }); + + $this->assertCount(11, $documents); + + $modifiedDocuments = array_filter($documents, function (Document $document) { + return $document->getAttribute('$permissions') == [ + Permission::read(Role::user('user3')), + Permission::create(Role::user('user3')), + Permission::update(Role::user('user3')), + Permission::delete(Role::user('user3')) + ]; + }); + + foreach ($modifiedDocuments as $document) { + $this->assertEquals('text📝 updated', $document->getAttribute('string')); + } + } + + public function testUpdateDocumentsRelationships(): void + { + if (!$this->getDatabase()->getAdapter()->getSupportForBatchOperations() || !$this->getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + Authorization::cleanRoles(); + Authorization::setRole(Role::any()->toString()); + + $this->getDatabase()->createCollection('testUpdateDocumentsRelationships1', attributes: [ + new Document([ + '$id' => ID::custom('string'), + 'type' => Database::VAR_STRING, + 'size' => 767, + 'required' => true, + ]) + ], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()) + ]); + + $this->getDatabase()->createCollection('testUpdateDocumentsRelationships2', attributes: [ + new Document([ + '$id' => ID::custom('string'), + 'type' => Database::VAR_STRING, + 'size' => 767, + 'required' => true, + ]) + ], permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()) + ]); + + $this->getDatabase()->createRelationship( + collection: 'testUpdateDocumentsRelationships1', + relatedCollection: 'testUpdateDocumentsRelationships2', + type: Database::RELATION_ONE_TO_ONE, + twoWay: true, + ); + + $this->getDatabase()->createDocument('testUpdateDocumentsRelationships1', new Document([ + '$id' => 'doc1', + 'string' => 'text📝', + ])); + + $this->getDatabase()->createDocument('testUpdateDocumentsRelationships2', new Document([ + '$id' => 'doc1', + 'string' => 'text📝', + 'testUpdateDocumentsRelationships1' => 'doc1' + ])); + + $sisterDocument = $this->getDatabase()->getDocument('testUpdateDocumentsRelationships2', 'doc1'); + $this->assertNotNull($sisterDocument); + + $this->getDatabase()->updateDocuments('testUpdateDocumentsRelationships1', new Document([ + 'string' => 'text📝 updated', + ])); + + $document = $this->getDatabase()->findOne('testUpdateDocumentsRelationships1'); + + $this->assertNotFalse($document); + $this->assertEquals('text📝 updated', $document->getAttribute('string')); + + $sisterDocument = $this->getDatabase()->getDocument('testUpdateDocumentsRelationships2', 'doc1'); + $this->assertNotNull($sisterDocument); + + $relationalDocument = $sisterDocument->getAttribute('testUpdateDocumentsRelationships1'); + $this->assertEquals('text📝 updated', $relationalDocument->getAttribute('string')); + + // Check relationship value updating between each other. + $this->getDatabase()->deleteRelationship('testUpdateDocumentsRelationships1', 'testUpdateDocumentsRelationships2'); + + $this->getDatabase()->createRelationship( + collection: 'testUpdateDocumentsRelationships1', + relatedCollection: 'testUpdateDocumentsRelationships2', + type: Database::RELATION_ONE_TO_MANY, + twoWay: true, + ); + + for ($i = 2; $i < 11; $i++) { + $this->getDatabase()->createDocument('testUpdateDocumentsRelationships1', new Document([ + '$id' => 'doc' . $i, + 'string' => 'text📝', + ])); + + $this->getDatabase()->createDocument('testUpdateDocumentsRelationships2', new Document([ + '$id' => 'doc' . $i, + 'string' => 'text📝', + 'testUpdateDocumentsRelationships1' => 'doc' . $i + ])); + } + + $this->getDatabase()->updateDocuments('testUpdateDocumentsRelationships2', new Document([ + 'testUpdateDocumentsRelationships1' => null + ])); + + $this->getDatabase()->updateDocuments('testUpdateDocumentsRelationships2', new Document([ + 'testUpdateDocumentsRelationships1' => 'doc1' + ])); + + $documents = $this->getDatabase()->find('testUpdateDocumentsRelationships2'); + + foreach ($documents as $document) { + $this->assertEquals('doc1', $document->getAttribute('testUpdateDocumentsRelationships1')->getId()); + } + } + public function testEvents(): void { Authorization::skip(function () { From 616aa2f17283696249428271b463f05448fdba83 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 1 Nov 2024 20:53:14 +1300 Subject: [PATCH 07/34] Merge pull request #447 from utopia-php/feat-batch-delete Batch Deletes --- src/Database/Adapter.php | 10 + src/Database/Adapter/MariaDB.php | 69 ++++ src/Database/Adapter/Mongo.php | 37 ++ src/Database/Adapter/Postgres.php | 70 ++++ src/Database/Database.php | 86 +++- tests/e2e/Adapter/Base.php | 643 ++++++++++++++++++++++++++++++ 6 files changed, 914 insertions(+), 1 deletion(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index d174a8759..99f4add78 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -589,6 +589,16 @@ abstract public function updateDocuments(string $collection, Document $updates, */ abstract public function deleteDocument(string $collection, string $id): bool; + /** + * Delete Documents + * + * @param string $collection + * @param array $ids + * + * @return int + */ + abstract public function deleteDocuments(string $collection, array $ids): int; + /** * Find Documents * diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 3df5cdd2d..6fdd187bd 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1678,6 +1678,75 @@ public function deleteDocument(string $collection, string $id): bool return $deleted; } + /** + * Delete Documents + * + * @param string $collection + * @param array $ids + * + * @return int + */ + public function deleteDocuments(string $collection, array $ids): int + { + try { + $name = $this->filter($collection); + $where = []; + + if ($this->sharedTables) { + $where[] = "_tenant = :_tenant"; + } + + $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($index) => ":_id_{$index}", \array_keys($ids))) . ")"; + + $sql = "DELETE FROM {$this->getSQLTable($name)} WHERE " . \implode(' AND ', $where); + + $sql = $this->trigger(Database::EVENT_DOCUMENTS_DELETE, $sql); + + $stmt = $this->getPDO()->prepare($sql); + + foreach ($ids as $id => $value) { + $stmt->bindValue(":_id_{$id}", $value); + } + + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } + + $sql = " + DELETE FROM {$this->getSQLTable($name . '_perms')} + WHERE _document IN (" . \implode(', ', \array_map(fn ($index) => ":_id_{$index}", \array_keys($ids))) . ") + "; + + if ($this->sharedTables) { + $sql .= ' AND _tenant = :_tenant'; + } + + $sql = $this->trigger(Database::EVENT_PERMISSIONS_DELETE, $sql); + + $stmtPermissions = $this->getPDO()->prepare($sql); + + foreach ($ids as $id => $value) { + $stmtPermissions->bindValue(":_id_{$id}", $value); + } + + if ($this->sharedTables) { + $stmtPermissions->bindValue(':_tenant', $this->tenant); + } + + if (!$stmt->execute()) { + throw new DatabaseException('Failed to delete documents'); + } + + if (!$stmtPermissions->execute()) { + throw new DatabaseException('Failed to delete permissions'); + } + } catch (\Throwable $e) { + throw new DatabaseException($e->getMessage(), $e->getCode(), $e); + } + + return $stmt->rowCount(); + } + /** * Find Documents * diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 61c0d7bb7..105fab4cb 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -931,6 +931,43 @@ public function deleteDocument(string $collection, string $id): bool return (!!$result); } + /** + * Delete Documents + * + * @param string $collection + * @param array $ids + * + * @return int + */ + public function deleteDocuments(string $collection, array $ids): int + { + $name = $this->getNamespace() . '_' . $this->filter($collection); + + $filters = $this->buildFilters([new Query(Query::TYPE_EQUAL, '_uid', $ids)]); + + if ($this->sharedTables) { + $filters['_tenant'] = (string)$this->getTenant(); + } + + $filters = $this->replaceInternalIdsKeys($filters, '$', '_', $this->operators); + $filters = $this->timeFilter($filters); + + $options = []; + + try { + $count = $this->client->delete( + collection: $name, + filters: $filters, + options: $options, + limit: 0 + ); + } catch (MongoException $e) { + $this->processException($e); + } + + return $count ?? 0; + } + /** * Update Attribute. * @param string $collection diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 0082f4cde..5fdc08e06 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1576,6 +1576,76 @@ public function deleteDocument(string $collection, string $id): bool return $deleted; } + + /** + * Delete Documents + * + * @param string $collection + * @param array $ids + * + * @return int + */ + public function deleteDocuments(string $collection, array $ids): int + { + try { + $name = $this->filter($collection); + $where = []; + + if ($this->sharedTables) { + $where[] = "_tenant = :_tenant"; + } + + $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($index) => ":_id_{$index}", \array_keys($ids))) . ")"; + + $sql = "DELETE FROM {$this->getSQLTable($name)} WHERE " . \implode(' AND ', $where); + + $sql = $this->trigger(Database::EVENT_DOCUMENTS_DELETE, $sql); + + $stmt = $this->getPDO()->prepare($sql); + + foreach ($ids as $id => $value) { + $stmt->bindValue(":_id_{$id}", $value); + } + + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } + + $sql = " + DELETE FROM {$this->getSQLTable($name . '_perms')} + WHERE _document IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ") + "; + + if ($this->sharedTables) { + $sql .= ' AND _tenant = :_tenant'; + } + + $sql = $this->trigger(Database::EVENT_PERMISSIONS_DELETE, $sql); + + $stmtPermissions = $this->getPDO()->prepare($sql); + + foreach ($ids as $id => $value) { + $stmtPermissions->bindValue(":_id_{$id}", $value); + } + + if ($this->sharedTables) { + $stmtPermissions->bindValue(':_tenant', $this->tenant); + } + + if (!$stmt->execute()) { + throw new DatabaseException('Failed to delete documents'); + } + + if (!$stmtPermissions->execute()) { + throw new DatabaseException('Failed to delete permissions'); + } + } catch (\Throwable $e) { + throw new DatabaseException($e->getMessage(), $e->getCode(), $e); + } + + return $stmt->rowCount(); + } + /** * Find Documents * diff --git a/src/Database/Database.php b/src/Database/Database.php index cbd099e61..efde2151f 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -114,6 +114,7 @@ class Database public const EVENT_DOCUMENT_FIND = 'document_find'; public const EVENT_DOCUMENT_CREATE = 'document_create'; public const EVENT_DOCUMENTS_CREATE = 'documents_create'; + public const EVENT_DOCUMENTS_DELETE = 'documents_delete'; public const EVENT_DOCUMENT_READ = 'document_read'; public const EVENT_DOCUMENT_UPDATE = 'document_update'; public const EVENT_DOCUMENTS_UPDATE = 'documents_update'; @@ -136,6 +137,7 @@ class Database public const EVENT_INDEX_DELETE = 'index_delete'; public const INSERT_BATCH_SIZE = 100; + public const DELETE_BATCH_SIZE = 100; protected Adapter $adapter; @@ -4976,7 +4978,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection $this->deleteDocument( $relatedCollection->getId(), - $value->getId() + ($value instanceof Document) ? $value->getId() : $value ); \array_pop($this->relationshipDeleteStack); @@ -5051,6 +5053,88 @@ private function deleteCascade(Document $collection, Document $relatedCollection } } + /** + * Delete Documents + * + * Deletes all documents which match the given query, will respect the relationship's onDelete optin. + * + * @param string $collection + * @param array $queries + * @param int $batchSize + * + * @return int + * + * @throws AuthorizationException + * @throws DatabaseException + * @throws RestrictedException + */ + public function deleteDocuments(string $collection, array $queries = [], int $batchSize = self::DELETE_BATCH_SIZE): int + { + if ($this->adapter->getSharedTables() && empty($this->adapter->getTenant())) { + throw new DatabaseException('Missing tenant. Tenant must be set when table sharing is enabled.'); + } + + $queries = Query::groupByType($queries)['filters']; + $collection = $this->silent(fn () => $this->getCollection($collection)); + $affectedDocumentIds = []; + + $deleted = $this->withTransaction(function () use ($collection, $queries, $batchSize, $affectedDocumentIds) { + $lastDocument = null; + + $documentSecurity = $collection->getAttribute('documentSecurity', false); + $skipAuth = $this->authorization->isValid(new Input(self::PERMISSION_DELETE, $collection->getDelete())); + + if (!$skipAuth && !$documentSecurity && $collection->getId() !== self::METADATA) { + throw new AuthorizationException($this->authorization->getDescription()); + } + + while (true) { + $affectedDocuments = $this->find($collection->getId(), array_merge( + empty($lastDocument) ? [ + Query::limit($batchSize), + ] : [ + Query::limit($batchSize), + Query::cursorAfter($lastDocument), + ], + $queries, + ), forPermission: Database::PERMISSION_DELETE); + + if (empty($affectedDocuments)) { + break; + } + + $affectedDocumentIds = array_merge($affectedDocumentIds, array_map(fn ($document) => $document->getId(), $affectedDocuments)); + + foreach ($affectedDocuments as $document) { + // Delete Relationships + if ($this->resolveRelationships) { + $document = $this->silent(fn () => $this->deleteDocumentRelationships($collection, $document)); + } + + $this->purgeRelatedDocuments($collection, $document->getId()); + $this->purgeCachedDocument($collection->getId(), $document->getId()); + } + + if (count($affectedDocuments) < $batchSize) { + break; + } else { + $lastDocument = end($affectedDocuments); + } + } + + if (empty($affectedDocumentIds)) { + return 0; + } + + $this->trigger(self::EVENT_DOCUMENTS_DELETE, $affectedDocumentIds); + + // Mass delete using adapter with query + return $this->adapter->deleteDocuments($collection->getId(), $affectedDocumentIds); + }); + + return $deleted; + } + /** * Cleans the all the collection's documents from the cache * And the all related cached documents. diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index b0c62fce2..d603d27f9 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15635,6 +15635,649 @@ public function testTransformations(): void $this->assertTrue($result->isEmpty()); } + public function propegateBulkDocuments(bool $documentSecurity = false): void + { + for ($i = 0; $i < 10; $i++) { + static::getDatabase()->createDocument('bulk_delete', new Document( + array_merge([ + '$id' => 'doc' . $i, + 'text' => 'value' . $i, + 'integer' => $i + ], $documentSecurity ? [ + '$permissions' => [ + Permission::create(Role::any()), + Permission::read(Role::any()), + ], + ] : []) + )); + } + } + + public function testDeleteBulkDocuments(): void + { + static::getDatabase()->createCollection( + 'bulk_delete', + attributes: [ + new Document([ + '$id' => 'text', + 'type' => Database::VAR_STRING, + 'size' => 100, + 'required' => true, + ]), + new Document([ + '$id' => 'integer', + 'type' => Database::VAR_INTEGER, + 'size' => 10, + 'required' => true, + ]) + ], + documentSecurity: false, + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::delete(Role::any()) + ] + ); + + $this->propegateBulkDocuments(); + + $docs = static::getDatabase()->find('bulk_delete'); + $this->assertCount(10, $docs); + + // TEST: Bulk Delete All Documents + $deleted = static::getDatabase()->deleteDocuments('bulk_delete'); + $this->assertEquals(10, $deleted); + + $docs = static::getDatabase()->find('bulk_delete'); + $this->assertCount(0, $docs); + + // TEST: Bulk delete documents with queries. + $this->propegateBulkDocuments(); + + $deleted = static::getDatabase()->deleteDocuments('bulk_delete', [ + Query::greaterThanEqual('integer', 5) + ]); + $this->assertEquals(5, $deleted); + + $docs = static::getDatabase()->find('bulk_delete'); + $this->assertCount(5, $docs); + + // TEST (FAIL): Bulk delete all documents with invalid collection permission + static::getDatabase()->updateCollection('bulk_delete', [], false); + try { + static::getDatabase()->deleteDocuments('bulk_delete'); + $this->fail('Bulk deleted documents with invalid collection permission'); + } catch (\Utopia\Database\Exception\Authorization) { + } + + static::getDatabase()->updateCollection('bulk_delete', [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::delete(Role::any()) + ], false); + $deleted = static::getDatabase()->deleteDocuments('bulk_delete'); + + $this->assertEquals(5, $deleted); + $this->assertEquals(0, count($this->getDatabase()->find('bulk_delete'))); + + // TEST: Make sure we can't delete documents we don't have permissions for + static::getDatabase()->updateCollection('bulk_delete', [ + Permission::create(Role::any()), + ], true); + $this->propegateBulkDocuments(true); + + $deleted = static::getDatabase()->deleteDocuments('bulk_delete'); + $this->assertEquals(0, $deleted); + + $documents = static::$authorization->skip(function () { + return static::getDatabase()->find('bulk_delete'); + }); + + $this->assertCount(10, $documents); + + static::getDatabase()->updateCollection('bulk_delete', [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::delete(Role::any()) + ], false); + static::getDatabase()->deleteDocuments('bulk_delete'); + $this->assertEquals(0, count($this->getDatabase()->find('bulk_delete'))); + + // Teardown + static::getDatabase()->deleteCollection('bulk_delete'); + } + + public function testDeleteBulkDocumentsOneToOneRelationship(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + $this->getDatabase()->createCollection('bulk_delete_person_o2o'); + $this->getDatabase()->createCollection('bulk_delete_library_o2o'); + + $this->getDatabase()->createAttribute('bulk_delete_person_o2o', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_o2o', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_o2o', 'area', Database::VAR_STRING, 255, true); + + // Restrict + $this->getDatabase()->createRelationship( + collection: 'bulk_delete_person_o2o', + relatedCollection: 'bulk_delete_library_o2o', + type: Database::RELATION_ONE_TO_ONE, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2o' => [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + $library = $person1->getAttribute('bulk_delete_library_o2o'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); + + // Delete person + try { + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2o'); + $this->fail('Failed to throw exception'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + + $this->getDatabase()->updateDocument('bulk_delete_person_o2o', 'person1', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2o' => null, + ])); + + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2o')); + $this->getDatabase()->deleteDocuments('bulk_delete_library_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2o')); + + // NULL + $this->getDatabase()->updateRelationship( + collection: 'bulk_delete_person_o2o', + id: 'bulk_delete_library_o2o', + onDelete: Database::RELATION_MUTATE_SET_NULL + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2o' => [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + $library = $person1->getAttribute('bulk_delete_library_o2o'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); + + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + + $this->getDatabase()->deleteDocuments('bulk_delete_library_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2o')); + $this->assertCount(1, $this->getDatabase()->find('bulk_delete_person_o2o')); + + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + $library = $person->getAttribute('bulk_delete_library_o2o'); + $this->assertNull($library); + + // NULL - Cleanup + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2o')); + $this->getDatabase()->deleteDocuments('bulk_delete_library_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2o')); + + // Cascade + $this->getDatabase()->updateRelationship( + collection: 'bulk_delete_person_o2o', + id: 'bulk_delete_library_o2o', + onDelete: Database::RELATION_MUTATE_CASCADE + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2o' => [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + $library = $person1->getAttribute('bulk_delete_library_o2o'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); + + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + + $this->getDatabase()->deleteDocuments('bulk_delete_library_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2o')); + $this->assertCount(1, $this->getDatabase()->find('bulk_delete_person_o2o')); + + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + $library = $person->getAttribute('bulk_delete_library_o2o'); + $this->assertEmpty($library); + $this->assertNotNull($library); + + // Test Bulk delete parent + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2o')); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2o' => [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); + $library = $person1->getAttribute('bulk_delete_library_o2o'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); + + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2o')); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2o')); + } + + public function testDeleteBulkDocumentsOneToManyRelationship(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + $this->getDatabase()->createCollection('bulk_delete_person_o2m'); + $this->getDatabase()->createCollection('bulk_delete_library_o2m'); + + $this->getDatabase()->createAttribute('bulk_delete_person_o2m', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_o2m', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_o2m', 'area', Database::VAR_STRING, 255, true); + + // Restrict + $this->getDatabase()->createRelationship( + collection: 'bulk_delete_person_o2m', + relatedCollection: 'bulk_delete_library_o2m', + type: Database::RELATION_ONE_TO_MANY, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2m', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2m' => [ + [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + [ + '$id' => 'library2', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 2', + 'area' => 'Area 2', + ], + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_o2m', 'person1'); + $libraries = $person1->getAttribute('bulk_delete_library_o2m'); + $this->assertCount(2, $libraries); + + // Delete person + try { + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2m'); + $this->fail('Failed to throw exception'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + + // Restrict Cleanup + $this->getDatabase()->deleteDocuments('bulk_delete_library_o2m'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2m')); + + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2m'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2m')); + + // NULL + $this->getDatabase()->updateRelationship( + collection: 'bulk_delete_person_o2m', + id: 'bulk_delete_library_o2m', + onDelete: Database::RELATION_MUTATE_SET_NULL + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2m', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2m' => [ + [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + [ + '$id' => 'library2', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 2', + 'area' => 'Area 2', + ], + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_o2m', 'person1'); + $libraries = $person1->getAttribute('bulk_delete_library_o2m'); + $this->assertCount(2, $libraries); + + $this->getDatabase()->deleteDocuments('bulk_delete_library_o2m'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2m')); + + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2m', 'person1'); + $libraries = $person->getAttribute('bulk_delete_library_o2m'); + $this->assertEmpty($libraries); + + // NULL - Cleanup + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2m'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2m')); + + + // Cascade + $this->getDatabase()->updateRelationship( + collection: 'bulk_delete_person_o2m', + id: 'bulk_delete_library_o2m', + onDelete: Database::RELATION_MUTATE_CASCADE + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2m', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_o2m' => [ + [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + [ + '$id' => 'library2', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 2', + 'area' => 'Area 2', + ], + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_o2m', 'person1'); + $libraries = $person1->getAttribute('bulk_delete_library_o2m'); + $this->assertCount(2, $libraries); + + $this->getDatabase()->deleteDocuments('bulk_delete_library_o2m'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_o2m')); + + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2m', 'person1'); + $libraries = $person->getAttribute('bulk_delete_library_o2m'); + $this->assertEmpty($libraries); + } + + public function testDeleteBulkDocumentsManyToManyRelationship(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + $this->getDatabase()->createCollection('bulk_delete_person_m2m'); + $this->getDatabase()->createCollection('bulk_delete_library_m2m'); + + $this->getDatabase()->createAttribute('bulk_delete_person_m2m', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_m2m', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_m2m', 'area', Database::VAR_STRING, 255, true); + + // Many-to-Many Relationship + $this->getDatabase()->createRelationship( + collection: 'bulk_delete_person_m2m', + relatedCollection: 'bulk_delete_library_m2m', + type: Database::RELATION_MANY_TO_MANY, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_m2m', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_m2m' => [ + [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + [ + '$id' => 'library2', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 2', + 'area' => 'Area 2', + ], + ], + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_m2m', 'person1'); + $libraries = $person1->getAttribute('bulk_delete_library_m2m'); + $this->assertCount(2, $libraries); + + // Delete person + try { + $this->getDatabase()->deleteDocuments('bulk_delete_person_m2m'); + $this->fail('Failed to throw exception'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + + // Restrict Cleanup + $this->getDatabase()->deleteRelationship('bulk_delete_person_m2m', 'bulk_delete_library_m2m'); + $this->getDatabase()->deleteDocuments('bulk_delete_library_m2m'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_m2m')); + + $this->getDatabase()->deleteDocuments('bulk_delete_person_m2m'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_m2m')); + } + + public function testDeleteBulkDocumentsManyToOneRelationship(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + $this->getDatabase()->createCollection('bulk_delete_person_m2o'); + $this->getDatabase()->createCollection('bulk_delete_library_m2o'); + + $this->getDatabase()->createAttribute('bulk_delete_person_m2o', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_m2o', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library_m2o', 'area', Database::VAR_STRING, 255, true); + + // Many-to-One Relationship + $this->getDatabase()->createRelationship( + collection: 'bulk_delete_person_m2o', + relatedCollection: 'bulk_delete_library_m2o', + type: Database::RELATION_MANY_TO_ONE, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_m2o', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library_m2o' => [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + ])); + + $person2 = $this->getDatabase()->createDocument('bulk_delete_person_m2o', new Document([ + '$id' => 'person2', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 2', + 'bulk_delete_library_m2o' => [ + '$id' => 'library1', + ] + ])); + + $person1 = $this->getDatabase()->getDocument('bulk_delete_person_m2o', 'person1'); + $library = $person1->getAttribute('bulk_delete_library_m2o'); + $this->assertEquals('library1', $library['$id']); + + // Delete library + try { + $this->getDatabase()->deleteDocuments('bulk_delete_library_m2o'); + $this->fail('Failed to throw exception'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + + $this->assertEquals(2, count($this->getDatabase()->find('bulk_delete_person_m2o'))); + + // Test delete people + $this->getDatabase()->deleteDocuments('bulk_delete_person_m2o'); + $this->assertEquals(0, count($this->getDatabase()->find('bulk_delete_person_m2o'))); + + // Restrict Cleanup + $this->getDatabase()->deleteDocuments('bulk_delete_library_m2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library_m2o')); + + $this->getDatabase()->deleteDocuments('bulk_delete_person_m2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_m2o')); + } + public function testUpdateDocuments(): void { if (!static::getDatabase()->getAdapter()->getSupportForBatchOperations()) { From 84558a6bdfc870a98035b50b43841dae1418a19d Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 1 Nov 2024 21:08:46 +1300 Subject: [PATCH 08/34] Fix tests --- src/Database/Database.php | 5 +++-- tests/e2e/Adapter/Base.php | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index efde2151f..3fb8af6ce 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5082,10 +5082,11 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $lastDocument = null; $documentSecurity = $collection->getAttribute('documentSecurity', false); - $skipAuth = $this->authorization->isValid(new Input(self::PERMISSION_DELETE, $collection->getDelete())); + $authorization = new Authorization(self::PERMISSION_DELETE); + $skipAuth = $authorization->isValid($collection->getDelete()); if (!$skipAuth && !$documentSecurity && $collection->getId() !== self::METADATA) { - throw new AuthorizationException($this->authorization->getDescription()); + throw new AuthorizationException($authorization->getDescription()); } while (true) { diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index d603d27f9..20c7930f3 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15729,7 +15729,7 @@ public function testDeleteBulkDocuments(): void $deleted = static::getDatabase()->deleteDocuments('bulk_delete'); $this->assertEquals(0, $deleted); - $documents = static::$authorization->skip(function () { + $documents = Authorization::skip(function () { return static::getDatabase()->find('bulk_delete'); }); From 39550cb0e32cac45bf5ea1deb96569b837c028b1 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 4 Nov 2024 15:02:23 +1300 Subject: [PATCH 09/34] Rollback in transaction --- src/Database/Adapter/SQL.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index b06492f3f..745d2e59a 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -34,6 +34,10 @@ public function startTransaction(): bool { try { if ($this->inTransaction === 0) { + if ($this->getPDO()->inTransaction()) { + $this->getPDO()->rollBack(); + } + $result = $this->getPDO()->beginTransaction(); } else { $result = true; From afb9e96e9f9efc4617170e662f13ba7fe24b1e53 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 4 Nov 2024 18:46:17 +1300 Subject: [PATCH 10/34] Min/max date validation based on adapter limits --- composer.json | 2 +- src/Database/Adapter.php | 102 ++++++++++-------- src/Database/Adapter/MariaDB.php | 42 +++++--- src/Database/Adapter/Mongo.php | 59 ++++++----- src/Database/Adapter/Postgres.php | 82 +++++++------- src/Database/Adapter/SQLite.php | 22 ++-- src/Database/Database.php | 48 +++++++-- src/Database/Validator/Datetime.php | 59 +++++------ src/Database/Validator/Queries/Documents.php | 16 ++- src/Database/Validator/Query/Filter.php | 20 ++-- src/Database/Validator/Structure.php | 18 ++-- tests/e2e/Adapter/Base.php | 5 +- tests/unit/Validator/DateTimeTest.php | 106 ++++++++++++++----- 13 files changed, 362 insertions(+), 219 deletions(-) diff --git a/composer.json b/composer.json index 995ee1ef1..a1ca02e4b 100755 --- a/composer.json +++ b/composer.json @@ -35,7 +35,7 @@ "require": { "ext-pdo": "*", "ext-mbstring": "*", - "php": ">=8.0", + "php": ">=8.3", "utopia-php/framework": "0.33.*", "utopia-php/cache": "0.10.*", "utopia-php/mongo": "0.3.*" diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 99f4add78..f0f63b659 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -234,6 +234,35 @@ public function resetMetadata(): self return $this; } + /** + * Set a global timeout for database queries in milliseconds. + * + * This function allows you to set a maximum execution time for all database + * queries executed using the library, or a specific event specified by the + * event parameter. Once this timeout is set, any database query that takes + * longer than the specified time will be automatically terminated by the library, + * and an appropriate error or exception will be raised to handle the timeout condition. + * + * @param int $milliseconds The timeout value in milliseconds for database queries. + * @param string $event The event the timeout should fire fore + * @return void + * + * @throws Exception The provided timeout value must be greater than or equal to 0. + */ + abstract public function setTimeout(int $milliseconds, string $event = Database::EVENT_ALL): void; + + /** + * Clears a global timeout for database queries. + * + * @param string $event + * @return void + */ + public function clearTimeout(string $event): void + { + // Clear existing callback + $this->before($event, 'timeout', null); + } + /** * Start a new transaction. * @@ -395,6 +424,14 @@ abstract public function createCollection(string $name, array $attributes = [], */ abstract public function deleteCollection(string $id): bool; + /** + * Analyze a collection updating it's metadata on the database engine + * + * @param string $collection + * @return bool + */ + abstract public function analyzeCollection(string $collection): bool; + /** * Create Attribute * @@ -687,6 +724,28 @@ abstract public function getLimitForAttributes(): int; */ abstract public function getLimitForIndexes(): int; + /** + * @return int + */ + abstract public function getMaxIndexLength(): int; + + /** + * Get the minimum supported DateTime value + * + * @return \DateTime + */ + abstract public function getMinDateTime(): \DateTime; + + /** + * Get the maximum supported DateTime value + * + * @return \DateTime + */ + public function getMaxDateTime(): \DateTime + { + return new \DateTime('9999-12-31 23:59:59'); + } + /** * Is schemas supported? * @@ -921,47 +980,4 @@ public function escapeWildcards(string $value): string * @throws Exception */ abstract public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value, string $updatedAt, int|float|null $min = null, int|float|null $max = null): bool; - - /** - * @return int - */ - abstract public function getMaxIndexLength(): int; - - - /** - * Set a global timeout for database queries in milliseconds. - * - * This function allows you to set a maximum execution time for all database - * queries executed using the library, or a specific event specified by the - * event parameter. Once this timeout is set, any database query that takes - * longer than the specified time will be automatically terminated by the library, - * and an appropriate error or exception will be raised to handle the timeout condition. - * - * @param int $milliseconds The timeout value in milliseconds for database queries. - * @param string $event The event the timeout should fire fore - * @return void - * - * @throws Exception The provided timeout value must be greater than or equal to 0. - */ - abstract public function setTimeout(int $milliseconds, string $event = Database::EVENT_ALL): void; - - /** - * Clears a global timeout for database queries. - * - * @param string $event - * @return void - */ - public function clearTimeout(string $event): void - { - // Clear existing callback - $this->before($event, 'timeout', null); - } - - /** - * Analyze a collection updating it's metadata on the database engine - * - * @param string $collection - * @return bool - */ - abstract public function analyzeCollection(string $collection): bool; } diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index ade82a750..0e72eee4e 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -313,6 +313,22 @@ public function deleteCollection(string $id): bool ->execute(); } + /** + * Analyze a collection updating it's metadata on the database engine + * + * @param string $collection + * @return bool + */ + public function analyzeCollection(string $collection): bool + { + $name = $this->filter($collection); + + $sql = "ANALYZE TABLE {$this->getSQLTable($name)}"; + + $stmt = $this->getPDO()->prepare($sql); + return $stmt->execute(); + } + /** * Create Attribute * @@ -2285,6 +2301,16 @@ protected function getPDOType(mixed $value): int }; } + public function getMinDateTime(): \DateTime + { + return new \DateTime('1000-01-01 00:00:00'); + } + + public function getMaxDateTime(): \DateTime + { + return new \DateTime('9999-12-31 23:59:59'); + } + /** * Is fulltext Wildcard index supported? * @@ -2372,20 +2398,4 @@ protected function processException(PDOException $e): void throw $e; } - - /** - * Analyze a collection updating it's metadata on the database engine - * - * @param string $collection - * @return bool - */ - public function analyzeCollection(string $collection): bool - { - $name = $this->filter($collection); - - $sql = "ANALYZE TABLE {$this->getSQLTable($name)}"; - - $stmt = $this->getPDO()->prepare($sql); - return $stmt->execute(); - } } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 105fab4cb..433068396 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -57,6 +57,22 @@ public function __construct(Client $client) $this->client->connect(); } + public function setTimeout(int $milliseconds, string $event = Database::EVENT_ALL): void + { + if (!$this->getSupportForTimeouts()) { + return; + } + + $this->timeout = $milliseconds; + } + + public function clearTimeout(string $event): void + { + parent::clearTimeout($event); + + $this->timeout = null; + } + public function startTransaction(): bool { return true; @@ -320,6 +336,17 @@ public function deleteCollection(string $id): bool return (!!$this->getClient()->dropCollection($id)); } + /** + * Analyze a collection updating it's metadata on the database engine + * + * @param string $collection + * @return bool + */ + public function analyzeCollection(string $collection): bool + { + return false; + } + /** * Create Attribute * @@ -1640,6 +1667,11 @@ public function getLimitForIndexes(): int return 64; } + public function getMinDateTime(): \DateTime + { + return new \DateTime('-9999-01-01 00:00:00'); + } + /** * Is schemas supported? * @@ -1894,31 +1926,4 @@ public function getMaxIndexLength(): int { return 0; } - - public function setTimeout(int $milliseconds, string $event = Database::EVENT_ALL): void - { - if (!$this->getSupportForTimeouts()) { - return; - } - - $this->timeout = $milliseconds; - } - - public function clearTimeout(string $event): void - { - parent::clearTimeout($event); - - $this->timeout = null; - } - - /** - * Analyze a collection updating it's metadata on the database engine - * - * @param string $collection - * @return bool - */ - public function analyzeCollection(string $collection): bool - { - return false; - } } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 4f6d6a3dc..983e112dd 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -26,6 +26,30 @@ class Postgres extends SQL * 4. Full-text search is different - to_tsvector() and to_tsquery() */ + /** + * Returns Max Execution Time + * @param int $milliseconds + * @param string $event + * @return void + * @throws DatabaseException + */ + public function setTimeout(int $milliseconds, string $event = Database::EVENT_ALL): void + { + if (!$this->getSupportForTimeouts()) { + return; + } + if ($milliseconds <= 0) { + throw new DatabaseException('Timeout must be greater than 0'); + } + $this->before($event, 'timeout', function ($sql) use ($milliseconds) { + return " + SET statement_timeout = {$milliseconds}; + {$sql}; + SET statement_timeout = 0; + "; + }); + } + /** * Create Database * @@ -256,6 +280,17 @@ public function deleteCollection(string $id): bool ->execute(); } + /** + * Analyze a collection updating it's metadata on the database engine + * + * @param string $collection + * @return bool + */ + public function analyzeCollection(string $collection): bool + { + return false; + } + /** * Create Attribute * @@ -2233,12 +2268,16 @@ protected function decodeArray(array $value): string return '{' . implode(",", $value) . '}'; } + public function getMinDateTime(): \DateTime + { + return new \DateTime('-4713-01-01 00:00:00'); + } + /** * Is fulltext Wildcard index supported? * * @return bool */ - // TODO: Fix full-text search logic for postgres and MariaDB public function getSupportForFulltextWildcardIndex(): bool { return false; @@ -2265,27 +2304,11 @@ public function getSupportForJSONOverlaps(): bool } /** - * Returns Max Execution Time - * @param int $milliseconds - * @param string $event - * @return void - * @throws DatabaseException + * @return string */ - public function setTimeout(int $milliseconds, string $event = Database::EVENT_ALL): void + public function getLikeOperator(): string { - if (!$this->getSupportForTimeouts()) { - return; - } - if ($milliseconds <= 0) { - throw new DatabaseException('Timeout must be greater than 0'); - } - $this->before($event, 'timeout', function ($sql) use ($milliseconds) { - return " - SET statement_timeout = {$milliseconds}; - {$sql}; - SET statement_timeout = 0; - "; - }); + return 'ILIKE'; } /** @@ -2314,23 +2337,4 @@ protected function processException(PDOException $e): void throw $e; } - - /** - * @return string - */ - public function getLikeOperator(): string - { - return 'ILIKE'; - } - - /** - * Analyze a collection updating it's metadata on the database engine - * - * @param string $collection - * @return bool - */ - public function analyzeCollection(string $collection): bool - { - return false; - } } diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 8303ed720..5d1945598 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -267,6 +267,17 @@ public function deleteCollection(string $id): bool return true; } + /** + * Analyze a collection updating it's metadata on the database engine + * + * @param string $collection + * @return bool + */ + public function analyzeCollection(string $collection): bool + { + return false; + } + /** * Update Attribute * @@ -1140,15 +1151,4 @@ protected function processException(PDOException $e): void throw $e; } - - /** - * Analyze a collection updating it's metadata on the database engine - * - * @param string $collection - * @return bool - */ - public function analyzeCollection(string $collection): bool - { - return false; - } } diff --git a/src/Database/Database.php b/src/Database/Database.php index 7fdad32f9..ffbc61175 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -3287,7 +3287,11 @@ public function createDocument(string $collection, Document $document): Document } } - $structure = new Structure($collection); + $structure = new Structure( + $collection, + $this->adapter->getMinDateTime(), + $this->adapter->getMaxDateTime(), + ); if (!$structure->isValid($document)) { throw new StructureException($structure->getDescription()); } @@ -3349,7 +3353,11 @@ public function createDocuments(string $collection, array $documents, int $batch $document = $this->encode($collection, $document); - $validator = new Structure($collection); + $validator = new Structure( + $collection, + $this->adapter->getMinDateTime(), + $this->adapter->getMaxDateTime(), + ); if (!$validator->isValid($document)) { throw new StructureException($validator->getDescription()); } @@ -3876,7 +3884,11 @@ public function updateDocument(string $collection, string $id, Document $documen $document = $this->encode($collection, $document); - $structureValidator = new Structure($collection); + $structureValidator = new Structure( + $collection, + $this->adapter->getMinDateTime(), + $this->adapter->getMaxDateTime(), + ); if (!$structureValidator->isValid($document)) { // Make sure updated structure still apply collection rules (if any) throw new StructureException($structureValidator->getDescription()); } @@ -3942,7 +3954,11 @@ public function updateDocuments(string $collection, Document $updates, array $qu $updates = $this->encode($collection, $updates); // Check new document structure - $validator = new PartialStructure($collection); + $validator = new PartialStructure( + $collection, + $this->adapter->getMinDateTime(), + $this->adapter->getMaxDateTime(), + ); if (!$validator->isValid($updates)) { throw new StructureException($validator->getDescription()); } @@ -5204,7 +5220,13 @@ public function find(string $collection, array $queries = [], string $forPermiss $indexes = $collection->getAttribute('indexes', []); if ($this->validate) { - $validator = new DocumentsValidator($attributes, $indexes, maxValuesCount: $this->maxQueryValues); + $validator = new DocumentsValidator( + $attributes, + $indexes, + $this->maxQueryValues, + $this->adapter->getMinDateTime(), + $this->adapter->getMaxDateTime(), + ); if (!$validator->isValid($queries)) { throw new QueryException($validator->getDescription()); } @@ -5384,7 +5406,13 @@ public function count(string $collection, array $queries = [], ?int $max = null) $indexes = $collection->getAttribute('indexes', []); if ($this->validate) { - $validator = new DocumentsValidator($attributes, $indexes, maxValuesCount: $this->maxQueryValues); + $validator = new DocumentsValidator( + $attributes, + $indexes, + $this->maxQueryValues, + $this->adapter->getMinDateTime(), + $this->adapter->getMaxDateTime(), + ); if (!$validator->isValid($queries)) { throw new QueryException($validator->getDescription()); } @@ -5430,7 +5458,13 @@ public function sum(string $collection, string $attribute, array $queries = [], $indexes = $collection->getAttribute('indexes', []); if ($this->validate) { - $validator = new DocumentsValidator($attributes, $indexes, maxValuesCount: $this->maxQueryValues); + $validator = new DocumentsValidator( + $attributes, + $indexes, + $this->maxQueryValues, + $this->adapter->getMinDateTime(), + $this->adapter->getMaxDateTime(), + ); if (!$validator->isValid($queries)) { throw new QueryException($validator->getDescription()); } diff --git a/src/Database/Validator/Datetime.php b/src/Database/Validator/Datetime.php index 812669d7a..0d5c887f6 100644 --- a/src/Database/Validator/Datetime.php +++ b/src/Database/Validator/Datetime.php @@ -13,33 +13,18 @@ class Datetime extends Validator public const PRECISION_ANY = 'any'; /** - * @var string + * @throws \Exception */ - protected string $precision = self::PRECISION_ANY; - - /** - * @var bool - */ - protected bool $requireDateInFuture = false; - - - /** - * @var int minimum offset from now in seconds - */ - protected int $offset = 0; - - /** - * @param int $offset minimum offset from now in seconds - */ - public function __construct(bool $requireDateInFuture = false, string $precision = self::PRECISION_ANY, int $offset = 0) - { + public function __construct( + private readonly \DateTime $min = new \DateTime('0000-01-01'), + private readonly \DateTime $max = new \DateTime('9999-12-31'), + private readonly bool $requireDateInFuture = false, + private readonly string $precision = self::PRECISION_ANY, + private readonly int $offset = 0, + ) { if ($offset < 0) { - throw new \Exception('Offset must be a positive number.'); + throw new \Exception('Offset must be a positive integer.'); } - - $this->requireDateInFuture = $requireDateInFuture; - $this->offset = $offset; - $this->precision = $precision; } /** @@ -51,16 +36,19 @@ public function getDescription(): string $message = 'Value must be valid date'; if ($this->offset > 0) { - $message .= " at least " . $this->offset . " seconds in future"; + $message .= " at least " . $this->offset . " seconds in the future and"; } elseif ($this->requireDateInFuture) { - $message .= " in future"; + $message .= " in the future and"; } if ($this->precision !== self::PRECISION_ANY) { $message .= " with " . $this->precision . " precision"; } - $message .= '.'; + $min = $this->min->format('Y-m-d H:i:s'); + $max = $this->max->format('Y-m-d H:i:s'); + + $message .= " between {$min} and {$max}."; return $message; } @@ -114,13 +102,24 @@ public function isValid($value): bool return false; } } - } catch (\Exception $e) { + } catch (\Exception) { return false; } - [$year] = explode('-', $value); + // Custom year validation to account for PHP allowing year overflow + $matches = []; + if (preg_match('/(?min->format('Y'); + $maxYear = (int)$this->max->format('Y'); + if ($year < $minYear || $year > $maxYear) { + return false; + } + } else { + return false; + } - if ((int)$year > 9999) { + if ($date < $this->min || $date > $this->max) { return false; } diff --git a/src/Database/Validator/Queries/Documents.php b/src/Database/Validator/Queries/Documents.php index d308f874d..abce8694f 100644 --- a/src/Database/Validator/Queries/Documents.php +++ b/src/Database/Validator/Queries/Documents.php @@ -22,8 +22,13 @@ class Documents extends IndexedQueries * @param array $indexes * @throws Exception */ - public function __construct(array $attributes, array $indexes, int $maxValuesCount = 100) - { + public function __construct( + array $attributes, + array $indexes, + int $maxValuesCount = 100, + \DateTime $minAllowedDate = new \DateTime('0000-01-01'), + \DateTime $maxAllowedDate = new \DateTime('9999-12-31'), + ) { $attributes[] = new Document([ '$id' => '$id', 'key' => '$id', @@ -53,7 +58,12 @@ public function __construct(array $attributes, array $indexes, int $maxValuesCou new Limit(), new Offset(), new Cursor(), - new Filter($attributes, $maxValuesCount), + new Filter( + $attributes, + $maxValuesCount, + $minAllowedDate, + $maxAllowedDate, + ), new Order($attributes), new Select($attributes), ]; diff --git a/src/Database/Validator/Query/Filter.php b/src/Database/Validator/Query/Filter.php index 1fb50f9fc..9c810efcc 100644 --- a/src/Database/Validator/Query/Filter.php +++ b/src/Database/Validator/Query/Filter.php @@ -18,19 +18,21 @@ class Filter extends Base */ protected array $schema = []; - private int $maxValuesCount; - /** * @param array $attributes * @param int $maxValuesCount + * @param \DateTime $minAllowedDate + * @param \DateTime $maxAllowedDate */ - public function __construct(array $attributes = [], int $maxValuesCount = 100) - { + public function __construct( + array $attributes = [], + private readonly int $maxValuesCount = 100, + private readonly \DateTime $minAllowedDate = new \DateTime('0000-01-01'), + private readonly \DateTime $maxAllowedDate = new \DateTime('9999-12-31'), + ) { foreach ($attributes as $attribute) { $this->schema[$attribute->getAttribute('key', $attribute->getAttribute('$id'))] = $attribute->getArrayCopy(); } - - $this->maxValuesCount = $maxValuesCount; } /** @@ -93,7 +95,6 @@ protected function isValidAttributeAndValues(string $attribute, array $values, s $attributeType = $attributeSchema['type']; foreach ($values as $value) { - $validator = null; switch ($attributeType) { @@ -114,7 +115,10 @@ protected function isValidAttributeAndValues(string $attribute, array $values, s break; case Database::VAR_DATETIME: - $validator = new DatetimeValidator(); + $validator = new DatetimeValidator( + min: $this->minAllowedDate, + max: $this->maxAllowedDate + ); break; case Database::VAR_RELATIONSHIP: diff --git a/src/Database/Validator/Structure.php b/src/Database/Validator/Structure.php index 2e4cfde97..7b2e8b6f5 100644 --- a/src/Database/Validator/Structure.php +++ b/src/Database/Validator/Structure.php @@ -17,11 +17,6 @@ class Structure extends Validator { - /** - * @var Document - */ - protected Document $collection; - /** * @var array> */ @@ -106,9 +101,11 @@ class Structure extends Validator * Structure constructor. * */ - public function __construct(Document $collection) - { - $this->collection = $collection; + public function __construct( + protected readonly Document $collection, + private readonly \DateTime $minAllowedDate = new \DateTime('0000-01-01'), + private readonly \DateTime $maxAllowedDate = new \DateTime('9999-12-31'), + ) { } /** @@ -342,7 +339,10 @@ protected function checkForInvalidAttributeValues(array $structure, array $keys) break; case Database::VAR_DATETIME: - $validators[] = new DatetimeValidator(); + $validators[] = new DatetimeValidator( + min: $this->minAllowedDate, + max: $this->maxAllowedDate + ); break; default: diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 20c7930f3..6930d8d93 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -6138,7 +6138,10 @@ public function testCreateDatetime(): void $this->assertGreaterThan('2020-08-16T19:30:08.363+00:00', $doc->getUpdatedAt()); $document = static::getDatabase()->getDocument('datetime', 'id1234'); - $dateValidator = new DatetimeValidator(); + + $min = static::getDatabase()->getAdapter()->getMinDateTime(); + $max = static::getDatabase()->getAdapter()->getMaxDateTime(); + $dateValidator = new DatetimeValidator($min, $max); $this->assertEquals(null, $document->getAttribute('date2')); $this->assertEquals(true, $dateValidator->isValid($document->getAttribute('date'))); $this->assertEquals(false, $dateValidator->isValid($document->getAttribute('date2'))); diff --git a/tests/unit/Validator/DateTimeTest.php b/tests/unit/Validator/DateTimeTest.php index d8ababb6f..106080c29 100644 --- a/tests/unit/Validator/DateTimeTest.php +++ b/tests/unit/Validator/DateTimeTest.php @@ -8,6 +8,19 @@ class DateTimeTest extends TestCase { + private \DateTime $minAllowed; + private \DateTime $maxAllowed; + private string $minString = '0000-01-01 00:00:00'; + private string $maxString = '9999-12-31 23:59:59'; + + public function __construct() + { + parent::__construct(); + + $this->minAllowed = new \DateTime($this->minString); + $this->maxAllowed = new \DateTime($this->maxString); + } + public function setUp(): void { } @@ -18,7 +31,7 @@ public function tearDown(): void public function testCreateDatetime(): void { - $dateValidator = new DatetimeValidator(); + $dateValidator = new DatetimeValidator($this->minAllowed, $this->maxAllowed); $this->assertGreaterThan(DateTime::addSeconds(new \DateTime(), -3), DateTime::now()); $this->assertEquals(true, $dateValidator->isValid("2022-12-04")); @@ -29,7 +42,7 @@ public function testCreateDatetime(): void $now = DateTime::now(); $this->assertEquals(23, strlen($now)); $this->assertGreaterThan('2020-1-1 11:31:52.680', $now); - $this->assertEquals('Value must be valid date.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); $date = '2022-07-02 18:31:52.680'; $dateObject = new \DateTime($date); @@ -51,63 +64,100 @@ public function testCreateDatetime(): void * Test for Failure */ $this->assertEquals(false, $dateValidator->isValid("2022-13-04 11:31:52.680")); + $this->assertEquals(false, $dateValidator->isValid("-0001-13-04 00:00:00")); + $this->assertEquals(false, $dateValidator->isValid("0000-00-00 00:00:00")); + $this->assertEquals(false, $dateValidator->isValid("10000-01-01 00:00:00")); } public function testPastDateValidation(): void { - $dateValidator = new DatetimeValidator(requireDateInFuture: true); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + requireDateInFuture: true, + ); + $this->assertEquals(false, $dateValidator->isValid(DateTime::addSeconds(new \DateTime(), -3))); $this->assertEquals(true, $dateValidator->isValid(DateTime::addSeconds(new \DateTime(), 5))); - $this->assertEquals('Value must be valid date in future.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date in the future and between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); + + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + requireDateInFuture: false + ); - $dateValidator = new DatetimeValidator(requireDateInFuture: false); $this->assertEquals(true, $dateValidator->isValid(DateTime::addSeconds(new \DateTime(), -3))); $this->assertEquals(true, $dateValidator->isValid(DateTime::addSeconds(new \DateTime(), 5))); - $this->assertEquals('Value must be valid date.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); } public function testDatePrecision(): void { - $dateValidator = new DatetimeValidator(precision: DatetimeValidator::PRECISION_SECONDS); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + precision: DatetimeValidator::PRECISION_SECONDS + ); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.255')))); $this->assertEquals(true, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.000')))); $this->assertEquals(true, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26')))); - $this->assertEquals('Value must be valid date with seconds precision.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date with seconds precision between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); - $dateValidator = new DatetimeValidator(precision: DatetimeValidator::PRECISION_MINUTES); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + precision: DatetimeValidator::PRECISION_MINUTES + ); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.255')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.000')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26')))); $this->assertEquals(true, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:00')))); - $this->assertEquals('Value must be valid date with minutes precision.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date with minutes precision between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); - $dateValidator = new DatetimeValidator(precision: DatetimeValidator::PRECISION_HOURS); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + precision: DatetimeValidator::PRECISION_HOURS + ); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.255')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.000')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:00')))); $this->assertEquals(true, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:00:00')))); - $this->assertEquals('Value must be valid date with hours precision.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date with hours precision between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); - $dateValidator = new DatetimeValidator(precision: DatetimeValidator::PRECISION_DAYS); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + precision: DatetimeValidator::PRECISION_DAYS + ); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.255')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.000')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:00')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:00:00')))); $this->assertEquals(true, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 00:00:00')))); - $this->assertEquals('Value must be valid date with days precision.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date with days precision between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); - $dateValidator = new DatetimeValidator(true, DatetimeValidator::PRECISION_MINUTES); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + precision: DatetimeValidator::PRECISION_MINUTES + ); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2019-04-08 19:36:26.255')))); $this->assertEquals(false, $dateValidator->isValid(DateTime::format(new \DateTime('2100-04-08 19:36:26.255')))); $this->assertEquals(true, $dateValidator->isValid(DateTime::format(new \DateTime('2100-04-08 19:36:00')))); - $this->assertEquals('Value must be valid date in future with minutes precision.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date with minutes precision between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); } public function testOffset(): void { - $dateValidator = new DatetimeValidator(offset: 60); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + offset: 60 + ); $time = (new \DateTime()); $this->assertEquals(false, $dateValidator->isValid(DateTime::format($time))); @@ -115,22 +165,30 @@ public function testOffset(): void $this->assertEquals(false, $dateValidator->isValid(DateTime::format($time))); $time = $time->add(new \DateInterval('PT20S')); $this->assertEquals(true, $dateValidator->isValid(DateTime::format($time))); - $this->assertEquals('Value must be valid date at least 60 seconds in future.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date at least 60 seconds in the future and between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); - $dateValidator = new DatetimeValidator(requireDateInFuture: true, offset: 60); + $dateValidator = new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + requireDateInFuture: true, + offset: 60 + ); $time = (new \DateTime()); $time = $time->add(new \DateInterval('PT50S')); $time = $time->add(new \DateInterval('PT20S')); $this->assertEquals(true, $dateValidator->isValid(DateTime::format($time))); - $this->assertEquals('Value must be valid date at least 60 seconds in future.', $dateValidator->getDescription()); + $this->assertEquals("Value must be valid date at least 60 seconds in the future and between {$this->minString} and {$this->maxString}.", $dateValidator->getDescription()); - $threwException = false; try { - $dateValidator = new DatetimeValidator(offset: -60); + new DatetimeValidator( + $this->minAllowed, + $this->maxAllowed, + offset: -60 + ); + $this->fail('Failed to throw exception when offset is negative.'); } catch (\Exception $e) { - $threwException = true; + $this->assertEquals('Offset must be a positive integer.', $e->getMessage()); } - $this->assertTrue($threwException); } } From a98a71b6d47f7375bd112ee51e845de0ed329337 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Nov 2024 13:52:45 +0200 Subject: [PATCH 11/34] Validate date --- composer.lock | 4 ++-- src/Database/Database.php | 4 ++-- src/Database/Validator/Datetime.php | 2 +- tests/e2e/Adapter/Base.php | 9 +++++++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/composer.lock b/composer.lock index 49c2e3ddf..cc1b58ef6 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d8b51c1a58da391a80c01cd5b691b739", + "content-hash": "0fffc3b6680e12db596e0fb8dd971606", "packages": [ { "name": "jean85/pretty-package-versions", @@ -2591,7 +2591,7 @@ "platform": { "ext-pdo": "*", "ext-mbstring": "*", - "php": ">=8.0" + "php": ">=8.3" }, "platform-dev": [], "plugin-api-version": "2.6.0" diff --git a/src/Database/Database.php b/src/Database/Database.php index ffbc61175..10a79b3cb 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -406,11 +406,11 @@ function (mixed $value) { self::addFilter( 'datetime', /** - * @param string|null $value + * @param mixed $value * @return string|null * @throws Exception */ - function (?string $value) { + function (mixed $value) { if (is_null($value)) { return null; } diff --git a/src/Database/Validator/Datetime.php b/src/Database/Validator/Datetime.php index 0d5c887f6..7950b1e07 100644 --- a/src/Database/Validator/Datetime.php +++ b/src/Database/Validator/Datetime.php @@ -60,7 +60,7 @@ public function getDescription(): string */ public function isValid($value): bool { - if (empty($value)) { + if (empty($value) || ! is_string($value)) { return false; } diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 6930d8d93..6fde24f1f 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -6119,6 +6119,15 @@ public function testCreateDatetime(): void $this->assertEquals(true, static::getDatabase()->createAttribute('datetime', 'date', Database::VAR_DATETIME, 0, true, null, true, false, null, [], ['datetime'])); $this->assertEquals(true, static::getDatabase()->createAttribute('datetime', 'date2', Database::VAR_DATETIME, 0, false, null, true, false, null, [], ['datetime'])); + try { + static::getDatabase()->createDocument('datetime', new Document([ + 'date' => ['2020-01-01'], // array + ])); + $this->fail('Failed to throw exception'); + } catch (Exception $e) { + $this->assertInstanceOf(StructureException::class, $e); + } + $doc = static::getDatabase()->createDocument('datetime', new Document([ '$id' => ID::custom('id1234'), '$permissions' => [ From 0b24787365dc3dc2c22c442a4d705e33dafa1037 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Nov 2024 14:00:54 +0200 Subject: [PATCH 12/34] lint --- src/Database/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 10a79b3cb..356a6e439 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -412,7 +412,7 @@ function (mixed $value) { */ function (mixed $value) { if (is_null($value)) { - return null; + return; } try { $value = new \DateTime($value); From 184ba52bb067d502a5c33f0992fad172beec4a38 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Nov 2024 14:11:21 +0200 Subject: [PATCH 13/34] revert lock --- composer.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.lock b/composer.lock index cc1b58ef6..49c2e3ddf 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "0fffc3b6680e12db596e0fb8dd971606", + "content-hash": "d8b51c1a58da391a80c01cd5b691b739", "packages": [ { "name": "jean85/pretty-package-versions", @@ -2591,7 +2591,7 @@ "platform": { "ext-pdo": "*", "ext-mbstring": "*", - "php": ">=8.3" + "php": ">=8.0" }, "platform-dev": [], "plugin-api-version": "2.6.0" From 4a367eb98c29c8be73188b5f72b213d37f87f539 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Nov 2024 14:17:44 +0200 Subject: [PATCH 14/34] return null --- src/Database/Database.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 356a6e439..ea148e221 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -407,12 +407,11 @@ function (mixed $value) { 'datetime', /** * @param mixed $value - * @return string|null - * @throws Exception + * @return mixed */ function (mixed $value) { if (is_null($value)) { - return; + return null; } try { $value = new \DateTime($value); From 2952a8f947aedecd72bffbb0894645dfb1d51b1e Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Nov 2024 14:24:14 +0200 Subject: [PATCH 15/34] lint --- src/Database/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index ea148e221..13645e36f 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -411,7 +411,7 @@ function (mixed $value) { */ function (mixed $value) { if (is_null($value)) { - return null; + return; } try { $value = new \DateTime($value); From d148ebab23ee68130c31dc9b2950ddaf03ea23c9 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 18:07:55 +1300 Subject: [PATCH 16/34] Add transaction exception --- src/Database/Adapter/SQL.php | 13 +++++++------ src/Database/Exception/Transaction.php | 9 +++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 src/Database/Exception/Transaction.php diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 745d2e59a..6023212af 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -9,6 +9,7 @@ use Utopia\Database\Database; use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; +use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Query; abstract class SQL extends Adapter @@ -43,11 +44,11 @@ public function startTransaction(): bool $result = true; } } catch (PDOException $e) { - throw new DatabaseException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e); + throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e); } if (!$result) { - throw new DatabaseException('Failed to start transaction'); + throw new TransactionException('Failed to start transaction'); } $this->inTransaction++; @@ -70,13 +71,13 @@ public function commitTransaction(): bool try { $result = $this->getPDO()->commit(); } catch (PDOException $e) { - throw new DatabaseException('Failed to commit transaction: ' . $e->getMessage(), $e->getCode(), $e); + throw new TransactionException('Failed to commit transaction: ' . $e->getMessage(), $e->getCode(), $e); } finally { $this->inTransaction--; } if (!$result) { - throw new DatabaseException('Failed to commit transaction'); + throw new TransactionException('Failed to commit transaction'); } return $result; @@ -94,13 +95,13 @@ public function rollbackTransaction(): bool try { $result = $this->getPDO()->rollBack(); } catch (PDOException $e) { - throw new DatabaseException('Failed to rollback transaction: ' . $e->getMessage(), $e->getCode(), $e); + throw new TransactionException('Failed to rollback transaction: ' . $e->getMessage(), $e->getCode(), $e); } finally { $this->inTransaction = 0; } if (!$result) { - throw new DatabaseException('Failed to rollback transaction'); + throw new TransactionException('Failed to rollback transaction'); } return $result; diff --git a/src/Database/Exception/Transaction.php b/src/Database/Exception/Transaction.php new file mode 100644 index 000000000..3a3ddf0af --- /dev/null +++ b/src/Database/Exception/Transaction.php @@ -0,0 +1,9 @@ + Date: Tue, 5 Nov 2024 18:08:21 +1300 Subject: [PATCH 17/34] Handle possible implicit commit --- src/Database/Adapter.php | 33 ++++++++++++++++++++++++--------- src/Database/Adapter/SQL.php | 5 +++++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index f0f63b659..3b7aedd91 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -315,16 +315,31 @@ public function inTransaction(): bool */ public function withTransaction(callable $callback): mixed { - $this->startTransaction(); - - try { - $result = $callback(); - $this->commitTransaction(); - return $result; - } catch (\Throwable $e) { - $this->rollbackTransaction(); - throw $e; + for ($attempts = 0; $attempts < 3; $attempts++) { + try { + $this->startTransaction(); + $result = $callback(); + $this->commitTransaction(); + return $result; + } catch (\Throwable $e) { + + try { + $this->rollbackTransaction(); + } catch (\Throwable) { + if ($attempts < 2) { + continue; + } + } + + if ($attempts < 2) { + continue; + } + + throw $e; + } } + + throw new TransactionException('Failed to execute transaction'); } /** diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 6023212af..06b7136d2 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -68,6 +68,11 @@ public function commitTransaction(): bool return true; } + if (!$this->getPDO()->inTransaction()) { + $this->inTransaction = 0; + return false; + } + try { $result = $this->getPDO()->commit(); } catch (PDOException $e) { From 6c10f97e50131e9fc655463064365a31480c0098 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 18:09:17 +1300 Subject: [PATCH 18/34] Reset counter to 0 only if commit succeeded --- src/Database/Adapter/SQL.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 06b7136d2..fbf3ad887 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -75,10 +75,9 @@ public function commitTransaction(): bool try { $result = $this->getPDO()->commit(); + $this->inTransaction = 0; } catch (PDOException $e) { throw new TransactionException('Failed to commit transaction: ' . $e->getMessage(), $e->getCode(), $e); - } finally { - $this->inTransaction--; } if (!$result) { From d64ce93f26e9b952b1ff2d98d51e90a1fbb229fc Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 18:11:16 +1300 Subject: [PATCH 19/34] Savepoints for nested transactions --- src/Database/Adapter/SQL.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index fbf3ad887..64ce0235e 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -41,6 +41,7 @@ public function startTransaction(): bool $result = $this->getPDO()->beginTransaction(); } else { + $this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction); $result = true; } } catch (PDOException $e) { @@ -97,11 +98,16 @@ public function rollbackTransaction(): bool } try { - $result = $this->getPDO()->rollBack(); + if ($this->inTransaction > 1) { + $this->getPDO()->exec('ROLLBACK TO transaction' . $this->inTransaction); + $this->inTransaction--; + $result = true; + } else { + $result = $this->getPDO()->rollBack(); + $this->inTransaction = 0; + } } catch (PDOException $e) { - throw new TransactionException('Failed to rollback transaction: ' . $e->getMessage(), $e->getCode(), $e); - } finally { - $this->inTransaction = 0; + throw new DatabaseException('Failed to rollback transaction: ' . $e->getMessage(), $e->getCode(), $e); } if (!$result) { From 3b2b22ecb8a7683c3a06b9849a27548e5c46ae53 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 18:56:13 +1300 Subject: [PATCH 20/34] Remove savepoints from postgres --- src/Database/Adapter.php | 1 - src/Database/Adapter/Postgres.php | 52 +++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 3b7aedd91..d78822fba 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -322,7 +322,6 @@ public function withTransaction(callable $callback): mixed $this->commitTransaction(); return $result; } catch (\Throwable $e) { - try { $this->rollbackTransaction(); } catch (\Throwable) { diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 983e112dd..4fa08fcb1 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -11,6 +11,7 @@ use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Exception\Timeout as TimeoutException; +use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Exception\Truncate as TruncateException; use Utopia\Database\Query; use Utopia\Database\Validator\Authorization; @@ -26,6 +27,57 @@ class Postgres extends SQL * 4. Full-text search is different - to_tsvector() and to_tsquery() */ + /** + * @inheritDoc + */ + public function startTransaction(): bool + { + try { + if ($this->inTransaction === 0) { + if ($this->getPDO()->inTransaction()) { + $this->getPDO()->rollBack(); + } + + $result = $this->getPDO()->beginTransaction(); + } else { + $result = true; + } + } catch (PDOException $e) { + throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e); + } + + if (!$result) { + throw new TransactionException('Failed to start transaction'); + } + + $this->inTransaction++; + + return $result; + } + + /** + * @inheritDoc + */ + public function rollbackTransaction(): bool + { + if ($this->inTransaction === 0) { + return false; + } + + try { + $result = $this->getPDO()->rollBack(); + $this->inTransaction = 0; + } catch (PDOException $e) { + throw new DatabaseException('Failed to rollback transaction: ' . $e->getMessage(), $e->getCode(), $e); + } + + if (!$result) { + throw new TransactionException('Failed to rollback transaction'); + } + + return $result; + } + /** * Returns Max Execution Time * @param int $milliseconds From 160110f2750aca3eb10592a5219d472d3f8261e0 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 19:23:59 +1300 Subject: [PATCH 21/34] Issue raw rollback to be certain no transactions are active --- src/Database/Adapter/SQL.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 64ce0235e..f03035c4b 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -37,6 +37,9 @@ public function startTransaction(): bool if ($this->inTransaction === 0) { if ($this->getPDO()->inTransaction()) { $this->getPDO()->rollBack(); + } else { + // If no active transaction, this has no effect. + $this->getPDO()->exec('ROLLBACK'); } $result = $this->getPDO()->beginTransaction(); From 6f9b6023923a318e5f7c2a879cd3c7616d30df1f Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 19:28:22 +1300 Subject: [PATCH 22/34] Throw rollback exception if max attempts reached --- src/Database/Adapter.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index d78822fba..e01c0e006 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -321,20 +321,23 @@ public function withTransaction(callable $callback): mixed $result = $callback(); $this->commitTransaction(); return $result; - } catch (\Throwable $e) { + } catch (\Throwable $action) { + try { $this->rollbackTransaction(); - } catch (\Throwable) { + } catch (\Throwable $rollback) { if ($attempts < 2) { continue; } + + throw $rollback; } if ($attempts < 2) { continue; } - throw $e; + throw $action; } } From 7b52365b3f7f70e81ac995098f034889aebf9cdd Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 20:28:29 +1300 Subject: [PATCH 23/34] 5ms sleep --- src/Database/Adapter.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index e01c0e006..a60d105c8 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -327,6 +327,7 @@ public function withTransaction(callable $callback): mixed $this->rollbackTransaction(); } catch (\Throwable $rollback) { if ($attempts < 2) { + \usleep(5000); // 5ms continue; } @@ -334,6 +335,7 @@ public function withTransaction(callable $callback): mixed } if ($attempts < 2) { + \usleep(5000); // 5ms continue; } From 1158f910d590816334415a2332b265f2f6c3a675 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 21:20:24 +1300 Subject: [PATCH 24/34] Fix stan --- src/Database/Adapter.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index a60d105c8..fd59b8300 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -6,6 +6,7 @@ use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Exception\Timeout as TimeoutException; +use Utopia\Database\Exception\Transaction as TransactionException; abstract class Adapter { From 8d1b7dc0d3492234f8ce7aa0b8e4270384d05a5e Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 5 Nov 2024 22:16:58 +1300 Subject: [PATCH 25/34] Prepared statements for savepoints --- src/Database/Adapter/SQL.php | 13 ++++++++----- src/Database/Adapter/SQLite.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index f03035c4b..3b837dfd5 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -39,13 +39,14 @@ public function startTransaction(): bool $this->getPDO()->rollBack(); } else { // If no active transaction, this has no effect. - $this->getPDO()->exec('ROLLBACK'); + $this->getPDO()->prepare('ROLLBACK')->execute(); } $result = $this->getPDO()->beginTransaction(); } else { - $this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction); - $result = true; + $result = $this->getPDO() + ->prepare('SAVEPOINT transaction' . $this->inTransaction) + ->execute(); } } catch (PDOException $e) { throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e); @@ -102,9 +103,11 @@ public function rollbackTransaction(): bool try { if ($this->inTransaction > 1) { - $this->getPDO()->exec('ROLLBACK TO transaction' . $this->inTransaction); + $result = $this->getPDO() + ->prepare('ROLLBACK TO transaction' . ($this->inTransaction - 1)) + ->execute(); + $this->inTransaction--; - $result = true; } else { $result = $this->getPDO()->rollBack(); $this->inTransaction = 0; diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 5d1945598..0b6bc7b99 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -12,6 +12,7 @@ use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Exception\NotFound as NotFoundException; use Utopia\Database\Exception\Timeout as TimeoutException; +use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Helpers\ID; /** @@ -30,6 +31,36 @@ */ class SQLite extends MariaDB { + /** + * @inheritDoc + */ + public function startTransaction(): bool + { + try { + if ($this->inTransaction === 0) { + if ($this->getPDO()->inTransaction()) { + $this->getPDO()->rollBack(); + } + + $result = $this->getPDO()->beginTransaction(); + } else { + $result = $this->getPDO() + ->prepare('SAVEPOINT transaction' . $this->inTransaction) + ->execute(); + } + } catch (PDOException $e) { + throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e); + } + + if (!$result) { + throw new TransactionException('Failed to start transaction'); + } + + $this->inTransaction++; + + return $result; + } + /** * Check if Database exists * Optionally check if collection exists in Database From 872012b5cce54d3dc1c04238787f9d6cdbfdc630 Mon Sep 17 00:00:00 2001 From: Fabian Gruber Date: Tue, 5 Nov 2024 18:06:22 +0100 Subject: [PATCH 26/34] chore: update utopia-php/cache to 0.11.0 --- composer.json | 2 +- composer.lock | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/composer.json b/composer.json index a1ca02e4b..66b0fad6d 100755 --- a/composer.json +++ b/composer.json @@ -37,7 +37,7 @@ "ext-mbstring": "*", "php": ">=8.3", "utopia-php/framework": "0.33.*", - "utopia-php/cache": "0.10.*", + "utopia-php/cache": "0.11.*", "utopia-php/mongo": "0.3.*" }, "require-dev": { diff --git a/composer.lock b/composer.lock index 49c2e3ddf..1a149c62a 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d8b51c1a58da391a80c01cd5b691b739", + "content-hash": "f7eec4bad737b741ae97c81db0532d29", "packages": [ { "name": "jean85/pretty-package-versions", @@ -216,16 +216,16 @@ }, { "name": "utopia-php/cache", - "version": "0.10.2", + "version": "0.11.0", "source": { "type": "git", "url": "https://github.com/utopia-php/cache.git", - "reference": "b22c6eb6d308de246b023efd0fc9758aee8b8247" + "reference": "8ebcab5aac7606331cef69b0081f6c9eff2e58bc" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/cache/zipball/b22c6eb6d308de246b023efd0fc9758aee8b8247", - "reference": "b22c6eb6d308de246b023efd0fc9758aee8b8247", + "url": "https://api.github.com/repos/utopia-php/cache/zipball/8ebcab5aac7606331cef69b0081f6c9eff2e58bc", + "reference": "8ebcab5aac7606331cef69b0081f6c9eff2e58bc", "shasum": "" }, "require": { @@ -236,7 +236,7 @@ }, "require-dev": { "laravel/pint": "1.2.*", - "phpstan/phpstan": "1.9.x-dev", + "phpstan/phpstan": "^1.12", "phpunit/phpunit": "^9.3", "vimeo/psalm": "4.13.1" }, @@ -260,9 +260,9 @@ ], "support": { "issues": "https://github.com/utopia-php/cache/issues", - "source": "https://github.com/utopia-php/cache/tree/0.10.2" + "source": "https://github.com/utopia-php/cache/tree/0.11.0" }, - "time": "2024-06-25T20:36:35+00:00" + "time": "2024-11-05T16:53:58+00:00" }, { "name": "utopia-php/framework", @@ -2591,7 +2591,7 @@ "platform": { "ext-pdo": "*", "ext-mbstring": "*", - "php": ">=8.0" + "php": ">=8.3" }, "platform-dev": [], "plugin-api-version": "2.6.0" From a21113139cc22c471ea11b911bde8a2cdc28c49c Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 14:35:32 +1300 Subject: [PATCH 27/34] Reset transaction count on retry loop failure --- docker-compose.yml | 2 +- src/Database/Adapter.php | 3 ++- src/Database/Adapter/Postgres.php | 3 +++ src/Database/Adapter/SQL.php | 9 ++------- tests/e2e/Adapter/Base.php | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 23b135ba9..5497f4e28 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,7 +5,7 @@ services: build: context: . args: - DEBUG: false + DEBUG: true networks: - database volumes: diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index fd59b8300..2b0cb04fd 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -323,7 +323,6 @@ public function withTransaction(callable $callback): mixed $this->commitTransaction(); return $result; } catch (\Throwable $action) { - try { $this->rollbackTransaction(); } catch (\Throwable $rollback) { @@ -332,6 +331,7 @@ public function withTransaction(callable $callback): mixed continue; } + $this->inTransaction = 0; throw $rollback; } @@ -340,6 +340,7 @@ public function withTransaction(callable $callback): mixed continue; } + $this->inTransaction = 0; throw $action; } } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 4fa08fcb1..4bd39827c 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -36,6 +36,9 @@ public function startTransaction(): bool if ($this->inTransaction === 0) { if ($this->getPDO()->inTransaction()) { $this->getPDO()->rollBack(); + } else { + // If no active transaction, this has no effect. + $this->getPDO()->prepare('ROLLBACK')->execute(); } $result = $this->getPDO()->beginTransaction(); diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 3b837dfd5..abf74e987 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -44,9 +44,7 @@ public function startTransaction(): bool $result = $this->getPDO()->beginTransaction(); } else { - $result = $this->getPDO() - ->prepare('SAVEPOINT transaction' . $this->inTransaction) - ->execute(); + $result = $this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction); } } catch (PDOException $e) { throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e); @@ -103,10 +101,7 @@ public function rollbackTransaction(): bool try { if ($this->inTransaction > 1) { - $result = $this->getPDO() - ->prepare('ROLLBACK TO transaction' . ($this->inTransaction - 1)) - ->execute(); - + $result = $this->getPDO()->exec('ROLLBACK TO transaction' . ($this->inTransaction - 1)); $this->inTransaction--; } else { $result = $this->getPDO()->rollBack(); diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 6fde24f1f..b4c3a8c5f 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -348,7 +348,7 @@ public function testVirtualRelationsAttributes(): void ])); $this->fail('Failed to throw exception'); } catch (Exception $e) { - $this->assertTrue($e instanceof RelationshipException); + $this->assertInstanceOf(RelationshipException::class, $e); } static::getDatabase()->deleteRelationship('v1', 'v2'); From 4ed35fdc2386721e00215574ac4964ba978e27fc Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 14:36:53 +1300 Subject: [PATCH 28/34] Dummy --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index ccdaa969e..783265d80 100755 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,7 +7,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="false"> + stopOnFailure="true"> ./tests/unit From d9f85c67b10cf2064955644237b2c7af37629bcb Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 14:36:58 +1300 Subject: [PATCH 29/34] Revert "Dummy" This reverts commit 4ed35fdc2386721e00215574ac4964ba978e27fc. --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index 783265d80..ccdaa969e 100755 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,7 +7,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="true"> + stopOnFailure="false"> ./tests/unit From 3b4b2492e5d0888fc254a31b2644764ca212da65 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 15:10:19 +1300 Subject: [PATCH 30/34] Ensure values is array --- src/Database/Query.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Database/Query.php b/src/Database/Query.php index aab17c195..1c82e7b4c 100644 --- a/src/Database/Query.php +++ b/src/Database/Query.php @@ -262,6 +262,10 @@ public static function parseQuery(array $query): self throw new QueryException('Invalid query attribute. Must be a string, got ' . \gettype($attribute)); } + if (!\is_array($values)) { + throw new QueryException('Invalid query values. Must be an array, got ' . \gettype($values)); + } + if (\in_array($method, self::LOGICAL_TYPES)) { foreach ($values as $index => $value) { $values[$index] = self::parseQuery($value); From c575e50d89976c7286d08dcb138ac50e26bd70ed Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 15:15:49 +1300 Subject: [PATCH 31/34] Ensure method is string --- src/Database/Query.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Database/Query.php b/src/Database/Query.php index 1c82e7b4c..7388453b2 100644 --- a/src/Database/Query.php +++ b/src/Database/Query.php @@ -254,6 +254,10 @@ public static function parseQuery(array $query): self $attribute = $query['attribute'] ?? ''; $values = $query['values'] ?? []; + if (!\is_string($method)) { + throw new QueryException('Invalid query method. Must be a string, got ' . \gettype($method)); + } + if (!self::isMethod($method)) { throw new QueryException('Invalid query method: ' . $method); } From 9fe321165712bdbdefecd192b51fb7c889acc4a9 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 15:15:54 +1300 Subject: [PATCH 32/34] Add tests --- tests/unit/QueryTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/QueryTest.php b/tests/unit/QueryTest.php index 9666ebf3a..d9ad6cd93 100644 --- a/tests/unit/QueryTest.php +++ b/tests/unit/QueryTest.php @@ -213,6 +213,13 @@ public function testParse(): void $this->assertEquals('actors', $queries[0]->getAttribute()); $this->assertEquals($json, '{"method":"or","values":[{"method":"equal","attribute":"actors","values":["Brad Pitt"]},{"method":"equal","attribute":"actors","values":["Johnny Depp"]}]}'); + try { + Query::parse('{"method":["equal"],"attribute":["title"],"values":["test"]}'); + $this->fail('Failed to throw exception'); + } catch (QueryException $e) { + $this->assertEquals('Invalid query method. Must be a string, got array', $e->getMessage()); + } + try { Query::parse('{"method":"equal","attribute":["title"],"values":["test"]}'); $this->fail('Failed to throw exception'); @@ -220,6 +227,13 @@ public function testParse(): void $this->assertEquals('Invalid query attribute. Must be a string, got array', $e->getMessage()); } + try { + Query::parse('{"method":"equal","attribute":"title","values":"test"}'); + $this->fail('Failed to throw exception'); + } catch (QueryException $e) { + $this->assertEquals('Invalid query values. Must be an array, got string', $e->getMessage()); + } + try { Query::parse('false'); $this->fail('Failed to throw exception'); From 7383ca946a197118058929b9dc60f9a973fdec13 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 15:19:22 +1300 Subject: [PATCH 33/34] Revert "Add tests" This reverts commit 9fe321165712bdbdefecd192b51fb7c889acc4a9. --- tests/unit/QueryTest.php | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/unit/QueryTest.php b/tests/unit/QueryTest.php index d9ad6cd93..9666ebf3a 100644 --- a/tests/unit/QueryTest.php +++ b/tests/unit/QueryTest.php @@ -213,13 +213,6 @@ public function testParse(): void $this->assertEquals('actors', $queries[0]->getAttribute()); $this->assertEquals($json, '{"method":"or","values":[{"method":"equal","attribute":"actors","values":["Brad Pitt"]},{"method":"equal","attribute":"actors","values":["Johnny Depp"]}]}'); - try { - Query::parse('{"method":["equal"],"attribute":["title"],"values":["test"]}'); - $this->fail('Failed to throw exception'); - } catch (QueryException $e) { - $this->assertEquals('Invalid query method. Must be a string, got array', $e->getMessage()); - } - try { Query::parse('{"method":"equal","attribute":["title"],"values":["test"]}'); $this->fail('Failed to throw exception'); @@ -227,13 +220,6 @@ public function testParse(): void $this->assertEquals('Invalid query attribute. Must be a string, got array', $e->getMessage()); } - try { - Query::parse('{"method":"equal","attribute":"title","values":"test"}'); - $this->fail('Failed to throw exception'); - } catch (QueryException $e) { - $this->assertEquals('Invalid query values. Must be an array, got string', $e->getMessage()); - } - try { Query::parse('false'); $this->fail('Failed to throw exception'); From b7110d5fe5d90c2d264a59e665a4730b9aad9675 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 6 Nov 2024 15:19:28 +1300 Subject: [PATCH 34/34] Reapply "Add tests" This reverts commit 7383ca946a197118058929b9dc60f9a973fdec13. --- tests/unit/QueryTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/QueryTest.php b/tests/unit/QueryTest.php index 9666ebf3a..d9ad6cd93 100644 --- a/tests/unit/QueryTest.php +++ b/tests/unit/QueryTest.php @@ -213,6 +213,13 @@ public function testParse(): void $this->assertEquals('actors', $queries[0]->getAttribute()); $this->assertEquals($json, '{"method":"or","values":[{"method":"equal","attribute":"actors","values":["Brad Pitt"]},{"method":"equal","attribute":"actors","values":["Johnny Depp"]}]}'); + try { + Query::parse('{"method":["equal"],"attribute":["title"],"values":["test"]}'); + $this->fail('Failed to throw exception'); + } catch (QueryException $e) { + $this->assertEquals('Invalid query method. Must be a string, got array', $e->getMessage()); + } + try { Query::parse('{"method":"equal","attribute":["title"],"values":["test"]}'); $this->fail('Failed to throw exception'); @@ -220,6 +227,13 @@ public function testParse(): void $this->assertEquals('Invalid query attribute. Must be a string, got array', $e->getMessage()); } + try { + Query::parse('{"method":"equal","attribute":"title","values":"test"}'); + $this->fail('Failed to throw exception'); + } catch (QueryException $e) { + $this->assertEquals('Invalid query values. Must be an array, got string', $e->getMessage()); + } + try { Query::parse('false'); $this->fail('Failed to throw exception');