From 03ba92cbf26954bf9e077f237b2015f2deec1d7d Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 3 Aug 2025 17:59:54 +0300 Subject: [PATCH 1/6] comments --- tests/e2e/Adapter/Scopes/PermissionTests.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/e2e/Adapter/Scopes/PermissionTests.php b/tests/e2e/Adapter/Scopes/PermissionTests.php index 6247c2294..4e26c955c 100644 --- a/tests/e2e/Adapter/Scopes/PermissionTests.php +++ b/tests/e2e/Adapter/Scopes/PermissionTests.php @@ -127,6 +127,9 @@ public function testUnsetPermissions(): void foreach ($documents as $document) { $this->assertEquals('Richard Nixon', $document->getAttribute('president')); + /** + * This is failing!!! + */ $this->assertEquals([], $document->getPermissions()); } } From 11b0d98a2becc061ba32908b1830ecd9d0c1d7da Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Aug 2025 12:01:33 +0300 Subject: [PATCH 2/6] Check $permissions keys --- phpunit.xml | 2 +- src/Database/Adapter/SQL.php | 16 +++++++++++++--- src/Database/Database.php | 7 +++++-- tests/e2e/Adapter/Base.php | 8 ++++---- tests/e2e/Adapter/Scopes/PermissionTests.php | 10 +++++++--- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/phpunit.xml b/phpunit.xml index 2a0531cfd..34365d48d 100755 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,7 +7,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="false" + stopOnFailure="true" > diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 936e32fb1..b592e6c21 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -11,6 +11,7 @@ use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Exception\NotFound as NotFoundException; +use Utopia\Database\Exception\Query as QueryException; use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Query; @@ -426,10 +427,20 @@ public function updateDocuments(string $collection, Document $updates, array $do $attributes['_createdAt'] = $updates->getCreatedAt(); } - if (!empty($updates->getPermissions())) { + if ($updates->offsetExists('$permissions')) { $attributes['_permissions'] = json_encode($updates->getPermissions()); } +// if ($updatePermissions) { +// $originalPermissions = $document->getPermissions(); +// $currentPermissions = $updates->getPermissions(); +// +// sort($originalPermissions); +// sort($currentPermissions); +// +// $skipPermissionsUpdate = ($originalPermissions === $currentPermissions); +// } + if (empty($attributes)) { return 0; } @@ -484,7 +495,7 @@ public function updateDocuments(string $collection, Document $updates, array $do $affected = $stmt->rowCount(); // Permissions logic - if (!empty($updates->getPermissions())) { + if ($updates->offsetExists('$permissions')) { $removeQueries = []; $removeBindValues = []; @@ -492,7 +503,6 @@ public function updateDocuments(string $collection, Document $updates, array $do $addBindValues = []; foreach ($documents as $index => $document) { - // Permissions logic $sql = " SELECT _type, _permission FROM {$this->getSQLTable($name . '_perms')} diff --git a/src/Database/Database.php b/src/Database/Database.php index 9f2674f0a..cc9b3bf36 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4375,13 +4375,16 @@ public function updateDocuments( if (!empty($cursor) && $cursor->getCollection() !== $collection->getId()) { throw new DatabaseException("Cursor document must be from the same Collection."); } + unset($updates['$id']); unset($updates['$tenant']); + if (($updates->getCreatedAt() === null || !$this->preserveDates)) { unset($updates['$createdAt']); } else { $updates['$createdAt'] = $updates->getCreatedAt(); } + if ($this->adapter->getSharedTables()) { $updates['$tenant'] = $this->adapter->getTenant(); } @@ -4431,7 +4434,7 @@ public function updateDocuments( } $this->withTransaction(function () use ($collection, $updates, &$batch) { - foreach ($batch as &$document) { + foreach ($batch as $index => $document) { $new = new Document(\array_merge($document->getArrayCopy(), $updates->getArrayCopy())); if ($this->resolveRelationships) { @@ -4451,7 +4454,7 @@ public function updateDocuments( throw new ConflictException('Document was updated after the request timestamp'); } - $document = $this->encode($collection, $document); + $batch[$index] = $this->encode($collection, $document); } $this->adapter->updateDocuments( diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index a57fe2748..14ebc1fe1 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -17,10 +17,10 @@ abstract class Base extends TestCase { - use CollectionTests; - use DocumentTests; - use AttributeTests; - use IndexTests; +// use CollectionTests; +// use DocumentTests; +// use AttributeTests; +// use IndexTests; use PermissionTests; use RelationshipTests; use GeneralTests; diff --git a/tests/e2e/Adapter/Scopes/PermissionTests.php b/tests/e2e/Adapter/Scopes/PermissionTests.php index 4e26c955c..f9207bc39 100644 --- a/tests/e2e/Adapter/Scopes/PermissionTests.php +++ b/tests/e2e/Adapter/Scopes/PermissionTests.php @@ -122,16 +122,20 @@ public function testUnsetPermissions(): void } $documents = $database->find(__FUNCTION__); + $this->assertEquals(0, count($documents)); + + Authorization::disable(); + $documents = $database->find(__FUNCTION__); + Authorization::reset(); $this->assertEquals(3, count($documents)); foreach ($documents as $document) { $this->assertEquals('Richard Nixon', $document->getAttribute('president')); - /** - * This is failing!!! - */ $this->assertEquals([], $document->getPermissions()); } + + $this->assertEquals('shmuel', 'fogel'); } public function testCreateDocumentsEmptyPermission(): void From 04fec5de2c54e4783d7f6a8a01428c4fad98a152 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Aug 2025 15:41:53 +0300 Subject: [PATCH 3/6] Run tests --- src/Database/Adapter/SQL.php | 15 ++---- src/Database/Database.php | 27 +++++++++-- tests/e2e/Adapter/Base.php | 8 ++-- tests/e2e/Adapter/Scopes/DocumentTests.php | 2 +- tests/e2e/Adapter/Scopes/PermissionTests.php | 48 ++++++++++++++++++-- 5 files changed, 78 insertions(+), 22 deletions(-) diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index b592e6c21..238e266bc 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -11,7 +11,6 @@ use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Exception\NotFound as NotFoundException; -use Utopia\Database\Exception\Query as QueryException; use Utopia\Database\Exception\Transaction as TransactionException; use Utopia\Database\Query; @@ -431,16 +430,6 @@ public function updateDocuments(string $collection, Document $updates, array $do $attributes['_permissions'] = json_encode($updates->getPermissions()); } -// if ($updatePermissions) { -// $originalPermissions = $document->getPermissions(); -// $currentPermissions = $updates->getPermissions(); -// -// sort($originalPermissions); -// sort($currentPermissions); -// -// $skipPermissionsUpdate = ($originalPermissions === $currentPermissions); -// } - if (empty($attributes)) { return 0; } @@ -503,6 +492,10 @@ public function updateDocuments(string $collection, Document $updates, array $do $addBindValues = []; foreach ($documents as $index => $document) { + if ($document->getAttribute('$skipPermissionsUpdate', false)) { + continue; + } + $sql = " SELECT _type, _permission FROM {$this->getSQLTable($name . '_perms')} diff --git a/src/Database/Database.php b/src/Database/Database.php index cc9b3bf36..3e2cc1a1e 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -3609,7 +3609,7 @@ public function createDocument(string $collection, Document $document): Document ->setAttribute('$createdAt', ($createdAt === null || !$this->preserveDates) ? $time : $createdAt) ->setAttribute('$updatedAt', ($updatedAt === null || !$this->preserveDates) ? $time : $updatedAt); - if (empty($document->getPermissions())){ + if (empty($document->getPermissions())) { $document->setAttribute('$permissions', []); } @@ -3712,7 +3712,7 @@ public function createDocuments( ->setAttribute('$createdAt', ($createdAt === null || !$this->preserveDates) ? $time : $createdAt) ->setAttribute('$updatedAt', ($updatedAt === null || !$this->preserveDates) ? $time : $updatedAt); - if (empty($document->getPermissions())){ + if (empty($document->getPermissions())) { $document->setAttribute('$permissions', []); } @@ -4433,8 +4433,27 @@ public function updateDocuments( break; } - $this->withTransaction(function () use ($collection, $updates, &$batch) { + $currentPermissions = $updates->getPermissions(); + sort($currentPermissions); + + $this->withTransaction(function () use ($collection, $updates, &$batch, $currentPermissions) { foreach ($batch as $index => $document) { + + $skipPermissionsUpdate = true; + + if ($updates->offsetExists('$permissions')) { + if (!$document->offsetExists('$permissions')) { + throw new QueryException('Permission document missing in select'); + } + + $originalPermissions = $document->getPermissions(); + sort($originalPermissions); + + $skipPermissionsUpdate = ($originalPermissions === $currentPermissions); + } + + $document->setAttribute('$skipPermissionsUpdate', $skipPermissionsUpdate); + $new = new Document(\array_merge($document->getArrayCopy(), $updates->getArrayCopy())); if ($this->resolveRelationships) { @@ -4465,6 +4484,8 @@ public function updateDocuments( }); foreach ($batch as $doc) { + $doc->removeAttribute('$skipPermissionsUpdate'); + $this->purgeCachedDocument($collection->getId(), $doc->getId()); $doc = $this->decode($collection, $doc); try { diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 14ebc1fe1..237595014 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -17,10 +17,10 @@ abstract class Base extends TestCase { -// use CollectionTests; -// use DocumentTests; -// use AttributeTests; -// use IndexTests; + // use CollectionTests; + // use DocumentTests; + // use AttributeTests; + // use IndexTests; use PermissionTests; use RelationshipTests; use GeneralTests; diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 40c319574..81ce0fae2 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -450,7 +450,7 @@ public function testSkipPermissions(): void $this->assertEquals(2, \count($results)); $this->assertEquals(2, $count); - foreach ($results as $result){ + foreach ($results as $result) { $this->assertArrayHasKey('$permissions', $result); $this->assertEquals([], $result->getAttribute('$permissions')); } diff --git a/tests/e2e/Adapter/Scopes/PermissionTests.php b/tests/e2e/Adapter/Scopes/PermissionTests.php index f9207bc39..5eb5a42c9 100644 --- a/tests/e2e/Adapter/Scopes/PermissionTests.php +++ b/tests/e2e/Adapter/Scopes/PermissionTests.php @@ -94,6 +94,49 @@ public function testUnsetPermissions(): void $this->assertEquals($permissions, $document->getPermissions()); } + /** + * Change permissions remove delete + */ + $permissions = [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + ]; + + $updates = new Document([ + '$permissions' => $permissions, + 'president' => 'Joe biden' + ]); + + /** + * @var $results Array + */ + $results = []; + $modified = $database->updateDocuments( + __FUNCTION__, + $updates, + onNext: function ($doc) use (&$results) { + $results[] = $doc; + } + ); + + $this->assertEquals(3, $modified); + + foreach ($results as $result) { + $this->assertEquals('Joe biden', $result->getAttribute('president')); + $this->assertEquals($permissions, $result->getPermissions()); + $this->assertArrayNotHasKey('$skipPermissionsUpdate', $result); + } + + $documents = $database->find(__FUNCTION__); + + $this->assertEquals(3, count($documents)); + + foreach ($documents as $document) { + $this->assertEquals('Joe biden', $document->getAttribute('president')); + $this->assertEquals($permissions, $document->getPermissions()); + } + /** * Unset permissions */ @@ -133,12 +176,11 @@ public function testUnsetPermissions(): void foreach ($documents as $document) { $this->assertEquals('Richard Nixon', $document->getAttribute('president')); $this->assertEquals([], $document->getPermissions()); + $this->assertArrayNotHasKey('$skipPermissionsUpdate', $document); } - - $this->assertEquals('shmuel', 'fogel'); } - public function testCreateDocumentsEmptyPermission(): void + public function testCreateDocumentsEmptyPermission(): void { /** @var Database $database */ $database = static::getDatabase(); From 5bcef6e18790b3ad83fa50a01516f89572fa06b7 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Aug 2025 15:48:57 +0300 Subject: [PATCH 4/6] formatting --- tests/e2e/Adapter/Scopes/PermissionTests.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/e2e/Adapter/Scopes/PermissionTests.php b/tests/e2e/Adapter/Scopes/PermissionTests.php index 5eb5a42c9..50e14c90c 100644 --- a/tests/e2e/Adapter/Scopes/PermissionTests.php +++ b/tests/e2e/Adapter/Scopes/PermissionTests.php @@ -44,9 +44,6 @@ public function testUnsetPermissions(): void ]); } - /** - * @var $results Array - */ $results = []; $count = $database->createDocuments(__FUNCTION__, $documents, onNext: function ($doc) use (&$results) { $results[] = $doc; @@ -66,9 +63,6 @@ public function testUnsetPermissions(): void 'president' => 'George Washington' ]); - /** - * @var $results Array - */ $results = []; $modified = $database->updateDocuments( __FUNCTION__, @@ -108,9 +102,6 @@ public function testUnsetPermissions(): void 'president' => 'Joe biden' ]); - /** - * @var $results Array - */ $results = []; $modified = $database->updateDocuments( __FUNCTION__, @@ -145,9 +136,6 @@ public function testUnsetPermissions(): void 'president' => 'Richard Nixon' ]); - /** - * @var $results Array - */ $results = []; $modified = $database->updateDocuments( __FUNCTION__, From 69f08e372f05d1728cd9fef7af5f622ada6bd12f Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Aug 2025 15:55:13 +0300 Subject: [PATCH 5/6] Remove comment --- tests/e2e/Adapter/Base.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 237595014..a57fe2748 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -17,10 +17,10 @@ abstract class Base extends TestCase { - // use CollectionTests; - // use DocumentTests; - // use AttributeTests; - // use IndexTests; + use CollectionTests; + use DocumentTests; + use AttributeTests; + use IndexTests; use PermissionTests; use RelationshipTests; use GeneralTests; From 3d8c85b0a28516687dd5483390666868dbcf8937 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 4 Aug 2025 15:57:53 +0300 Subject: [PATCH 6/6] stopOnFailure --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index 34365d48d..2a0531cfd 100755 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,7 +7,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="true" + stopOnFailure="false" >