From 3fe3bc7d51f0ca94236ad4d62f044161e34cb469 Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Mon, 17 Jul 2023 22:50:44 +0530 Subject: [PATCH 1/3] added logic for throwing no exception when no change in document in update document --- src/Database/Database.php | 23 +++++++++++++++++------ tests/Database/Base.php | 5 ++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index e9bd5b1b6..deb44ec60 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2791,7 +2791,6 @@ public function updateDocument(string $collection, string $id, Document $documen } $time = DateTime::now(); - $document->setAttribute('$updatedAt', $time); $old = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this $collection = $this->silent(fn () => $this->getCollection($collection)); @@ -2801,7 +2800,17 @@ public function updateDocument(string $collection, string $id, Document $documen if ($collection->getId() !== self::METADATA) { $documentSecurity = $collection->getAttribute('documentSecurity', false); - if (!$validator->isValid([ + $skipPermission = true; + foreach ($document as $key=>$value) { + if ($old->getAttribute($key) instanceof Document) { + continue; + } + if ($old->getAttribute($key) !== $value) { + $skipPermission = false; + break; + } + } + if (!$skipPermission && !$validator->isValid([ ...$collection->getUpdate(), ...($documentSecurity ? $old->getUpdate() : []) ])) { @@ -2809,6 +2818,8 @@ public function updateDocument(string $collection, string $id, Document $documen } } + $document->setAttribute('$updatedAt', $time); + // Check if document was updated after the request timestamp $oldUpdatedAt = new \DateTime($old->getUpdatedAt()); if (!is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) { @@ -3010,16 +3021,16 @@ private function updateDocumentRelationships(Document $collection, Document $old $removedDocuments = \array_diff($oldIds, $newIds); foreach ($removedDocuments as $relation) { - $relation = $this->skipRelationships(fn () => $this->getDocument( + $relation = $this->skipRelationships(fn () => Authorization::skip(fn () => $this->getDocument( $relatedCollection->getId(), $relation - )); + ))); - $this->skipRelationships(fn () => $this->updateDocument( + $this->skipRelationships(fn () => Authorization::skip(fn () =>$this->updateDocument( $relatedCollection->getId(), $relation->getId(), $relation->setAttribute($twoWayKey, null) - )); + ))); //removing relationship from 10 id document in collection B by updating it. } foreach ($value as $relation) { diff --git a/tests/Database/Base.php b/tests/Database/Base.php index 83f8b3027..c0716f047 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -2729,7 +2729,7 @@ public function testWritePermissionsUpdateFailure(Document $document): Document Permission::delete(Role::any()), ], 'string' => 'textđź“ť', - 'integer' => 5, + 'integer' => 6, 'bigint' => 8589934592, // 2^33 'float' => 5.55, 'boolean' => true, @@ -10597,7 +10597,6 @@ public function testCollectionPermissionsUpdateWorks(array $data): array public function testCollectionPermissionsUpdateThrowsException(array $data): void { [$collection, $document] = $data; - Authorization::cleanRoles(); Authorization::setRole(Role::any()->toString()); @@ -10605,7 +10604,7 @@ public function testCollectionPermissionsUpdateThrowsException(array $data): voi $document = static::getDatabase()->updateDocument( $collection->getId(), $document->getId(), - $document->setAttribute('test', 'ipsum') + $document->setAttribute('test', 'lorem') ); } From 18786780e3cc3510e26ff0717e7e0f962a758ed0 Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Tue, 18 Jul 2023 00:34:56 +0530 Subject: [PATCH 2/3] removed unnecessary changes --- src/Database/Database.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index deb44ec60..9d4a9273e 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -3021,16 +3021,16 @@ private function updateDocumentRelationships(Document $collection, Document $old $removedDocuments = \array_diff($oldIds, $newIds); foreach ($removedDocuments as $relation) { - $relation = $this->skipRelationships(fn () => Authorization::skip(fn () => $this->getDocument( + $relation = $this->skipRelationships(fn () => $this->getDocument( $relatedCollection->getId(), $relation - ))); + )); - $this->skipRelationships(fn () => Authorization::skip(fn () =>$this->updateDocument( + $this->skipRelationships(fn () => $this->updateDocument( $relatedCollection->getId(), $relation->getId(), $relation->setAttribute($twoWayKey, null) - ))); //removing relationship from 10 id document in collection B by updating it. + )); } foreach ($value as $relation) { From 657cbc177bd8292de5390c9bef3ad27d568a9535 Mon Sep 17 00:00:00 2001 From: Prateek Banga Date: Tue, 18 Jul 2023 15:01:06 +0530 Subject: [PATCH 3/3] Added test case for no authorization exception when no change to update Document --- src/Database/Database.php | 4 +++- tests/Database/Base.php | 41 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 9d4a9273e..9638857dc 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2791,8 +2791,8 @@ public function updateDocument(string $collection, string $id, Document $documen } $time = DateTime::now(); - $old = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this + $collection = $this->silent(fn () => $this->getCollection($collection)); $validator = new Authorization(self::PERMISSION_UPDATE); @@ -2801,7 +2801,9 @@ public function updateDocument(string $collection, string $id, Document $documen $documentSecurity = $collection->getAttribute('documentSecurity', false); $skipPermission = true; + //compare if the document any changes foreach ($document as $key=>$value) { + //skip the nested documents as they will be checked later in recursions. if ($old->getAttribute($key) instanceof Document) { continue; } diff --git a/tests/Database/Base.php b/tests/Database/Base.php index c0716f047..f5860c10d 100644 --- a/tests/Database/Base.php +++ b/tests/Database/Base.php @@ -2739,6 +2739,47 @@ public function testWritePermissionsUpdateFailure(Document $document): Document return $document; } + + /** + * @depends testCreateDocument + */ + public function testNoChangeUpdateDocumentWithoutPermission(Document $document): Document + { + Authorization::cleanRoles(); + Authorization::setRole(Role::any()->toString()); + + + $document = static::getDatabase()->createDocument('documents', new Document([ + 'string' => 'textđź“ť', + 'integer' => 5, + 'bigint' => 8589934592, // 2^33 + 'float' => 5.55, + 'boolean' => true, + 'colors' => ['pink', 'green', 'blue'], + ])); + Authorization::cleanRoles(); + //no changes in document + $documentToUpdate = new Document([ + '$id' => ID::custom($document->getId()), + 'string' => 'textđź“ť', + 'integer' => 5, + 'bigint' => 8589934592, // 2^33 + 'float' => 5.55, + 'boolean' => true, + 'colors' => ['pink', 'green', 'blue'], + '$collection' => 'documents', + ]); + $documentToUpdate->setAttribute('$createdAt', $document->getAttribute('$createdAt')); + $documentToUpdate->setAttribute('$updatedAt', $document->getAttribute('$updatedAt')); + $updatedDocument = static::getDatabase()->updateDocument('documents', $document->getId(), $documentToUpdate); + + + // Document should be updated without any problem and without any authorization exception as there is no change in document. + $this->assertEquals(true, $updatedDocument->getUpdatedAt() > $document->getUpdatedAt()); + + return $document; + } + public function testExceptionAttributeLimit(): void { if ($this->getDatabase()->getLimitForAttributes() > 0) {