From 89b111cff1779e3015bcca586dd86a4a730cc8f7 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 1 Aug 2023 01:40:45 +0530 Subject: [PATCH 1/6] fixes comparison check and permission issues in relationships Skipping authorization for the db calls that are only made for updating document logic but not required by user. Fixes the bug in comparing new document and old document in updateDocument method. Adds a test case to verify. --- src/Database/Database.php | 98 ++++++++++++++++++++++++--------------- tests/Database/Base.php | 67 +++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 38 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 038b47e44..d1b1a10a0 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2752,7 +2752,7 @@ private function relateDocuments( } // Try to get the related document - $related = $this->getDocument($relatedCollection->getId(), $relation->getId()); + $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $relation->getId())); if ($related->isEmpty()) { // If the related document doesn't exist, create it, inheriting permissions if none are set @@ -2773,7 +2773,7 @@ private function relateDocuments( if ($relationType === Database::RELATION_MANY_TO_MANY) { $junction = $this->getJunctionCollection($collection, $relatedCollection, $side); - $this->createDocument($junction, new Document([ + Authorization::skip(fn () => $this->createDocument($junction, new Document([ $key => $related->getId(), $twoWayKey => $document->getId(), '$permissions' => [ @@ -2781,7 +2781,7 @@ private function relateDocuments( Permission::update(Role::any()), Permission::delete(Role::any()), ] - ])); + ]))); } return $related->getId(); @@ -2882,24 +2882,48 @@ public function updateDocument(string $collection, string $id, Document $documen $collection = $this->silent(fn () => $this->getCollection($collection)); + $relationships = \array_filter($collection->getAttribute('attributes', []), function ($attribute) { + return $attribute['type'] === Database::VAR_RELATIONSHIP; + }); + $validator = new Authorization(self::PERMISSION_UPDATE); $shouldUpdate = false; if ($collection->getId() !== self::METADATA) { $documentSecurity = $collection->getAttribute('documentSecurity', false); - // Compare if the document has any changes - foreach ($document as $key=>$value) { - // Skip the nested documents as they will be checked later in recursions. - $oldAttributeValue = $old->getAttribute($key); - if ($oldAttributeValue instanceof Document) { - continue; - } - if ($oldAttributeValue !== $value) { - $shouldUpdate = true; - break; + $compareChanges = (function (array | Document $document, array | Document $old) use (&$compareChanges, &$relationships): bool { + $isDifferent = false; + // Compare if the document has any changes + foreach ($document as $key=>$value) { + $isRelationKey = false; + // Check if key is relationship key, no need to compare relationships + foreach ($relationships as $relationship) { + if ($key === $relationship->getAttribute('key')) { + $isRelationKey = true; + break; + } + } + if ($isRelationKey) { + continue; + } + $oldAttributeValue = ($old instanceof Document) ? $old->getAttribute($key) : $old[$key] ?? null; + // Skip the nested documents as they will be checked later in recursions. + if ($oldAttributeValue instanceof Document) { + continue; + } elseif (\is_array($oldAttributeValue) && \is_array($value) && !($value instanceof Document)) { + if ($compareChanges($value, $oldAttributeValue)) { + $isDifferent = true; + } + } elseif ($oldAttributeValue !== $value) { + $isDifferent = true; + } } - } + return $isDifferent; + }); + + $shouldUpdate = $compareChanges($document, $old); + if ($shouldUpdate && !$validator->isValid([ ...$collection->getUpdate(), ...($documentSecurity ? $old->getUpdate() : []) @@ -2996,7 +3020,7 @@ private function updateDocumentRelationships(Document $collection, Document $old case Database::RELATION_ONE_TO_ONE: if (!$twoWay) { if (\is_string($value)) { - $related = $this->getDocument($relatedCollection->getId(), $value); + $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $value)); if ($related->isEmpty()) { // If no such document exists in related collection // For one-one we need to update the related key to old value, either null if no relation exists or old related document id @@ -3021,13 +3045,13 @@ private function updateDocumentRelationships(Document $collection, Document $old switch (\gettype($value)) { case 'string': - $related = $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value)); + $related = Authorization::skip(fn () => $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value))); if ( $oldValue?->getId() !== $value - && $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ + && Authorization::skip(fn () => $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ Query::equal($twoWayKey, [$value]), - ])) + ]))) ) { // Have to do this here because otherwise relations would be updated before the database can throw the unique violation throw new DuplicateException('Document already has a related document'); @@ -3041,13 +3065,13 @@ private function updateDocumentRelationships(Document $collection, Document $old break; case 'object': if ($value instanceof Document) { - $related = $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value->getId())); + $related = Authorization::skip(fn () => $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value->getId()))); if ( $oldValue?->getId() !== $value->getId() - && $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ + && Authorization::skip(fn () => $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ Query::equal($twoWayKey, [$value->getId()]), - ])) + ]))) ) { // Have to do this here because otherwise relations would be updated before the database can throw the unique violation throw new DuplicateException('Document already has a related document'); @@ -3077,10 +3101,10 @@ private function updateDocumentRelationships(Document $collection, Document $old // no break case 'NULL': if (!\is_null($oldValue?->getId())) { - $oldRelated = $this->skipRelationships( + $oldRelated = Authorization::skip(fn () => $this->skipRelationships( fn () => $this->getDocument($relatedCollection->getId(), $oldValue->getId()) - ); + )); $this->skipRelationships(fn () => $this->updateDocument( $relatedCollection->getId(), $oldRelated->getId(), @@ -3121,24 +3145,24 @@ private function updateDocumentRelationships(Document $collection, Document $old $removedDocuments = \array_diff($oldIds, $newIds); foreach ($removedDocuments as $relation) { - $relation = $this->skipRelationships(fn () => $this->getDocument( + $relation = Authorization::skip(fn () => $this->skipRelationships(fn () => $this->getDocument( $relatedCollection->getId(), $relation - )); + ))); - $this->skipRelationships(fn () => $this->updateDocument( + Authorization::skip(fn () => $this->skipRelationships(fn () => $this->updateDocument( $relatedCollection->getId(), $relation->getId(), $relation->setAttribute($twoWayKey, null) - )); + ))); } foreach ($value as $relation) { if (\is_string($relation)) { - $related = $this->skipRelationships( + $related = Authorization::skip(fn () => $this->skipRelationships( fn () => $this->getDocument($relatedCollection->getId(), $relation) - ); + )); if ($related->isEmpty()) { continue; @@ -3150,7 +3174,7 @@ private function updateDocumentRelationships(Document $collection, Document $old $related->setAttribute($twoWayKey, $document->getId()) )); } elseif ($relation instanceof Document) { - $related = $this->getDocument($relatedCollection->getId(), $relation->getId()); + $related = Authorization::skip(fn () =>$this->getDocument($relatedCollection->getId(), $relation->getId())); if ($related->isEmpty()) { if (!isset($value['$permissions'])) { @@ -3177,7 +3201,7 @@ private function updateDocumentRelationships(Document $collection, Document $old } if (\is_string($value)) { - $related = $this->getDocument($relatedCollection->getId(), $value); + $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $value)); if ($related->isEmpty()) { //If no such document exists in related collection //For one-one we need to update the related key to old value, either null if no relation exists or old related document id @@ -3185,7 +3209,7 @@ private function updateDocumentRelationships(Document $collection, Document $old } $this->deleteCachedDocument($relatedCollection->getId(), $value); } elseif ($value instanceof Document) { - $related = $this->getDocument($relatedCollection->getId(), $value->getId()); + $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $value->getId())); if ($related->isEmpty()) { if (!isset($value['$permissions'])) { @@ -3237,24 +3261,24 @@ private function updateDocumentRelationships(Document $collection, Document $old foreach ($removedDocuments as $relation) { $junction = $this->getJunctionCollection($collection, $relatedCollection, $side); - $junctions = $this->find($junction, [ + $junctions = Authorization::skip(fn () => $this->find($junction, [ Query::equal($key, [$relation]), Query::equal($twoWayKey, [$document->getId()]), Query::limit(PHP_INT_MAX) - ]); + ])); foreach ($junctions as $junction) { - $this->deleteDocument($junction->getCollection(), $junction->getId()); + Authorization::skip(fn () => $this->deleteDocument($junction->getCollection(), $junction->getId())); } } foreach ($value as $relation) { if (\is_string($relation)) { - if (\in_array($relation, $oldIds) || $this->getDocument($relatedCollection->getId(), $relation)->isEmpty()) { + if (\in_array($relation, $oldIds) || Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $relation)->isEmpty())) { continue; } } elseif ($relation instanceof Document) { - $related = $this->getDocument($relatedCollection->getId(), $relation->getId()); + $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $relation->getId())); if ($related->isEmpty()) { if (!isset($value['$permissions'])) { diff --git a/tests/Database/Base.php b/tests/Database/Base.php index 85599ceec..f751a7d5f 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -11315,7 +11315,7 @@ public function testCollectionPermissionsRelationshipsUpdateThrowsException(arra $document = static::getDatabase()->updateDocument( $collection->getId(), $document->getId(), - $document->setAttribute('test', 'ipsum') + $document->setAttribute('test', $document->getAttribute('test').'value') ); } @@ -11354,6 +11354,71 @@ public function testCollectionPermissionsRelationshipsDeleteWorks(array $data): )); } + public function testCreateRelationDocumentWithoutUpdatePermission(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + Authorization::cleanRoles(); + Authorization::setRole(Role::user('a')->toString()); + + static::getDatabase()->createCollection('parentRelationTest', [], [], [ + Permission::read(Role::user('a')), + Permission::create(Role::user('a')), + Permission::update(Role::user('a')), + Permission::delete(Role::user('a')) + ]); + static::getDatabase()->createCollection('childRelationTest', [], [], [ + Permission::create(Role::user('a')), + Permission::read(Role::user('a')), + ]); + static::getDatabase()->createAttribute('parentRelationTest', 'name', Database::VAR_STRING, 255, false); + static::getDatabase()->createAttribute('childRelationTest', 'name', Database::VAR_STRING, 255, false); + + static::getDatabase()->createRelationship( + collection: 'parentRelationTest', + relatedCollection: 'childRelationTest', + type: Database::RELATION_ONE_TO_MANY, + id: 'childs' + ); + + // Create document with relationship with nested data + $parent = static::getDatabase()->createDocument('parentRelationTest', new Document([ + '$id' => 'parent1', + 'name' => 'Parent 1', + 'childs' => [ + [ + '$id' => 'child1', + 'name' => 'Child 1', + ], + ], + ])); + $this->assertEquals(1, \count($parent['childs'])); + $updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', new Document([ + '$id' => 'parent1', + 'name'=>'Parent 1', + '$collection' => 'parentRelationTest', + 'childs' => [ + new Document([ + '$id' => 'child1', + '$collection' => 'childRelationTest' + ]), + new Document([ + '$id' => 'child2', + 'name' => 'Child 2', + '$collection' => 'childRelationTest' + ]), + ] + ])); + + $this->assertEquals(2, \count($updatedParent['childs'])); + + static::getDatabase()->deleteCollection('parentRelationTest'); + static::getDatabase()->deleteCollection('childRelationTest'); + } + public function testLabels(): void { $this->assertInstanceOf('Utopia\Database\Document', static::getDatabase()->createCollection( From adb879a476750768d9288f1eb76c564e0676f21a Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 1 Aug 2023 01:50:13 +0530 Subject: [PATCH 2/6] removes unnecessary condition checking --- src/Database/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index d1b1a10a0..5025121a3 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2911,7 +2911,7 @@ public function updateDocument(string $collection, string $id, Document $documen // Skip the nested documents as they will be checked later in recursions. if ($oldAttributeValue instanceof Document) { continue; - } elseif (\is_array($oldAttributeValue) && \is_array($value) && !($value instanceof Document)) { + } elseif (\is_array($oldAttributeValue) && \is_array($value)) { if ($compareChanges($value, $oldAttributeValue)) { $isDifferent = true; } From 2a9e76f1d8512130b2b0b81afb9dd9cc2747761a Mon Sep 17 00:00:00 2001 From: prateek banga Date: Wed, 2 Aug 2023 12:06:19 +0530 Subject: [PATCH 3/6] removes unnecessary code --- src/Database/Database.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 753ed7812..a3c0ea313 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2885,10 +2885,6 @@ public function updateDocument(string $collection, string $id, Document $documen return $attribute['type'] === Database::VAR_RELATIONSHIP; }); - $relationships = \array_filter($collection->getAttribute('attributes', []), function ($attribute) { - return $attribute['type'] === Database::VAR_RELATIONSHIP; - }); - $validator = new Authorization(self::PERMISSION_UPDATE); $shouldUpdate = false; From ab0eee8e1ea76c8c53335640e77ded2e64f9ddfb Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 18 Aug 2023 21:21:57 +0530 Subject: [PATCH 4/6] removes skip auth from get relation documents --- src/Database/Database.php | 47 ++++++++++++++++++++------------------- tests/Database/Base.php | 36 +++++++++++++----------------- 2 files changed, 40 insertions(+), 43 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 972ddcb08..ed560f824 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2793,7 +2793,7 @@ private function relateDocuments( } // Try to get the related document - $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $relation->getId())); + $related = $this->getDocument($relatedCollection->getId(), $relation->getId()); if ($related->isEmpty()) { // If the related document doesn't exist, create it, inheriting permissions if none are set @@ -2814,7 +2814,7 @@ private function relateDocuments( if ($relationType === Database::RELATION_MANY_TO_MANY) { $junction = $this->getJunctionCollection($collection, $relatedCollection, $side); - Authorization::skip(fn () => $this->createDocument($junction, new Document([ + $this->createDocument($junction, new Document([ $key => $related->getId(), $twoWayKey => $document->getId(), '$permissions' => [ @@ -2822,7 +2822,7 @@ private function relateDocuments( Permission::update(Role::any()), Permission::delete(Role::any()), ] - ]))); + ])); } return $related->getId(); @@ -3115,7 +3115,7 @@ private function updateDocumentRelationships(Document $collection, Document $old case Database::RELATION_ONE_TO_ONE: if (!$twoWay) { if (\is_string($value)) { - $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $value)); + $related = $this->getDocument($relatedCollection->getId(), $value); if ($related->isEmpty()) { // If no such document exists in related collection // For one-one we need to update the related key to null if no relation exists @@ -3140,7 +3140,7 @@ private function updateDocumentRelationships(Document $collection, Document $old switch (\gettype($value)) { case 'string': - $related = Authorization::skip(fn () => $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value))); + $related = $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value)); if ($related->isEmpty()) { // If no such document exists in related collection // For one-one we need to update the related key to null if no relation exists @@ -3149,9 +3149,9 @@ private function updateDocumentRelationships(Document $collection, Document $old } if ( $oldValue?->getId() !== $value - && Authorization::skip(fn () => $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ + && $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ Query::equal($twoWayKey, [$value]), - ]))) + ])) ) { // Have to do this here because otherwise relations would be updated before the database can throw the unique violation throw new DuplicateException('Document already has a related document'); @@ -3165,13 +3165,13 @@ private function updateDocumentRelationships(Document $collection, Document $old break; case 'object': if ($value instanceof Document) { - $related = Authorization::skip(fn () => $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value->getId()))); + $related = $this->skipRelationships(fn () => $this->getDocument($relatedCollection->getId(), $value->getId())); if ( $oldValue?->getId() !== $value->getId() - && Authorization::skip(fn () => $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ + && $this->skipRelationships(fn () => $this->findOne($relatedCollection->getId(), [ Query::equal($twoWayKey, [$value->getId()]), - ]))) + ])) ) { // Have to do this here because otherwise relations would be updated before the database can throw the unique violation throw new DuplicateException('Document already has a related document'); @@ -3201,10 +3201,10 @@ private function updateDocumentRelationships(Document $collection, Document $old // no break case 'NULL': if (!\is_null($oldValue?->getId())) { - $oldRelated = Authorization::skip(fn () => $this->skipRelationships( + $oldRelated = $this->skipRelationships( fn () => $this->getDocument($relatedCollection->getId(), $oldValue->getId()) - )); + ); $this->skipRelationships(fn () => $this->updateDocument( $relatedCollection->getId(), $oldRelated->getId(), @@ -3259,10 +3259,10 @@ private function updateDocumentRelationships(Document $collection, Document $old foreach ($value as $relation) { if (\is_string($relation)) { - $related = Authorization::skip(fn () => $this->skipRelationships( + $related = $this->skipRelationships( fn () => $this->getDocument($relatedCollection->getId(), $relation) - )); + ); if ($related->isEmpty()) { continue; @@ -3274,7 +3274,7 @@ private function updateDocumentRelationships(Document $collection, Document $old $related->setAttribute($twoWayKey, $document->getId()) )); } elseif ($relation instanceof Document) { - $related = Authorization::skip(fn () =>$this->getDocument($relatedCollection->getId(), $relation->getId())); + $related = $this->getDocument($relatedCollection->getId(), $relation->getId()); if ($related->isEmpty()) { if (!isset($value['$permissions'])) { @@ -3285,11 +3285,12 @@ private function updateDocumentRelationships(Document $collection, Document $old $relation->setAttribute($twoWayKey, $document->getId()) ); } else { - $this->updateDocument( + // We need to skip the relationship when updating document so that comparison check does not fail + $this->skipRelationships(fn () => $this->updateDocument( $relatedCollection->getId(), $related->getId(), $relation->setAttribute($twoWayKey, $document->getId()) - ); + )); } } else { throw new DatabaseException('Invalid value for relationship'); @@ -3301,7 +3302,7 @@ private function updateDocumentRelationships(Document $collection, Document $old } if (\is_string($value)) { - $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $value)); + $related = $this->getDocument($relatedCollection->getId(), $value); if ($related->isEmpty()) { // If no such document exists in related collection // For many-one we need to update the related key to null if no relation exists @@ -3309,7 +3310,7 @@ private function updateDocumentRelationships(Document $collection, Document $old } $this->deleteCachedDocument($relatedCollection->getId(), $value); } elseif ($value instanceof Document) { - $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $value->getId())); + $related = $this->getDocument($relatedCollection->getId(), $value->getId()); if ($related->isEmpty()) { if (!isset($value['$permissions'])) { @@ -3361,11 +3362,11 @@ private function updateDocumentRelationships(Document $collection, Document $old foreach ($removedDocuments as $relation) { $junction = $this->getJunctionCollection($collection, $relatedCollection, $side); - $junctions = Authorization::skip(fn () => $this->find($junction, [ + $junctions = $this->find($junction, [ Query::equal($key, [$relation]), Query::equal($twoWayKey, [$document->getId()]), Query::limit(PHP_INT_MAX) - ])); + ]); foreach ($junctions as $junction) { Authorization::skip(fn () => $this->deleteDocument($junction->getCollection(), $junction->getId())); @@ -3374,11 +3375,11 @@ private function updateDocumentRelationships(Document $collection, Document $old foreach ($value as $relation) { if (\is_string($relation)) { - if (\in_array($relation, $oldIds) || Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $relation)->isEmpty())) { + if (\in_array($relation, $oldIds) || $this->getDocument($relatedCollection->getId(), $relation)->isEmpty()) { continue; } } elseif ($relation instanceof Document) { - $related = Authorization::skip(fn () => $this->getDocument($relatedCollection->getId(), $relation->getId())); + $related = $this->getDocument($relatedCollection->getId(), $relation->getId()); if ($related->isEmpty()) { if (!isset($value['$permissions'])) { diff --git a/tests/Database/Base.php b/tests/Database/Base.php index 846461f30..09aa8f2bb 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -11870,39 +11870,35 @@ public function testCreateRelationDocumentWithoutUpdatePermission(): void collection: 'parentRelationTest', relatedCollection: 'childRelationTest', type: Database::RELATION_ONE_TO_MANY, - id: 'childs' + id: 'children' ); // Create document with relationship with nested data $parent = static::getDatabase()->createDocument('parentRelationTest', new Document([ '$id' => 'parent1', 'name' => 'Parent 1', - 'childs' => [ + 'children' => [ [ '$id' => 'child1', 'name' => 'Child 1', ], ], ])); - $this->assertEquals(1, \count($parent['childs'])); - $updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', new Document([ - '$id' => 'parent1', - 'name'=>'Parent 1', - '$collection' => 'parentRelationTest', - 'childs' => [ - new Document([ - '$id' => 'child1', - '$collection' => 'childRelationTest' - ]), - new Document([ - '$id' => 'child2', - 'name' => 'Child 2', - '$collection' => 'childRelationTest' - ]), - ] - ])); + $doc = static::getDatabase()->skipRelationships( + fn () => static::getDatabase()->getDocument('childRelationTest', 'child1') + ); + $this->assertEquals(1, \count($parent['children'])); + $parent->setAttribute('children', [ + [ + '$id' => 'child1', + ], + [ + '$id' => 'child2', + ], + ]); + $updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', $parent); - $this->assertEquals(2, \count($updatedParent['childs'])); + $this->assertEquals(2, \count($updatedParent['children'])); static::getDatabase()->deleteCollection('parentRelationTest'); static::getDatabase()->deleteCollection('childRelationTest'); From 78ea9d68344afd62bec986955bff2acbd3b92918 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Mon, 21 Aug 2023 23:48:51 +0530 Subject: [PATCH 5/6] fix test case --- tests/Database/Base.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/Database/Base.php b/tests/Database/Base.php index 09aa8f2bb..56d3757e5 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -11884,21 +11884,15 @@ public function testCreateRelationDocumentWithoutUpdatePermission(): void ], ], ])); - $doc = static::getDatabase()->skipRelationships( - fn () => static::getDatabase()->getDocument('childRelationTest', 'child1') - ); - $this->assertEquals(1, \count($parent['children'])); + $this->assertEquals('child1', $parent->getAttribute('children')[0]->getId()); $parent->setAttribute('children', [ - [ - '$id' => 'child1', - ], [ '$id' => 'child2', ], ]); $updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', $parent); - $this->assertEquals(2, \count($updatedParent['children'])); + $this->assertEquals('child2', $updatedParent->getAttribute('children')[0]->getId()); static::getDatabase()->deleteCollection('parentRelationTest'); static::getDatabase()->deleteCollection('childRelationTest'); From fc8c21a83ea7541d3f7b1777189e37a62b44c1cd Mon Sep 17 00:00:00 2001 From: prateek banga Date: Tue, 22 Aug 2023 00:35:29 +0530 Subject: [PATCH 6/6] removes skip relationship --- src/Database/Database.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index ed560f824..bef242d5d 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -3285,12 +3285,11 @@ private function updateDocumentRelationships(Document $collection, Document $old $relation->setAttribute($twoWayKey, $document->getId()) ); } else { - // We need to skip the relationship when updating document so that comparison check does not fail - $this->skipRelationships(fn () => $this->updateDocument( + $this->updateDocument( $relatedCollection->getId(), $related->getId(), $relation->setAttribute($twoWayKey, $document->getId()) - )); + ); } } else { throw new DatabaseException('Invalid value for relationship');