Skip to content

Commit 42bfc0d

Browse files
abnegateabnegate
authored
Merge pull request #598 from utopia-php/fix-upsert-mismatch
Fix upsert mismatch
2 parents 4abe536 + da31aff commit 42bfc0d

File tree

2 files changed

+136
-8
lines changed

2 files changed

+136
-8
lines changed

‎src/Database/Database.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4916,8 +4916,8 @@ public function createOrUpdateDocumentsWithIncrease(
49164916
$batchSize = \min(Database::INSERT_BATCH_SIZE, \max(1, $batchSize));
49174917
$collection = $this->silent(fn () => $this->getCollection($collection));
49184918
$documentSecurity = $collection->getAttribute('documentSecurity', false);
4919+
$collectionAttributes = $collection->getAttribute('attributes', []);
49194920
$time = DateTime::now();
4920-
49214921
$created = 0;
49224922
$updated = 0;
49234923

@@ -4974,7 +4974,8 @@ public function createOrUpdateDocumentsWithIncrease(
49744974
$document
49754975
->setAttribute('$id', empty($document->getId()) ? ID::unique() : $document->getId())
49764976
->setAttribute('$collection', $collection->getId())
4977-
->setAttribute('$updatedAt', empty($updatedAt) || !$this->preserveDates ? $time : $updatedAt);
4977+
->setAttribute('$updatedAt', empty($updatedAt) || !$this->preserveDates ? $time : $updatedAt)
4978+
->removeAttribute('$internalId');
49784979

49794980
if ($old->isEmpty()) {
49804981
$createdAt = $document->getCreatedAt();
@@ -4983,6 +4984,17 @@ public function createOrUpdateDocumentsWithIncrease(
49834984
$document['$createdAt'] = $old->getCreatedAt();
49844985
}
49854986

4987+
// Force matching optional parameter sets
4988+
// Doesn't use decode as that intentionally skips null defaults to reduce payload size
4989+
foreach ($collectionAttributes as $attr) {
4990+
if (!$attr->getAttribute('required') && !\array_key_exists($attr['$id'], (array)$document)) {
4991+
$document->setAttribute(
4992+
$attr['$id'],
4993+
$old->getAttribute($attr['$id'], ($attr['default'] ?? null))
4994+
);
4995+
}
4996+
}
4997+
49864998
if (!$updatesPermissions) {
49874999
$document->setAttribute('$permissions', $old->getPermissions());
49885000
}

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

Lines changed: 122 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ public function testCreateDocumentsWithDifferentAttributes(): void
322322
static::getDatabase()->deleteCollection($collection);
323323
}
324324

325-
public function testCreateOrUpdateDocuments(): void
325+
public function testUpsertDocuments(): void
326326
{
327327
if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) {
328328
$this->expectNotToPerformAssertions();
@@ -362,9 +362,13 @@ public function testCreateOrUpdateDocuments(): void
362362
];
363363

364364
$results = [];
365-
$count = static::getDatabase()->createOrUpdateDocuments(__FUNCTION__, $documents, onNext: function ($doc) use (&$results) {
366-
$results[] = $doc;
367-
});
365+
$count = static::getDatabase()->createOrUpdateDocuments(
366+
__FUNCTION__,
367+
$documents,
368+
onNext: function ($doc) use (&$results) {
369+
$results[] = $doc;
370+
}
371+
);
368372

369373
$this->assertEquals(2, $count);
370374

@@ -432,7 +436,7 @@ public function testCreateOrUpdateDocuments(): void
432436
}
433437
}
434438

435-
public function testCreateOrUpdateDocumentsInc(): void
439+
public function testUpsertDocumentsInc(): void
436440
{
437441
if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) {
438442
$this->expectNotToPerformAssertions();
@@ -501,7 +505,7 @@ public function testCreateOrUpdateDocumentsInc(): void
501505
}
502506
}
503507

504-
public function testCreateOrUpdateDocumentsPermissions(): void
508+
public function testUpsertDocumentsPermissions(): void
505509
{
506510
if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) {
507511
$this->expectNotToPerformAssertions();
@@ -587,6 +591,118 @@ public function testCreateOrUpdateDocumentsPermissions(): void
587591
$this->assertEquals($newPermissions, $document->getPermissions());
588592
}
589593

