diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 8f9170fb7..bc8f376ab 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1445,12 +1445,14 @@ public function createOrUpdateDocuments( if (!empty($toRemove)) { $removeQueries[] = "( _document = :_uid_{$index} - {$this->getTenantQuery($collection, tenantCount: \count($toRemove))} + " . ($this->sharedTables ? " AND _tenant = :_tenant_{$index}" : '') . " AND _type = '{$type}' AND _permission IN (" . \implode(',', \array_map(fn ($i) => ":remove_{$type}_{$index}_{$i}", \array_keys($toRemove))) . ") )"; $removeBindValues[":_uid_{$index}"] = $document->getId(); - $removeBindValues[":_tenant_{$index}"] = $document->getTenant(); + if ($this->sharedTables) { + $removeBindValues[":_tenant_{$index}"] = $document->getTenant(); + } foreach ($toRemove as $i => $perm) { $removeBindValues[":remove_{$type}_{$index}_{$i}"] = $perm; } diff --git a/src/Database/Database.php b/src/Database/Database.php index 1ebfc1d8b..01c9493dd 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4920,7 +4920,7 @@ public function createOrUpdateDocumentsWithIncrease( $time = DateTime::now(); $created = 0; $updated = 0; - + $seenIds = []; foreach ($documents as $key => $document) { if ($this->getSharedTables() && $this->getTenantPerDocument()) { $old = Authorization::skip(fn () => $this->withTenant($document->getTenant(), fn () => $this->silent(fn () => $this->getDocument( @@ -5028,12 +5028,19 @@ public function createOrUpdateDocumentsWithIncrease( $document = $this->silent(fn () => $this->createDocumentRelationships($collection, $document)); } + $seenIds[] = $document->getId(); + $documents[$key] = new Change( old: $old, new: $document ); } + // Required because *some* DBs will allow duplicate IDs for upsert + if (\count($seenIds) !== \count(\array_unique($seenIds))) { + throw new DuplicateException('Duplicate document IDs found in the input array.'); + } + foreach (\array_chunk($documents, $batchSize) as $chunk) { /** * @var array $chunk diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index a4393ea0a..4434ca841 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -735,6 +735,75 @@ public function testUpsertDocumentsNoop(): void $this->assertEquals(0, $count); } + public function testUpsertDuplicateIds(): void + { + $db = static::getDatabase(); + if (!$db->getAdapter()->getSupportForUpserts()) { + $this->expectNotToPerformAssertions(); + return; + } + + $db->createCollection(__FUNCTION__); + $db->createAttribute(__FUNCTION__, 'num', Database::VAR_INTEGER, 0, true); + + $doc1 = new Document(['$id' => 'dup', 'num' => 1]); + $doc2 = new Document(['$id' => 'dup', 'num' => 2]); + + try { + $db->createOrUpdateDocuments(__FUNCTION__, [$doc1, $doc2]); + $this->fail('Failed to throw exception'); + } catch (\Throwable $e) { + $this->assertInstanceOf(DuplicateException::class, $e, $e->getMessage()); + } + } + + public function testUpsertMixedPermissionDelta(): void + { + $db = static::getDatabase(); + if (!$db->getAdapter()->getSupportForUpserts()) { + $this->expectNotToPerformAssertions(); + return; + } + + $db->createCollection(__FUNCTION__); + $db->createAttribute(__FUNCTION__, 'v', Database::VAR_INTEGER, 0, true); + + $d1 = $db->createDocument(__FUNCTION__, new Document([ + '$id' => 'a', + 'v' => 0, + '$permissions' => [ + Permission::update(Role::any()) + ] + ])); + $d2 = $db->createDocument(__FUNCTION__, new Document([ + '$id' => 'b', + 'v' => 0, + '$permissions' => [ + Permission::update(Role::any()) + ] + ])); + + // d1 adds write, d2 removes update + $d1->setAttribute('$permissions', [ + Permission::read(Role::any()), + Permission::update(Role::any()) + ]); + $d2->setAttribute('$permissions', [ + Permission::read(Role::any()) + ]); + + $db->createOrUpdateDocuments(__FUNCTION__, [$d1, $d2]); + + $this->assertEquals([ + Permission::read(Role::any()), + Permission::update(Role::any()), + ], $db->getDocument(__FUNCTION__, 'a')->getPermissions()); + + $this->assertEquals([ + Permission::read(Role::any()), + ], $db->getDocument(__FUNCTION__, 'b')->getPermissions()); + } + public function testRespectNulls(): Document { static::getDatabase()->createCollection('documents_nulls');