Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cascading deletes across multiple levels #269

Merged
merged 3 commits into from
May 16, 2023
Merged

Fix cascading deletes across multiple levels #269

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f49fa56
Fix cascading deletes across multiple levels
abnegate May 2, 2023
6594bae
Rename db instance vars to more clearly reflect usage
abnegate May 16, 2023
0f3f93a
Don't check depth for cascading deletes
abnegate May 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 142 additions & 30 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ class Database
protected bool $resolveRelationships = true;

private int $relationshipFetchDepth = 1;
private int $relationshipDeleteDepth = 1;

/**
* Stack of collection IDs when creating or updating related documents
Expand All @@ -273,10 +274,15 @@ class Database
private array $relationshipWriteStack = [];

/**
* @var array<array<string, mixed>>
* @var array<Document>
*/
private array $relationshipFetchMap = [];

/**
* @var array<Document>
*/
private array $relationshipDeleteMap = [];
abnegate marked this conversation as resolved.
Show resolved Hide resolved

/**
* @param Adapter $adapter
* @param Cache $cache
Expand Down Expand Up @@ -2237,11 +2243,9 @@ private function populateDocumentRelationships(Document $collection, Document $d
{
$attributes = $collection->getAttribute('attributes', []);

$relationships = \array_filter(
$attributes,
fn ($attribute) =>
$attribute['type'] === Database::VAR_RELATIONSHIP
);
$relationships = \array_filter($attributes, function ($attribute) {
return $attribute['type'] === Database::VAR_RELATIONSHIP;
});

foreach ($relationships as $relationship) {
$key = $relationship['key'];
Expand Down Expand Up @@ -2881,7 +2885,6 @@ private function updateDocumentRelationships(Document $collection, Document $old

if ($stackCount >= Database::RELATION_MAX_DEPTH - 1 && $this->relationshipWriteStack[$stackCount - 1] !== $relatedCollection->getId()) {
$document->removeAttribute($key);

continue;
}

Expand Down Expand Up @@ -3377,6 +3380,7 @@ public function deleteDocument(string $collection, string $id): bool

/**
* @throws Exception
* @throws Throwable
*/
private function deleteDocumentRelationships(Document $collection, Document $document): Document
{
Expand All @@ -3396,6 +3400,13 @@ private function deleteDocumentRelationships(Document $collection, Document $doc
$onDelete = $relationship['options']['onDelete'];
$side = $relationship['options']['side'];

$relationship->setAttribute('collection', $collection->getId());
$relationship->setAttribute('document', $document->getId());

if ($this->relationshipDeleteDepth >= Database::RELATION_MAX_DEPTH) {
abnegate marked this conversation as resolved.
Show resolved Hide resolved
return $document;
}

switch ($onDelete) {
case Database::RELATION_MUTATE_RESTRICT:
$this->deleteRestrict($relatedCollection, $document, $value, $relationType, $twoWay, $twoWayKey, $side);
Expand All @@ -3404,7 +3415,51 @@ private function deleteDocumentRelationships(Document $collection, Document $doc
$this->deleteSetNull($collection, $relatedCollection, $document, $value, $relationType, $twoWay, $twoWayKey, $side);
break;
case Database::RELATION_MUTATE_CASCADE:
$this->deleteCascade($collection, $relatedCollection, $document, $key, $value, $relationType, $twoWay, $twoWayKey, $side);
foreach ($this->relationshipDeleteMap as $processedRelationship) {
$existingKey = $processedRelationship['key'];
$existingCollection = $processedRelationship['collection'];
$existingRelatedCollection = $processedRelationship['options']['relatedCollection'];
$existingTwoWayKey = $processedRelationship['options']['twoWayKey'];
$existingSide = $processedRelationship['options']['side'];

// If this relationship has already been fetched for this document, skip it
$reflexive = $processedRelationship == $relationship;

// If this relationship is the same as a previously fetched relationship, but on the other side, skip it
$symmetric = $existingKey === $twoWayKey
&& $existingTwoWayKey === $key
&& $existingRelatedCollection === $collection->getId()
&& $existingCollection === $relatedCollection->getId()
&& $existingSide !== $side;

// If this relationship is not directly related but relates across multiple collections, skip it.
//
// These conditions ensure that a relationship is considered transitive if it has the same
// two-way key and related collection, but is on the opposite side of the relationship (the first and second conditions).
//
// They also ensure that a relationship is considered transitive if it has the same key and related
// collection as an existing relationship, but a different two-way key (the third condition),
// or the same two-way key as an existing relationship, but a different key (the fourth condition).
$transitive = (($existingKey === $twoWayKey
&& $existingCollection === $relatedCollection->getId()
&& $existingSide !== $side)
|| ($existingTwoWayKey === $key
&& $existingRelatedCollection === $collection->getId()
&& $existingSide !== $side)
|| ($existingKey === $key
&& $existingTwoWayKey !== $twoWayKey
&& $existingRelatedCollection === $relatedCollection->getId()
&& $existingSide !== $side)
|| ($existingKey !== $key
&& $existingTwoWayKey === $twoWayKey
&& $existingRelatedCollection === $relatedCollection->getId()
&& $existingSide !== $side));

if ($reflexive || $symmetric || $transitive) {
break 2;
}
}
$this->deleteCascade($collection, $relatedCollection, $document, $key, $value, $relationType, $twoWayKey, $side, $relationship);
break;
}
}
Expand All @@ -3414,9 +3469,17 @@ private function deleteDocumentRelationships(Document $collection, Document $doc

/**
* @throws Exception
* @throws Throwable
*/
private function deleteRestrict(Document $relatedCollection, Document $document, mixed $value, string $relationType, bool $twoWay, string $twoWayKey, string $side): void
{
private function deleteRestrict(
Document $relatedCollection,
Document $document,
mixed $value,
string $relationType,
bool $twoWay,
string $twoWayKey,
string $side
): void {
if ($value instanceof Document && $value->isEmpty()) {
$value = null;
}
Expand All @@ -3443,6 +3506,7 @@ private function deleteRestrict(Document $relatedCollection, Document $document,
return;
}


$this->skipRelationships(fn () => $this->updateDocument(
$relatedCollection->getId(),
$related->getId(),
Expand All @@ -3465,8 +3529,16 @@ private function deleteRestrict(Document $relatedCollection, Document $document,
}
}

private function deleteSetNull(Document $collection, Document $relatedCollection, Document $document, mixed $value, string $relationType, bool $twoWay, string $twoWayKey, string $side): void
{
private function deleteSetNull(
Document $collection,
Document $relatedCollection,
Document $document,
mixed $value,
string $relationType,
bool $twoWay,
string $twoWayKey,
string $side
): void {
switch ($relationType) {
case Database::RELATION_ONE_TO_ONE:
if (!$twoWay && $side === Database::RELATION_SIDE_PARENT) {
Expand Down Expand Up @@ -3553,28 +3625,55 @@ private function deleteSetNull(Document $collection, Document $relatedCollection
}
}

private function deleteCascade(Document $collection, Document $relatedCollection, Document $document, string $key, mixed $value, string $relationType, bool $twoWay, string $twoWayKey, string $side): void
{
/**
* @throws AuthorizationException
* @throws ConflictException
* @throws Throwable
*/
private function deleteCascade(
Document $collection,
Document $relatedCollection,
Document $document,
string $key,
mixed $value,
string $relationType,
string $twoWayKey,
string $side,
Document $relationship
): void {
switch ($relationType) {
case Database::RELATION_ONE_TO_ONE:
if ($value !== null) {
$this->skipRelationships(fn () =>
$this->relationshipDeleteDepth++;
$this->relationshipDeleteMap[] = $relationship;

$this->deleteDocument(
$relatedCollection->getId(),
$value->getId()
));
);

$this->relationshipDeleteDepth--;
\array_pop($this->relationshipDeleteMap);
}
break;
case Database::RELATION_ONE_TO_MANY:
if ($side === Database::RELATION_SIDE_CHILD) {
break;
}

$this->relationshipDeleteDepth++;
$this->relationshipDeleteMap[] = $relationship;

foreach ($value as $relation) {
$this->skipRelationships(fn () => $this->deleteDocument(
$this->deleteDocument(
$relatedCollection->getId(),
$relation->getId()
));
);
}

$this->relationshipDeleteDepth--;
\array_pop($this->relationshipDeleteMap);

break;
case Database::RELATION_MANY_TO_ONE:
if ($side === Database::RELATION_SIDE_PARENT) {
Expand All @@ -3586,12 +3685,19 @@ private function deleteCascade(Document $collection, Document $relatedCollection
Query::limit(PHP_INT_MAX)
]);

$this->relationshipDeleteDepth++;
$this->relationshipDeleteMap[] = $relationship;

foreach ($value as $relation) {
$this->skipRelationships(fn () => $this->deleteDocument(
$this->deleteDocument(
$relatedCollection->getId(),
$relation->getId()
));
);
}

$this->relationshipDeleteDepth--;
\array_pop($this->relationshipDeleteMap);

break;
case Database::RELATION_MANY_TO_MANY:
$junction = $this->getJunctionCollection($collection, $relatedCollection, $side);
Expand All @@ -3601,20 +3707,25 @@ private function deleteCascade(Document $collection, Document $relatedCollection
Query::limit(PHP_INT_MAX)
]);

$this->relationshipDeleteDepth++;
$this->relationshipDeleteMap[] = $relationship;

foreach ($junctions as $document) {
$this->skipRelationships(function () use ($document, $junction, $relatedCollection, $key, $side) {
if ($side === Database::RELATION_SIDE_PARENT) {
$this->deleteDocument(
$relatedCollection->getId(),
$document->getAttribute($key)
);
}
if ($side === Database::RELATION_SIDE_PARENT) {
$this->deleteDocument(
$junction,
$document->getId()
$relatedCollection->getId(),
$document->getAttribute($key)
);
});
}
$this->deleteDocument(
$junction,
$document->getId()
);
}

$this->relationshipDeleteDepth--;
\array_pop($this->relationshipDeleteMap);

break;
}
}
Expand All @@ -3625,6 +3736,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection
* @param string $collection
*
* @return bool
* @throws Exception
*/
public function deleteCachedCollection(string $collection): bool
{
Expand Down
65 changes: 65 additions & 0 deletions tests/Database/Base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10328,6 +10328,71 @@ public function testManyToManyRelationshipKeyWithSymbols(): void
$this->assertEquals($doc1->getId(), $doc2->getAttribute('$symbols_coll.ection8')[0]->getId());
}

public function testCascadeMultiDelete(): void
{
if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) {
$this->expectNotToPerformAssertions();
return;
}

static::getDatabase()->createCollection('cascadeMultiDelete1');
static::getDatabase()->createCollection('cascadeMultiDelete2');
static::getDatabase()->createCollection('cascadeMultiDelete3');

static::getDatabase()->createRelationship(
collection: 'cascadeMultiDelete1',
relatedCollection: 'cascadeMultiDelete2',
type: Database::RELATION_ONE_TO_MANY,
twoWay: true,
onDelete: Database::RELATION_MUTATE_CASCADE
);

static::getDatabase()->createRelationship(
collection: 'cascadeMultiDelete2',
relatedCollection: 'cascadeMultiDelete3',
type: Database::RELATION_ONE_TO_MANY,
twoWay: true,
onDelete: Database::RELATION_MUTATE_CASCADE
);

$root = static::getDatabase()->createDocument('cascadeMultiDelete1', new Document([
'$id' => 'cascadeMultiDelete1',
'$permissions' => [
Permission::read(Role::any()),
Permission::delete(Role::any())
],
'cascadeMultiDelete2' => [
[
'$id' => 'cascadeMultiDelete2',
'$permissions' => [
Permission::read(Role::any()),
Permission::delete(Role::any())
],
'cascadeMultiDelete3' => [
[
'$id' => 'cascadeMultiDelete3',
'$permissions' => [
Permission::read(Role::any()),
Permission::delete(Role::any())
],
],
],
],
],
]));

$this->assertCount(1, $root->getAttribute('cascadeMultiDelete2'));
$this->assertCount(1, $root->getAttribute('cascadeMultiDelete2')[0]->getAttribute('cascadeMultiDelete3'));

$this->assertEquals(true, static::getDatabase()->deleteDocument('cascadeMultiDelete1', $root->getId()));

$multi2 = static::getDatabase()->getDocument('cascadeMultiDelete2', 'cascadeMultiDelete2');
$this->assertEquals(true, $multi2->isEmpty());

$multi3 = static::getDatabase()->getDocument('cascadeMultiDelete3', 'cascadeMultiDelete3');
$this->assertEquals(true, $multi3->isEmpty());
}

public function testCollectionUpdate(): Document
{
$collection = static::getDatabase()->createCollection('collectionUpdate', permissions: [
Expand Down