Skip to content

Commit 7b2622d

Browse files
abnegateabnegate
committed
Merge remote-tracking branch 'origin/0.69.x'
# Conflicts: # tests/e2e/Adapter/Scopes/DocumentTests.php
2 parents 4670879 + 3db51ec commit 7b2622d

File tree

3 files changed

+115
-3
lines changed

3 files changed

+115
-3
lines changed

‎src/Database/Adapter/MariaDB.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,6 +1195,8 @@ public function createOrUpdateDocuments(
11951195
= $document->getTenant();
11961196
}
11971197

1198+
\ksort($attributes);
1199+
11981200
$columns = [];
11991201
foreach (\array_keys($attributes) as $key => $attr) {
12001202
/**
@@ -1288,12 +1290,14 @@ public function createOrUpdateDocuments(
12881290
if (!empty($toRemove)) {
12891291
$removeQueries[] = "(
12901292
_document = :_uid_{$index}
1291-
{$this->getTenantQuery($collection, tenantCount: \count($toRemove))}
1293+
" . ($this->sharedTables ? " AND _tenant = :_tenant_{$index}" : '') . "
12921294
AND _type = '{$type}'
12931295
AND _permission IN (" . \implode(',', \array_map(fn ($i) => ":remove_{$type}_{$index}_{$i}", \array_keys($toRemove))) . ")
12941296
)";
12951297
$removeBindValues[":_uid_{$index}"] = $document->getId();
1296-
$removeBindValues[":_tenant_{$index}"] = $document->getTenant();
1298+
if ($this->sharedTables) {
1299+
$removeBindValues[":_tenant_{$index}"] = $document->getTenant();
1300+
}
12971301
foreach ($toRemove as $i => $perm) {
12981302
$removeBindValues[":remove_{$type}_{$index}_{$i}"] = $perm;
12991303
}

‎src/Database/Database.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4920,7 +4920,7 @@ public function createOrUpdateDocumentsWithIncrease(
49204920
$time = DateTime::now();
49214921
$created = 0;
49224922
$updated = 0;
4923-
4923+
$seenIds = [];
49244924
foreach ($documents as $key => $document) {
49254925
if ($this->getSharedTables() && $this->getTenantPerDocument()) {
49264926
$old = Authorization::skip(fn () => $this->withTenant($document->getTenant(), fn () => $this->silent(fn () => $this->getDocument(
@@ -5028,12 +5028,19 @@ public function createOrUpdateDocumentsWithIncrease(
50285028
$document = $this->silent(fn () => $this->createDocumentRelationships($collection, $document));
50295029
}
50305030

5031+
$seenIds[] = $document->getId();
5032+
50315033
$documents[$key] = new Change(
50325034
old: $old,
50335035
new: $document
50345036
);
50355037
}
50365038

5039+
// Required because *some* DBs will allow duplicate IDs for upsert
5040+
if (\count($seenIds) !== \count(\array_unique($seenIds))) {
5041+
throw new DuplicateException('Duplicate document IDs found in the input array.');
5042+
}
5043+
50375044
foreach (\array_chunk($documents, $batchSize) as $chunk) {
50385045
/**
50395046
* @var array<Change> $chunk

‎tests/e2e/Adapter/Scopes/DocumentTests.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,38 @@ public function testUpsertDocumentsAttributeMismatch(): void
746746
$this->assertEquals(null, $existingDocument->getAttribute('last'));
747747
$this->assertEquals('second', $newDocument->getAttribute('first'));
748748
$this->assertEquals('last', $newDocument->getAttribute('last'));
749+
750+
$doc3 = new Document([
751+
'$id' => 'third',
752+
'last' => 'last',
753+
'first' => 'third',
754+
]);
755+
756+
$doc4 = new Document([
757+
'$id' => 'fourth',
758+
'first' => 'fourth',
759+
'last' => 'last',
760+
]);
761+
762+
// Ensure mismatch of attribute orders is allowed
763+
$docs = $database->createOrUpdateDocuments(__FUNCTION__, [
764+
$doc3,
765+
$doc4
766+
]);
767+
768+
$this->assertEquals(2, $docs);
769+
$this->assertEquals('third', $doc3->getAttribute('first'));
770+
$this->assertEquals('last', $doc3->getAttribute('last'));
771+
$this->assertEquals('fourth', $doc4->getAttribute('first'));
772+
$this->assertEquals('last', $doc4->getAttribute('last'));
773+
774+
$doc3 = $database->getDocument(__FUNCTION__, 'third');
775+
$doc4 = $database->getDocument(__FUNCTION__, 'fourth');
776+
777+
$this->assertEquals('third', $doc3->getAttribute('first'));
778+
$this->assertEquals('last', $doc3->getAttribute('last'));
779+
$this->assertEquals('fourth', $doc4->getAttribute('first'));
780+
$this->assertEquals('last', $doc4->getAttribute('last'));
749781
}
750782

751783
public function testUpsertDocumentsNoop(): void
@@ -777,6 +809,75 @@ public function testUpsertDocumentsNoop(): void
777809
$this->assertEquals(0, $count);
778810
}
779811

812+
public function testUpsertDuplicateIds(): void
813+
{
814+
$db = static::getDatabase();
815+
if (!$db->getAdapter()->getSupportForUpserts()) {
816+
$this->expectNotToPerformAssertions();
817+
return;
818+
}
819+
820+
$db->createCollection(__FUNCTION__);
821+
$db->createAttribute(__FUNCTION__, 'num', Database::VAR_INTEGER, 0, true);
822+
823+
$doc1 = new Document(['$id' => 'dup', 'num' => 1]);
824+
$doc2 = new Document(['$id' => 'dup', 'num' => 2]);
825+
826+
try {
827+
$db->createOrUpdateDocuments(__FUNCTION__, [$doc1, $doc2]);
828+
$this->fail('Failed to throw exception');
829+
} catch (\Throwable $e) {
830+
$this->assertInstanceOf(DuplicateException::class, $e, $e->getMessage());
831+
}
832+
}
833+
834+
public function testUpsertMixedPermissionDelta(): void
835+
{
836+
$db = static::getDatabase();
837+
if (!$db->getAdapter()->getSupportForUpserts()) {
838+
$this->expectNotToPerformAssertions();
839+
return;
840+
}
841+
842+
$db->createCollection(__FUNCTION__);
843+
$db->createAttribute(__FUNCTION__, 'v', Database::VAR_INTEGER, 0, true);
844+
845+
$d1 = $db->createDocument(__FUNCTION__, new Document([
846+
'$id' => 'a',
847+
'v' => 0,
848+
'$permissions' => [
849+
Permission::update(Role::any())
850+
]
851+
]));
852+
$d2 = $db->createDocument(__FUNCTION__, new Document([
853+
'$id' => 'b',
854+
'v' => 0,
855+
'$permissions' => [
856+
Permission::update(Role::any())
857+
]
858+
]));
859+
860+
// d1 adds write, d2 removes update
861+
$d1->setAttribute('$permissions', [
862+
Permission::read(Role::any()),
863+
Permission::update(Role::any())
864+
]);
865+
$d2->setAttribute('$permissions', [
866+
Permission::read(Role::any())
867+
]);
868+
869+
$db->createOrUpdateDocuments(__FUNCTION__, [$d1, $d2]);
870+
871+
$this->assertEquals([
872+
Permission::read(Role::any()),
873+
Permission::update(Role::any()),
874+
], $db->getDocument(__FUNCTION__, 'a')->getPermissions());
875+
876+
$this->assertEquals([
877+
Permission::read(Role::any()),
878+
], $db->getDocument(__FUNCTION__, 'b')->getPermissions());
879+
}
880+
780881
public function testRespectNulls(): Document
781882
{
782883
/** @var Database $database */

0 commit comments

Comments
 (0)