From 9ff077cf1be96ca667ab9ea26e6485214cd76159 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 29 May 2025 21:20:23 +1200 Subject: [PATCH 1/4] Remove internal ID on upsert as creates will never contain it --- src/Database/Database.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 0c7dfbb41..6f8737b01 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4974,7 +4974,8 @@ public function createOrUpdateDocumentsWithIncrease( $document ->setAttribute('$id', empty($document->getId()) ? ID::unique() : $document->getId()) ->setAttribute('$collection', $collection->getId()) - ->setAttribute('$updatedAt', empty($updatedAt) || !$this->preserveDates ? $time : $updatedAt); + ->setAttribute('$updatedAt', empty($updatedAt) || !$this->preserveDates ? $time : $updatedAt) + ->removeAttribute('$internalId'); if ($old->isEmpty()) { $createdAt = $document->getCreatedAt(); From 4756f2249075fa98eac0f78ab3055015364df9b1 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 29 May 2025 21:24:21 +1200 Subject: [PATCH 2/4] Force include optional attributes on upsert --- src/Database/Database.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 6f8737b01..e3b965528 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4916,8 +4916,8 @@ public function createOrUpdateDocumentsWithIncrease( $batchSize = \min(Database::INSERT_BATCH_SIZE, \max(1, $batchSize)); $collection = $this->silent(fn () => $this->getCollection($collection)); $documentSecurity = $collection->getAttribute('documentSecurity', false); + $collectionAttributes = $collection->getAttribute('attributes', []); $time = DateTime::now(); - $created = 0; $updated = 0; @@ -4980,6 +4980,14 @@ public function createOrUpdateDocumentsWithIncrease( if ($old->isEmpty()) { $createdAt = $document->getCreatedAt(); $document->setAttribute('$createdAt', empty($createdAt) || !$this->preserveDates ? $time : $createdAt); + + // Force matching optional parameter sets + // Doesn't use decode as that intentionally skips null defaults to reduce payload size + foreach ($collectionAttributes as $attr) { + if (!$attr->getAttribute('required') && !$document->isSet($attr['$id'])) { + $document->setAttribute($attr['$id'], $attr['default'] ?? null); + } + } } else { $document['$createdAt'] = $old->getCreatedAt(); } From 477cb44634472c1edb926ae211f9d12dde9b2765 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 29 May 2025 21:24:28 +1200 Subject: [PATCH 3/4] Tests --- tests/e2e/Adapter/Scopes/DocumentTests.php | 96 ++++++++++++++++++++-- 1 file changed, 90 insertions(+), 6 deletions(-) diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 35ab3d191..196079ac8 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -322,7 +322,7 @@ public function testCreateDocumentsWithDifferentAttributes(): void static::getDatabase()->deleteCollection($collection); } - public function testCreateOrUpdateDocuments(): void + public function testUpsertDocuments(): void { if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) { $this->expectNotToPerformAssertions(); @@ -362,9 +362,13 @@ public function testCreateOrUpdateDocuments(): void ]; $results = []; - $count = static::getDatabase()->createOrUpdateDocuments(__FUNCTION__, $documents, onNext: function ($doc) use (&$results) { - $results[] = $doc; - }); + $count = static::getDatabase()->createOrUpdateDocuments( + __FUNCTION__, + $documents, + onNext: function ($doc) use (&$results) { + $results[] = $doc; + } + ); $this->assertEquals(2, $count); @@ -432,7 +436,7 @@ public function testCreateOrUpdateDocuments(): void } } - public function testCreateOrUpdateDocumentsInc(): void + public function testUpsertDocumentsInc(): void { if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) { $this->expectNotToPerformAssertions(); @@ -501,7 +505,7 @@ public function testCreateOrUpdateDocumentsInc(): void } } - public function testCreateOrUpdateDocumentsPermissions(): void + public function testUpsertDocumentsPermissions(): void { if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) { $this->expectNotToPerformAssertions(); @@ -587,6 +591,86 @@ public function testCreateOrUpdateDocumentsPermissions(): void $this->assertEquals($newPermissions, $document->getPermissions()); } + public function testUpsertDocumentsAttributeMismatch(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForUpserts()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection(__FUNCTION__, permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], documentSecurity: false); + $database->createAttribute(__FUNCTION__, 'first', Database::VAR_STRING, 128, true); + $database->createAttribute(__FUNCTION__, 'last', Database::VAR_STRING, 128, false); + + $existingDocument = $database->createDocument(__FUNCTION__, new Document([ + '$id' => 'first', + 'first' => 'first', + 'last' => 'last', + ])); + + $newDocument = new Document([ + '$id' => 'second', + 'first' => 'second', + ]); + + $docs = $database->createOrUpdateDocuments(__FUNCTION__, [ + $existingDocument->setAttribute('first', 'updated'), + $newDocument, + ]); + + $this->assertEquals(2, $docs); + $this->assertEquals('updated', $existingDocument->getAttribute('first')); + $this->assertEquals('second', $newDocument->getAttribute('first')); + $this->assertEquals('', $newDocument->getAttribute('last')); + + try { + $database->createOrUpdateDocuments(__FUNCTION__, [ + $existingDocument->removeAttribute('first'), + $newDocument + ]); + $this->fail('Failed to throw exception'); + } catch (Throwable $e) { + $this->assertTrue($e instanceof StructureException, $e->getMessage()); + } + } + + public function testUpsertDocumentsNoop(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) { + $this->expectNotToPerformAssertions(); + return; + } + + static::getDatabase()->createCollection(__FUNCTION__); + static::getDatabase()->createAttribute(__FUNCTION__, 'string', Database::VAR_STRING, 128, true); + + $document = new Document([ + '$id' => 'first', + 'string' => 'textđź“ť', + '$permissions' => [ + Permission::read(Role::any()), + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + ]); + + $count = static::getDatabase()->createOrUpdateDocuments(__FUNCTION__, [$document]); + $this->assertEquals(1, $count); + + // No changes, should return 0 + $count = static::getDatabase()->createOrUpdateDocuments(__FUNCTION__, [$document]); + $this->assertEquals(0, $count); + } + public function testRespectNulls(): Document { static::getDatabase()->createCollection('documents_nulls'); From da31affbe7e4ad88fbc5492bc89c50b639786726 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 29 May 2025 22:10:56 +1200 Subject: [PATCH 4/4] Handle reverse missing optional cases --- src/Database/Database.php | 19 +++++++------ tests/e2e/Adapter/Scopes/DocumentTests.php | 32 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index e3b965528..1ebfc1d8b 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4980,18 +4980,21 @@ public function createOrUpdateDocumentsWithIncrease( if ($old->isEmpty()) { $createdAt = $document->getCreatedAt(); $document->setAttribute('$createdAt', empty($createdAt) || !$this->preserveDates ? $time : $createdAt); - - // Force matching optional parameter sets - // Doesn't use decode as that intentionally skips null defaults to reduce payload size - foreach ($collectionAttributes as $attr) { - if (!$attr->getAttribute('required') && !$document->isSet($attr['$id'])) { - $document->setAttribute($attr['$id'], $attr['default'] ?? null); - } - } } else { $document['$createdAt'] = $old->getCreatedAt(); } + // Force matching optional parameter sets + // Doesn't use decode as that intentionally skips null defaults to reduce payload size + foreach ($collectionAttributes as $attr) { + if (!$attr->getAttribute('required') && !\array_key_exists($attr['$id'], (array)$document)) { + $document->setAttribute( + $attr['$id'], + $old->getAttribute($attr['$id'], ($attr['default'] ?? null)) + ); + } + } + if (!$updatesPermissions) { $document->setAttribute('$permissions', $old->getPermissions()); } diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 196079ac8..30fcbda9e 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -621,6 +621,7 @@ public function testUpsertDocumentsAttributeMismatch(): void 'first' => 'second', ]); + // Ensure missing optionals on new document is allowed $docs = $database->createOrUpdateDocuments(__FUNCTION__, [ $existingDocument->setAttribute('first', 'updated'), $newDocument, @@ -628,6 +629,7 @@ public function testUpsertDocumentsAttributeMismatch(): void $this->assertEquals(2, $docs); $this->assertEquals('updated', $existingDocument->getAttribute('first')); + $this->assertEquals('last', $existingDocument->getAttribute('last')); $this->assertEquals('second', $newDocument->getAttribute('first')); $this->assertEquals('', $newDocument->getAttribute('last')); @@ -640,6 +642,36 @@ public function testUpsertDocumentsAttributeMismatch(): void } catch (Throwable $e) { $this->assertTrue($e instanceof StructureException, $e->getMessage()); } + + // Ensure missing optionals on existing document is allowed + $docs = $database->createOrUpdateDocuments(__FUNCTION__, [ + $existingDocument + ->setAttribute('first', 'first') + ->removeAttribute('last'), + $newDocument + ->setAttribute('last', 'last') + ]); + + $this->assertEquals(2, $docs); + $this->assertEquals('first', $existingDocument->getAttribute('first')); + $this->assertEquals('last', $existingDocument->getAttribute('last')); + $this->assertEquals('second', $newDocument->getAttribute('first')); + $this->assertEquals('last', $newDocument->getAttribute('last')); + + // Ensure set null on existing document is allowed + $docs = $database->createOrUpdateDocuments(__FUNCTION__, [ + $existingDocument + ->setAttribute('first', 'first') + ->setAttribute('last', null), + $newDocument + ->setAttribute('last', 'last') + ]); + + $this->assertEquals(1, $docs); + $this->assertEquals('first', $existingDocument->getAttribute('first')); + $this->assertEquals(null, $existingDocument->getAttribute('last')); + $this->assertEquals('second', $newDocument->getAttribute('first')); + $this->assertEquals('last', $newDocument->getAttribute('last')); } public function testUpsertDocumentsNoop(): void