diff --git a/src/Database/Database.php b/src/Database/Database.php index 0c7dfbb41..1ebfc1d8b 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; @@ -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(); @@ -4983,6 +4984,17 @@ public function createOrUpdateDocumentsWithIncrease( $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 35ab3d191..30fcbda9e 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,118 @@ 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', + ]); + + // Ensure missing optionals on new document is allowed + $docs = $database->createOrUpdateDocuments(__FUNCTION__, [ + $existingDocument->setAttribute('first', 'updated'), + $newDocument, + ]); + + $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')); + + try { + $database->createOrUpdateDocuments(__FUNCTION__, [ + $existingDocument->removeAttribute('first'), + $newDocument + ]); + $this->fail('Failed to throw exception'); + } 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 + { + 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');