From d909c034bf97bd012cd6012cc70dcb0170fd03cf Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 24 Dec 2023 18:42:00 +0200 Subject: [PATCH 1/3] Fix --- src/Database/Adapter/MariaDB.php | 18 +++- src/Database/Adapter/Postgres.php | 28 +++-- src/Database/Database.php | 16 ++- tests/e2e/Adapter/Base.php | 167 ++++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+), 16 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 0efa96df8..a6e7f2952 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -532,9 +532,17 @@ public function deleteRelationship( switch ($type) { case Database::RELATION_ONE_TO_ONE: - $sql = "ALTER TABLE {$table} DROP COLUMN `{$key}`;"; - if ($twoWay) { - $sql .= "ALTER TABLE {$relatedTable} DROP COLUMN `{$twoWayKey}`;"; + if ($side === Database::RELATION_SIDE_PARENT) { + $sql = "ALTER TABLE {$table} DROP COLUMN `{$key}`;"; + if ($twoWay) { + $sql .= "ALTER TABLE {$relatedTable} DROP COLUMN `{$twoWayKey}`;"; + } + } + else if ($side === Database::RELATION_SIDE_CHILD) { + $sql = "ALTER TABLE {$relatedTable} DROP COLUMN `{$twoWayKey}`;"; + if ($twoWay) { + $sql .= "ALTER TABLE {$table} DROP COLUMN `{$key}`;"; + } } break; case Database::RELATION_ONE_TO_MANY: @@ -545,9 +553,9 @@ public function deleteRelationship( } break; case Database::RELATION_MANY_TO_ONE: - if ($twoWay && $side === Database::RELATION_SIDE_CHILD) { + if ($side === Database::RELATION_SIDE_CHILD) { $sql = "ALTER TABLE {$relatedTable} DROP COLUMN `{$twoWayKey}`;"; - } else { + } elseif ($twoWay) { $sql = "ALTER TABLE {$table} DROP COLUMN `{$key}`;"; } break; diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index e52be9b53..ffd8bb6ca 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -525,25 +525,35 @@ public function deleteRelationship( $relatedTable = $this->getSQLTable($relatedName); $key = $this->filter($key); + $sql = ''; + switch ($type) { case Database::RELATION_ONE_TO_ONE: - $sql = "ALTER TABLE {$table} DROP COLUMN \"{$key}\";"; - if ($twoWay) { - $sql .= "ALTER TABLE {$relatedTable} DROP COLUMN \"{$twoWayKey}\";"; + if ($side === Database::RELATION_SIDE_PARENT) { + $sql = "ALTER TABLE {$table} DROP COLUMN \"{$key}\";"; + if ($twoWay) { + $sql .= "ALTER TABLE {$relatedTable} DROP COLUMN \"{$twoWayKey}\";"; + } + } + else if ($side === Database::RELATION_SIDE_CHILD) { + $sql = "ALTER TABLE {$relatedTable} DROP COLUMN \"{$twoWayKey}\";"; + if ($twoWay) { + $sql .= "ALTER TABLE {$table} DROP COLUMN \"{$key}\";"; + } } break; case Database::RELATION_ONE_TO_MANY: if ($side === Database::RELATION_SIDE_PARENT) { $sql = "ALTER TABLE {$relatedTable} DROP COLUMN \"{$twoWayKey}\";"; - } else { + } elseif ($twoWay) { $sql = "ALTER TABLE {$table} DROP COLUMN \"{$key}\";"; } break; case Database::RELATION_MANY_TO_ONE: - if ($side === Database::RELATION_SIDE_PARENT) { - $sql = "ALTER TABLE {$table} DROP COLUMN \"{$key}\";"; - } else { + if ($side === Database::RELATION_SIDE_CHILD) { $sql = "ALTER TABLE {$relatedTable} DROP COLUMN \"{$twoWayKey}\";"; + } elseif ($twoWay) { + $sql = "ALTER TABLE {$table} DROP COLUMN \"{$key}\";"; } break; case Database::RELATION_MANY_TO_MANY: @@ -564,6 +574,10 @@ public function deleteRelationship( throw new DatabaseException('Invalid relationship type'); } + if (empty($sql)) { + return true; + } + $sql = $this->trigger(Database::EVENT_ATTRIBUTE_DELETE, $sql); return $this->getPDO() diff --git a/src/Database/Database.php b/src/Database/Database.php index ca7920ac1..bc232ede1 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2186,9 +2186,17 @@ public function deleteRelationship(string $collection, string $id): bool switch ($type) { case self::RELATION_ONE_TO_ONE: - $this->deleteIndex($collection->getId(), $indexKey); - if ($twoWay) { + if ($side === Database::RELATION_SIDE_PARENT) { + $this->deleteIndex($collection->getId(), $indexKey); + if ($twoWay) { + $this->deleteIndex($relatedCollection->getId(), $twoWayIndexKey); + } + } + if ($side === Database::RELATION_SIDE_CHILD) { $this->deleteIndex($relatedCollection->getId(), $twoWayIndexKey); + if ($twoWay) { + $this->deleteIndex($collection->getId(), $indexKey); + } } break; case self::RELATION_ONE_TO_MANY: @@ -2199,9 +2207,9 @@ public function deleteRelationship(string $collection, string $id): bool } break; case self::RELATION_MANY_TO_ONE: - if ($twoWay && $side === Database::RELATION_SIDE_CHILD) { + if ($side === Database::RELATION_SIDE_CHILD) { $this->deleteIndex($relatedCollection->getId(), $twoWayIndexKey); - } else { + } elseif ($twoWay) { $this->deleteIndex($collection->getId(), $indexKey); } break; diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 6eb5f4f37..01b4c62d7 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -78,6 +78,173 @@ public function testCreateExistsDelete(): void $this->assertEquals(true, static::getDatabase()->create()); } + public function testDeleteRelatedCollection(): void + { + if (!static::getDatabase()->getAdapter()->getSupportForRelationships()) { + $this->expectNotToPerformAssertions(); + return; + } + + + static::getDatabase()->createCollection('c1'); + static::getDatabase()->createCollection('c2'); + + // ONE_TO_ONE + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_ONE, + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c1')); + $collection = static::getDatabase()->getCollection('c2'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c1'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_ONE, + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c2')); + $collection = static::getDatabase()->getCollection('c1'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c2'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_ONE, + twoWay: true + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c1')); + $collection = static::getDatabase()->getCollection('c2'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c1'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_ONE, + twoWay: true + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c2')); + $collection = static::getDatabase()->getCollection('c1'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + // ONE_TO_MANY + static::getDatabase()->createCollection('c2'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_MANY, + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c1')); + $collection = static::getDatabase()->getCollection('c2'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c1'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_MANY, + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c2')); + $collection = static::getDatabase()->getCollection('c1'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c2'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_MANY, + twoWay: true + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c1')); + $collection = static::getDatabase()->getCollection('c2'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c1'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_ONE_TO_MANY, + twoWay: true + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c2')); + $collection = static::getDatabase()->getCollection('c1'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + + // RELATION_MANY_TO_ONE + static::getDatabase()->createCollection('c2'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_MANY_TO_ONE, + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c1')); + $collection = static::getDatabase()->getCollection('c2'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c1'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_MANY_TO_ONE, + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c2')); + $collection = static::getDatabase()->getCollection('c1'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c2'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_MANY_TO_ONE, + twoWay: true + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c1')); + $collection = static::getDatabase()->getCollection('c2'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + static::getDatabase()->createCollection('c1'); + static::getDatabase()->createRelationship( + collection: 'c1', + relatedCollection: 'c2', + type: Database::RELATION_MANY_TO_ONE, + twoWay: true + ); + + $this->assertEquals(true, static::getDatabase()->deleteCollection('c2')); + $collection = static::getDatabase()->getCollection('c1'); + $this->assertCount(0, $collection->getAttribute('attributes')); + $this->assertCount(0, $collection->getAttribute('indexes')); + + + } + /** * @throws Exception|Throwable */ From 0a1853f27aa0d42006e14a6d9a7a95846ae23226 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 24 Dec 2023 18:44:04 +0200 Subject: [PATCH 2/3] formatting --- src/Database/Adapter/MariaDB.php | 3 +-- src/Database/Adapter/Postgres.php | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index a6e7f2952..5e960247b 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -537,8 +537,7 @@ public function deleteRelationship( if ($twoWay) { $sql .= "ALTER TABLE {$relatedTable} DROP COLUMN `{$twoWayKey}`;"; } - } - else if ($side === Database::RELATION_SIDE_CHILD) { + } elseif ($side === Database::RELATION_SIDE_CHILD) { $sql = "ALTER TABLE {$relatedTable} DROP COLUMN `{$twoWayKey}`;"; if ($twoWay) { $sql .= "ALTER TABLE {$table} DROP COLUMN `{$key}`;"; diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index ffd8bb6ca..ff3416f1a 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -534,8 +534,7 @@ public function deleteRelationship( if ($twoWay) { $sql .= "ALTER TABLE {$relatedTable} DROP COLUMN \"{$twoWayKey}\";"; } - } - else if ($side === Database::RELATION_SIDE_CHILD) { + } elseif ($side === Database::RELATION_SIDE_CHILD) { $sql = "ALTER TABLE {$relatedTable} DROP COLUMN \"{$twoWayKey}\";"; if ($twoWay) { $sql .= "ALTER TABLE {$table} DROP COLUMN \"{$key}\";"; From 9a5dbf87e01562a03703a68f9e0442e4ff228d97 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 19 Jan 2024 18:57:59 +1300 Subject: [PATCH 3/3] Apply suggestions from code review --- tests/e2e/Adapter/Base.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 01b4c62d7..5fd42ea15 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -85,7 +85,6 @@ public function testDeleteRelatedCollection(): void return; } - static::getDatabase()->createCollection('c1'); static::getDatabase()->createCollection('c2'); @@ -190,7 +189,6 @@ public function testDeleteRelatedCollection(): void $this->assertCount(0, $collection->getAttribute('attributes')); $this->assertCount(0, $collection->getAttribute('indexes')); - // RELATION_MANY_TO_ONE static::getDatabase()->createCollection('c2'); static::getDatabase()->createRelationship( @@ -241,8 +239,6 @@ public function testDeleteRelatedCollection(): void $collection = static::getDatabase()->getCollection('c1'); $this->assertCount(0, $collection->getAttribute('attributes')); $this->assertCount(0, $collection->getAttribute('indexes')); - - } /**