diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index b651e33ca1..32ba866115 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3265,7 +3265,6 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') } $isBulk = true; - if (!empty($data)) { // Single document provided, convert to single item array // But remember that it was single to respond with a single document @@ -3307,8 +3306,15 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') Database::PERMISSION_DELETE, ]; + // If bulk, we need to validate permissions explicitly per document if ($isBulk) { $permissions = $document['$permissions'] ?? null; + if (!empty($permissions)) { + $validator = new Permissions(); + if (!$validator->isValid($permissions)) { + throw new Exception(Exception::GENERAL_BAD_REQUEST, $validator->getDescription()); + } + } } $permissions = Permission::aggregate($permissions, $allowedPermissions); @@ -3434,17 +3440,19 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') $document['$collection'] = $collection->getId(); // Determine the source ID depending on whether it's a bulk operation. - $sourceId = $isBulk ? $document['$id'] : $documentId; + $sourceId = $isBulk + ? ($document['$id'] ?? ID::unique()) + : $documentId; - // For bulk operations, ensure $id is provided. - if ($isBulk && empty($sourceId)) { - throw new Exception( - Exception::DOCUMENT_INVALID_STRUCTURE, - '$id must be set in each document when creating bulk documents' - ); + // If bulk, we need to validate ID explicitly + if ($isBulk) { + $validator = new CustomId(); + if (!$validator->isValid($sourceId)) { + throw new Exception(Exception::GENERAL_BAD_REQUEST, $validator->getDescription()); + } } - // Assign a unique id if needed, otherwise use the provided id. + // Assign a unique ID if needed, otherwise use the provided ID. $document['$id'] = $sourceId === 'unique()' ? ID::unique() : $sourceId; $document = new Document($document); $setPermissions($document, $permissions); @@ -4254,17 +4262,18 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents') throw new Exception(Exception::GENERAL_QUERY_INVALID, $e->getMessage()); } - if (!\is_null($permissions)) { - $data['$permissions'] = $permissions; + if ($data['$permissions']) { + $validator = new Permissions(); + if (!$validator->isValid($data['$permissions'])) { + throw new Exception(Exception::GENERAL_BAD_REQUEST, $validator->getDescription()); + } } - $partialDocument = new Document($data); - $documents = []; $modified = $dbForProject->updateDocuments( 'database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), - $partialDocument, + new Document($data), $queries, onNext: function (Document $document) use ($plan, &$documents) { if (\count($documents) < ($plan['databasesBatchSize'] ?? APP_LIMIT_DATABASE_BATCH)) { diff --git a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php index d327b669da..a27815f4ed 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomClientTest.php @@ -872,7 +872,6 @@ class DatabasesCustomClientTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); - // We should be allowed to link and create a new document from collection 3 to collection 2. $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $collection1['body']['$id'] . '/documents/' . $collection1['body']['$id'], array_merge([ 'content-type' => 'application/json', diff --git a/tests/e2e/Services/Databases/DatabasesCustomServerTest.php b/tests/e2e/Services/Databases/DatabasesCustomServerTest.php index 4cb0c84efd..829960b54f 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomServerTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomServerTest.php @@ -4199,6 +4199,20 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); $this->assertCount(3, $response['body']['documents']); + // TEST SUCCESS - $id is auto-assigned if not included in bulk documents + $response = $this->client->call(Client::METHOD_POST, "/databases/{$databaseId}/collections/{$data['$id']}/documents", array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'documents' => [ + [ + 'number' => 1, + ] + ], + ]); + + $this->assertEquals(201, $response['headers']['status-code']); + // TEST FAIL - Can't use data and document together $response = $this->client->call(Client::METHOD_POST, "/databases/{$databaseId}/collections/{$data['$id']}/documents", array_merge([ 'content-type' => 'application/json', @@ -4233,13 +4247,14 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); - // TEST FAIL - Can't miss $id in bulk documents + // TEST FAIL - Can't include invalid ID in bulk documents $response = $this->client->call(Client::METHOD_POST, "/databases/{$databaseId}/collections/{$data['$id']}/documents", array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'documents' => [ [ + '$id' => '$invalid', 'number' => 1, ] ], @@ -4278,13 +4293,38 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); + // TEST FAIL - Can't include invalid permissions in nested documents + $response = $this->client->call(Client::METHOD_POST, "/databases/{$databaseId}/collections/{$data['$id']}/documents", array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'documents' => [ + [ + '$id' => ID::unique(), + '$permissions' => ['invalid'], + 'number' => 1, + ], + ], + ]); + // TEST FAIL - Can't bulk create in a collection with relationships - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ + $collection2 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => 'Bulk Related', + 'documentSecurity' => true, + 'permissions' => [], + ]); + + $response = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ], $this->getHeaders()), [ - 'relatedCollectionId' => ID::unique(), + 'relatedCollectionId' => $collection2['body']['$id'], 'type' => 'manyToOne', 'twoWay' => true, 'onDelete' => 'cascade', @@ -4309,7 +4349,7 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); } - public function testBulkUpdates(): void + public function testBulkUpdate(): void { // Create database $database = $this->client->call(Client::METHOD_POST, '/databases', [ @@ -4540,13 +4580,23 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(10, $documents['body']['total']); // TEST: Fail - Can't bulk update in a collection with relationships + $collection2 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => 'Bulk Related', + 'documentSecurity' => true, + 'permissions' => [], + ]); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ + $response = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ], $this->getHeaders()), [ - 'relatedCollectionId' => ID::unique(), + 'relatedCollectionId' => $collection2['body']['$id'], 'type' => 'manyToOne', 'twoWay' => true, 'onDelete' => 'cascade', @@ -4573,7 +4623,7 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); } - public function testBulkUpserts(): void + public function testBulkUpsert(): void { // Create database $database = $this->client->call(Client::METHOD_POST, '/databases', [ @@ -4723,13 +4773,23 @@ class DatabasesCustomServerTest extends Scope ], $response['body']['documents'][1]['$permissions']); // TEST: Fail - Can't bulk upsert in a collection with relationships + $collection2 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => 'Bulk Related', + 'documentSecurity' => true, + 'permissions' => [], + ]); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ + $response = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ], $this->getHeaders()), [ - 'relatedCollectionId' => ID::unique(), + 'relatedCollectionId' => $collection2['body']['$id'], 'type' => 'manyToOne', 'twoWay' => true, 'onDelete' => 'cascade', @@ -4756,7 +4816,7 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); } - public function testBulkDeletes(): void + public function testBulkDelete(): void { // Create database $database = $this->client->call(Client::METHOD_POST, '/databases', [ @@ -5054,13 +5114,23 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(0, $documents['body']['total']); // TEST: Fail - Can't bulk delete in a collection with relationships + $collection2 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => 'Bulk Related', + 'documentSecurity' => true, + 'permissions' => [], + ]); - $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ + $response = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['$id'] . '/attributes/relationship', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] ], $this->getHeaders()), [ - 'relatedCollectionId' => ID::unique(), + 'relatedCollectionId' => $collection2['body']['$id'], 'type' => 'manyToOne', 'twoWay' => true, 'onDelete' => 'cascade',