From c7317b04683df8c3583514415f989755c2a3f35d Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 13 Apr 2023 15:59:57 +1200 Subject: [PATCH 1/4] Fix auto-setting custom ID on nested documents --- app/controllers/api/databases.php | 48 ++++++++++++++++--- .../e2e/Services/Databases/DatabasesBase.php | 22 ++++++++- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index b309b74894..4e3661e9ac 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -2743,8 +2743,11 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') if (empty($related)) { continue; } - if (!\is_array($related)) { - $related = [$related]; + + if (\is_array($related) && \array_values($related) === $related) { + $relations = $related; + } else { + $relations = [$related]; } $relatedCollectionId = $relationship->getAttribute('relatedCollection'); @@ -2752,7 +2755,15 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') fn() => $dbForProject->getDocument('database_' . $database->getInternalId(), $relatedCollectionId) ); - foreach ($related as $relation) { + foreach ($relations as &$relation) { + if ( + \is_array($related) + && \array_values($related) !== $related + && !isset($relation['$id']) + ) { + $relation['$id'] = ID::unique(); + $relation = new Document($relation); + } if ($relation instanceof Document) { $current = Authorization::skip( fn() => $dbForProject->getDocument('database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId(), $relation->getId()) @@ -2761,7 +2772,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') if ($current->isEmpty()) { $type = Database::PERMISSION_CREATE; - if (!isset($relation['$id']) || $relation['$id'] === 'unique()') { + if (isset($relation['$id']) && $relation['$id'] === 'unique()') { $relation['$id'] = ID::unique(); } } else { @@ -2774,6 +2785,12 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') $checkPermissions($relatedCollection, $relation, $type); } } + + if (\is_array($related) && \array_values($related) === $related) { + $document->setAttribute($relationship->getAttribute('key'), \array_values($relations)); + } else { + $document->setAttribute($relationship->getAttribute('key'), \reset($relations)); + } } }; @@ -3321,8 +3338,11 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum if (empty($related)) { continue; } - if (!\is_array($related)) { - $related = [$related]; + + if (\is_array($related) && \array_values($related) === $related) { + $relations = $related; + } else { + $relations = [$related]; } $relatedCollectionId = $relationship->getAttribute('relatedCollection'); @@ -3331,6 +3351,14 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum ); foreach ($related as $relation) { + if ( + \is_array($relation) + && \array_values($relation) !== $relation + && !isset($relation['$id']) + ) { + $relation['$id'] = ID::unique(); + $relation = new Document($relation); + } if ($relation instanceof Document) { $oldDocument = Authorization::skip(fn() => $dbForProject->getDocument( 'database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId(), @@ -3340,7 +3368,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum if ($oldDocument->isEmpty()) { $type = Database::PERMISSION_CREATE; - if (!isset($relation['$id']) || $relation['$id'] === 'unique()') { + if (isset($relation['$id']) && $relation['$id'] === 'unique()') { $relation['$id'] = ID::unique(); } } else { @@ -3353,6 +3381,12 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum $checkPermissions($relatedCollection, $relation, $oldDocument, $type); } } + + if (\is_array($related) && \array_values($related) === $related) { + $document->setAttribute($relationship->getAttribute('key'), \array_values($relations)); + } else { + $document->setAttribute($relationship->getAttribute('key'), \reset($relations)); + } } }; diff --git a/tests/e2e/Services/Databases/DatabasesBase.php b/tests/e2e/Services/Databases/DatabasesBase.php index 42f83ea046..a2cb48cf93 100644 --- a/tests/e2e/Services/Databases/DatabasesBase.php +++ b/tests/e2e/Services/Databases/DatabasesBase.php @@ -3339,6 +3339,26 @@ trait DatabasesBase $this->assertEquals('Library 1', $person1['body']['library']['libraryName']); + // Create without nested ID + $person2 = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $person['body']['$id'] . '/documents', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'documentId' => ID::unique(), + 'data' => [ + 'library' => [ + 'libraryName' => 'Library 2', + ], + ], + 'permissions' => [ + Permission::read(Role::user($this->getUser()['$id'])), + Permission::update(Role::user($this->getUser()['$id'])), + Permission::delete(Role::user($this->getUser()['$id'])), + ] + ]); + + $this->assertEquals('Library 2', $person2['body']['library']['libraryName']); + // Ensure IDs were set and internal IDs removed $this->assertEquals($databaseId, $person1['body']['$databaseId']); $this->assertEquals($databaseId, $person1['body']['library']['$databaseId']); @@ -3901,7 +3921,7 @@ trait DatabasesBase ]); $this->assertEquals(200, $response['headers']['status-code']); - $this->assertEquals(1, count($response['body']['documents'])); + $this->assertEquals(2, count($response['body']['documents'])); $this->assertEquals(null, $response['body']['documents'][0]['fullName']); $this->assertArrayNotHasKey("libraries", $response['body']['documents'][0]); } From 975b044de08ff5653f89b63f140d29cf7c94e092 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 13 Apr 2023 16:08:53 +1200 Subject: [PATCH 2/4] Iterate relation by reference on update --- app/controllers/api/databases.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 4e3661e9ac..0fbb74f564 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3350,7 +3350,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum fn() => $dbForProject->getDocument('database_' . $database->getInternalId(), $relatedCollectionId) ); - foreach ($related as $relation) { + foreach ($related as &$relation) { if ( \is_array($relation) && \array_values($relation) !== $relation From 08be9c4c890635d60561c9ec8dd99e200edda670 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 13 Apr 2023 16:35:42 +1200 Subject: [PATCH 3/4] Update changelog --- CHANGES.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 21501b1bd5..5ab6e586cf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,8 @@ +# Version 1.3.2 + +## Bugs +- Fixed auto-setting custom ID on nested documents [#5363](https://github.com/appwrite/appwrite/pull/5363) + # Version 1.3.1 ## Bugs From 0dea80c9dac6982f4dd22390d0a42dc356e1a91c Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 14 Apr 2023 22:03:16 +1200 Subject: [PATCH 4/4] Cache whether relation is a list --- app/controllers/api/databases.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 0fbb74f564..5146c945ea 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -2744,7 +2744,9 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') continue; } - if (\is_array($related) && \array_values($related) === $related) { + $isList = \is_array($related) && \array_values($related) === $related; + + if ($isList) { $relations = $related; } else { $relations = [$related]; @@ -2786,7 +2788,7 @@ App::post('/v1/databases/:databaseId/collections/:collectionId/documents') } } - if (\is_array($related) && \array_values($related) === $related) { + if ($isList) { $document->setAttribute($relationship->getAttribute('key'), \array_values($relations)); } else { $document->setAttribute($relationship->getAttribute('key'), \reset($relations)); @@ -3339,7 +3341,9 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum continue; } - if (\is_array($related) && \array_values($related) === $related) { + $isList = \is_array($related) && \array_values($related) === $related; + + if ($isList) { $relations = $related; } else { $relations = [$related]; @@ -3382,7 +3386,7 @@ App::patch('/v1/databases/:databaseId/collections/:collectionId/documents/:docum } } - if (\is_array($related) && \array_values($related) === $related) { + if ($isList) { $document->setAttribute($relationship->getAttribute('key'), \array_values($relations)); } else { $document->setAttribute($relationship->getAttribute('key'), \reset($relations));