From 9447d46ee2211e09fb2f2f05305e94c5afb2e18c Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 11 Aug 2023 17:30:09 +0530 Subject: [PATCH 1/6] fixes edge case in comparison check --- src/Database/Database.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Database/Database.php b/src/Database/Database.php index dbe891f20..6524abee6 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2940,6 +2940,10 @@ public function updateDocument(string $collection, string $id, Document $documen foreach ($document as $key => $value) { // Skip the nested documents as they will be checked later in recursions. if (\array_key_exists($key, $relationships)) { + // No need to compare nested documents more than max depth. + if (count($this->relationshipWriteStack) >= Database::RELATION_MAX_DEPTH - 1) { + continue; + } $relationType = (string) $relationships[$key]['options']['relationType']; $side = (string) $relationships[$key]['options']['side']; From 8ccf0633a60ac671abbee5e2aa4175c7b1e36c09 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 11 Aug 2023 22:01:13 +0530 Subject: [PATCH 2/6] adds test case --- tests/Database/Base.php | 102 +++++++++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 32 deletions(-) diff --git a/tests/Database/Base.php b/tests/Database/Base.php index 18c39e63b..b8bc950f2 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -3326,49 +3326,87 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void $this->expectNotToPerformAssertions(); return; } + $attribute = new Document([ + '$id' => ID::custom("name"), + 'type' => Database::VAR_STRING, + 'size' => 100, + 'required' => false, + 'default' => null, + 'signed' => false, + 'array' => false, + 'filters' => [], + ]); - static::getDatabase()->createCollection('parentRelationTest'); - static::getDatabase()->createCollection('childRelationTest'); - - static::getDatabase()->createAttribute('parentRelationTest', 'name', Database::VAR_STRING, 255, false); - static::getDatabase()->createAttribute('childRelationTest', 'name', Database::VAR_STRING, 255, false); + $permission = [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::delete(Role::any()), + ]; + for($i=1; $i < 6; $i++){ + static::getDatabase()->createCollection("level{$i}", [$attribute], [], $permission); + } - static::getDatabase()->createRelationship( - collection: 'parentRelationTest', - relatedCollection: 'childRelationTest', - type: Database::RELATION_ONE_TO_MANY, - id: 'children' - ); + for($i = 1; $i < 5; $i++){ + $collectionId = $i; + $relatedCollectionId = $i+1; + static::getDatabase()->createRelationship( + collection: "level{$collectionId}", + relatedCollection: "level{$relatedCollectionId}", + type: Database::RELATION_ONE_TO_MANY, + id: "level{$relatedCollectionId}" + ); + } // Create document with relationship with nested data - $parent = static::getDatabase()->createDocument('parentRelationTest', new Document([ - '$id' => 'parent1', - '$permissions' => [ - Permission::read(Role::any()), - ], - 'name' => 'Parent 1', - 'children' => [ + $level1 = static::getDatabase()->createDocument('level1', new Document([ + '$id' => 'level1', + '$permissions' => [], + 'name' => 'Level 1', + 'level2' => [ [ - '$id' => 'child1', - '$permissions' => [ - Permission::read(Role::any()), + '$id' => 'level2', + '$permissions' => [], + 'name' => 'Level 2', + 'level3' => [ + [ + '$id' => 'level3', + '$permissions' => [], + 'name' => 'Level haha', + 'level4' => [ + [ + '$id' => 'level4', + '$permissions' => [], + 'name' => 'Level 4', + 'level5' => [ + [ + '$id' => 'level5', + '$permissions' => [], + 'name' => 'Level 5', + + ], + ], + + ], + ], + + ], ], - 'name' => 'Child 1', ], ], ])); - $this->assertEquals(1, \count($parent['children'])); - - $parent->setAttribute('children', ['child1']); - - $updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', $parent); - - $this->assertEquals($updatedParent->getUpdatedAt(), $parent->getUpdatedAt()); - $this->assertEquals($updatedParent->getAttribute('children')[0]->getUpdatedAt(), $parent->getAttribute('children')[0]->getUpdatedAt()); + $this->assertEquals(1, \count($level1['level2'])); + + $updateLevel1 = static::getDatabase()->updateDocument('level1', 'level1', $level1); + + $this->assertEquals($level1, $updateLevel1); + + $this->expectException(AuthorizationException::class); + static::getDatabase()->updateDocument('level1', 'level1', $level1->setAttribute('name', 'haha')); - static::getDatabase()->deleteCollection('parentRelationTest'); - static::getDatabase()->deleteCollection('childRelationTest'); + for($i=1; $i < 6; $i++){ + static::getDatabase()->deleteCollection("level{$i}"); + } } public function testExceptionAttributeLimit(): void From 244b08812121f743537751cb03e8d7787cebbdf3 Mon Sep 17 00:00:00 2001 From: prateek banga Date: Fri, 11 Aug 2023 22:02:20 +0530 Subject: [PATCH 3/6] lint fix --- tests/Database/Base.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/Database/Base.php b/tests/Database/Base.php index b8bc950f2..1a2395f3d 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -3342,11 +3342,11 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void Permission::create(Role::any()), Permission::delete(Role::any()), ]; - for($i=1; $i < 6; $i++){ + for ($i=1; $i < 6; $i++) { static::getDatabase()->createCollection("level{$i}", [$attribute], [], $permission); } - for($i = 1; $i < 5; $i++){ + for ($i = 1; $i < 5; $i++) { $collectionId = $i; $relatedCollectionId = $i+1; static::getDatabase()->createRelationship( @@ -3382,13 +3382,13 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void '$id' => 'level5', '$permissions' => [], 'name' => 'Level 5', - + ], ], - + ], ], - + ], ], ], @@ -3396,15 +3396,15 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void ])); $this->assertEquals(1, \count($level1['level2'])); - + $updateLevel1 = static::getDatabase()->updateDocument('level1', 'level1', $level1); - + $this->assertEquals($level1, $updateLevel1); - + $this->expectException(AuthorizationException::class); static::getDatabase()->updateDocument('level1', 'level1', $level1->setAttribute('name', 'haha')); - for($i=1; $i < 6; $i++){ + for ($i=1; $i < 6; $i++) { static::getDatabase()->deleteCollection("level{$i}"); } } From da4c162edf1db9d64850109fbdc5e31227b8b18d Mon Sep 17 00:00:00 2001 From: prateek banga Date: Mon, 14 Aug 2023 10:54:34 +0530 Subject: [PATCH 4/6] adds assertion to update level3 doc --- tests/Database/Base.php | 76 ++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/tests/Database/Base.php b/tests/Database/Base.php index 1a2395f3d..a8b282168 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -3337,13 +3337,13 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void 'filters' => [], ]); - $permission = [ + $permissions = [ Permission::read(Role::any()), Permission::create(Role::any()), Permission::delete(Role::any()), ]; for ($i=1; $i < 6; $i++) { - static::getDatabase()->createCollection("level{$i}", [$attribute], [], $permission); + static::getDatabase()->createCollection("level{$i}", [$attribute], [], $permissions); } for ($i = 1; $i < 5; $i++) { @@ -3352,7 +3352,7 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void static::getDatabase()->createRelationship( collection: "level{$collectionId}", relatedCollection: "level{$relatedCollectionId}", - type: Database::RELATION_ONE_TO_MANY, + type: Database::RELATION_ONE_TO_ONE, id: "level{$relatedCollectionId}" ); } @@ -3363,46 +3363,52 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void '$permissions' => [], 'name' => 'Level 1', 'level2' => [ - [ - '$id' => 'level2', + '$id' => 'level2', + '$permissions' => [], + 'name' => 'Level 2', + 'level3' => [ + '$id' => 'level3', '$permissions' => [], - 'name' => 'Level 2', - 'level3' => [ - [ - '$id' => 'level3', + 'name' => 'Level haha', + 'level4' => [ + '$id' => 'level4', + '$permissions' => [], + 'name' => 'Level 4', + 'level5' => [ + '$id' => 'level5', '$permissions' => [], - 'name' => 'Level haha', - 'level4' => [ - [ - '$id' => 'level4', - '$permissions' => [], - 'name' => 'Level 4', - 'level5' => [ - [ - '$id' => 'level5', - '$permissions' => [], - 'name' => 'Level 5', - - ], - ], - - ], - ], - - ], + 'name' => 'Level 5', + ] ], ], ], - ])); - - $this->assertEquals(1, \count($level1['level2'])); + ])); + static::getDatabase()->updateDocument('level1', $level1->getId(), new Document($level1->getArrayCopy())); + $updatedLevel1 = static::getDatabase()->getDocument('level1', $level1->getId()); + $this->assertEquals($level1, $updatedLevel1); - $updateLevel1 = static::getDatabase()->updateDocument('level1', 'level1', $level1); + try { + static::getDatabase()->updateDocument('level1', $level1->getId(), $level1->setAttribute('name', 'haha')); + $this->fail('Failed to throw exception'); + } catch(Exception $e) { + $this->assertInstanceOf(AuthorizationException::class, $e); + } + $level1->setAttribute('name','Level 1'); + static::getDatabase()->updateCollection('level3', [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], false); + $level2 = $level1->getAttribute('level2'); + $level3 = $level2->getAttribute('level3'); - $this->assertEquals($level1, $updateLevel1); + $level3->setAttribute('name', 'updated value'); + $level2->setAttribute('level3', $level3); + $level1->setAttribute('level2', $level2); - $this->expectException(AuthorizationException::class); - static::getDatabase()->updateDocument('level1', 'level1', $level1->setAttribute('name', 'haha')); + $level1 = static::getDatabase()->updateDocument('level1', $level1->getId(), $level1); + $this->assertEquals('updated value', $level1['level2']['level3']['name']); for ($i=1; $i < 6; $i++) { static::getDatabase()->deleteCollection("level{$i}"); From 97d367a77c212bb15d5c0ef2dec30849d74fb0bc Mon Sep 17 00:00:00 2001 From: prateek banga Date: Mon, 14 Aug 2023 10:56:02 +0530 Subject: [PATCH 5/6] lint fix --- tests/Database/Base.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Database/Base.php b/tests/Database/Base.php index a8b282168..df17e5228 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -3382,7 +3382,7 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void ], ], ], - ])); + ])); static::getDatabase()->updateDocument('level1', $level1->getId(), new Document($level1->getArrayCopy())); $updatedLevel1 = static::getDatabase()->getDocument('level1', $level1->getId()); $this->assertEquals($level1, $updatedLevel1); @@ -3393,7 +3393,7 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void } catch(Exception $e) { $this->assertInstanceOf(AuthorizationException::class, $e); } - $level1->setAttribute('name','Level 1'); + $level1->setAttribute('name', 'Level 1'); static::getDatabase()->updateCollection('level3', [ Permission::read(Role::any()), Permission::create(Role::any()), @@ -3407,7 +3407,7 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void $level2->setAttribute('level3', $level3); $level1->setAttribute('level2', $level2); - $level1 = static::getDatabase()->updateDocument('level1', $level1->getId(), $level1); + $level1 = static::getDatabase()->updateDocument('level1', $level1->getId(), $level1); $this->assertEquals('updated value', $level1['level2']['level3']['name']); for ($i=1; $i < 6; $i++) { From 557c630c68ade920f3e169f38f5064b133d710de Mon Sep 17 00:00:00 2001 From: prateek banga Date: Mon, 14 Aug 2023 14:25:08 +0530 Subject: [PATCH 6/6] changes name value in test case --- tests/Database/Base.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Database/Base.php b/tests/Database/Base.php index df17e5228..75030e84c 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -3369,7 +3369,7 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void 'level3' => [ '$id' => 'level3', '$permissions' => [], - 'name' => 'Level haha', + 'name' => 'Level 3', 'level4' => [ '$id' => 'level4', '$permissions' => [],