From 0598fe76602166d58381d62a39be502015ce2f2a Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Mon, 27 Oct 2025 14:18:55 +0530 Subject: [PATCH 1/3] fix - update document replacing the exisint document instead of partial updates in case of schemaless --- src/Database/Adapter/Mongo.php | 4 +- src/Database/Database.php | 22 ++++++++- tests/e2e/Adapter/Scopes/DocumentTests.php | 6 ++- tests/e2e/Adapter/Scopes/SchemalessTests.php | 49 ++++++++++++++++++-- 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 82afb0eaa..9ce134b9a 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -1447,9 +1447,9 @@ public function updateDocument(Document $collection, string $id, Document $docum unset($record['_id']); // Don't update _id $options = $this->getTransactionOptions(); - $updateQuery = [ + $updateQuery = $this->getSupportForAttributes() ? [ '$set' => $record, - ]; + ] : $record; $this->client->update($name, $filters, $updateQuery, $options); } catch (MongoException $e) { throw $this->processException($e); diff --git a/src/Database/Database.php b/src/Database/Database.php index 1c6c5de0a..b9308a7fe 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4913,8 +4913,15 @@ public function updateDocument(string $collection, string $id, Document $documen $skipPermissionsUpdate = ($originalPermissions === $currentPermissions); } $createdAt = $document->getCreatedAt(); - - $document = \array_merge($old->getArrayCopy(), $document->getArrayCopy()); + if ($this->adapter->getSupportForAttributes()) { + $document = \array_merge($old->getArrayCopy(), $document->getArrayCopy()); + } else { + $oldArray = $old->getArrayCopy(); + $newArray = $document->getArrayCopy(); + $internalKeys = array_map(fn ($attr) => $attr['$id'], self::INTERNAL_ATTRIBUTES); + $internalAttrs = array_intersect_key($oldArray, array_flip($internalKeys)); + $document = array_merge($internalAttrs, $newArray); + } $document['$collection'] = $old->getAttribute('$collection'); // Make sure user doesn't switch collection ID $document['$createdAt'] = ($createdAt === null || !$this->preserveDates) ? $old->getCreatedAt() : $createdAt; @@ -5022,6 +5029,17 @@ public function updateDocument(string $collection, string $id, Document $documen } } + // to check addition and removal of fields in case of schemaless + if (!$this->adapter->getSupportForAttributes()) { + $internalFields = array_map(fn ($attr) => $attr['$id'], self::INTERNAL_ATTRIBUTES); + + $oldKeys = array_keys(array_diff_key($old->getArrayCopy(), array_flip($internalFields))); + $newKeys = array_keys(array_diff_key($document->getArrayCopy(), array_flip($internalFields))); + if (count($oldKeys) !== count($newKeys) || array_diff($oldKeys, $newKeys)) { + $shouldUpdate = true; + } + } + $updatePermissions = [ ...$collection->getUpdate(), ...($documentSecurity ? $old->getUpdate() : []) diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 311a026d9..36f2d9103 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -6561,7 +6561,11 @@ public function testUpdateDocumentSuccessiveCallsDoNotResetDefaults(): void { /** @var Database $database */ $database = static::getDatabase(); - + // // skipping as schemaless doesn't look to the attributes metadata + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } $collectionId = 'successive_update_single'; Authorization::cleanRoles(); Authorization::setRole(Role::any()->toString()); diff --git a/tests/e2e/Adapter/Scopes/SchemalessTests.php b/tests/e2e/Adapter/Scopes/SchemalessTests.php index ee0985682..01c6664d8 100644 --- a/tests/e2e/Adapter/Scopes/SchemalessTests.php +++ b/tests/e2e/Adapter/Scopes/SchemalessTests.php @@ -345,8 +345,8 @@ public function testSchemalessUpdateDocumentWithQuery(): void $this->assertEquals('updated', $updatedDoc->getAttribute('status')); $this->assertEquals('2023-01-01', $updatedDoc->getAttribute('lastModified')); $this->assertEquals('added', $updatedDoc->getAttribute('newAttribute')); - $this->assertEquals('user', $updatedDoc->getAttribute('type')); // Existing attributes preserved - $this->assertEquals(100, $updatedDoc->getAttribute('score')); + $this->assertArrayNotHasKey('score', $updatedDoc); + $this->assertArrayNotHasKey('type', $updatedDoc); $retrievedDoc = $database->getDocument($colName, 'doc1'); $this->assertEquals('updated', $retrievedDoc->getAttribute('status')); @@ -361,7 +361,7 @@ public function testSchemalessUpdateDocumentWithQuery(): void $this->assertEquals('value1', $updatedDoc2->getAttribute('customField1')); $this->assertEquals(42, $updatedDoc2->getAttribute('customField2')); $this->assertEquals(['array', 'of', 'values'], $updatedDoc2->getAttribute('customField3')); - $this->assertEquals('admin', $updatedDoc2->getAttribute('type')); // Original attributes preserved + $this->assertArrayNotHasKey('type', $updatedDoc2); $database->deleteCollection($colName); } @@ -1155,4 +1155,47 @@ public function testSchemalessDates(): void $database->deleteCollection($col); } + + public function testSchemalessRemoveAttributesByUpdate(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + // in schemaless, if attributes are created and then if values are not provided then they are replaced with the default attribute automatically in the encode + if ($database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + + $col = uniqid('sl_update_remove'); + $database->createCollection($col); + + $permissions = [ + Permission::read(Role::any()), + Permission::write(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()) + ]; + + $single = $database->createDocument($col, new Document(['$id' => 'docS', 'key' => 'single', 'extra' => 'yes', '$permissions' => $permissions])); + $this->assertEquals('docS', $single->getId()); + $this->assertEquals('yes', $single->getAttribute('extra')); + $this->assertEquals('single', $single->getAttribute('key')); + + // before removing attribute + $doc = $database->getDocument($col, 'docS'); + $this->assertEquals('yes', $doc->getAttribute('extra')); + $this->assertEquals('single', $doc->getAttribute('key')); + + // removing attribute + $doc = $database->updateDocument($col, 'docS', new Document(['$id' => 'docS','key' => 'single2'])); + $this->assertEquals('single2', $doc->getAttribute('key')); + $this->assertArrayNotHasKey('extra', $doc); + + $doc = $database->getDocument($col, 'docS'); + $this->assertEquals('single2', $doc->getAttribute('key')); + $this->assertArrayNotHasKey('extra', $doc); + + $database->deleteCollection($col); + } } From 9a1496e7231ee31c07da6e2d1131851be42ce15a Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 29 Oct 2025 17:48:12 +0530 Subject: [PATCH 2/3] added bulk update support for document replacement in the updateDocuments --- src/Database/Adapter/Mongo.php | 72 +++++++++++++++++--- src/Database/Database.php | 33 ++++++--- tests/e2e/Adapter/Scopes/DocumentTests.php | 16 ++++- tests/e2e/Adapter/Scopes/SchemalessTests.php | 64 +++++++++++++++++ 4 files changed, 163 insertions(+), 22 deletions(-) diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 9ce134b9a..cc9dc1279 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -1445,11 +1445,12 @@ public function updateDocument(Document $collection, string $id, Document $docum try { unset($record['_id']); // Don't update _id - $options = $this->getTransactionOptions(); - $updateQuery = $this->getSupportForAttributes() ? [ - '$set' => $record, - ] : $record; + $updates = match (true) { + $this->getSupportForAttributes() => ['$set' => $record], + default => $record, + }; + $updateQuery = $updates; $this->client->update($name, $filters, $updateQuery, $options); } catch (MongoException $e) { throw $this->processException($e); @@ -1486,20 +1487,23 @@ public function updateDocuments(Document $collection, Document $updates, array $ $filters['_tenant'] = $this->getTenantFilters($collection->getId()); } - $record = $updates->getArrayCopy(); - $record = $this->replaceChars('$', '_', $record); - - $updateQuery = [ - '$set' => $record, - ]; + $record = $this->replaceChars('$', '_', $updates->getArrayCopy()); try { + if (!$this->getSupportForAttributes()) { + return $this->updateDocumentsForSchemaless($name, $documents, $record, $filters, $options); + } + + $updateQuery = [ + '$set' => $record, + ]; + return $this->client->update( $name, $filters, $updateQuery, options: $options, - multi: true, + multi: true ); } catch (MongoException $e) { throw $this->processException($e); @@ -3189,4 +3193,50 @@ public function getTenantQuery(string $collection, string $alias = ''): string { return ''; } + + /** + * @param string $collection + * @param array $oldDocuments + * @param array $updates + * @param array $filters + * @param array $options + * @return int + */ + private function updateDocumentsForSchemaless(string $collection, array $oldDocuments, array $updates, array $filters, array $options): int + { + $internalKeys = array_map( + fn ($attr) => str_replace('$', '_', $attr['$id']), + Database::INTERNAL_ATTRIBUTES + ); + $unsetUnion = []; + + foreach ($oldDocuments as $doc) { + if ($doc->offsetExists('$skipPermissionsUpdate')) { + $doc->removeAttribute('$skipPermissionsUpdate'); + } + $attrs = $doc->getAttributes(); + $attributes = $this->replaceChars('$', '_', $attrs); + foreach (array_keys($attributes) as $attributeKey) { + if (!array_key_exists($attributeKey, $updates) && !in_array($attributeKey, $internalKeys, true)) { + $unsetUnion[$attributeKey] = 1; + } + } + } + + $updateQuery = [ + '$set' => $updates, + ]; + + if (!empty($unsetUnion)) { + $updateQuery['$unset'] = $unsetUnion; + } + + return $this->client->update( + $collection, + $filters, + $updateQuery, + options: $options, + multi: true + ); + } } diff --git a/src/Database/Database.php b/src/Database/Database.php index b9308a7fe..49c1a0cae 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4913,15 +4913,7 @@ public function updateDocument(string $collection, string $id, Document $documen $skipPermissionsUpdate = ($originalPermissions === $currentPermissions); } $createdAt = $document->getCreatedAt(); - if ($this->adapter->getSupportForAttributes()) { - $document = \array_merge($old->getArrayCopy(), $document->getArrayCopy()); - } else { - $oldArray = $old->getArrayCopy(); - $newArray = $document->getArrayCopy(); - $internalKeys = array_map(fn ($attr) => $attr['$id'], self::INTERNAL_ATTRIBUTES); - $internalAttrs = array_intersect_key($oldArray, array_flip($internalKeys)); - $document = array_merge($internalAttrs, $newArray); - } + $document = $this->mergeDocuments($old, $document); $document['$collection'] = $old->getAttribute('$collection'); // Make sure user doesn't switch collection ID $document['$createdAt'] = ($createdAt === null || !$this->preserveDates) ? $old->getCreatedAt() : $createdAt; @@ -8378,4 +8370,27 @@ protected function encodeSpatialData(mixed $value, string $type): string throw new DatabaseException('Unknown spatial type: ' . $type); } } + + /** + * @param Document $old + * @param Document $new + * @return array + */ + protected function mergeDocuments(Document $old, Document $new): array + { + switch (true) { + case $this->adapter->getSupportForAttributes(): + return \array_merge($old->getArrayCopy(), $new->getArrayCopy()); + + // schemaless behaviour + default: + $oldArray = $old->getArrayCopy(); + $newArray = $new->getArrayCopy(); + + $internalKeys = array_map(fn ($attr) => $attr['$id'], self::INTERNAL_ATTRIBUTES); + $internalAttrs = array_intersect_key($oldArray, array_flip($internalKeys)); + + return array_merge($internalAttrs, $newArray); + } + } } diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 36f2d9103..0e56d6f57 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -6170,14 +6170,26 @@ public function testCreateUpdateDocumentsMismatch(): void $this->assertEquals(2, $database->updateDocuments($colName, new Document(['key' => 'new doc']))); $doc = $database->getDocument($colName, 'doc4'); $this->assertEquals('doc4', $doc->getId()); - $this->assertEquals('value', $doc->getAttribute('value')); + if (!$database->getAdapter()->getSupportForAttributes()) { + // not assertArrayNotHasKey as createAttribute is done previously + // value would be returned in attributes but schemaless updateDocuments do replace the old document with new document + // so null + $this->assertNull($doc->getAttribute('value')); + } else { + $this->assertEquals('value', $doc->getAttribute('value')); + + } $addedDocs = $database->find($colName); $this->assertCount(2, $addedDocs); foreach ($addedDocs as $doc) { $this->assertNotEmpty($doc->getPermissions()); $this->assertCount(3, $doc->getPermissions()); - $this->assertEquals('value', $doc->getAttribute('value')); + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->assertNull($doc->getAttribute('value')); + } else { + $this->assertEquals('value', $doc->getAttribute('value')); + } } $database->deleteCollection($colName); } diff --git a/tests/e2e/Adapter/Scopes/SchemalessTests.php b/tests/e2e/Adapter/Scopes/SchemalessTests.php index 01c6664d8..6d2638ecd 100644 --- a/tests/e2e/Adapter/Scopes/SchemalessTests.php +++ b/tests/e2e/Adapter/Scopes/SchemalessTests.php @@ -1196,6 +1196,70 @@ public function testSchemalessRemoveAttributesByUpdate(): void $this->assertEquals('single2', $doc->getAttribute('key')); $this->assertArrayNotHasKey('extra', $doc); + // Test updateDocuments - bulk update with attribute removal + $docs = [ + new Document(['$id' => 'docA', 'key' => 'keepA', 'extra' => 'removeA', '$permissions' => $permissions]), + new Document(['$id' => 'docB', 'key' => 'keepB', 'extra' => 'removeB', 'more' => 'data', '$permissions' => $permissions]), + new Document(['$id' => 'docC', 'key' => 'keepC', 'extra' => 'removeC', '$permissions' => $permissions]), + ]; + $this->assertEquals(3, $database->createDocuments($col, $docs)); + + // Verify all documents have both 'key' and 'extra' + $allDocs = $database->find($col); + $this->assertCount(4, $allDocs); // 3 new + 1 old (docS) + + foreach ($allDocs as $doc) { + if (in_array($doc->getId(), ['docA', 'docB', 'docC'])) { + $this->assertArrayHasKey('extra', $doc->getAttributes()); + } + } + + // Bulk update all new docs: keep 'key', remove 'extra' and 'more' + $updatedCount = $database->updateDocuments($col, new Document(['key' => 'updatedBulk']), [Query::startsWith('$id', 'doc'),Query::notEqual('$id', 'docS')]); + $this->assertEquals(3, $updatedCount); + + // Verify 'extra' and 'more' fields are removed from all updated docs + $updatedDocs = $database->find($col, [Query::startsWith('$id', 'doc'),Query::notEqual('$id', 'docS')]); + $this->assertCount(3, $updatedDocs); + + foreach ($updatedDocs as $doc) { + $this->assertEquals('updatedBulk', $doc->getAttribute('key')); + $this->assertArrayNotHasKey('extra', $doc->getAttributes()); + if ($doc->getId() === 'docB') { + $this->assertArrayNotHasKey('more', $doc->getAttributes()); + } + } + + // Verify docS (not in update query) is unchanged + $docS = $database->getDocument($col, 'docS'); + $this->assertEquals('single2', $docS->getAttribute('key')); + $this->assertArrayNotHasKey('extra', $docS); + + // Update documents with query filter - partial update + $database->updateDocuments( + $col, + new Document(['label' => 'tagged']), + [Query::equal('$id', ['docA', 'docC'])] + ); + + $docA = $database->getDocument($col, 'docA'); + $docB = $database->getDocument($col, 'docB'); + $docC = $database->getDocument($col, 'docC'); + + $this->assertEquals('tagged', $docA->getAttribute('label')); + $this->assertArrayNotHasKey('label', $docB->getAttributes()); + $this->assertEquals('tagged', $docC->getAttribute('label')); + + // Verify 'key' is still preserved in docB and not in others + $this->assertEquals('updatedBulk', $docB->getAttribute('key')); + + foreach (['docA','docC'] as $doc) { + $this->assertArrayNotHasKey('key', $database->getDocument($col, $doc)->getAttributes()); + } + + // verify docs is still preserved(untouched) + $docS = $database->getDocument($col, 'docS'); + $this->assertEquals('single2', $docS->getAttribute('key')); $database->deleteCollection($col); } } From ef4809c625d36fa9993305234e38bd8e3492306c Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 29 Oct 2025 18:02:38 +0530 Subject: [PATCH 3/3] added few more tests for edge cases --- tests/e2e/Adapter/Scopes/SchemalessTests.php | 63 ++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/e2e/Adapter/Scopes/SchemalessTests.php b/tests/e2e/Adapter/Scopes/SchemalessTests.php index 6d2638ecd..6cbb7964c 100644 --- a/tests/e2e/Adapter/Scopes/SchemalessTests.php +++ b/tests/e2e/Adapter/Scopes/SchemalessTests.php @@ -1260,6 +1260,69 @@ public function testSchemalessRemoveAttributesByUpdate(): void // verify docs is still preserved(untouched) $docS = $database->getDocument($col, 'docS'); $this->assertEquals('single2', $docS->getAttribute('key')); + + // 1) Null Value Preservation vs Removal + $docNull = $database->createDocument($col, new Document([ + '$id' => 'docNull', + '$permissions' => $permissions, + 'keepMe' => 'k1', + 'removeMe' => 'r1', + 'nullable' => 'n1' + ])); + $this->assertEquals('docNull', $docNull->getId()); + + $docNullUpdated = $database->updateDocument($col, 'docNull', new Document([ + 'keepMe' => 'k2', + 'nullable' => null // explicitly set null should be preserved as key with null value + ])); + $this->assertEquals('k2', $docNullUpdated->getAttribute('keepMe')); + $this->assertArrayHasKey('nullable', $docNullUpdated->getAttributes()); + $this->assertNull($docNullUpdated->getAttribute('nullable')); + $this->assertArrayNotHasKey('removeMe', $docNullUpdated->getAttributes()); + + $docNullRefetch = $database->getDocument($col, 'docNull'); + $this->assertEquals('k2', $docNullRefetch->getAttribute('keepMe')); + $this->assertArrayHasKey('nullable', $docNullRefetch->getAttributes()); + $this->assertNull($docNullRefetch->getAttribute('nullable')); + $this->assertArrayNotHasKey('removeMe', $docNullRefetch->getAttributes()); + + // 2) Internal attributes preservation ($id, $sequence, $permissions) + $before = $database->getDocument($col, 'docS'); + $this->assertEquals('docS', $before->getId()); + $beforeSequence = $before->getSequence(); + $beforePermissions = $before->getPermissions(); + $this->assertNotEmpty($beforeSequence); + $this->assertGreaterThanOrEqual(1, count($beforePermissions)); + + $after = $database->updateDocument($col, 'docS', new Document(['key' => 'single3'])); + $this->assertEquals('docS', $after->getId()); + $this->assertEquals('single3', $after->getAttribute('key')); + $this->assertEquals($beforeSequence, $after->getSequence()); + foreach ($beforePermissions as $perm) { + $this->assertContains($perm, $after->getPermissions()); + } + + $afterRefetch = $database->getDocument($col, 'docS'); + $this->assertEquals('docS', $afterRefetch->getId()); + $this->assertEquals($beforeSequence, $afterRefetch->getSequence()); + foreach ($beforePermissions as $perm) { + $this->assertContains($perm, $afterRefetch->getPermissions()); + } + + // 3) Update with empty document (removes all non-internal attributes) + $noOp = $database->updateDocument($col, 'docS', new Document([])); + $this->assertEquals('docS', $noOp->getId()); + $this->assertArrayNotHasKey('key', $noOp->getAttributes()); + + $noOpRefetch = $database->getDocument($col, 'docS'); + $this->assertArrayNotHasKey('key', $noOpRefetch->getAttributes()); + // Internal attributes should still be present + $this->assertEquals('docS', $noOpRefetch->getId()); + $this->assertNotEmpty($noOpRefetch->getSequence()); + $this->assertEquals($col, $noOpRefetch->getCollection()); + $this->assertTrue(is_string($noOpRefetch->getAttribute('$createdAt'))); + $this->assertTrue(is_string($noOpRefetch->getAttribute('$updatedAt'))); + $this->assertGreaterThanOrEqual(1, count($noOpRefetch->getPermissions())); $database->deleteCollection($col); } }