594+
public function testUpsertDocumentsAttributeMismatch(): void
595+
{
596+
/** @var Database $database */
597+
$database = static::getDatabase();
598+
599+
if (!$database->getAdapter()->getSupportForUpserts()) {
600+
$this->expectNotToPerformAssertions();
601+
return;
602+
}
603+
604+
$database->createCollection(__FUNCTION__, permissions: [
605+
Permission::create(Role::any()),
606+
Permission::read(Role::any()),
607+
Permission::update(Role::any()),
608+
Permission::delete(Role::any()),
609+
], documentSecurity: false);
610+
$database->createAttribute(__FUNCTION__, 'first', Database::VAR_STRING, 128, true);
611+
$database->createAttribute(__FUNCTION__, 'last', Database::VAR_STRING, 128, false);
612+
613+
$existingDocument = $database->createDocument(__FUNCTION__, new Document([
614+
'$id' => 'first',
615+
'first' => 'first',
616+
'last' => 'last',
617+
]));
618+
619+
$newDocument = new Document([
620+
'$id' => 'second',
621+
'first' => 'second',
622+
]);
623+
624+
// Ensure missing optionals on new document is allowed
625+
$docs = $database->createOrUpdateDocuments(__FUNCTION__, [
626+
$existingDocument->setAttribute('first', 'updated'),
627+
$newDocument,
628+
]);
629+
630+
$this->assertEquals(2, $docs);
631+
$this->assertEquals('updated', $existingDocument->getAttribute('first'));
632+
$this->assertEquals('last', $existingDocument->getAttribute('last'));
633+
$this->assertEquals('second', $newDocument->getAttribute('first'));
634+
$this->assertEquals('', $newDocument->getAttribute('last'));
635+
636+
try {
637+
$database->createOrUpdateDocuments(__FUNCTION__, [
638+
$existingDocument->removeAttribute('first'),
639+
$newDocument
640+
]);
641+
$this->fail('Failed to throw exception');
642+
} catch (Throwable $e) {
643+
$this->assertTrue($e instanceof StructureException, $e->getMessage());
644+
}
645+
646+
// Ensure missing optionals on existing document is allowed
647+
$docs = $database->createOrUpdateDocuments(__FUNCTION__, [
648+
$existingDocument
649+
->setAttribute('first', 'first')
650+
->removeAttribute('last'),
651+
$newDocument
652+
->setAttribute('last', 'last')
653+
]);
654+
655+
$this->assertEquals(2, $docs);
656+
$this->assertEquals('first', $existingDocument->getAttribute('first'));
657+
$this->assertEquals('last', $existingDocument->getAttribute('last'));
658+
$this->assertEquals('second', $newDocument->getAttribute('first'));
659+
$this->assertEquals('last', $newDocument->getAttribute('last'));
660+
661+
// Ensure set null on existing document is allowed
662+
$docs = $database->createOrUpdateDocuments(__FUNCTION__, [
663+
$existingDocument
664+
->setAttribute('first', 'first')
665+
->setAttribute('last', null),
666+
$newDocument
667+
->setAttribute('last', 'last')
668+
]);
669+
670+
$this->assertEquals(1, $docs);
671+
$this->assertEquals('first', $existingDocument->getAttribute('first'));
672+
$this->assertEquals(null, $existingDocument->getAttribute('last'));
673+
$this->assertEquals('second', $newDocument->getAttribute('first'));
674+
$this->assertEquals('last', $newDocument->getAttribute('last'));
675+
}
676+
677+
public function testUpsertDocumentsNoop(): void
678+
{
679+
if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) {
680+
$this->expectNotToPerformAssertions();
681+
return;
682+
}
683+
684+
static::getDatabase()->createCollection(__FUNCTION__);
685+
static::getDatabase()->createAttribute(__FUNCTION__, 'string', Database::VAR_STRING, 128, true);
686+
687+
$document = new Document([
688+
'$id' => 'first',
689+
'string' => 'text📝',
690+
'$permissions' => [
691+
Permission::read(Role::any()),
692+
Permission::create(Role::any()),
693+
Permission::update(Role::any()),
694+
Permission::delete(Role::any()),
695+
],
696+
]);
697+
698+
$count = static::getDatabase()->createOrUpdateDocuments(__FUNCTION__, [$document]);
699+
$this->assertEquals(1, $count);
700+
701+
// No changes, should return 0
702+
$count = static::getDatabase()->createOrUpdateDocuments(__FUNCTION__, [$document]);
703+
$this->assertEquals(0, $count);
704+
}
705+
590706
public function testRespectNulls(): Document
591707
{
592708
static::getDatabase()->createCollection('documents_nulls');

0 commit comments

Comments
 (0)