Skip to content

Commit 8e5038a

Browse files
abnegateabnegate
authored
Merge pull request #621 from utopia-php/dat-573
Add error handling for document updates with onError callback
2 parents fd24f36 + 48eec93 commit 8e5038a

7 files changed

Lines changed: 459 additions & 21 deletions

File tree

‎src/Database/Database.php‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Utopia\Database;
44

55
use Exception;
6+
use Throwable;
67
use Utopia\Cache\Cache;
78
use Utopia\CLI\Console;
89
use Utopia\Database\Exception as DatabaseException;
@@ -4273,6 +4274,7 @@ public function updateDocument(string $collection, string $id, Document $documen
42734274
* @param array<Query> $queries
42744275
* @param int $batchSize
42754276
* @param callable|null $onNext
4277+
* @param callable|null $onError
42764278
* @return int
42774279
* @throws AuthorizationException
42784280
* @throws ConflictException
@@ -4289,6 +4291,7 @@ public function updateDocuments(
42894291
array $queries = [],
42904292
int $batchSize = self::INSERT_BATCH_SIZE,
42914293
?callable $onNext = null,
4294+
?callable $onError = null,
42924295
): int {
42934296
if ($updates->isEmpty()) {
42944297
return 0;
@@ -4389,30 +4392,29 @@ public function updateDocuments(
43894392
break;
43904393
}
43914394

4392-
foreach ($batch as &$document) {
4393-
$new = new Document(\array_merge($document->getArrayCopy(), $updates->getArrayCopy()));
4395+
$this->withTransaction(function () use ($collection, $updates, &$batch) {
4396+
foreach ($batch as &$document) {
4397+
$new = new Document(\array_merge($document->getArrayCopy(), $updates->getArrayCopy()));
43944398

4395-
if ($this->resolveRelationships) {
4396-
$this->silent(fn () => $this->updateDocumentRelationships($collection, $document, $new));
4397-
}
4398-
4399-
$document = $new;
4399+
if ($this->resolveRelationships) {
4400+
$this->silent(fn () => $this->updateDocumentRelationships($collection, $document, $new));
4401+
}
44004402

4401-
// Check if document was updated after the request timestamp
4402-
try {
4403-
$oldUpdatedAt = new \DateTime($document->getUpdatedAt());
4404-
} catch (Exception $e) {
4405-
throw new DatabaseException($e->getMessage(), $e->getCode(), $e);
4406-
}
4403+
$document = $new;
44074404

4408-
if (!is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) {
4409-
throw new ConflictException('Document was updated after the request timestamp');
4410-
}
4405+
// Check if document was updated after the request timestamp
4406+
try {
4407+
$oldUpdatedAt = new \DateTime($document->getUpdatedAt());
4408+
} catch (Exception $e) {
4409+
throw new DatabaseException($e->getMessage(), $e->getCode(), $e);
4410+
}
44114411

4412-
$document = $this->encode($collection, $document);
4413-
}
4412+
if (!is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) {
4413+
throw new ConflictException('Document was updated after the request timestamp');
4414+
}
44144415

4415-
$this->withTransaction(function () use ($collection, $updates, $batch) {
4416+
$document = $this->encode($collection, $document);
4417+
}
44164418
$this->adapter->updateDocuments(
44174419
$collection->getId(),
44184420
$updates,
@@ -4423,7 +4425,11 @@ public function updateDocuments(
44234425
foreach ($batch as $doc) {
44244426
$this->purgeCachedDocument($collection->getId(), $doc->getId());
44254427
$doc = $this->decode($collection, $doc);
4426-
$onNext && $onNext($doc);
4428+
try {
4429+
$onNext && $onNext($doc);
4430+
} catch (Throwable $th) {
4431+
$onError ? $onError($th) : throw $th;
4432+
}
44274433
$modified++;
44284434
}
44294435

‎src/Database/Mirror.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,7 @@ public function updateDocuments(
713713
array $queries = [],
714714
int $batchSize = self::INSERT_BATCH_SIZE,
715715
?callable $onNext = null,
716+
?callable $onError = null,
716717
): int {
717718
$modified = 0;
718719

@@ -724,7 +725,8 @@ public function updateDocuments(
724725
function ($doc) use ($onNext, &$modified) {
725726
$onNext && $onNext($doc);
726727
$modified++;
727-
}
728+
},
729+
$onError
728730
);
729731

730732
if (

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3427,6 +3427,7 @@ public function testUpdateDocument(Document $document): Document
34273427
return $document;
34283428
}
34293429

3430+
34303431
/**
34313432
* @depends testUpdateDocument
34323433
*/
@@ -3691,6 +3692,102 @@ public function testUpdateDocuments(): void
36913692
Authorization::cleanRoles();
36923693
Authorization::setRole(Role::any()->toString());
36933694
}
3695+
3696+
public function testUpdateDocumentsWithCallbackSupport(): void
3697+
{
3698+
/** @var Database $database */
3699+
$database = static::getDatabase();
3700+
3701+
if (!$database->getAdapter()->getSupportForBatchOperations()) {
3702+
$this->expectNotToPerformAssertions();
3703+
return;
3704+
}
3705+
3706+
$collection = 'update_callback';
3707+
Authorization::cleanRoles();
3708+
Authorization::setRole(Role::any()->toString());
3709+
3710+
$database->createCollection($collection, attributes: [
3711+
new Document([
3712+
'$id' => ID::custom('string'),
3713+
'type' => Database::VAR_STRING,
3714+
'format' => '',
3715+
'size' => 100,
3716+
'signed' => true,
3717+
'required' => false,
3718+
'default' => null,
3719+
'array' => false,
3720+
'filters' => [],
3721+
]),
3722+
new Document([
3723+
'$id' => ID::custom('integer'),
3724+
'type' => Database::VAR_INTEGER,
3725+
'format' => '',
3726+
'size' => 10000,
3727+
'signed' => true,
3728+
'required' => false,
3729+
'default' => null,
3730+
'array' => false,
3731+
'filters' => [],
3732+
]),
3733+
], permissions: [
3734+
Permission::read(Role::any()),
3735+
Permission::create(Role::any()),
3736+
Permission::update(Role::any()),
3737+
Permission::delete(Role::any())
3738+
], documentSecurity: false);
3739+
3740+
for ($i = 0; $i < 10; $i++) {
3741+
$database->createDocument($collection, new Document([
3742+
'$id' => 'doc' . $i,
3743+
'string' => 'text📝 ' . $i,
3744+
'integer' => $i
3745+
]));
3746+
}
3747+
// Test onNext is throwing the error without the onError
3748+
// a non existent document to test the error thrown
3749+
try {
3750+
$results = [];
3751+
$count = $database->updateDocuments($collection, new Document([
3752+
'string' => 'text📝 updated',
3753+
]), [
3754+
Query::greaterThanEqual('integer', 100),
3755+
], onNext: function ($doc) use (&$results) {
3756+
$results[] = $doc;
3757+
throw new Exception("Error thrown to test that update doesn't stop and error is caught");
3758+
});
3759+
} catch (Exception $e) {
3760+
$this->assertInstanceOf(Exception::class, $e);
3761+
$this->assertEquals("Error thrown to test that update doesn't stop and error is caught", $e->getMessage());
3762+
}
3763+
3764+
// Test Update half of the documents
3765+
$results = [];
3766+
$count = $database->updateDocuments($collection, new Document([
3767+
'string' => 'text📝 updated',
3768+
]), [
3769+
Query::greaterThanEqual('integer', 5),
3770+
], onNext: function ($doc) use (&$results) {
3771+
$results[] = $doc;
3772+
throw new Exception("Error thrown to test that update doesn't stop and error is caught");
3773+
}, onError:function ($e) {
3774+
$this->assertInstanceOf(Exception::class, $e);
3775+
$this->assertEquals("Error thrown to test that update doesn't stop and error is caught", $e->getMessage());
3776+
});
3777+
3778+
$this->assertEquals(5, $count);
3779+
3780+
foreach ($results as $document) {
3781+
$this->assertEquals('text📝 updated', $document->getAttribute('string'));
3782+
}
3783+
3784+
$updatedDocuments = $database->find($collection, [
3785+
Query::greaterThanEqual('integer', 5),
3786+
]);
3787+
3788+
$this->assertCount(5, $updatedDocuments);
3789+
}
3790+
36943791
/**
36953792
* @depends testCreateDocument
36963793
*/

‎tests/e2e/Adapter/Scopes/Relationships/ManyToManyTests.php‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Utopia\Database\Database;
77
use Utopia\Database\Document;
88
use Utopia\Database\Exception\Restricted as RestrictedException;
9+
use Utopia\Database\Exception\Structure;
910
use Utopia\Database\Helpers\ID;
1011
use Utopia\Database\Helpers\Permission;
1112
use Utopia\Database\Helpers\Role;
@@ -1595,4 +1596,89 @@ public function testDeleteBulkDocumentsManyToManyRelationship(): void
15951596
$this->getDatabase()->deleteDocuments('bulk_delete_person_m2m');
15961597
$this->assertCount(0, $this->getDatabase()->find('bulk_delete_person_m2m'));
15971598
}
1599+
public function testUpdateParentAndChild_ManyToMany(): void
1600+
{
1601+
/** @var Database $database */
1602+
$database = static::getDatabase();
1603+
1604+
if (
1605+
!$database->getAdapter()->getSupportForRelationships() ||
1606+
!$database->getAdapter()->getSupportForBatchOperations()
1607+
) {
1608+
$this->expectNotToPerformAssertions();
1609+
return;
1610+
}
1611+
1612+
$parentCollection = 'parent_combined_m2m';
1613+
$childCollection = 'child_combined_m2m';
1614+
1615+
$database->createCollection($parentCollection);
1616+
$database->createCollection($childCollection);
1617+
1618+
$database->createAttribute($parentCollection, 'name', Database::VAR_STRING, 255, true);
1619+
$database->createAttribute($childCollection, 'name', Database::VAR_STRING, 255, true);
1620+
$database->createAttribute($childCollection, 'parentNumber', Database::VAR_INTEGER, 0, false);
1621+
1622+
1623+
$database->createRelationship(
1624+
collection: $parentCollection,
1625+
relatedCollection: $childCollection,
1626+
type: Database::RELATION_MANY_TO_MANY,
1627+
id: 'parentNumber'
1628+
);
1629+
1630+
$database->createDocument($parentCollection, new Document([
1631+
'$id' => 'parent1',
1632+
'$permissions' => [
1633+
Permission::read(Role::any()),
1634+
Permission::update(Role::any()),
1635+
Permission::delete(Role::any()),
1636+
],
1637+
'name' => 'Parent 1',
1638+
]));
1639+
1640+
$database->createDocument($childCollection, new Document([
1641+
'$id' => 'child1',
1642+
'$permissions' => [
1643+
Permission::read(Role::any()),
1644+
Permission::update(Role::any()),
1645+
Permission::delete(Role::any()),
1646+
],
1647+
'name' => 'Child 1',
1648+
'parentNumber' => null,
1649+
]));
1650+
1651+
$database->updateDocuments(
1652+
$parentCollection,
1653+
new Document(['name' => 'Parent 1 Updated']),
1654+
[Query::equal('$id', ['parent1'])]
1655+
);
1656+
1657+
$parentDoc = $database->getDocument($parentCollection, 'parent1');
1658+
$this->assertEquals('Parent 1 Updated', $parentDoc->getAttribute('name'), 'Parent should be updated');
1659+
1660+
$childDoc = $database->getDocument($childCollection, 'child1');
1661+
$this->assertEquals('Child 1', $childDoc->getAttribute('name'), 'Child should remain unchanged');
1662+
1663+
// invalid update to child
1664+
try {
1665+
$database->updateDocuments(
1666+
$childCollection,
1667+
new Document(['parentNumber' => 'not-a-number']),
1668+
[Query::equal('$id', ['child1'])]
1669+
);
1670+
$this->fail('Expected exception was not thrown for invalid parentNumber type');
1671+
} catch (\Throwable $e) {
1672+
$this->assertInstanceOf(Structure::class, $e);
1673+
}
1674+
1675+
// parent remains unaffected
1676+
$parentDocAfter = $database->getDocument($parentCollection, 'parent1');
1677+
$this->assertEquals('Parent 1 Updated', $parentDocAfter->getAttribute('name'), 'Parent should not be affected by failed child update');
1678+
1679+
$database->deleteCollection($parentCollection);
1680+
$database->deleteCollection($childCollection);
1681+
}
1682+
1683+
15981684
}

0 commit comments

Comments
 (0)