Skip to content

Commit 94ec1c4

Browse files
abnegateclaude
abnegate
and
claude
committed
fix(relationships): keep related documents for a create retried after a lock conflict
Relating a new document's relationships removes or replaces its relationship values and changes the related documents nested in it. When the parent insert (or the commit) then hit a lock conflict, withTransaction() ran the create closure again on the same, already changed document, so the retried attempt created the parent without its children or links, on both the prepared and the one-by-one paths. The relationship hook now copies the document and the documents nested in it before relating changes them, and createDocument() restores those copies before a retried attempt relates again. The copy is taken only for a top-level create whose relationship values are documents or arrays, so a create without relationship values pays only a per-relationship-attribute check, and nested creates, createDocuments() and upserts copy nothing. updateDocument() already rebuilds its document from the input on every attempt, which the new test covers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5d8595e commit 94ec1c4

3 files changed

Lines changed: 197 additions & 7 deletions

File tree

‎src/Database/Hook/Relationships.php‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,15 +281,24 @@ public function withSnapshot(Snapshot $snapshot, callable $callback): mixed
281281
}
282282

283283
/**
284-
* {@inheritDoc}
284+
* Relate the related documents of a new document, removing or replacing its relationship values with what is
285+
* stored for them.
286+
*
287+
* @param array<int, array{Document, array<string, mixed>}>|null $copies Given an array, receives the document
288+
* and the documents nested in it as
289+
* they were before relating changed
290+
* them, for restore() to give back to an
291+
* attempt the transaction retries
285292
*
286293
* @throws DuplicateException If a related document already exists
287294
* @throws RelationshipException If a relationship constraint is violated
288295
*/
289-
public function afterDocumentCreate(Document $collection, Document $document): Document
296+
public function afterDocumentCreate(Document $collection, Document $document, ?array &$copies = null): Document
290297
{
291298
$coroutine = $this->coroutine();
292-
$relate = fn (?PreparedCreate $prepared): Document => $this->relate($collection, $document, $coroutine, $prepared);
299+
$relate = function (?PreparedCreate $prepared) use ($collection, $document, $coroutine, &$copies): Document {
300+
return $this->relate($collection, $document, $coroutine, $prepared, $copies);
301+
};
293302

294303
if (! $this->canPrepare($coroutine) || ! $this->hasRelatedDocuments($collection, $document)) {
295304
return $relate(null);
@@ -647,9 +656,11 @@ private function copy(Document $document, array &$copies): void
647656
}
648657

649658
/**
659+
* Give each copied document back the attributes it was copied with.
660+
*
650661
* @param array<int, array{Document, array<string, mixed>}> $copies
651662
*/
652-
private function restore(array $copies): void
663+
public function restore(array $copies): void
653664
{
654665
foreach ($copies as [$document, $attributes]) {
655666
$document->exchangeArray($attributes);
@@ -692,7 +703,13 @@ private function writePrepared(PreparedCreate $prepared): void
692703
$this->db->createPrepared($documents);
693704
}
694705

695-
private function relate(Document $collection, Document $document, int $coroutine, ?PreparedCreate $prepared): Document
706+
/**
707+
* @param array<int, array{Document, array<string, mixed>}>|null $copies Receives the document and the
708+
* documents nested in it before
709+
* relating changes them, unless the
710+
* relating is nested in another write
711+
*/
712+
private function relate(Document $collection, Document $document, int $coroutine, ?PreparedCreate $prepared, ?array &$copies = null): Document
696713
{
697714
$relationships = $this->relationships($collection);
698715
$writeStack = $this->writeStacks[$coroutine] ?? [];
@@ -702,6 +719,9 @@ private function relate(Document $collection, Document $document, int $coroutine
702719
/** @var string $key */
703720
$key = $relationship->getAttribute('key', $relationship->getId());
704721
$value = $document->getAttribute($key);
722+
if ($copies !== null && $stackCount === 0 && (\is_array($value) || $value instanceof Document)) {
723+
$this->copy($document, $copies);
724+
}
705725
$rel = RelationshipVO::fromArray(['collection' => $collection->getId()] + $relationship->getArrayCopy());
706726
$relatedCollection = $prepared === null
707727
? $this->db->getCollection($rel->getRelatedCollection())

‎src/Database/Traits/Documents.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,10 +1059,17 @@ public function createDocument(string $collection, Document $document): Document
10591059

10601060
$document = $this->prepareDocument($collection, $document);
10611061

1062-
$document = $this->withMutation(Event::DocumentCreate, $document, function () use ($collection, $document) {
1062+
/** @var array<int, array{Document, array<string, mixed>}> $copies */
1063+
$copies = [];
1064+
$document = $this->withMutation(Event::DocumentCreate, $document, function () use ($collection, $document, &$copies) {
10631065
$hook = $this->relationshipHook;
10641066
if ($hook?->isEnabled()) {
1065-
$document = $this->silent(fn () => $hook->afterDocumentCreate($collection, $document));
1067+
if ($copies !== []) {
1068+
$hook->restore($copies);
1069+
}
1070+
$document = $this->silent(function () use ($hook, $collection, $document, &$copies): Document {
1071+
return $hook->afterDocumentCreate($collection, $document, $copies);
1072+
});
10661073
}
10671074

10681075
$document = $this->adapter->createDocument($collection, $document);

‎tests/unit/Relationships/ParentConflictRetryTest.php‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
<?php
2+
3+
namespace Tests\Unit\Relationships;
4+
5+
use PDO;
6+
use PHPUnit\Framework\Attributes\DataProvider;
7+
use PHPUnit\Framework\TestCase;
8+
use Tests\Unit\Support\RelationshipSQLite;
9+
use Utopia\Cache\Adapter\None;
10+
use Utopia\Cache\Cache;
11+
use Utopia\Database\Attribute;
12+
use Utopia\Database\Collection;
13+
use Utopia\Database\Database;
14+
use Utopia\Database\Document;
15+
use Utopia\Database\Exception\Contention;
16+
use Utopia\Database\Helpers\Permission;
17+
use Utopia\Database\Helpers\Role;
18+
use Utopia\Database\Hook\Relationships;
19+
use Utopia\Database\Relationship;
20+
use Utopia\Database\Validator\Authorization;
21+
22+
/**
23+
* A lock conflict on writing a document, after its related documents were related, rolls the whole attempt back and
24+
* the transaction runs it again: the retried attempt must relate the documents it was given once more.
25+
*/
26+
final class ParentConflictRetryTest extends TestCase
27+
{
28+
/**
29+
* @return iterable<string, array{bool}>
30+
*/
31+
public static function modes(): iterable
32+
{
33+
yield 'prepared' => [true];
34+
yield 'one by one' => [false];
35+
}
36+
37+
#[DataProvider('modes')]
38+
public function testARetriedCreateKeepsItsNewRelatedDocuments(bool $prepare): void
39+
{
40+
$database = $this->database($prepare, 'createDocument');
41+
42+
$database->createDocument('parents', new Document([
43+
'$id' => 'p1',
44+
'name' => 'p1',
45+
'children' => [new Document([
46+
'$id' => 'c1',
47+
'name' => 'c1',
48+
'toys' => [new Document(['$id' => 't1', 'name' => 't1'])],
49+
])],
50+
]));
51+
52+
$this->assertSame([['p1', 'p1']], $this->stored($database, 'parents', 'name'));
53+
$this->assertSame([['c1', 'p1']], $this->stored($database, 'children', 'parent'));
54+
$this->assertSame([['t1', 'c1']], $this->stored($database, 'toys', 'child'));
55+
}
56+
57+
#[DataProvider('modes')]
58+
public function testARetriedCreateKeepsItsRelatedDocumentIds(bool $prepare): void
59+
{
60+
$database = $this->database($prepare, 'createDocument');
61+
$database->createDocument('children', new Document(['$id' => 'c1', 'name' => 'c1']));
62+
63+
$database->createDocument('parents', new Document(['$id' => 'p1', 'name' => 'p1', 'children' => ['c1']]));
64+
65+
$this->assertSame([['p1', 'p1']], $this->stored($database, 'parents', 'name'));
66+
$this->assertSame([['c1', 'p1']], $this->stored($database, 'children', 'parent'));
67+
}
68+
69+
#[DataProvider('modes')]
70+
public function testARetriedUpdateKeepsItsNewRelatedDocuments(bool $prepare): void
71+
{
72+
$database = $this->database($prepare, 'updateDocument');
73+
$database->createDocument('parents', new Document(['$id' => 'p1', 'name' => 'p1']));
74+
75+
$database->updateDocument('parents', 'p1', new Document([
76+
'name' => 'renamed',
77+
'children' => [new Document([
78+
'$id' => 'c1',
79+
'name' => 'c1',
80+
'toys' => [new Document(['$id' => 't1', 'name' => 't1'])],
81+
])],
82+
]));
83+
84+
$this->assertSame([['p1', 'renamed']], $this->stored($database, 'parents', 'name'));
85+
$this->assertSame([['c1', 'p1']], $this->stored($database, 'children', 'parent'));
86+
$this->assertSame([['t1', 'c1']], $this->stored($database, 'toys', 'child'));
87+
}
88+
89+
/**
90+
* A database whose first write of the given operation to the parents collection meets a lock conflict.
91+
*/
92+
private function database(bool $prepare, string $operation): Database
93+
{
94+
$adapter = new class (new PDO('sqlite::memory:'), $operation) extends RelationshipSQLite {
95+
private bool $conflicted = false;
96+
97+
public function __construct(PDO $pdo, private readonly string $operation)
98+
{
99+
parent::__construct($pdo);
100+
}
101+
102+
#[\Override]
103+
public function createDocument(Document $collection, Document $document): Document
104+
{
105+
$this->conflict('createDocument', $collection);
106+
107+
return parent::createDocument($collection, $document);
108+
}
109+
110+
#[\Override]
111+
public function updateDocument(Document $collection, string $id, Document $document, bool $skipPermissions): Document
112+
{
113+
$this->conflict('updateDocument', $collection);
114+
115+
return parent::updateDocument($collection, $id, $document, $skipPermissions);
116+
}
117+
118+
private function conflict(string $operation, Document $collection): void
119+
{
120+
if ($this->conflicted || $operation !== $this->operation || $collection->getId() !== 'parents') {
121+
return;
122+
}
123+
124+
$this->conflicted = true;
125+
$this->getPDO()->exec('ROLLBACK');
126+
127+
throw new Contention('Deadlock found when trying to get lock');
128+
}
129+
};
130+
131+
$authorization = new Authorization();
132+
$authorization->addRole(Role::any()->toString());
133+
$database = new Database($adapter, new Cache(new None()));
134+
$database->setAuthorization($authorization)->setDatabase('parent_conflict')->setNamespace('conflict');
135+
$database->create();
136+
$database->addHook(new Relationships($database, prepare: $prepare));
137+
138+
$permissions = [
139+
Permission::create(Role::any()),
140+
Permission::read(Role::any()),
141+
Permission::update(Role::any()),
142+
Permission::delete(Role::any()),
143+
];
144+
foreach (['parents', 'children', 'toys'] as $collection) {
145+
$database->createCollection(new Collection(id: $collection, attributes: [Attribute::string(key: 'name', size: 64)], permissions: $permissions, documentSecurity: false));
146+
}
147+
$database->createRelationship(Relationship::oneToMany(collection: 'parents', relatedCollection: 'children', twoWay: true, key: 'children', twoWayKey: 'parent'));
148+
$database->createRelationship(Relationship::oneToMany(collection: 'children', relatedCollection: 'toys', twoWay: true, key: 'toys', twoWayKey: 'child'));
149+
150+
return $database;
151+
}
152+
153+
/**
154+
* @return list<array{string, mixed}>
155+
*/
156+
private function stored(Database $database, string $collection, string $attribute): array
157+
{
158+
return \array_map(
159+
static fn (Document $document): array => [$document->getId(), $document->getAttribute($attribute)],
160+
$database->skipRelationships(static fn (): array => $database->find($collection)),
161+
);
162+
}
163+
}

0 commit comments

Comments
 (0)