From 170f8d50d0d5896623c3a6a09016a88a4a457aaf Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 10 Sep 2024 10:27:13 +0200 Subject: [PATCH 01/23] Initial Push for Batch Deletes --- src/Database/Adapter.php | 10 +++ src/Database/Adapter/MariaDB.php | 55 ++++++++++++ src/Database/Adapter/Mongo.php | 13 +++ src/Database/Adapter/Postgres.php | 55 ++++++++++++ src/Database/Database.php | 78 ++++++++++++++++ tests/e2e/Adapter/Base.php | 145 ++++++++++++++++++++++++++++++ 6 files changed, 356 insertions(+) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 4c2aa326a..bf010faaa 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -587,6 +587,16 @@ abstract public function updateDocuments(string $collection, array $documents, i */ abstract public function deleteDocument(string $collection, string $id): bool; + /** + * Delete Documents + * + * @param string $collection + * @param array<\Utopia\Database\Query> $queries + * + * @return bool + */ + abstract public function deleteDocuments(string $collection, array $queries): bool; + /** * Find Documents * diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index aaaa260fe..6256fec37 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1635,6 +1635,61 @@ public function deleteDocument(string $collection, string $id): bool return $deleted; } + /** + * Delete Documents + * + * @param string $collection + * @param array<\Utopia\Database\Query> $queries + * + * @return bool + */ + public function deleteDocuments(string $collection, array $queries): bool + { + $name = $this->filter($collection); + $where = []; + + $queries = array_map(fn ($query) => clone $query, $queries); + + $conditions = $this->getSQLConditions($queries); + if(!empty($conditions)) { + $where[] = $conditions; + } + + if ($this->sharedTables) { + $where[] = "table_main._tenant = :_tenant"; + } + + $sqlWhere = !empty($where) ? 'WHERE ' . implode(' AND ', $where) : ''; + + $selections = $this->getAttributeSelections($queries); + + $sql = " + USE {$this->database}; + DELETE {$this->getAttributeProjection($selections, 'table_main')} + FROM {$this->getSQLTable($name)} as table_main + {$sqlWhere}; + "; + + $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); + + $stmt = $this->getPDO()->prepare($sql); + + foreach ($queries as $query) { + $this->bindConditionValue($stmt, $query); + } + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } + + try { + $stmt->execute(); + } catch (PDOException $e) { + $this->processException($e); + } + + return true; + } + /** * Find Documents * diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index feb1d9a15..62b411ca3 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -909,6 +909,19 @@ public function deleteDocument(string $collection, string $id): bool return (!!$result); } + /** + * Delete Documents + * + * @param string $collection + * @param array<\Utopia\Database\Query> $queries + * + * @return bool + */ + public function deleteDocuments(string $collection, array $queries): bool + { + throw new \Exception('Not Implemented'); + } + /** * Update Attribute. * @param string $collection diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 8c25731d6..7ea145f85 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1558,6 +1558,61 @@ public function deleteDocument(string $collection, string $id): bool return $deleted; } + + /** + * Delete Documents + * + * @param string $collection + * @param array<\Utopia\Database\Query> $queries + * + * @return bool + */ + public function deleteDocuments(string $collection, array $queries): bool + { + $name = $this->filter($collection); + $where = []; + + $queries = array_map(fn ($query) => clone $query, $queries); + + $conditions = $this->getSQLConditions($queries); + if(!empty($conditions)) { + $where[] = $conditions; + } + + if ($this->sharedTables) { + $where[] = "table_main._tenant = :_tenant"; + } + + $sqlWhere = !empty($where) ? 'WHERE ' . implode(' AND ', $where) : ''; + + $selections = $this->getAttributeSelections($queries); + + $sql = " + SELECT {$this->getAttributeProjection($selections, 'table_main')} + FROM {$this->getSQLTable($name)} as table_main + {$sqlWhere} + "; + + $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); + + $stmt = $this->getPDO()->prepare($sql); + + foreach ($queries as $query) { + $this->bindConditionValue($stmt, $query); + } + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } + + try { + $stmt->execute(); + } catch (PDOException $e) { + $this->processException($e); + } + + return true; + } + /** * Find Documents * diff --git a/src/Database/Database.php b/src/Database/Database.php index 437f18881..58b9d0000 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4944,6 +4944,84 @@ 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 + * + * @return bool + * + * @throws AuthorizationException + * @throws DatabaseException + * @throws RestrictedException + */ + public function deleteDocuments(string $collection, array $queries = []): bool + { + if ($this->adapter->getSharedTables() && empty($this->adapter->getTenant())) { + throw new DatabaseException('Missing tenant. Tenant must be set when table sharing is enabled.'); + } + + $collection = $this->silent(fn () => $this->getCollection($collection)); + + $deleted = $this->withTransaction(function () use ($collection, $queries) { + $lastDocument = null; + while (true) { + $affectedDocuments = $this->find($collection->getId(), array_merge( + empty($lastDocument) ? [ + Query::limit(100), + ] : [ + Query::limit(100), + Query::cursorAfter($lastDocument), + ], + $queries, + )); + + if (empty($affectedDocuments)) { + break; + } + + foreach ($affectedDocuments as $document) { + $validator = new Authorization(self::PERMISSION_DELETE); + + if ($collection->getId() !== self::METADATA) { + $documentSecurity = $collection->getAttribute('documentSecurity', false); + if (!$validator->isValid([ + ...$collection->getDelete(), + ...($documentSecurity ? $document->getDelete() : []) + ])) { + throw new AuthorizationException($validator->getDescription()); + } + } + + // Delete Relationships + if ($this->resolveRelationships) { + $document = $this->silent(fn () => $this->deleteDocumentRelationships($collection, $document)); + } + + // Fire events + $this->trigger(self::EVENT_DOCUMENT_DELETE, $document); + + $this->purgeRelatedDocuments($collection, $document->getId()); + $this->purgeCachedDocument($collection->getId(), $document->getId()); + } + + if (count($affectedDocuments) < 100) { + break; + } else { + $lastDocument = end($affectedDocuments); + } + } + + // Mass delete using adapter with query + return $this->adapter->deleteDocuments($collection->getId(), $queries); + }); + + 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 72587d44a..ff2998633 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15608,4 +15608,149 @@ public function testEvents(): void $database->delete('hellodb'); }); } + + public function propegateBulkDocuments(bool $documentSecurity = false): void + { + for ($i = 0; $i < 10; $i++) { + var_dump(new Document( + array_merge([ + '$id' => 'doc' . $i, + 'text' => 'value' . $i, + 'integer' => $i + ], $documentSecurity ? [ + '$permissions' => [ + Permission::delete(Role::any()), + Permission::create(Role::any()), + Permission::read(Role::any()), + ], + ] : []) + )); + + 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 + static::getDatabase()->deleteDocuments('bulk_delete'); + + $docs = static::getDatabase()->find('bulk_delete'); + $this->assertCount(0, $docs); + + // TEST: Bulk delete documents with queries. + $this->propegateBulkDocuments(); + + static::getDatabase()->deleteDocuments('bulk_delete', [ + Query::greaterThanEqual('integer', 5) + ]); + + $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); + static::getDatabase()->deleteDocuments('bulk_delete'); + $this->assertEquals(0, count($this->getDatabase()->find('bulk_delete'))); + + // TEST (FAIL): Bulk delete all documents with invalid document permissions + // Authorization::setRole(Role::any()->toString()); + static::getDatabase()->updateCollection('bulk_delete', [ + Permission::create(Role::any()), + ], true); + $this->propegateBulkDocuments(true); + + try { + static::getDatabase()->deleteDocuments('bulk_delete'); + $this->fail('Bulk deleted documents with invalid document permission'); + } catch (\Utopia\Database\Exception\Authorization) { + } + + 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 testDeleteBulkDocumentsRelationships(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + static::getDatabase()->createCollection('bulk_delete_r_1'); + static::getDatabase()->createCollection('bulk_delete_r_2'); + + // ONE_TO_ONE + static::getDatabase()->createRelationship( + collection: 'bulk_delete_r_1', + relatedCollection: 'bulk_delete_r_2', + type: Database::RELATION_ONE_TO_ONE, + ); + + // Restrict + + // NULL + + // Cascade + } } From 79e65da983634578b676c7458d5cad879a3dfbbe Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 10 Sep 2024 19:46:46 +0900 Subject: [PATCH 02/23] Update src/Database/Adapter/MariaDB.php Co-authored-by: Jake Barnby --- src/Database/Adapter/MariaDB.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 6256fec37..825ecefdb 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1664,9 +1664,7 @@ public function deleteDocuments(string $collection, array $queries): bool $selections = $this->getAttributeSelections($queries); $sql = " - USE {$this->database}; - DELETE {$this->getAttributeProjection($selections, 'table_main')} - FROM {$this->getSQLTable($name)} as table_main + DELETE FROM {$this->getSQLTable($name)} {$sqlWhere}; "; From b0798700db1d37a4f531b0f005e897f3dfca66a5 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 17 Sep 2024 14:16:55 +0900 Subject: [PATCH 03/23] Address comments, add MongoDB and Postgres Support --- composer.lock | 30 +++++++++++++++--------------- src/Database/Adapter/MariaDB.php | 2 -- src/Database/Adapter/Mongo.php | 27 ++++++++++++++++++++++++++- src/Database/Adapter/Postgres.php | 5 +---- src/Database/Database.php | 10 ++++++---- tests/e2e/Adapter/Base.php | 14 -------------- 6 files changed, 48 insertions(+), 40 deletions(-) diff --git a/composer.lock b/composer.lock index b98610e74..32455f27d 100644 --- a/composer.lock +++ b/composer.lock @@ -136,20 +136,20 @@ }, { "name": "symfony/polyfill-php80", - "version": "v1.30.0", + "version": "v1.31.0", "source": { "type": "git", "url": "https://github.com/symfony/polyfill-php80.git", - "reference": "77fa7995ac1b21ab60769b7323d600a991a90433" + "reference": "60328e362d4c2c802a54fcbf04f9d3fb892b4cf8" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-php80/zipball/77fa7995ac1b21ab60769b7323d600a991a90433", - "reference": "77fa7995ac1b21ab60769b7323d600a991a90433", + "url": "https://api.github.com/repos/symfony/polyfill-php80/zipball/60328e362d4c2c802a54fcbf04f9d3fb892b4cf8", + "reference": "60328e362d4c2c802a54fcbf04f9d3fb892b4cf8", "shasum": "" }, "require": { - "php": ">=7.1" + "php": ">=7.2" }, "type": "library", "extra": { @@ -196,7 +196,7 @@ "shim" ], "support": { - "source": "https://github.com/symfony/polyfill-php80/tree/v1.30.0" + "source": "https://github.com/symfony/polyfill-php80/tree/v1.31.0" }, "funding": [ { @@ -212,7 +212,7 @@ "type": "tidelift" } ], - "time": "2024-05-31T15:07:36+00:00" + "time": "2024-09-09T11:45:10+00:00" }, { "name": "utopia-php/cache", @@ -506,16 +506,16 @@ }, { "name": "laravel/pint", - "version": "v1.17.2", + "version": "v1.17.3", "source": { "type": "git", "url": "https://github.com/laravel/pint.git", - "reference": "e8a88130a25e3f9d4d5785e6a1afca98268ab110" + "reference": "9d77be916e145864f10788bb94531d03e1f7b482" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laravel/pint/zipball/e8a88130a25e3f9d4d5785e6a1afca98268ab110", - "reference": "e8a88130a25e3f9d4d5785e6a1afca98268ab110", + "url": "https://api.github.com/repos/laravel/pint/zipball/9d77be916e145864f10788bb94531d03e1f7b482", + "reference": "9d77be916e145864f10788bb94531d03e1f7b482", "shasum": "" }, "require": { @@ -526,13 +526,13 @@ "php": "^8.1.0" }, "require-dev": { - "friendsofphp/php-cs-fixer": "^3.61.1", - "illuminate/view": "^10.48.18", + "friendsofphp/php-cs-fixer": "^3.64.0", + "illuminate/view": "^10.48.20", "larastan/larastan": "^2.9.8", "laravel-zero/framework": "^10.4.0", "mockery/mockery": "^1.6.12", "nunomaduro/termwind": "^1.15.1", - "pestphp/pest": "^2.35.0" + "pestphp/pest": "^2.35.1" }, "bin": [ "builds/pint" @@ -568,7 +568,7 @@ "issues": "https://github.com/laravel/pint/issues", "source": "https://github.com/laravel/pint" }, - "time": "2024-08-06T15:11:54+00:00" + "time": "2024-09-03T15:00:28+00:00" }, { "name": "myclabs/deep-copy", diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 825ecefdb..7b1537d11 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1661,8 +1661,6 @@ public function deleteDocuments(string $collection, array $queries): bool $sqlWhere = !empty($where) ? 'WHERE ' . implode(' AND ', $where) : ''; - $selections = $this->getAttributeSelections($queries); - $sql = " DELETE FROM {$this->getSQLTable($name)} {$sqlWhere}; diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 62b411ca3..41c3c147e 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -919,7 +919,32 @@ public function deleteDocument(string $collection, string $id): bool */ public function deleteDocuments(string $collection, array $queries): bool { - throw new \Exception('Not Implemented'); + $name = $this->getNamespace() . '_' . $this->filter($collection); + $queries = array_map(fn ($query) => clone $query, $queries); + + $filters = $this->buildFilters($queries); + + if ($this->sharedTables) { + $filters['_tenant'] = (string)$this->getTenant(); + } + + $filters = $this->replaceInternalIdsKeys($filters, '$', '_', $this->operators); + $filters = $this->timeFilter($filters); + + $options = []; + + try { + $res = $this->client->delete( + collection: $name, + filters: $filters, + options: $options, + limit: 0 + ); + } catch (MongoException $e) { + $this->processException($e); + } + + return true; } /** diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 7ea145f85..9162ea12e 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1585,11 +1585,8 @@ public function deleteDocuments(string $collection, array $queries): bool $sqlWhere = !empty($where) ? 'WHERE ' . implode(' AND ', $where) : ''; - $selections = $this->getAttributeSelections($queries); - $sql = " - SELECT {$this->getAttributeProjection($selections, 'table_main')} - FROM {$this->getSQLTable($name)} as table_main + DELETE FROM {$this->getSQLTable($name)} {$sqlWhere} "; diff --git a/src/Database/Database.php b/src/Database/Database.php index 58b9d0000..944c3bea0 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4951,6 +4951,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection * * @param string $collection * @param array $queries + * @param int $batchSize * * @return bool * @@ -4958,22 +4959,23 @@ private function deleteCascade(Document $collection, Document $relatedCollection * @throws DatabaseException * @throws RestrictedException */ - public function deleteDocuments(string $collection, array $queries = []): bool + public function deleteDocuments(string $collection, array $queries = [], int $batchSize = 100): bool { 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)); - $deleted = $this->withTransaction(function () use ($collection, $queries) { + $deleted = $this->withTransaction(function () use ($collection, $queries, $batchSize) { $lastDocument = null; while (true) { $affectedDocuments = $this->find($collection->getId(), array_merge( empty($lastDocument) ? [ - Query::limit(100), + Query::limit($batchSize), ] : [ - Query::limit(100), + Query::limit($batchSize), Query::cursorAfter($lastDocument), ], $queries, diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index ff2998633..345605abb 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15612,20 +15612,6 @@ public function testEvents(): void public function propegateBulkDocuments(bool $documentSecurity = false): void { for ($i = 0; $i < 10; $i++) { - var_dump(new Document( - array_merge([ - '$id' => 'doc' . $i, - 'text' => 'value' . $i, - 'integer' => $i - ], $documentSecurity ? [ - '$permissions' => [ - Permission::delete(Role::any()), - Permission::create(Role::any()), - Permission::read(Role::any()), - ], - ] : []) - )); - static::getDatabase()->createDocument('bulk_delete', new Document( array_merge([ '$id' => 'doc' . $i, From a931aab7f429f30a35894065bdcf5badf44e8bb7 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 17 Sep 2024 14:39:37 +0900 Subject: [PATCH 04/23] Make event test the last test --- tests/e2e/Adapter/Base.php | 226 ++++++++++++++++++------------------- 1 file changed, 113 insertions(+), 113 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 345605abb..903c7284e 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -993,7 +993,7 @@ public function testQueryTimeout(): void Query::notEqual('longtext', 'appwrite'), ]); $this->fail('Failed to throw exception'); - } catch(TimeoutException $ex) { + } catch (TimeoutException $ex) { static::getDatabase()->clearTimeout(); static::getDatabase()->deleteCollection('global-timeouts'); } @@ -1843,7 +1843,7 @@ public function testCreateDocument(): Document 'empty' => [], ])); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertTrue($e instanceof StructureException); $this->assertStringContainsString('Invalid document structure: Attribute "float_unsigned" has invalid type. Value must be a valid range between 0 and', $e->getMessage()); } @@ -1862,7 +1862,7 @@ public function testCreateDocument(): Document 'empty' => [], ])); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertTrue($e instanceof StructureException); $this->assertEquals('Invalid document structure: Attribute "bigint_unsigned" has invalid type. Value must be a valid range between 0 and 9,223,372,036,854,775,807', $e->getMessage()); } @@ -2253,7 +2253,7 @@ public function testListDocumentSearch(): void public function testEmptyTenant(): void { - if(static::getDatabase()->getAdapter()->getSharedTables()) { + if (static::getDatabase()->getAdapter()->getSharedTables()) { $this->expectNotToPerformAssertions(); return; } @@ -2432,7 +2432,7 @@ public function testUpdateDocumentConflict(Document $document): void return $this->getDatabase()->updateDocument($document->getCollection(), $document->getId(), $document); }); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertTrue($e instanceof ConflictException); $this->assertEquals('Document was updated after the request timestamp', $e->getMessage()); } @@ -2573,7 +2573,7 @@ public function testArrayAttribute(): void try { $database->createDocument($collection, new Document([])); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid document structure: Missing required attribute "booleans"', $e->getMessage()); } @@ -2593,7 +2593,7 @@ public function testArrayAttribute(): void 'short' => ['More than 5 size'], ])); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid document structure: Attribute "short[\'0\']" has invalid type. Value must be a valid string and no longer than 5 chars', $e->getMessage()); } @@ -2602,7 +2602,7 @@ public function testArrayAttribute(): void 'names' => ['Joe', 100], ])); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid document structure: Attribute "names[\'1\']" has invalid type. Value must be a valid string and no longer than 255 chars', $e->getMessage()); } @@ -2611,7 +2611,7 @@ public function testArrayAttribute(): void 'age' => 1.5, ])); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid document structure: Attribute "age" has invalid type. Value must be a valid integer', $e->getMessage()); } @@ -2620,7 +2620,7 @@ public function testArrayAttribute(): void 'age' => -100, ])); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid document structure: Attribute "age" has invalid type. Value must be a valid range between 0 and 2,147,483,647', $e->getMessage()); } @@ -2649,7 +2649,7 @@ public function testArrayAttribute(): void try { $database->createIndex($collection, 'indx', Database::INDEX_FULLTEXT, ['names']); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { if ($this->getDatabase()->getAdapter()->getSupportForFulltextIndex()) { $this->assertEquals('"Fulltext" index is forbidden on array attributes', $e->getMessage()); } else { @@ -2660,7 +2660,7 @@ public function testArrayAttribute(): void try { $database->createIndex($collection, 'indx', Database::INDEX_KEY, ['numbers', 'names'], [100,100]); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('An index may only contain one array attribute', $e->getMessage()); } @@ -2681,7 +2681,7 @@ public function testArrayAttribute(): void try { $database->createIndex($collection, 'indx_numbers', Database::INDEX_KEY, ['tv_show', 'numbers'], [], []); // [700, 255] $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Index length is longer than the maximum: 768', $e->getMessage()); } } @@ -2692,7 +2692,7 @@ public function testArrayAttribute(): void try { $database->createIndex($collection, 'indx4', Database::INDEX_KEY, ['age', 'names'], [10, 255], []); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Cannot set a length on "integer" attributes', $e->getMessage()); } @@ -2705,7 +2705,7 @@ public function testArrayAttribute(): void Query::equal('names', ['Joe']), ]); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid query: Cannot query equal on attribute "names" because it is an array.', $e->getMessage()); } @@ -2714,7 +2714,7 @@ public function testArrayAttribute(): void Query::contains('age', [10]) ]); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid query: Cannot query contains on attribute "age" because it is not an array or string.', $e->getMessage()); } @@ -3150,7 +3150,7 @@ public function testFindContains(): void Query::contains('price', [10.5]), ]); $this->fail('Failed to throw exception'); - } catch(Throwable $e) { + } catch (Throwable $e) { $this->assertEquals('Invalid query: Cannot query contains on attribute "price" because it is not an array or string.', $e->getMessage()); $this->assertTrue($e instanceof DatabaseException); } @@ -4057,7 +4057,7 @@ public function testOrSingleQuery(): void ]) ]); $this->fail('Failed to throw exception'); - } catch(Exception $e) { + } catch (Exception $e) { $this->assertEquals('Invalid query: Or queries require at least two queries', $e->getMessage()); } } @@ -4117,7 +4117,7 @@ public function testAndSingleQuery(): void ]) ]); $this->fail('Failed to throw exception'); - } catch(Exception $e) { + } catch (Exception $e) { $this->assertEquals('Invalid query: And queries require at least two queries', $e->getMessage()); } } @@ -4965,7 +4965,7 @@ public function testStructureValidationAfterRelationsAttribute(): void 'name' => 'Frozen', // Unknown attribute 'name' after relation attribute ])); $this->fail('Failed to throw exception'); - } catch(Exception $e) { + } catch (Exception $e) { $this->assertInstanceOf(StructureException::class, $e); } } @@ -5040,7 +5040,7 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void try { static::getDatabase()->updateDocument('level1', $level1->getId(), $level1->setAttribute('name', 'haha')); $this->fail('Failed to throw exception'); - } catch(Exception $e) { + } catch (Exception $e) { $this->assertInstanceOf(AuthorizationException::class, $e); } $level1->setAttribute('name', 'Level 1'); @@ -15517,98 +15517,6 @@ public function testTransformations(): void $this->assertTrue($result->isEmpty()); } - public function testEvents(): void - { - Authorization::skip(function () { - $database = static::getDatabase(); - - $events = [ - Database::EVENT_DATABASE_CREATE, - Database::EVENT_DATABASE_LIST, - Database::EVENT_COLLECTION_CREATE, - Database::EVENT_COLLECTION_LIST, - Database::EVENT_COLLECTION_READ, - Database::EVENT_ATTRIBUTE_CREATE, - Database::EVENT_ATTRIBUTE_UPDATE, - Database::EVENT_INDEX_CREATE, - Database::EVENT_DOCUMENT_CREATE, - Database::EVENT_DOCUMENT_UPDATE, - Database::EVENT_DOCUMENT_READ, - Database::EVENT_DOCUMENT_FIND, - Database::EVENT_DOCUMENT_FIND, - Database::EVENT_DOCUMENT_COUNT, - Database::EVENT_DOCUMENT_SUM, - Database::EVENT_DOCUMENT_INCREASE, - Database::EVENT_DOCUMENT_DECREASE, - Database::EVENT_INDEX_DELETE, - Database::EVENT_DOCUMENT_DELETE, - Database::EVENT_ATTRIBUTE_DELETE, - Database::EVENT_COLLECTION_DELETE, - Database::EVENT_DATABASE_DELETE, - ]; - - $database->on(Database::EVENT_ALL, 'test', function ($event, $data) use (&$events) { - $shifted = array_shift($events); - - $this->assertEquals($shifted, $event); - }); - - if ($this->getDatabase()->getAdapter()->getSupportForSchemas()) { - $database->setDatabase('hellodb'); - $database->create(); - } else { - array_shift($events); - } - - $database->list(); - - $database->setDatabase($this->testDatabase); - - $collectionId = ID::unique(); - $database->createCollection($collectionId); - $database->listCollections(); - $database->getCollection($collectionId); - $database->createAttribute($collectionId, 'attr1', Database::VAR_INTEGER, 2, false); - $database->updateAttributeRequired($collectionId, 'attr1', true); - $indexId1 = 'index2_' . uniqid(); - $database->createIndex($collectionId, $indexId1, Database::INDEX_KEY, ['attr1']); - - $document = $database->createDocument($collectionId, new Document([ - '$id' => 'doc1', - 'attr1' => 10, - '$permissions' => [ - Permission::delete(Role::any()), - Permission::update(Role::any()), - Permission::read(Role::any()), - ], - ])); - - $executed = false; - $database->on(Database::EVENT_ALL, 'should-not-execute', function ($event, $data) use (&$executed) { - $executed = true; - }); - - $database->silent(function () use ($database, $collectionId, $document) { - $database->updateDocument($collectionId, 'doc1', $document->setAttribute('attr1', 15)); - $database->getDocument($collectionId, 'doc1'); - $database->find($collectionId); - $database->findOne($collectionId); - $database->count($collectionId); - $database->sum($collectionId, 'attr1'); - $database->increaseDocumentAttribute($collectionId, $document->getId(), 'attr1'); - $database->decreaseDocumentAttribute($collectionId, $document->getId(), 'attr1'); - }, ['should-not-execute']); - - $this->assertFalse($executed); - - $database->deleteIndex($collectionId, $indexId1); - $database->deleteDocument($collectionId, 'doc1'); - $database->deleteAttribute($collectionId, 'attr1'); - $database->deleteCollection($collectionId); - $database->delete('hellodb'); - }); - } - public function propegateBulkDocuments(bool $documentSecurity = false): void { for ($i = 0; $i < 10; $i++) { @@ -15739,4 +15647,96 @@ public function testDeleteBulkDocumentsRelationships(): void // Cascade } + + public function testEvents(): void + { + Authorization::skip(function () { + $database = static::getDatabase(); + + $events = [ + Database::EVENT_DATABASE_CREATE, + Database::EVENT_DATABASE_LIST, + Database::EVENT_COLLECTION_CREATE, + Database::EVENT_COLLECTION_LIST, + Database::EVENT_COLLECTION_READ, + Database::EVENT_ATTRIBUTE_CREATE, + Database::EVENT_ATTRIBUTE_UPDATE, + Database::EVENT_INDEX_CREATE, + Database::EVENT_DOCUMENT_CREATE, + Database::EVENT_DOCUMENT_UPDATE, + Database::EVENT_DOCUMENT_READ, + Database::EVENT_DOCUMENT_FIND, + Database::EVENT_DOCUMENT_FIND, + Database::EVENT_DOCUMENT_COUNT, + Database::EVENT_DOCUMENT_SUM, + Database::EVENT_DOCUMENT_INCREASE, + Database::EVENT_DOCUMENT_DECREASE, + Database::EVENT_INDEX_DELETE, + Database::EVENT_DOCUMENT_DELETE, + Database::EVENT_ATTRIBUTE_DELETE, + Database::EVENT_COLLECTION_DELETE, + Database::EVENT_DATABASE_DELETE, + ]; + + $database->on(Database::EVENT_ALL, 'test', function ($event, $data) use (&$events) { + $shifted = array_shift($events); + + $this->assertEquals($shifted, $event); + }); + + if ($this->getDatabase()->getAdapter()->getSupportForSchemas()) { + $database->setDatabase('hellodb'); + $database->create(); + } else { + array_shift($events); + } + + $database->list(); + + $database->setDatabase($this->testDatabase); + + $collectionId = ID::unique(); + $database->createCollection($collectionId); + $database->listCollections(); + $database->getCollection($collectionId); + $database->createAttribute($collectionId, 'attr1', Database::VAR_INTEGER, 2, false); + $database->updateAttributeRequired($collectionId, 'attr1', true); + $indexId1 = 'index2_' . uniqid(); + $database->createIndex($collectionId, $indexId1, Database::INDEX_KEY, ['attr1']); + + $document = $database->createDocument($collectionId, new Document([ + '$id' => 'doc1', + 'attr1' => 10, + '$permissions' => [ + Permission::delete(Role::any()), + Permission::update(Role::any()), + Permission::read(Role::any()), + ], + ])); + + $executed = false; + $database->on(Database::EVENT_ALL, 'should-not-execute', function ($event, $data) use (&$executed) { + $executed = true; + }); + + $database->silent(function () use ($database, $collectionId, $document) { + $database->updateDocument($collectionId, 'doc1', $document->setAttribute('attr1', 15)); + $database->getDocument($collectionId, 'doc1'); + $database->find($collectionId); + $database->findOne($collectionId); + $database->count($collectionId); + $database->sum($collectionId, 'attr1'); + $database->increaseDocumentAttribute($collectionId, $document->getId(), 'attr1'); + $database->decreaseDocumentAttribute($collectionId, $document->getId(), 'attr1'); + }, ['should-not-execute']); + + $this->assertFalse($executed); + + $database->deleteIndex($collectionId, $indexId1); + $database->deleteDocument($collectionId, 'doc1'); + $database->deleteAttribute($collectionId, 'attr1'); + $database->deleteCollection($collectionId); + $database->delete('hellodb'); + }); + } } From 4f3e91e575ead30d1eeeb200cbff6b9c48061079 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 17 Sep 2024 14:43:36 +0900 Subject: [PATCH 05/23] Run Linter --- src/Database/Adapter/MariaDB.php | 22 +++++++++++----------- src/Database/Adapter/Mongo.php | 11 +++++------ src/Database/Adapter/Postgres.php | 16 ++++++++-------- src/Database/Adapter/SQL.php | 6 +++--- src/Database/Adapter/SQLite.php | 12 ++++++------ src/Database/Database.php | 8 ++++---- src/Database/Exception.php | 2 +- src/Database/Query.php | 4 ++-- src/Database/Validator/Datetime.php | 18 +++++++++--------- src/Database/Validator/Index.php | 14 +++++++------- src/Database/Validator/Queries.php | 4 ++-- src/Database/Validator/Query/Filter.php | 18 +++++++++--------- src/Database/Validator/Structure.php | 2 +- tests/unit/Validator/DateTimeTest.php | 2 +- 14 files changed, 69 insertions(+), 70 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 7b1537d11..a3776fa98 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -687,7 +687,7 @@ public function createIndex(string $collection, string $id, string $type, array $attributes[$i] = "`{$attr}`{$length} {$order}"; - if(!empty($collectionAttribute['array']) && $this->castIndexArray()) { + if (!empty($collectionAttribute['array']) && $this->castIndexArray()) { $attributes[$i] = '(CAST(' . $attr . ' AS char(' . Database::ARRAY_INDEX_LENGTH . ') ARRAY))'; } } @@ -871,7 +871,7 @@ public function createDocument(string $collection, Document $document): Document $stmtPermissions->execute(); } } catch (\Throwable $e) { - if($e instanceof PDOException) { + if ($e instanceof PDOException) { switch ($e->getCode()) { case 1062: case 23000: @@ -919,7 +919,7 @@ public function createDocuments(string $collection, array $documents, int $batch $attributes['_createdAt'] = $document->getCreatedAt(); $attributes['_updatedAt'] = $document->getUpdatedAt(); $attributes['_permissions'] = \json_encode($document->getPermissions()); - if(!empty($document->getInternalId())) { + if (!empty($document->getInternalId())) { $attributes['_id'] = $document->getInternalId(); } @@ -999,7 +999,7 @@ public function createDocuments(string $collection, array $documents, int $batch } } } catch (\Throwable $e) { - if($e instanceof PDOException) { + if ($e instanceof PDOException) { switch ($e->getCode()) { case 1062: case 23000: @@ -1247,7 +1247,7 @@ public function updateDocument(string $collection, Document $document): Document } } catch (\Throwable $e) { - if($e instanceof PDOException) { + if ($e instanceof PDOException) { switch ($e->getCode()) { case 1062: case 23000: @@ -1504,7 +1504,7 @@ public function updateDocuments(string $collection, array $documents, int $batch } } } catch (\Throwable $e) { - if($e instanceof PDOException) { + if ($e instanceof PDOException) { switch ($e->getCode()) { case 1062: case 23000: @@ -1651,7 +1651,7 @@ public function deleteDocuments(string $collection, array $queries): bool $queries = array_map(fn ($query) => clone $query, $queries); $conditions = $this->getSQLConditions($queries); - if(!empty($conditions)) { + if (!empty($conditions)) { $where[] = $conditions; } @@ -1786,7 +1786,7 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, } $conditions = $this->getSQLConditions($queries); - if(!empty($conditions)) { + if (!empty($conditions)) { $where[] = $conditions; } @@ -1914,7 +1914,7 @@ public function count(string $collection, array $queries = [], ?int $max = null) $queries = array_map(fn ($query) => clone $query, $queries); $conditions = $this->getSQLConditions($queries); - if(!empty($conditions)) { + if (!empty($conditions)) { $where[] = $conditions; } @@ -2131,7 +2131,7 @@ protected function getSQLCondition(Query $query): string return "`table_main`.{$attribute} {$this->getSQLOperator($query->getMethod())}"; case Query::TYPE_CONTAINS: - if($this->getSupportForJSONOverlaps() && $query->onArray()) { + if ($this->getSupportForJSONOverlaps() && $query->onArray()) { return "JSON_OVERLAPS(`table_main`.{$attribute}, :{$placeholder}_0)"; } @@ -2157,7 +2157,7 @@ protected function getSQLCondition(Query $query): string */ protected function getSQLType(string $type, int $size, bool $signed = true, bool $array = false): string { - if($array === true) { + if ($array === true) { return 'JSON'; } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 41c3c147e..729e06755 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -3,7 +3,6 @@ namespace Utopia\Database\Adapter; use Exception; - use MongoDB\BSON\ObjectId; use MongoDB\BSON\Regex; use MongoDB\BSON\UTCDateTime; @@ -291,7 +290,7 @@ public function getSizeOfCollection(string $collection): int } else { throw new DatabaseException('No size found'); } - } catch(Exception $e) { + } catch (Exception $e) { throw new DatabaseException('Failed to get collection size: ' . $e->getMessage()); } } @@ -935,8 +934,8 @@ public function deleteDocuments(string $collection, array $queries): bool try { $res = $this->client->delete( - collection: $name, - filters: $filters, + collection: $name, + filters: $filters, options: $options, limit: 0 ); @@ -1406,7 +1405,7 @@ protected function buildFilters(array $queries, string $separator = '$and'): arr $queries = Query::groupByType($queries)['filters']; foreach ($queries as $query) { /* @var $query Query */ - if($query->isNested()) { + if ($query->isNested()) { $operator = $this->getQueryOperator($query->getMethod()); $filters[$separator][] = $this->buildFilters($query->getValues(), $operator); } else { @@ -1460,7 +1459,7 @@ protected function buildFilter(Query $query): array } elseif ($operator == '$ne' && \is_array($value)) { $filter[$attribute]['$nin'] = $value; } elseif ($operator == '$in') { - if($query->getMethod() === Query::TYPE_CONTAINS && !$query->onArray()) { + if ($query->getMethod() === Query::TYPE_CONTAINS && !$query->onArray()) { $filter[$attribute]['$regex'] = new Regex(".*{$this->escapeWildcards($value)}.*", 'i'); } else { $filter[$attribute]['$in'] = $query->getValues(); diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 9162ea12e..eb667c4a4 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -841,7 +841,7 @@ public function createDocument(string $collection, Document $document): Document $queryPermissions = $this->trigger(Database::EVENT_PERMISSIONS_CREATE, $queryPermissions); $stmtPermissions = $this->getPDO()->prepare($queryPermissions); - if($sqlTenant) { + if ($sqlTenant) { $stmtPermissions->bindValue(':_tenant', $this->tenant); } } @@ -900,7 +900,7 @@ public function createDocuments(string $collection, array $documents, int $batch $attributes['_updatedAt'] = $document->getUpdatedAt(); $attributes['_permissions'] = \json_encode($document->getPermissions()); - if($this->sharedTables) { + if ($this->sharedTables) { $attributes['_tenant'] = $this->tenant; } @@ -1232,7 +1232,7 @@ public function updateDocuments(string $collection, array $documents, int $batch $attributes['_updatedAt'] = $document->getUpdatedAt(); $attributes['_permissions'] = json_encode($document->getPermissions()); - if($this->sharedTables) { + if ($this->sharedTables) { $attributes['_tenant'] = $this->tenant; } @@ -1424,7 +1424,7 @@ public function updateDocuments(string $collection, array $documents, int $batch $stmtAddPermissions->bindValue($key, $value, $this->getPDOType($value)); } - if($this->sharedTables) { + if ($this->sharedTables) { $stmtAddPermissions->bindValue(':_tenant', $this->tenant); } @@ -1575,7 +1575,7 @@ public function deleteDocuments(string $collection, array $queries): bool $queries = array_map(fn ($query) => clone $query, $queries); $conditions = $this->getSQLConditions($queries); - if(!empty($conditions)) { + if (!empty($conditions)) { $where[] = $conditions; } @@ -1708,7 +1708,7 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, } $conditions = $this->getSQLConditions($queries); - if(!empty($conditions)) { + if (!empty($conditions)) { $where[] = $conditions; } @@ -1836,7 +1836,7 @@ public function count(string $collection, array $queries = [], ?int $max = null) $queries = array_map(fn ($query) => clone $query, $queries); $conditions = $this->getSQLConditions($queries); - if(!empty($conditions)) { + if (!empty($conditions)) { $where[] = $conditions; } @@ -2076,7 +2076,7 @@ protected function getFulltextValue(string $value): string */ protected function getSQLType(string $type, int $size, bool $signed = true, bool $array = false): string { - if($array === true) { + if ($array === true) { return 'JSONB'; } diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index dd6d822c6..e73515a57 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -818,14 +818,14 @@ protected function bindConditionValue(mixed $stmt, Query $query): void return; } - if($query->isNested()) { + if ($query->isNested()) { foreach ($query->getValues() as $value) { $this->bindConditionValue($stmt, $value); } return; } - if($this->getSupportForJSONOverlaps() && $query->onArray() && $query->getMethod() == Query::TYPE_CONTAINS) { + if ($this->getSupportForJSONOverlaps() && $query->onArray() && $query->getMethod() == Query::TYPE_CONTAINS) { $placeholder = $this->getSQLPlaceholder($query) . '_0'; $stmt->bindValue($placeholder, json_encode($query->getValues()), PDO::PARAM_STR); return; @@ -1071,7 +1071,7 @@ public function getSQLConditions(array $queries = [], string $separator = 'AND') continue; } - if($query->isNested()) { + if ($query->isNested()) { $conditions[] = $this->getSQLConditions($query->getValues(), $query->getMethod()); } else { $conditions[] = $this->getSQLCondition($query); diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 9f7441f6b..193f9b19c 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -448,7 +448,7 @@ public function createDocument(string $collection, Document $document): Document $attributes['_updatedAt'] = $document->getUpdatedAt(); $attributes['_permissions'] = json_encode($document->getPermissions()); - if($this->sharedTables) { + if ($this->sharedTables) { $attributes['_tenant'] = $this->tenant; } @@ -522,7 +522,7 @@ public function createDocument(string $collection, Document $document): Document $stmtPermissions = $this->getPDO()->prepare($queryPermissions); - if($this->sharedTables) { + if ($this->sharedTables) { $stmtPermissions->bindValue(':_tenant', $this->tenant); } } @@ -567,7 +567,7 @@ public function updateDocument(string $collection, Document $document): Document $attributes['_updatedAt'] = $document->getUpdatedAt(); $attributes['_permissions'] = json_encode($document->getPermissions()); - if($this->sharedTables) { + if ($this->sharedTables) { $attributes['_tenant'] = $this->tenant; } @@ -701,7 +701,7 @@ public function updateDocument(string $collection, Document $document): Document $stmtAddPermissions = $this->getPDO()->prepare($sql); $stmtAddPermissions->bindValue(":_uid", $document->getId()); - if($this->sharedTables) { + if ($this->sharedTables) { $stmtAddPermissions->bindValue(":_tenant", $this->tenant); } @@ -814,7 +814,7 @@ public function updateDocuments(string $collection, array $documents, int $batch $attributes['_updatedAt'] = $document->getUpdatedAt(); $attributes['_permissions'] = json_encode($document->getPermissions()); - if($this->sharedTables) { + if ($this->sharedTables) { $attributes['_tenant'] = $this->tenant; } @@ -1012,7 +1012,7 @@ public function updateDocuments(string $collection, array $documents, int $batch $stmtAddPermissions->bindValue($key, $value, $this->getPDOType($value)); } - if($this->sharedTables) { + if ($this->sharedTables) { $stmtAddPermissions->bindValue(':_tenant', $this->tenant); } diff --git a/src/Database/Database.php b/src/Database/Database.php index 944c3bea0..b3f42f047 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4331,7 +4331,7 @@ public function increaseDocumentAttribute(string $collection, string $id, string /* @var $document Document */ $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this - if($document->isEmpty()) { + if ($document->isEmpty()) { return false; } @@ -4426,7 +4426,7 @@ public function decreaseDocumentAttribute(string $collection, string $id, string /* @var $document Document */ $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this - if($document->isEmpty()) { + if ($document->isEmpty()) { return false; } @@ -4521,7 +4521,7 @@ public function deleteDocument(string $collection, string $id): bool $this->getDocument($collection->getId(), $id, forUpdate: true) )); - if($document->isEmpty()) { + if ($document->isEmpty()) { return false; } @@ -5190,7 +5190,7 @@ public function find(string $collection, array $queries = []): array $results = $skipAuth ? Authorization::skip($getResults) : $getResults(); - foreach ($results as &$node) { + foreach ($results as &$node) { if ($this->resolveRelationships && (empty($selects) || !empty($nestedSelections))) { $node = $this->silent(fn () => $this->populateDocumentRelationships($collection, $node, $nestedSelections)); } diff --git a/src/Database/Exception.php b/src/Database/Exception.php index 64ad3c997..94099c6ae 100644 --- a/src/Database/Exception.php +++ b/src/Database/Exception.php @@ -8,7 +8,7 @@ class Exception extends \Exception { public function __construct(string $message, int|string $code = 0, Throwable $previous = null) { - if(\is_string($code)) { + if (\is_string($code)) { if (\is_numeric($code)) { $code = (int) $code; } else { diff --git a/src/Database/Query.php b/src/Database/Query.php index 6af553415..aab17c195 100644 --- a/src/Database/Query.php +++ b/src/Database/Query.php @@ -297,7 +297,7 @@ public function toArray(): array { $array = ['method' => $this->method]; - if(!empty($this->attribute)) { + if (!empty($this->attribute)) { $array['attribute'] = $this->attribute; } @@ -690,7 +690,7 @@ public static function groupByType(array $queries): array */ public function isNested(): bool { - if(in_array($this->getMethod(), self::LOGICAL_TYPES)) { + if (in_array($this->getMethod(), self::LOGICAL_TYPES)) { return true; } diff --git a/src/Database/Validator/Datetime.php b/src/Database/Validator/Datetime.php index e8ffee5ff..812669d7a 100644 --- a/src/Database/Validator/Datetime.php +++ b/src/Database/Validator/Datetime.php @@ -33,7 +33,7 @@ class Datetime extends Validator */ public function __construct(bool $requireDateInFuture = false, string $precision = self::PRECISION_ANY, int $offset = 0) { - if($offset < 0) { + if ($offset < 0) { throw new \Exception('Offset must be a positive number.'); } @@ -50,13 +50,13 @@ public function getDescription(): string { $message = 'Value must be valid date'; - if($this->offset > 0) { + if ($this->offset > 0) { $message .= " at least " . $this->offset . " seconds in future"; - } elseif($this->requireDateInFuture) { + } elseif ($this->requireDateInFuture) { $message .= " in future"; } - if($this->precision !== self::PRECISION_ANY) { + if ($this->precision !== self::PRECISION_ANY) { $message .= " with " . $this->precision . " precision"; } @@ -84,9 +84,9 @@ public function isValid($value): bool return false; } - if($this->offset !== 0) { + if ($this->offset !== 0) { $diff = $date->getTimestamp() - $now->getTimestamp(); - if($diff <= $this->offset) { + if ($diff <= $this->offset) { return false; } } @@ -109,12 +109,12 @@ public function isValid($value): bool break; } - foreach($denyConstants as $constant) { - if(\intval($date->format($constant)) !== 0) { + foreach ($denyConstants as $constant) { + if (\intval($date->format($constant)) !== 0) { return false; } } - } catch(\Exception $e) { + } catch (\Exception $e) { return false; } diff --git a/src/Database/Validator/Index.php b/src/Database/Validator/Index.php index b24e39f73..fdaa1efe0 100644 --- a/src/Database/Validator/Index.php +++ b/src/Database/Validator/Index.php @@ -126,30 +126,30 @@ public function checkArrayIndex(Document $index): bool foreach ($attributes as $attributePosition => $attributeName) { $attribute = $this->attributes[\strtolower($attributeName)] ?? new Document(); - if($attribute->getAttribute('array', false)) { + if ($attribute->getAttribute('array', false)) { // Database::INDEX_UNIQUE Is not allowed! since mariaDB VS MySQL makes the unique Different on values - if($index->getAttribute('type') != Database::INDEX_KEY) { + if ($index->getAttribute('type') != Database::INDEX_KEY) { $this->message = '"' . ucfirst($index->getAttribute('type')) . '" index is forbidden on array attributes'; return false; } - if(empty($lengths[$attributePosition])) { + if (empty($lengths[$attributePosition])) { $this->message = 'Index length for array not specified'; return false; } $arrayAttributes[] = $attribute->getAttribute('key', ''); - if(count($arrayAttributes) > 1) { + if (count($arrayAttributes) > 1) { $this->message = 'An index may only contain one array attribute'; return false; } $direction = $orders[$attributePosition] ?? ''; - if(!empty($direction)) { + if (!empty($direction)) { $this->message = 'Invalid index order "' . $direction . '" on array attribute "'. $attribute->getAttribute('key', '') .'"'; return false; } - } elseif($attribute->getAttribute('type') !== Database::VAR_STRING && !empty($lengths[$attributePosition])) { + } elseif ($attribute->getAttribute('type') !== Database::VAR_STRING && !empty($lengths[$attributePosition])) { $this->message = 'Cannot set a length on "'. $attribute->getAttribute('type') . '" attributes'; return false; } @@ -188,7 +188,7 @@ public function checkIndexLength(Document $index): bool break; } - if($attribute->getAttribute('array', false)) { + if ($attribute->getAttribute('array', false)) { $attributeSize = Database::ARRAY_INDEX_LENGTH; $indexLength = Database::ARRAY_INDEX_LENGTH; } diff --git a/src/Database/Validator/Queries.php b/src/Database/Validator/Queries.php index 2e4aac71a..b1d67aad0 100644 --- a/src/Database/Validator/Queries.php +++ b/src/Database/Validator/Queries.php @@ -71,8 +71,8 @@ public function isValid($value): bool } } - if($query->isNested()) { - if(!self::isValid($query->getValues())) { + if ($query->isNested()) { + if (!self::isValid($query->getValues())) { return false; } } diff --git a/src/Database/Validator/Query/Filter.php b/src/Database/Validator/Query/Filter.php index 635fa4732..1fb50f9fc 100644 --- a/src/Database/Validator/Query/Filter.php +++ b/src/Database/Validator/Query/Filter.php @@ -131,29 +131,29 @@ protected function isValidAttributeAndValues(string $attribute, array $values, s } } - if($attributeSchema['type'] === 'relationship') { + if ($attributeSchema['type'] === 'relationship') { /** * We can not disable relationship query since we have logic that use it, * so instead we validate against the relation type */ $options = $attributeSchema['options']; - if($options['relationType'] === Database::RELATION_ONE_TO_ONE && $options['twoWay'] === false && $options['side'] === Database::RELATION_SIDE_CHILD) { + if ($options['relationType'] === Database::RELATION_ONE_TO_ONE && $options['twoWay'] === false && $options['side'] === Database::RELATION_SIDE_CHILD) { $this->message = 'Cannot query on virtual relationship attribute'; return false; } - if($options['relationType'] === Database::RELATION_ONE_TO_MANY && $options['side'] === Database::RELATION_SIDE_PARENT) { + if ($options['relationType'] === Database::RELATION_ONE_TO_MANY && $options['side'] === Database::RELATION_SIDE_PARENT) { $this->message = 'Cannot query on virtual relationship attribute'; return false; } - if($options['relationType'] === Database::RELATION_MANY_TO_ONE && $options['side'] === Database::RELATION_SIDE_CHILD) { + if ($options['relationType'] === Database::RELATION_MANY_TO_ONE && $options['side'] === Database::RELATION_SIDE_CHILD) { $this->message = 'Cannot query on virtual relationship attribute'; return false; } - if($options['relationType'] === Database::RELATION_MANY_TO_MANY) { + if ($options['relationType'] === Database::RELATION_MANY_TO_MANY) { $this->message = 'Cannot query on virtual relationship attribute'; return false; } @@ -161,7 +161,7 @@ protected function isValidAttributeAndValues(string $attribute, array $values, s $array = $attributeSchema['array'] ?? false; - if( + if ( !$array && $method === Query::TYPE_CONTAINS && $attributeSchema['type'] !== Database::VAR_STRING @@ -170,7 +170,7 @@ protected function isValidAttributeAndValues(string $attribute, array $values, s return false; } - if( + if ( $array && !in_array($method, [Query::TYPE_CONTAINS, Query::TYPE_IS_NULL, Query::TYPE_IS_NOT_NULL]) ) { @@ -253,12 +253,12 @@ public function isValid($value): bool case Query::TYPE_AND: $filters = Query::groupByType($value->getValues())['filters']; - if(count($value->getValues()) !== count($filters)) { + if (count($value->getValues()) !== count($filters)) { $this->message = \ucfirst($method) . ' queries can only contain filter queries'; return false; } - if(count($filters) < 2) { + if (count($filters) < 2) { $this->message = \ucfirst($method) . ' queries require at least two queries'; return false; } diff --git a/src/Database/Validator/Structure.php b/src/Database/Validator/Structure.php index 12b753824..991a32240 100644 --- a/src/Database/Validator/Structure.php +++ b/src/Database/Validator/Structure.php @@ -257,7 +257,7 @@ public function isValid($document): bool continue; } - if($type === Database::VAR_RELATIONSHIP) { + if ($type === Database::VAR_RELATIONSHIP) { continue; } diff --git a/tests/unit/Validator/DateTimeTest.php b/tests/unit/Validator/DateTimeTest.php index b9157e8b8..d8ababb6f 100644 --- a/tests/unit/Validator/DateTimeTest.php +++ b/tests/unit/Validator/DateTimeTest.php @@ -128,7 +128,7 @@ public function testOffset(): void $threwException = false; try { $dateValidator = new DatetimeValidator(offset: -60); - } catch(\Exception $e) { + } catch (\Exception $e) { $threwException = true; } $this->assertTrue($threwException); From c3115b5ea2973cc70825187f470b30d4b1090608 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 17 Sep 2024 15:29:12 +0900 Subject: [PATCH 06/23] Update Authorisation Validator --- src/Database/Database.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index b7adc87ee..827ef51ef 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5012,15 +5012,14 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } foreach ($affectedDocuments as $document) { - $validator = new Authorization(self::PERMISSION_DELETE); - if ($collection->getId() !== self::METADATA) { $documentSecurity = $collection->getAttribute('documentSecurity', false); - if (!$validator->isValid([ + $isValid = $this->authorization->isValid(new Input(self::PERMISSION_DELETE, [ ...$collection->getDelete(), ...($documentSecurity ? $document->getDelete() : []) - ])) { - throw new AuthorizationException($validator->getDescription()); + ])); + if (!$isValid) { + throw new AuthorizationException($this->authorization->getDescription()); } } From 04d801572983a50de691bedc6d8c1363ec107e6b Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 1 Oct 2024 15:58:20 +0900 Subject: [PATCH 07/23] Use ID based deletion, add more tests --- src/Database/Adapter.php | 4 +- src/Database/Adapter/MariaDB.php | 85 +++++++++++++++++---------- src/Database/Adapter/Mongo.php | 7 +-- src/Database/Adapter/Postgres.php | 85 +++++++++++++++++---------- src/Database/Database.php | 29 ++++++---- tests/e2e/Adapter/Base.php | 96 ++++++++++++++++++++++++++++--- 6 files changed, 219 insertions(+), 87 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index c655e1e59..eec087661 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -621,11 +621,11 @@ abstract public function deleteDocument(string $collection, string $id): bool; * Delete Documents * * @param string $collection - * @param array<\Utopia\Database\Query> $queries + * @param array $ids * * @return bool */ - abstract public function deleteDocuments(string $collection, array $queries): bool; + abstract public function deleteDocuments(string $collection, array $ids): bool; /** * Find Documents diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 504d131f1..129a0b229 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1636,51 +1636,76 @@ public function deleteDocument(string $collection, string $id): bool * Delete Documents * * @param string $collection - * @param array<\Utopia\Database\Query> $queries + * @param array $ids * * @return bool */ - public function deleteDocuments(string $collection, array $queries): bool + public function deleteDocuments(string $collection, array $ids): bool { - $name = $this->filter($collection); - $where = []; + try { + $name = $this->filter($collection); + $where = []; - $queries = array_map(fn ($query) => clone $query, $queries); + if ($this->sharedTables) { + $where[] = "_tenant = :_tenant"; + } - $conditions = $this->getSQLConditions($queries); - if (!empty($conditions)) { - $where[] = $conditions; - } + $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ")"; - if ($this->sharedTables) { - $where[] = "table_main._tenant = :_tenant"; - } + $sqlWhere = !empty($where) ? 'WHERE ' . \implode(' AND ', $where) : ''; - $sqlWhere = !empty($where) ? 'WHERE ' . implode(' AND ', $where) : ''; + $sql = " + DELETE FROM {$this->getSQLTable($name)} + {$sqlWhere}; + "; - $sql = " - DELETE FROM {$this->getSQLTable($name)} - {$sqlWhere}; - "; + $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); - $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); + $stmt = $this->getPDO()->prepare($sql); - $stmt = $this->getPDO()->prepare($sql); + foreach ($ids as $id => $value) { + $stmt->bindValue(":_id_{$id}", $value); + } - foreach ($queries as $query) { - $this->bindConditionValue($stmt, $query); - } - if ($this->sharedTables) { - $stmt->bindValue(':_tenant', $this->tenant); - } + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } - try { - $stmt->execute(); - } catch (PDOException $e) { - $this->processException($e); + $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'); + } + + $deleted = $stmt->rowCount(); + + if (!$stmtPermissions->execute()) { + throw new DatabaseException('Failed to delete permissions'); + } + } catch (\Throwable $e) { + throw new DatabaseException($e->getMessage(), $e->getCode(), $e); } - return true; + return $deleted; } /** diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index c54ed20ea..7841c51b0 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -912,16 +912,15 @@ public function deleteDocument(string $collection, string $id): bool * Delete Documents * * @param string $collection - * @param array<\Utopia\Database\Query> $queries + * @param array $ids * * @return bool */ - public function deleteDocuments(string $collection, array $queries): bool + public function deleteDocuments(string $collection, array $ids): bool { $name = $this->getNamespace() . '_' . $this->filter($collection); - $queries = array_map(fn ($query) => clone $query, $queries); - $filters = $this->buildFilters($queries); + $filters = $this->buildFilters([new Query(Query::TYPE_EQUAL, '_uid', $ids)]); if ($this->sharedTables) { $filters['_tenant'] = (string)$this->getTenant(); diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 92b91ca4e..16788d26b 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1561,51 +1561,76 @@ public function deleteDocument(string $collection, string $id): bool * Delete Documents * * @param string $collection - * @param array<\Utopia\Database\Query> $queries + * @param array $ids * * @return bool */ - public function deleteDocuments(string $collection, array $queries): bool + public function deleteDocuments(string $collection, array $ids): bool { - $name = $this->filter($collection); - $where = []; + try { + $name = $this->filter($collection); + $where = []; - $queries = array_map(fn ($query) => clone $query, $queries); + if ($this->sharedTables) { + $where[] = "_tenant = :_tenant"; + } - $conditions = $this->getSQLConditions($queries); - if (!empty($conditions)) { - $where[] = $conditions; - } + $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ")"; - if ($this->sharedTables) { - $where[] = "table_main._tenant = :_tenant"; - } + $sqlWhere = !empty($where) ? 'WHERE ' . \implode(' AND ', $where) : ''; - $sqlWhere = !empty($where) ? 'WHERE ' . implode(' AND ', $where) : ''; + $sql = " + DELETE FROM {$this->getSQLTable($name)} + {$sqlWhere}; + "; - $sql = " - DELETE FROM {$this->getSQLTable($name)} - {$sqlWhere} - "; + $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); - $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); + $stmt = $this->getPDO()->prepare($sql); - $stmt = $this->getPDO()->prepare($sql); + foreach ($ids as $id => $value) { + $stmt->bindValue(":_id_{$id}", $value); + } - foreach ($queries as $query) { - $this->bindConditionValue($stmt, $query); - } - if ($this->sharedTables) { - $stmt->bindValue(':_tenant', $this->tenant); - } + if ($this->sharedTables) { + $stmt->bindValue(':_tenant', $this->tenant); + } - try { - $stmt->execute(); - } catch (PDOException $e) { - $this->processException($e); + $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'); + } + + $deleted = $stmt->rowCount(); + + if (!$stmtPermissions->execute()) { + throw new DatabaseException('Failed to delete permissions'); + } + } catch (\Throwable $e) { + throw new DatabaseException($e->getMessage(), $e->getCode(), $e); } - return true; + return $deleted; } /** diff --git a/src/Database/Database.php b/src/Database/Database.php index 827ef51ef..7bb9826ab 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4993,24 +4993,29 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $queries = Query::groupByType($queries)['filters']; $collection = $this->silent(fn () => $this->getCollection($collection)); + $affectedDocumentIds = []; - $deleted = $this->withTransaction(function () use ($collection, $queries, $batchSize) { + $deleted = $this->withTransaction(function () use ($collection, $queries, $batchSize, $affectedDocumentIds) { $lastDocument = null; while (true) { - $affectedDocuments = $this->find($collection->getId(), array_merge( - empty($lastDocument) ? [ - Query::limit($batchSize), - ] : [ - Query::limit($batchSize), - Query::cursorAfter($lastDocument), - ], - $queries, - )); + $affectedDocuments = $this->skipRelationships(function () use ($collection, $queries, $batchSize, $lastDocument) { + return $this->find($collection->getId(), array_merge( + empty($lastDocument) ? [ + Query::limit($batchSize), + ] : [ + Query::limit($batchSize), + Query::cursorAfter($lastDocument), + ], + $queries, + )); + }); if (empty($affectedDocuments)) { break; } + $affectedDocumentIds = array_merge($affectedDocumentIds, array_map(fn ($document) => $document->getId(), $affectedDocuments)); + foreach ($affectedDocuments as $document) { if ($collection->getId() !== self::METADATA) { $documentSecurity = $collection->getAttribute('documentSecurity', false); @@ -5035,7 +5040,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $this->purgeCachedDocument($collection->getId(), $document->getId()); } - if (count($affectedDocuments) < 100) { + if (count($affectedDocuments) < $batchSize) { break; } else { $lastDocument = end($affectedDocuments); @@ -5043,7 +5048,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } // Mass delete using adapter with query - return $this->adapter->deleteDocuments($collection->getId(), $queries); + return $this->adapter->deleteDocuments($collection->getId(), $affectedDocumentIds); }); return $deleted; diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 8b340b0bd..d155e338c 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15642,27 +15642,105 @@ public function testDeleteBulkDocuments(): void static::getDatabase()->deleteCollection('bulk_delete'); } - - public function testDeleteBulkDocumentsRelationships(): void + public function testDeleteBulkDocumentsOneToOneRelationship(): void { if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { $this->expectNotToPerformAssertions(); return; } - static::getDatabase()->createCollection('bulk_delete_r_1'); - static::getDatabase()->createCollection('bulk_delete_r_2'); + $this->getDatabase()->createCollection('bulk_delete_person'); + $this->getDatabase()->createCollection('bulk_delete_library'); - // ONE_TO_ONE - static::getDatabase()->createRelationship( - collection: 'bulk_delete_r_1', - relatedCollection: 'bulk_delete_r_2', + $this->getDatabase()->createAttribute('bulk_delete_person', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library', 'name', Database::VAR_STRING, 255, true); + $this->getDatabase()->createAttribute('bulk_delete_library', 'area', Database::VAR_STRING, 255, true); + + // Restrict + $this->getDatabase()->createRelationship( + collection: 'bulk_delete_person', + relatedCollection: 'bulk_delete_library', type: Database::RELATION_ONE_TO_ONE, + onDelete: Database::RELATION_MUTATE_RESTRICT ); - // Restrict + $person1 = $this->getDatabase()->createDocument('bulk_delete_person', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library' => [ + '$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', 'person1'); + $library = $person1->getAttribute('bulk_delete_library'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person', $library); + + // Delete person + try { + $this->getDatabase()->deleteDocuments('bulk_delete_person'); + $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', 'person1', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library' => null, + ])); + + $this->getDatabase()->deleteDocuments('bulk_delete_person'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); + $this->getDatabase()->deleteDocuments('bulk_delete_library'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); + $this->assertTrue($this->getDatabase()->deleteRelationship('bulk_delete_person', 'bulk_delete_library')); // NULL + $this->getDatabase()->createRelationship( + collection: 'bulk_delete_person', + relatedCollection: 'bulk_delete_library', + type: Database::RELATION_ONE_TO_ONE, + onDelete: Database::RELATION_MUTATE_SET_NULL + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library' => [ + '$id' => 'library1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Library 1', + 'area' => 'Area 1', + ], + ])); // Cascade } From 87a6e3a835301a8e88c66712bddde8fc2f83239a Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 1 Oct 2024 17:22:54 +0900 Subject: [PATCH 08/23] Linter and CodeQL --- src/Database/Adapter.php | 2 +- src/Database/Adapter/MariaDB.php | 6 ++---- src/Database/Adapter/Postgres.php | 6 ++---- src/Database/Database.php | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index eec087661..349193da1 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -621,7 +621,7 @@ abstract public function deleteDocument(string $collection, string $id): bool; * Delete Documents * * @param string $collection - * @param array $ids + * @param array $ids * * @return bool */ diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 129a0b229..3649b63da 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1696,16 +1696,14 @@ public function deleteDocuments(string $collection, array $ids): bool throw new DatabaseException('Failed to delete documents'); } - $deleted = $stmt->rowCount(); - if (!$stmtPermissions->execute()) { throw new DatabaseException('Failed to delete permissions'); - } + } } catch (\Throwable $e) { throw new DatabaseException($e->getMessage(), $e->getCode(), $e); } - return $deleted; + return true; } /** diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 16788d26b..cc5ccce6c 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1621,16 +1621,14 @@ public function deleteDocuments(string $collection, array $ids): bool throw new DatabaseException('Failed to delete documents'); } - $deleted = $stmt->rowCount(); - if (!$stmtPermissions->execute()) { throw new DatabaseException('Failed to delete permissions'); - } + } } catch (\Throwable $e) { throw new DatabaseException($e->getMessage(), $e->getCode(), $e); } - return $deleted; + return true; } /** diff --git a/src/Database/Database.php b/src/Database/Database.php index 7bb9826ab..1cacdf3f6 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4998,7 +4998,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $deleted = $this->withTransaction(function () use ($collection, $queries, $batchSize, $affectedDocumentIds) { $lastDocument = null; while (true) { - $affectedDocuments = $this->skipRelationships(function () use ($collection, $queries, $batchSize, $lastDocument) { + $affectedDocuments = $this->skipRelationships(function () use ($collection, $queries, $batchSize, $lastDocument) { return $this->find($collection->getId(), array_merge( empty($lastDocument) ? [ Query::limit($batchSize), From 22abde7ade5e1bdc4b11d1c4f4560650056a0d76 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 1 Oct 2024 17:34:42 +0900 Subject: [PATCH 09/23] CodeQL --- src/Database/Adapter/MariaDB.php | 7 +------ src/Database/Adapter/Postgres.php | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 3649b63da..ab9254afc 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1652,12 +1652,7 @@ public function deleteDocuments(string $collection, array $ids): bool $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ")"; - $sqlWhere = !empty($where) ? 'WHERE ' . \implode(' AND ', $where) : ''; - - $sql = " - DELETE FROM {$this->getSQLTable($name)} - {$sqlWhere}; - "; + $sql = "DELETE FROM {$this->getSQLTable($name)} WHERE " . \implode(' AND ', $where); $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index cc5ccce6c..8c667dce5 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1577,12 +1577,7 @@ public function deleteDocuments(string $collection, array $ids): bool $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ")"; - $sqlWhere = !empty($where) ? 'WHERE ' . \implode(' AND ', $where) : ''; - - $sql = " - DELETE FROM {$this->getSQLTable($name)} - {$sqlWhere}; - "; + $sql = "DELETE FROM {$this->getSQLTable($name)} WHERE " . \implode(' AND ', $where); $sql = $this->trigger(Database::EVENT_DOCUMENT_DELETE, $sql); From 19e6029edf64d1fe777165f902865b7064b9a850 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 2 Oct 2024 12:28:58 +0900 Subject: [PATCH 10/23] Continue to add more tests --- src/Database/Adapter/MariaDB.php | 4 ++ src/Database/Adapter/Postgres.php | 4 ++ tests/e2e/Adapter/Base.php | 71 +++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index ab9254afc..4788c67fc 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1642,6 +1642,10 @@ public function deleteDocument(string $collection, string $id): bool */ public function deleteDocuments(string $collection, array $ids): bool { + if (empty($ids)) { + return true; + } + try { $name = $this->filter($collection); $where = []; diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 8c667dce5..437c44914 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1567,6 +1567,10 @@ public function deleteDocument(string $collection, string $id): bool */ public function deleteDocuments(string $collection, array $ids): bool { + if (empty($ids)) { + return true; + } + try { $name = $this->filter($collection); $where = []; diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index d155e338c..66aa4290b 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15742,7 +15742,78 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void ], ])); + $person1 = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); + $library = $person1->getAttribute('bulk_delete_library'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person', $library); + + $person = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); + + $this->getDatabase()->deleteDocuments('bulk_delete_library'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); + $this->assertCount(1, $this->getDatabase()->find('bulk_delete_person')); + + $person = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); + $library = $person->getAttribute('bulk_delete_library'); + $this->assertNull($library); + + // NULL - Cleanup + $this->getDatabase()->deleteDocuments('bulk_delete_person'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); + $this->getDatabase()->deleteDocuments('bulk_delete_library'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); + $this->assertTrue($this->getDatabase()->deleteRelationship('bulk_delete_person', 'bulk_delete_library')); + // Cascade + $this->getDatabase()->createRelationship( + collection: 'bulk_delete_person', + relatedCollection: 'bulk_delete_library', + type: Database::RELATION_ONE_TO_ONE, + onDelete: Database::RELATION_MUTATE_CASCADE + ); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library' => [ + '$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', 'person1'); + $library = $person1->getAttribute('bulk_delete_library'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person', $library); + + $person = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); + + $this->getDatabase()->deleteDocuments('bulk_delete_library'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); + $this->assertCount(1, $this->getDatabase()->find('bulk_delete_person')); + + $person = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); + $library = $person->getAttribute('bulk_delete_library'); + $this->assertEmpty($library); + $this->assertNotNull($library); + + // Cascade - Cleanup + $this->getDatabase()->deleteDocuments('bulk_delete_person'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); + $this->getDatabase()->deleteDocuments('bulk_delete_library'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); + $this->assertTrue($this->getDatabase()->deleteRelationship('bulk_delete_person', 'bulk_delete_library')); } public function testEvents(): void From d3264dcae23f55d344ce2db4ae8d3a84103b5235 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 2 Oct 2024 12:45:38 +0900 Subject: [PATCH 11/23] Fix Cascade --- src/Database/Database.php | 2 +- tests/e2e/Adapter/Base.php | 48 +++++++++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 1cacdf3f6..7116c9740 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4895,7 +4895,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection $this->deleteDocument( $relatedCollection->getId(), - $value->getId() + ($value instanceof Document) ? $value->getId() : $value ); \array_pop($this->relationshipDeleteStack); diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 66aa4290b..6851db6e2 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15712,13 +15712,11 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); $this->getDatabase()->deleteDocuments('bulk_delete_library'); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); - $this->assertTrue($this->getDatabase()->deleteRelationship('bulk_delete_person', 'bulk_delete_library')); // NULL - $this->getDatabase()->createRelationship( + $this->getDatabase()->updateRelationship( collection: 'bulk_delete_person', - relatedCollection: 'bulk_delete_library', - type: Database::RELATION_ONE_TO_ONE, + id: 'bulk_delete_library', onDelete: Database::RELATION_MUTATE_SET_NULL ); @@ -15762,13 +15760,11 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); $this->getDatabase()->deleteDocuments('bulk_delete_library'); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); - $this->assertTrue($this->getDatabase()->deleteRelationship('bulk_delete_person', 'bulk_delete_library')); // Cascade - $this->getDatabase()->createRelationship( + $this->getDatabase()->updateRelationship( collection: 'bulk_delete_person', - relatedCollection: 'bulk_delete_library', - type: Database::RELATION_ONE_TO_ONE, + id: 'bulk_delete_library', onDelete: Database::RELATION_MUTATE_CASCADE ); @@ -15808,12 +15804,42 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void $this->assertEmpty($library); $this->assertNotNull($library); - // Cascade - Cleanup + // Test Bulk delete parent + $this->getDatabase()->deleteDocuments('bulk_delete_person'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); + + $person1 = $this->getDatabase()->createDocument('bulk_delete_person', new Document([ + '$id' => 'person1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Person 1', + 'bulk_delete_library' => [ + '$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', 'person1'); + $library = $person1->getAttribute('bulk_delete_library'); + $this->assertEquals('library1', $library['$id']); + $this->assertArrayNotHasKey('bulk_delete_person', $library); + $this->getDatabase()->deleteDocuments('bulk_delete_person'); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); - $this->getDatabase()->deleteDocuments('bulk_delete_library'); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); - $this->assertTrue($this->getDatabase()->deleteRelationship('bulk_delete_person', 'bulk_delete_library')); + + $this->getDatabase()->deleteDocuments('bulk_delete_person'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); + $this->getDatabase()->deleteDocuments('bulk_delete_library'); } public function testEvents(): void From 1241b0ac6c20c530429408b1fa41b207a650fe9f Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 2 Oct 2024 12:48:49 +0900 Subject: [PATCH 12/23] Run Linter --- docker-compose.yml | 2 +- tests/e2e/Adapter/Base.php | 2 +- 2 files changed, 2 insertions(+), 2 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/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 6851db6e2..e0ef94059 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15760,7 +15760,7 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); $this->getDatabase()->deleteDocuments('bulk_delete_library'); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); - + // Cascade $this->getDatabase()->updateRelationship( collection: 'bulk_delete_person', From e6eae09eeb40ed9fcd3204024e558d611f7966a5 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Fri, 4 Oct 2024 17:00:25 +0900 Subject: [PATCH 13/23] Remove unnecessary cleanup --- tests/e2e/Adapter/Base.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index e0ef94059..b4162b9f0 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15836,10 +15836,6 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void $this->getDatabase()->deleteDocuments('bulk_delete_person'); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); - - $this->getDatabase()->deleteDocuments('bulk_delete_person'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); - $this->getDatabase()->deleteDocuments('bulk_delete_library'); } public function testEvents(): void From f1a9eb5ebf9a2dc7adcc2d314781af29beefcc9b Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Fri, 4 Oct 2024 17:02:05 +0900 Subject: [PATCH 14/23] Disable debug in compose file --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 5497f4e28..23b135ba9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,7 +5,7 @@ services: build: context: . args: - DEBUG: true + DEBUG: false networks: - database volumes: From dbe77e979a235b9286d70a8849182754c929b13e Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Thu, 10 Oct 2024 11:16:49 +0900 Subject: [PATCH 15/23] Add more tests --- docker-compose.yml | 2 +- src/Database/Database.php | 20 +- tests/e2e/Adapter/Base.php | 453 ++++++++++++++++++++++++++++++++----- 3 files changed, 404 insertions(+), 71 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/Database.php b/src/Database/Database.php index 7116c9740..417e5bea8 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4998,17 +4998,15 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $deleted = $this->withTransaction(function () use ($collection, $queries, $batchSize, $affectedDocumentIds) { $lastDocument = null; while (true) { - $affectedDocuments = $this->skipRelationships(function () use ($collection, $queries, $batchSize, $lastDocument) { - return $this->find($collection->getId(), array_merge( - empty($lastDocument) ? [ - Query::limit($batchSize), - ] : [ - Query::limit($batchSize), - Query::cursorAfter($lastDocument), - ], - $queries, - )); - }); + $affectedDocuments = $this->find($collection->getId(), array_merge( + empty($lastDocument) ? [ + Query::limit($batchSize), + ] : [ + Query::limit($batchSize), + Query::cursorAfter($lastDocument), + ], + $queries, + )); if (empty($affectedDocuments)) { break; diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index b4162b9f0..4892b6318 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15649,22 +15649,22 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void return; } - $this->getDatabase()->createCollection('bulk_delete_person'); - $this->getDatabase()->createCollection('bulk_delete_library'); + $this->getDatabase()->createCollection('bulk_delete_person_o2o'); + $this->getDatabase()->createCollection('bulk_delete_library_o2o'); - $this->getDatabase()->createAttribute('bulk_delete_person', 'name', Database::VAR_STRING, 255, true); - $this->getDatabase()->createAttribute('bulk_delete_library', 'name', Database::VAR_STRING, 255, true); - $this->getDatabase()->createAttribute('bulk_delete_library', 'area', Database::VAR_STRING, 255, true); + $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', - relatedCollection: 'bulk_delete_library', + 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', new Document([ + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ '$id' => 'person1', '$permissions' => [ Permission::read(Role::any()), @@ -15672,7 +15672,7 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void Permission::delete(Role::any()), ], 'name' => 'Person 1', - 'bulk_delete_library' => [ + 'bulk_delete_library_o2o' => [ '$id' => 'library1', '$permissions' => [ Permission::read(Role::any()), @@ -15684,20 +15684,20 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void ], ])); - $person1 = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); - $library = $person1->getAttribute('bulk_delete_library'); + $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', $library); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); // Delete person try { - $this->getDatabase()->deleteDocuments('bulk_delete_person'); + $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', 'person1', new Document([ + $this->getDatabase()->updateDocument('bulk_delete_person_o2o', 'person1', new Document([ '$id' => 'person1', '$permissions' => [ Permission::read(Role::any()), @@ -15705,22 +15705,22 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void Permission::delete(Role::any()), ], 'name' => 'Person 1', - 'bulk_delete_library' => null, + 'bulk_delete_library_o2o' => null, ])); - $this->getDatabase()->deleteDocuments('bulk_delete_person'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); - $this->getDatabase()->deleteDocuments('bulk_delete_library'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); + $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', - id: 'bulk_delete_library', + collection: 'bulk_delete_person_o2o', + id: 'bulk_delete_library_o2o', onDelete: Database::RELATION_MUTATE_SET_NULL ); - $person1 = $this->getDatabase()->createDocument('bulk_delete_person', new Document([ + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ '$id' => 'person1', '$permissions' => [ Permission::read(Role::any()), @@ -15728,7 +15728,7 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void Permission::delete(Role::any()), ], 'name' => 'Person 1', - 'bulk_delete_library' => [ + 'bulk_delete_library_o2o' => [ '$id' => 'library1', '$permissions' => [ Permission::read(Role::any()), @@ -15740,35 +15740,35 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void ], ])); - $person1 = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); - $library = $person1->getAttribute('bulk_delete_library'); + $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', $library); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); - $person = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); - $this->getDatabase()->deleteDocuments('bulk_delete_library'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); - $this->assertCount(1, $this->getDatabase()->find('bulk_delete_person')); + $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', 'person1'); - $library = $person->getAttribute('bulk_delete_library'); + $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'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); - $this->getDatabase()->deleteDocuments('bulk_delete_library'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); + $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', - id: 'bulk_delete_library', + collection: 'bulk_delete_person_o2o', + id: 'bulk_delete_library_o2o', onDelete: Database::RELATION_MUTATE_CASCADE ); - $person1 = $this->getDatabase()->createDocument('bulk_delete_person', new Document([ + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ '$id' => 'person1', '$permissions' => [ Permission::read(Role::any()), @@ -15776,7 +15776,7 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void Permission::delete(Role::any()), ], 'name' => 'Person 1', - 'bulk_delete_library' => [ + 'bulk_delete_library_o2o' => [ '$id' => 'library1', '$permissions' => [ Permission::read(Role::any()), @@ -15788,27 +15788,27 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void ], ])); - $person1 = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); - $library = $person1->getAttribute('bulk_delete_library'); + $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', $library); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); - $person = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); + $person = $this->getDatabase()->getDocument('bulk_delete_person_o2o', 'person1'); - $this->getDatabase()->deleteDocuments('bulk_delete_library'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_library')); - $this->assertCount(1, $this->getDatabase()->find('bulk_delete_person')); + $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', 'person1'); - $library = $person->getAttribute('bulk_delete_library'); + $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'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); + $this->getDatabase()->deleteDocuments('bulk_delete_person_o2o'); + $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2o')); - $person1 = $this->getDatabase()->createDocument('bulk_delete_person', new Document([ + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_o2o', new Document([ '$id' => 'person1', '$permissions' => [ Permission::read(Role::any()), @@ -15816,7 +15816,7 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void Permission::delete(Role::any()), ], 'name' => 'Person 1', - 'bulk_delete_library' => [ + 'bulk_delete_library_o2o' => [ '$id' => 'library1', '$permissions' => [ Permission::read(Role::any()), @@ -15828,14 +15828,349 @@ public function testDeleteBulkDocumentsOneToOneRelationship(): void ], ])); - $person1 = $this->getDatabase()->getDocument('bulk_delete_person', 'person1'); - $library = $person1->getAttribute('bulk_delete_library'); + $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', $library); + $this->assertArrayNotHasKey('bulk_delete_person_o2o', $library); - $this->getDatabase()->deleteDocuments('bulk_delete_person'); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person')); - $this->assertCount(0, $this->getDatabase()->find('bulk_delete_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 testEvents(): void From c75b4bf3b105992d6e9bcd2c97a176a947fae65a Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Thu, 10 Oct 2024 11:17:31 +0900 Subject: [PATCH 16/23] Remove debug flag in compose file --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 5497f4e28..23b135ba9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,7 +5,7 @@ services: build: context: . args: - DEBUG: true + DEBUG: false networks: - database volumes: From 4121f7f62899cc0b4f36696108f87cf5adf28805 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Thu, 10 Oct 2024 11:21:22 +0900 Subject: [PATCH 17/23] Run Linter --- tests/e2e/Adapter/Base.php | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 4892b6318..82694ec7a 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15964,7 +15964,7 @@ public function testDeleteBulkDocumentsOneToManyRelationship(): void // NULL - Cleanup $this->getDatabase()->deleteDocuments('bulk_delete_person_o2m'); $this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_o2m')); - + // Cascade $this->getDatabase()->updateRelationship( @@ -16023,14 +16023,14 @@ public function testDeleteBulkDocumentsManyToManyRelationship(): void $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', @@ -16038,7 +16038,7 @@ public function testDeleteBulkDocumentsManyToManyRelationship(): void type: Database::RELATION_MANY_TO_MANY, onDelete: Database::RELATION_MUTATE_RESTRICT ); - + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_m2m', new Document([ '$id' => 'person1', '$permissions' => [ @@ -16070,11 +16070,11 @@ public function testDeleteBulkDocumentsManyToManyRelationship(): void ], ], ])); - + $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'); @@ -16082,30 +16082,30 @@ public function testDeleteBulkDocumentsManyToManyRelationship(): void } 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', @@ -16113,7 +16113,7 @@ public function testDeleteBulkDocumentsManyToOneRelationship(): void type: Database::RELATION_MANY_TO_ONE, onDelete: Database::RELATION_MUTATE_RESTRICT ); - + $person1 = $this->getDatabase()->createDocument('bulk_delete_person_m2o', new Document([ '$id' => 'person1', '$permissions' => [ @@ -16146,11 +16146,11 @@ public function testDeleteBulkDocumentsManyToOneRelationship(): void '$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'); @@ -16164,11 +16164,11 @@ public function testDeleteBulkDocumentsManyToOneRelationship(): void // 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')); } From 66e0736104adb4d3fb6d165a70a1ae7e2e471b0c Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 16 Oct 2024 15:41:28 +0900 Subject: [PATCH 18/23] Use new event for bulk delete --- src/Database/Database.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 417e5bea8..22d056d63 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'; @@ -4997,6 +4998,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $deleted = $this->withTransaction(function () use ($collection, $queries, $batchSize, $affectedDocumentIds) { $lastDocument = null; + while (true) { $affectedDocuments = $this->find($collection->getId(), array_merge( empty($lastDocument) ? [ @@ -5031,9 +5033,6 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $document = $this->silent(fn () => $this->deleteDocumentRelationships($collection, $document)); } - // Fire events - $this->trigger(self::EVENT_DOCUMENT_DELETE, $document); - $this->purgeRelatedDocuments($collection, $document->getId()); $this->purgeCachedDocument($collection->getId(), $document->getId()); } @@ -5045,6 +5044,8 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } } + $this->trigger(self::EVENT_DOCUMENTS_DELETE, $affectedDocumentIds); + // Mass delete using adapter with query return $this->adapter->deleteDocuments($collection->getId(), $affectedDocumentIds); }); From d34b44df06420a5594b246534c5ceb8b526a710e Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Fri, 18 Oct 2024 14:39:24 +0900 Subject: [PATCH 19/23] Address comments --- src/Database/Adapter.php | 3 ++- src/Database/Adapter/MariaDB.php | 13 +++++++------ src/Database/Adapter/Mongo.php | 7 ++++--- src/Database/Adapter/Postgres.php | 9 +++++---- src/Database/Adapter/SQL.php | 4 ++-- src/Database/Adapter/SQLite.php | 4 ++-- src/Database/Database.php | 31 +++++++++++++++---------------- tests/e2e/Adapter/Base.php | 15 ++++++++------- 8 files changed, 45 insertions(+), 41 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 349193da1..4e614ee27 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -640,10 +640,11 @@ abstract public function deleteDocuments(string $collection, array $ids): 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 diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 4788c67fc..fb7c5afe3 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1654,11 +1654,11 @@ public function deleteDocuments(string $collection, array $ids): bool $where[] = "_tenant = :_tenant"; } - $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ")"; + $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_DOCUMENT_DELETE, $sql); + $sql = $this->trigger(Database::EVENT_DOCUMENTS_DELETE, $sql); $stmt = $this->getPDO()->prepare($sql); @@ -1672,7 +1672,7 @@ public function deleteDocuments(string $collection, array $ids): bool $sql = " DELETE FROM {$this->getSQLTable($name . '_perms')} - WHERE _document IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ") + WHERE _document IN (" . \implode(', ', \array_map(fn ($index) => ":_id_{$index}", \array_keys($ids))) . ") "; if ($this->sharedTables) { @@ -1705,7 +1705,7 @@ public function deleteDocuments(string $collection, array $ids): bool return true; } - /** + /** * Find Documents * * @param string $collection @@ -1716,11 +1716,12 @@ public function deleteDocuments(string $collection, array $ids): 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 = $this->authorization->getRoles(); @@ -1810,7 +1811,7 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, } if ($this->authorization->getStatus()) { - $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 7841c51b0..924315275 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -966,7 +966,7 @@ public function updateAttribute(string $collection, string $id, string $type, in return true; } - /** + /** * Find Documents * * Find data sets using chosen queries @@ -979,12 +979,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); @@ -998,7 +999,7 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, // permissions if ($this->authorization->getStatus()) { // skip if authorization is disabled $roles = \implode('|', $this->authorization->getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; + $filters['_permissions']['$in'] = [new Regex("{$forPermission}\(\".*(?:{$roles}).*\"\)", 'i')]; } $options = []; diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 437c44914..0bfffd3d5 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1579,11 +1579,11 @@ public function deleteDocuments(string $collection, array $ids): bool $where[] = "_tenant = :_tenant"; } - $where[] = "_uid IN (" . \implode(', ', \array_map(fn ($id) => ":_id_{$id}", \array_keys($ids))) . ")"; + $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_DOCUMENT_DELETE, $sql); + $sql = $this->trigger(Database::EVENT_DOCUMENTS_DELETE, $sql); $stmt = $this->getPDO()->prepare($sql); @@ -1643,13 +1643,14 @@ public function deleteDocuments(string $collection, array $ids): 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 = $this->authorization->getRoles(); @@ -1737,7 +1738,7 @@ public function find(string $collection, array $queries = [], ?int $limit = 25, } if ($this->authorization->getStatus()) { - $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..58fdbc52b 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -968,7 +968,7 @@ 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 { $roles = array_map(fn (string $role) => $this->getPDO()->quote($role), $roles); @@ -981,7 +981,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 193f9b19c..74984a260 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -1175,14 +1175,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 22d056d63..2c5c6f7db 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -137,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; @@ -4986,7 +4987,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection * @throws DatabaseException * @throws RestrictedException */ - public function deleteDocuments(string $collection, array $queries = [], int $batchSize = 100): bool + public function deleteDocuments(string $collection, array $queries = [], int $batchSize = self::DELETE_BATCH_SIZE): bool { if ($this->adapter->getSharedTables() && empty($this->adapter->getTenant())) { throw new DatabaseException('Missing tenant. Tenant must be set when table sharing is enabled.'); @@ -4998,6 +4999,13 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $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( @@ -5008,7 +5016,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba Query::cursorAfter($lastDocument), ], $queries, - )); + ), forPermission: Database::PERMISSION_DELETE); if (empty($affectedDocuments)) { break; @@ -5017,17 +5025,6 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $affectedDocumentIds = array_merge($affectedDocumentIds, array_map(fn ($document) => $document->getId(), $affectedDocuments)); foreach ($affectedDocuments as $document) { - if ($collection->getId() !== self::METADATA) { - $documentSecurity = $collection->getAttribute('documentSecurity', false); - $isValid = $this->authorization->isValid(new Input(self::PERMISSION_DELETE, [ - ...$collection->getDelete(), - ...($documentSecurity ? $document->getDelete() : []) - ])); - if (!$isValid) { - throw new AuthorizationException($this->authorization->getDescription()); - } - } - // Delete Relationships if ($this->resolveRelationships) { $document = $this->silent(fn () => $this->deleteDocumentRelationships($collection, $document)); @@ -5097,13 +5094,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.'); @@ -5127,7 +5125,7 @@ public function find(string $collection, array $queries = []): array $documentSecurity = $collection->getAttribute('documentSecurity', false); - $skipAuth = $this->authorization->isValid(new Input(self::PERMISSION_READ, $collection->getRead())); + $skipAuth = $this->authorization->isValid(new Input($forPermission, $collection->getPermissionsByType($forPermission))); if (!$skipAuth && !$documentSecurity && $collection->getId() !== self::METADATA) { throw new AuthorizationException($this->authorization->getDescription()); @@ -5214,7 +5212,8 @@ public function find(string $collection, array $queries = []): array $orderAttributes, $orderTypes, $cursor, - $cursorDirection ?? Database::CURSOR_AFTER + $cursorDirection ?? Database::CURSOR_AFTER, + $forPermission ); $results = $skipAuth ? $this->authorization->skip($getResults) : $getResults(); diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 82694ec7a..68d024a64 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15617,18 +15617,19 @@ public function testDeleteBulkDocuments(): void static::getDatabase()->deleteDocuments('bulk_delete'); $this->assertEquals(0, count($this->getDatabase()->find('bulk_delete'))); - // TEST (FAIL): Bulk delete all documents with invalid document permissions - // Authorization::setRole(Role::any()->toString()); + // 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); - try { - static::getDatabase()->deleteDocuments('bulk_delete'); - $this->fail('Bulk deleted documents with invalid document permission'); - } catch (\Utopia\Database\Exception\Authorization) { - } + static::getDatabase()->deleteDocuments('bulk_delete'); + + $documents = static::$authorization->skip(function () { + return static::getDatabase()->find('bulk_delete'); + }); + + $this->assertCount(10, $documents); static::getDatabase()->updateCollection('bulk_delete', [ Permission::create(Role::any()), From afd0d9c5e6a0fa8e2715161f98ac7e5b13f72a75 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Fri, 18 Oct 2024 14:42:16 +0900 Subject: [PATCH 20/23] Run Linter --- src/Database/Adapter/MariaDB.php | 2 +- src/Database/Adapter/Mongo.php | 2 +- src/Database/Database.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index fb7c5afe3..d7067e600 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1705,7 +1705,7 @@ public function deleteDocuments(string $collection, array $ids): bool return true; } - /** + /** * Find Documents * * @param string $collection diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 924315275..e2882241c 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -966,7 +966,7 @@ public function updateAttribute(string $collection, string $id, string $type, in return true; } - /** + /** * Find Documents * * Find data sets using chosen queries diff --git a/src/Database/Database.php b/src/Database/Database.php index 2c5c6f7db..119d75c75 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4999,7 +4999,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba $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())); From a65da29e4f051288c0a6af52cd87485cf7c6f749 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 28 Oct 2024 11:58:21 +0900 Subject: [PATCH 21/23] Fix mongoDB attempting to delete an empty array of documents --- src/Database/Adapter/Mongo.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index e2882241c..374e6fcbd 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -920,6 +920,10 @@ public function deleteDocuments(string $collection, array $ids): bool { $name = $this->getNamespace() . '_' . $this->filter($collection); + if (empty($ids)) { + return true; + } + $filters = $this->buildFilters([new Query(Query::TYPE_EQUAL, '_uid', $ids)]); if ($this->sharedTables) { From b3cab3bac046c2b24c6254d517570fe4fa899f84 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 28 Oct 2024 13:55:08 +0900 Subject: [PATCH 22/23] Move empty affected document check to database.php --- src/Database/Adapter/MariaDB.php | 4 ---- src/Database/Adapter/Mongo.php | 4 ---- src/Database/Adapter/Postgres.php | 4 ---- src/Database/Database.php | 4 ++++ 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index d7067e600..3103d3b90 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1642,10 +1642,6 @@ public function deleteDocument(string $collection, string $id): bool */ public function deleteDocuments(string $collection, array $ids): bool { - if (empty($ids)) { - return true; - } - try { $name = $this->filter($collection); $where = []; diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 374e6fcbd..e2882241c 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -920,10 +920,6 @@ public function deleteDocuments(string $collection, array $ids): bool { $name = $this->getNamespace() . '_' . $this->filter($collection); - if (empty($ids)) { - return true; - } - $filters = $this->buildFilters([new Query(Query::TYPE_EQUAL, '_uid', $ids)]); if ($this->sharedTables) { diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 0bfffd3d5..068e80b8b 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1567,10 +1567,6 @@ public function deleteDocument(string $collection, string $id): bool */ public function deleteDocuments(string $collection, array $ids): bool { - if (empty($ids)) { - return true; - } - try { $name = $this->filter($collection); $where = []; diff --git a/src/Database/Database.php b/src/Database/Database.php index 119d75c75..4f87d8403 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5041,6 +5041,10 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } } + if (empty($affectedDocumentIds)) { + return false; + } + $this->trigger(self::EVENT_DOCUMENTS_DELETE, $affectedDocumentIds); // Mass delete using adapter with query From a1a5f536d5acc8309c0893188487b41d96864157 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Fri, 1 Nov 2024 16:20:39 +0900 Subject: [PATCH 23/23] Address comments --- src/Database/Adapter.php | 4 ++-- src/Database/Adapter/MariaDB.php | 6 +++--- src/Database/Adapter/Mongo.php | 8 ++++---- src/Database/Adapter/Postgres.php | 6 +++--- src/Database/Database.php | 6 +++--- tests/e2e/Adapter/Base.php | 13 +++++++++---- 6 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 4e614ee27..8807570ff 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -623,9 +623,9 @@ abstract public function deleteDocument(string $collection, string $id): bool; * @param string $collection * @param array $ids * - * @return bool + * @return int */ - abstract public function deleteDocuments(string $collection, array $ids): bool; + 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 3103d3b90..7616f8b8c 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1638,9 +1638,9 @@ public function deleteDocument(string $collection, string $id): bool * @param string $collection * @param array $ids * - * @return bool + * @return int */ - public function deleteDocuments(string $collection, array $ids): bool + public function deleteDocuments(string $collection, array $ids): int { try { $name = $this->filter($collection); @@ -1698,7 +1698,7 @@ public function deleteDocuments(string $collection, array $ids): bool throw new DatabaseException($e->getMessage(), $e->getCode(), $e); } - return true; + return $stmt->rowCount(); } /** diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index e2882241c..c88f6d2f6 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -914,9 +914,9 @@ public function deleteDocument(string $collection, string $id): bool * @param string $collection * @param array $ids * - * @return bool + * @return int */ - public function deleteDocuments(string $collection, array $ids): bool + public function deleteDocuments(string $collection, array $ids): int { $name = $this->getNamespace() . '_' . $this->filter($collection); @@ -932,7 +932,7 @@ public function deleteDocuments(string $collection, array $ids): bool $options = []; try { - $res = $this->client->delete( + $count = $this->client->delete( collection: $name, filters: $filters, options: $options, @@ -942,7 +942,7 @@ public function deleteDocuments(string $collection, array $ids): bool $this->processException($e); } - return true; + return $count ?? 0; } /** diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 068e80b8b..7b0e33868 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1563,9 +1563,9 @@ public function deleteDocument(string $collection, string $id): bool * @param string $collection * @param array $ids * - * @return bool + * @return int */ - public function deleteDocuments(string $collection, array $ids): bool + public function deleteDocuments(string $collection, array $ids): int { try { $name = $this->filter($collection); @@ -1623,7 +1623,7 @@ public function deleteDocuments(string $collection, array $ids): bool throw new DatabaseException($e->getMessage(), $e->getCode(), $e); } - return true; + return $stmt->rowCount(); } /** diff --git a/src/Database/Database.php b/src/Database/Database.php index 4f87d8403..508c56b3f 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4981,13 +4981,13 @@ private function deleteCascade(Document $collection, Document $relatedCollection * @param array $queries * @param int $batchSize * - * @return bool + * @return int * * @throws AuthorizationException * @throws DatabaseException * @throws RestrictedException */ - public function deleteDocuments(string $collection, array $queries = [], int $batchSize = self::DELETE_BATCH_SIZE): bool + 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.'); @@ -5042,7 +5042,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } if (empty($affectedDocumentIds)) { - return false; + return 0; } $this->trigger(self::EVENT_DOCUMENTS_DELETE, $affectedDocumentIds); diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 68d024a64..94f610dab 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15586,7 +15586,8 @@ public function testDeleteBulkDocuments(): void $this->assertCount(10, $docs); // TEST: Bulk Delete All Documents - static::getDatabase()->deleteDocuments('bulk_delete'); + $deleted = static::getDatabase()->deleteDocuments('bulk_delete'); + $this->assertEquals(10, $deleted); $docs = static::getDatabase()->find('bulk_delete'); $this->assertCount(0, $docs); @@ -15594,9 +15595,10 @@ public function testDeleteBulkDocuments(): void // TEST: Bulk delete documents with queries. $this->propegateBulkDocuments(); - static::getDatabase()->deleteDocuments('bulk_delete', [ + $deleted = static::getDatabase()->deleteDocuments('bulk_delete', [ Query::greaterThanEqual('integer', 5) ]); + $this->assertEquals(5, $deleted); $docs = static::getDatabase()->find('bulk_delete'); $this->assertCount(5, $docs); @@ -15614,7 +15616,9 @@ public function testDeleteBulkDocuments(): void Permission::read(Role::any()), Permission::delete(Role::any()) ], false); - static::getDatabase()->deleteDocuments('bulk_delete'); + $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 @@ -15623,7 +15627,8 @@ public function testDeleteBulkDocuments(): void ], true); $this->propegateBulkDocuments(true); - static::getDatabase()->deleteDocuments('bulk_delete'); + $deleted = static::getDatabase()->deleteDocuments('bulk_delete'); + $this->assertEquals(0, $deleted); $documents = static::$authorization->skip(function () { return static::getDatabase()->find('bulk_delete');