Skip to content

Commit af610c0

Browse files
HarshMN2345HarshMN2345
committed
Fix one-way relationship twoWayKey handling
1 parent 504c45e commit af610c0

File tree

5 files changed

+137
-39
lines changed

5 files changed

+137
-39
lines changed

‎src/Database/Database.php‎

Lines changed: 61 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3536,6 +3536,26 @@ private function cleanupRelationship(
35363536
);
35373537
}
35383538

3539+
private function getOneWayRelationshipInternalKey(string $collectionId, string $id): string
3540+
{
3541+
$candidate = $collectionId . '_' . $id;
3542+
3543+
if (\strlen($candidate) <= self::LENGTH_KEY) {
3544+
return $candidate;
3545+
}
3546+
3547+
return 'rel_' . \md5($collectionId . ':' . $id);
3548+
}
3549+
3550+
private function getRelationshipTwoWayKey(array|Document $relationship): string
3551+
{
3552+
$options = \is_array($relationship)
3553+
? ($relationship['options'] ?? [])
3554+
: $relationship->getAttribute('options', []);
3555+
3556+
return $options['internalTwoWayKey'] ?? $options['twoWayKey'] ?? '';
3557+
}
3558+
35393559
/**
35403560
* Create a relationship attribute
35413561
*
@@ -3576,6 +3596,7 @@ public function createRelationship(
35763596
}
35773597

35783598
$id ??= $relatedCollection->getId();
3599+
$publicTwoWayKey = $twoWay ? ($twoWayKey ?? $collection->getId()) : null;
35793600

35803601
$attributes = $collection->getAttribute('attributes', []);
35813602
/** @var array<Document> $attributes */
@@ -3588,14 +3609,16 @@ public function createRelationship(
35883609
$twoWay
35893610
&& $attribute->getAttribute('type') === self::VAR_RELATIONSHIP
35903611
&& isset($attribute->getAttribute('options')['twoWayKey'])
3591-
&& \strtolower($attribute->getAttribute('options')['twoWayKey']) === \strtolower($twoWayKey)
3612+
&& \strtolower($attribute->getAttribute('options')['twoWayKey']) === \strtolower((string) $publicTwoWayKey)
35923613
&& $attribute->getAttribute('options')['relatedCollection'] === $relatedCollection->getId()
35933614
) {
35943615
throw new DuplicateException('Related attribute already exists');
35953616
}
35963617
}
35973618

3598-
$twoWayKey ??= $collection->getId();
3619+
$twoWayKey = $twoWay
3620+
? (string) $publicTwoWayKey
3621+
: $this->getOneWayRelationshipInternalKey($collection->getId(), $id);
35993622

36003623
$relationship = new Document([
36013624
'$id' => ID::custom($id),
@@ -3607,7 +3630,8 @@ public function createRelationship(
36073630
'relatedCollection' => $relatedCollection->getId(),
36083631
'relationType' => $type,
36093632
'twoWay' => $twoWay,
3610-
'twoWayKey' => $twoWayKey,
3633+
'twoWayKey' => $publicTwoWayKey,
3634+
'internalTwoWayKey' => $twoWayKey,
36113635
'onDelete' => $onDelete,
36123636
'side' => Database::RELATION_SIDE_PARENT,
36133637
],
@@ -3623,7 +3647,8 @@ public function createRelationship(
36233647
'relatedCollection' => $collection->getId(),
36243648
'relationType' => $type,
36253649
'twoWay' => $twoWay,
3626-
'twoWayKey' => $id,
3650+
'twoWayKey' => $twoWay ? $id : null,
3651+
'internalTwoWayKey' => $id,
36273652
'onDelete' => $onDelete,
36283653
'side' => Database::RELATION_SIDE_CHILD,
36293654
],
@@ -3898,23 +3923,27 @@ public function updateRelationship(
38983923

38993924
// Determine if we need to alter the database (rename columns/indexes)
39003925
$oldAttribute = $attributes[$attributeIndex];
3901-
$oldTwoWayKey = $oldAttribute['options']['twoWayKey'];
3926+
$oldTwoWayKey = $this->getRelationshipTwoWayKey($oldAttribute);
3927+
$oldPublicTwoWayKey = $oldAttribute['options']['twoWayKey'] ?? null;
3928+
$actualTwoWay = $twoWay ?? $oldAttribute['options']['twoWay'];
3929+
$actualOnDelete = $onDelete ?? $oldAttribute['options']['onDelete'];
3930+
$actualNewKey = $newKey ?? $id;
3931+
$actualPublicNewTwoWayKey = $actualTwoWay ? ($newTwoWayKey ?? $oldPublicTwoWayKey ?? $collection->getId()) : null;
3932+
$actualNewTwoWayKey = $actualTwoWay
3933+
? (string) $actualPublicNewTwoWayKey
3934+
: $this->getOneWayRelationshipInternalKey($collection->getId(), $actualNewKey);
39023935
$altering = (!\is_null($newKey) && $newKey !== $id)
3903-
|| (!\is_null($newTwoWayKey) && $newTwoWayKey !== $oldTwoWayKey);
3936+
|| ($actualNewTwoWayKey !== $oldTwoWayKey);
39043937

39053938
// Validate new keys don't already exist
39063939
if (
3907-
!\is_null($newTwoWayKey)
3908-
&& \in_array($newTwoWayKey, \array_map(fn ($attribute) => $attribute['key'], $relatedCollection->getAttribute('attributes', [])))
3940+
$actualTwoWay
3941+
&& !\is_null($actualPublicNewTwoWayKey)
3942+
&& \in_array($actualPublicNewTwoWayKey, \array_map(fn ($attribute) => $attribute['key'], $relatedCollection->getAttribute('attributes', [])))
39093943
) {
39103944
throw new DuplicateException('Related attribute already exists');
39113945
}
39123946

3913-
$actualNewKey = $newKey ?? $id;
3914-
$actualNewTwoWayKey = $newTwoWayKey ?? $oldTwoWayKey;
3915-
$actualTwoWay = $twoWay ?? $oldAttribute['options']['twoWay'];
3916-
$actualOnDelete = $onDelete ?? $oldAttribute['options']['onDelete'];
3917-
39183947
$adapterUpdated = false;
39193948
if ($altering) {
39203949
try {
@@ -3959,22 +3988,24 @@ public function updateRelationship(
39593988
}
39603989

39613990
try {
3962-
$this->updateAttributeMeta($collection->getId(), $id, function ($attribute) use ($actualNewKey, $actualNewTwoWayKey, $actualTwoWay, $actualOnDelete, $relatedCollection, $type, $side) {
3991+
$this->updateAttributeMeta($collection->getId(), $id, function ($attribute) use ($actualNewKey, $actualNewTwoWayKey, $actualPublicNewTwoWayKey, $actualTwoWay, $actualOnDelete, $relatedCollection, $type, $side) {
39633992
$attribute->setAttribute('$id', $actualNewKey);
39643993
$attribute->setAttribute('key', $actualNewKey);
39653994
$attribute->setAttribute('options', [
39663995
'relatedCollection' => $relatedCollection->getId(),
39673996
'relationType' => $type,
39683997
'twoWay' => $actualTwoWay,
3969-
'twoWayKey' => $actualNewTwoWayKey,
3998+
'twoWayKey' => $actualPublicNewTwoWayKey,
3999+
'internalTwoWayKey' => $actualNewTwoWayKey,
39704000
'onDelete' => $actualOnDelete,
39714001
'side' => $side,
39724002
]);
39734003
});
39744004

39754005
$this->updateAttributeMeta($relatedCollection->getId(), $oldTwoWayKey, function ($twoWayAttribute) use ($actualNewKey, $actualNewTwoWayKey, $actualTwoWay, $actualOnDelete) {
39764006
$options = $twoWayAttribute->getAttribute('options', []);
3977-
$options['twoWayKey'] = $actualNewKey;
4007+
$options['twoWayKey'] = $actualTwoWay ? $actualNewKey : null;
4008+
$options['internalTwoWayKey'] = $actualNewKey;
39784009
$options['twoWay'] = $actualTwoWay;
39794010
$options['onDelete'] = $actualOnDelete;
39804011

@@ -4111,7 +4142,8 @@ function ($index) use ($newKey) {
41114142
try {
41124143
$this->updateAttributeMeta($relatedCollection->getId(), $actualNewTwoWayKey, function ($twoWayAttribute) use ($oldTwoWayKey, $id, $oldAttribute) {
41134144
$options = $twoWayAttribute->getAttribute('options', []);
4114-
$options['twoWayKey'] = $id;
4145+
$options['twoWayKey'] = $oldAttribute['options']['twoWay'] ? $id : null;
4146+
$options['internalTwoWayKey'] = $id;
41154147
$options['twoWay'] = $oldAttribute['options']['twoWay'];
41164148
$options['onDelete'] = $oldAttribute['options']['onDelete'];
41174149
$twoWayAttribute->setAttribute('$id', $oldTwoWayKey);
@@ -4205,7 +4237,7 @@ public function deleteRelationship(string $collection, string $id): bool
42054237
$relatedCollection = $relationship['options']['relatedCollection'];
42064238
$type = $relationship['options']['relationType'];
42074239
$twoWay = $relationship['options']['twoWay'];
4208-
$twoWayKey = $relationship['options']['twoWayKey'];
4240+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
42094241
$side = $relationship['options']['side'];
42104242

42114243
$relatedCollection = $this->silent(fn () => $this->getCollection($relatedCollection));
@@ -4989,7 +5021,7 @@ private function populateDocumentsRelationships(
49895021

49905022
// Get two-way relationship info
49915023
$twoWay = $relationship['options']['twoWay'];
4992-
$twoWayKey = $relationship['options']['twoWayKey'];
5024+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
49935025

49945026
// Queue if:
49955027
// 1. No explicit selects (fetch all recursively), OR
@@ -5167,7 +5199,7 @@ private function populateOneToManyRelationshipsBatch(
51675199
): array {
51685200
$key = $relationship['key'];
51695201
$twoWay = $relationship['options']['twoWay'];
5170-
$twoWayKey = $relationship['options']['twoWayKey'];
5202+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
51715203
$side = $relationship['options']['side'];
51725204
$relatedCollection = $this->getCollection($relationship['options']['relatedCollection']);
51735205

@@ -5265,7 +5297,7 @@ private function populateManyToOneRelationshipsBatch(
52655297
): array {
52665298
$key = $relationship['key'];
52675299
$twoWay = $relationship['options']['twoWay'];
5268-
$twoWayKey = $relationship['options']['twoWayKey'];
5300+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
52695301
$side = $relationship['options']['side'];
52705302
$relatedCollection = $this->getCollection($relationship['options']['relatedCollection']);
52715303

@@ -5360,7 +5392,7 @@ private function populateManyToManyRelationshipsBatch(
53605392
): array {
53615393
$key = $relationship['key'];
53625394
$twoWay = $relationship['options']['twoWay'];
5363-
$twoWayKey = $relationship['options']['twoWayKey'];
5395+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
53645396
$side = $relationship['options']['side'];
53655397
$relatedCollection = $this->getCollection($relationship['options']['relatedCollection']);
53665398
$collection = $this->getCollection($relationship->getAttribute('collection'));
@@ -5756,7 +5788,7 @@ private function createDocumentRelationships(Document $collection, Document $doc
57565788
$relatedCollection = $this->getCollection($relationship['options']['relatedCollection']);
57575789
$relationType = $relationship['options']['relationType'];
57585790
$twoWay = $relationship['options']['twoWay'];
5759-
$twoWayKey = $relationship['options']['twoWayKey'];
5791+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
57605792
$side = $relationship['options']['side'];
57615793

57625794
if ($stackCount >= Database::RELATION_MAX_DEPTH - 1 && $this->relationshipWriteStack[$stackCount - 1] !== $relatedCollection->getId()) {
@@ -6585,7 +6617,7 @@ private function updateDocumentRelationships(Document $collection, Document $old
65856617
$relatedCollection = $this->getCollection($relationship['options']['relatedCollection']);
65866618
$relationType = (string)$relationship['options']['relationType'];
65876619
$twoWay = (bool)$relationship['options']['twoWay'];
6588-
$twoWayKey = (string)$relationship['options']['twoWayKey'];
6620+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
65896621
$side = (string)$relationship['options']['side'];
65906622

65916623
if (Operator::isOperator($value)) {
@@ -7673,7 +7705,7 @@ private function deleteDocumentRelationships(Document $collection, Document $doc
76737705
$relatedCollection = $this->getCollection($relationship['options']['relatedCollection']);
76747706
$relationType = $relationship['options']['relationType'];
76757707
$twoWay = $relationship['options']['twoWay'];
7676-
$twoWayKey = $relationship['options']['twoWayKey'];
7708+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
76777709
$onDelete = $relationship['options']['onDelete'];
76787710
$side = $relationship['options']['side'];
76797711

@@ -7692,7 +7724,7 @@ private function deleteDocumentRelationships(Document $collection, Document $doc
76927724
$existingKey = $processedRelationship['key'];
76937725
$existingCollection = $processedRelationship['collection'];
76947726
$existingRelatedCollection = $processedRelationship['options']['relatedCollection'];
7695-
$existingTwoWayKey = $processedRelationship['options']['twoWayKey'];
7727+
$existingTwoWayKey = $this->getRelationshipTwoWayKey($processedRelationship);
76967728
$existingSide = $processedRelationship['options']['side'];
76977729

76987730
// If this relationship has already been fetched for this document, skip it
@@ -9522,7 +9554,7 @@ private function processNestedRelationshipPath(string $startCollection, array $q
95229554
'toCollection' => $relationship['options']['relatedCollection'],
95239555
'relationType' => $relationship['options']['relationType'],
95249556
'side' => $relationship['options']['side'],
9525-
'twoWayKey' => $relationship['options']['twoWayKey'],
9557+
'twoWayKey' => $this->getRelationshipTwoWayKey($relationship),
95269558
];
95279559

95289560
$currentCollection = $relationship['options']['relatedCollection'];
@@ -9896,7 +9928,7 @@ private function resolveRelationshipGroupToIds(
98969928
return null;
98979929
}
98989930

9899-
$twoWayKey = $relationship->getAttribute('options')['twoWayKey'];
9931+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
99009932
$relatedCollectionDoc = $this->silent(fn () => $this->getCollection($relatedCollection));
99019933
$junction = $this->getJunctionCollection($collection, $relatedCollectionDoc, $side);
99029934

@@ -9924,7 +9956,7 @@ private function resolveRelationshipGroupToIds(
99249956
])
99259957
));
99269958

9927-
$twoWayKey = $relationship->getAttribute('options')['twoWayKey'];
9959+
$twoWayKey = $this->getRelationshipTwoWayKey($relationship);
99289960
$parentIds = [];
99299961

99309962
foreach ($matchingDocs as $doc) {

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function testManyToManyOneWayRelationship(): void
5050
$this->assertEquals('song', $attribute['options']['relatedCollection']);
5151
$this->assertEquals(Database::RELATION_MANY_TO_MANY, $attribute['options']['relationType']);
5252
$this->assertEquals(false, $attribute['options']['twoWay']);
53-
$this->assertEquals('playlist', $attribute['options']['twoWayKey']);
53+
$this->assertEquals(null, $attribute['options']['twoWayKey']);
5454
}
5555
}
5656

@@ -325,6 +325,38 @@ public function testManyToManyOneWayRelationship(): void
325325
$this->assertEquals(null, $songs);
326326
}
327327

328+
public function testManyToManyOneWayRelationshipIgnoresProvidedTwoWayKey(): void
329+
{
330+
/** @var Database $database */
331+
$database = $this->getDatabase();
332+
333+
if (!$database->getAdapter()->getSupportForRelationships()) {
334+
$this->expectNotToPerformAssertions();
335+
return;
336+
}
337+
338+
$database->createCollection('course');
339+
$database->createCollection('tag');
340+
341+
$database->createAttribute('course', 'name', Database::VAR_STRING, 255, true);
342+
$database->createAttribute('tag', 'name', Database::VAR_STRING, 255, true);
343+
344+
$database->createRelationship(
345+
collection: 'course',
346+
relatedCollection: 'tag',
347+
type: Database::RELATION_MANY_TO_MANY,
348+
twoWay: false,
349+
id: 'tags',
350+
twoWayKey: 'shouldBeIgnored'
351+
);
352+
353+
$collection = $database->getCollection('course');
354+
$attributes = $collection->getAttribute('attributes', []);
355+
$relationship = \array_values(\array_filter($attributes, fn (Document $attribute) => $attribute->getId() === 'tags'))[0];
356+
357+
$this->assertEquals(null, $relationship['options']['twoWayKey']);
358+
}
359+
328360
public function testManyToManyTwoWayRelationship(): void
329361
{
330362
/** @var Database $database */

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,22 +50,22 @@ public function testManyToOneOneWayRelationship(): void
5050
$this->assertEquals('movie', $attribute['options']['relatedCollection']);
5151
$this->assertEquals(Database::RELATION_MANY_TO_ONE, $attribute['options']['relationType']);
5252
$this->assertEquals(false, $attribute['options']['twoWay']);
53-
$this->assertEquals('review', $attribute['options']['twoWayKey']);
53+
$this->assertEquals(null, $attribute['options']['twoWayKey']);
5454
}
5555
}
5656

5757
// Check metadata for related collection
5858
$collection = $database->getCollection('movie');
5959
$attributes = $collection->getAttribute('attributes', []);
6060
foreach ($attributes as $attribute) {
61-
if ($attribute['key'] === 'review') {
61+
if ($attribute['key'] === 'review_movie') {
6262
$this->assertEquals('relationship', $attribute['type']);
63-
$this->assertEquals('review', $attribute['$id']);
64-
$this->assertEquals('review', $attribute['key']);
63+
$this->assertEquals('review_movie', $attribute['$id']);
64+
$this->assertEquals('review_movie', $attribute['key']);
6565
$this->assertEquals('review', $attribute['options']['relatedCollection']);
6666
$this->assertEquals(Database::RELATION_MANY_TO_ONE, $attribute['options']['relationType']);
6767
$this->assertEquals(false, $attribute['options']['twoWay']);
68-
$this->assertEquals('movie', $attribute['options']['twoWayKey']);
68+
$this->assertEquals(null, $attribute['options']['twoWayKey']);
6969
}
7070
}
7171

@@ -853,9 +853,11 @@ public function testManyToOneOneWayRelationshipDoesNotValidateTwoWayKeyDuplicate
853853
$collection = $database->getCollection('critic');
854854
$attributes = $collection->getAttribute('attributes', []);
855855
$keys = \array_map(fn (Document $attribute) => $attribute->getId(), $attributes);
856+
$favoriteFilm = \array_values(\array_filter($attributes, fn (Document $attribute) => $attribute->getId() === 'favoriteFilm'))[0];
856857

857858
$this->assertContains('favoriteFilm', $keys);
858859
$this->assertContains('leastFavoriteFilm', $keys);
860+
$this->assertEquals(null, $favoriteFilm['options']['twoWayKey']);
859861
}
860862

861863
public function testManyToOneTwoWayRelationshipStillValidatesTwoWayKeyDuplicates(): void

0 commit comments

Comments
 (0)