Add Migrator::drop support to drop linked FKs first (#1027)

src/Persistence/Sql/Connection.php

@@ -16,6 +16,8 @@
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Result as DbalResult;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
+use Doctrine\DBAL\Schema\SqliteSchemaManager;
+use Doctrine\DBAL\Schema\TableDiff;
 
 /**
  * Class for establishing and maintaining connection with your database.
@@ -448,6 +450,33 @@ public function getDatabasePlatform(): AbstractPlatform
      */
     public function createSchemaManager(): AbstractSchemaManager
     {
-        return $this->getConnection()->createSchemaManager();
+        $dbalConnection = $this->getConnection();
+        $platform = $this->getDatabasePlatform();
+        if ($platform instanceof SqlitePlatform) {
+            // @phpstan-ignore-next-line
+            return new class($dbalConnection, $platform) extends SqliteSchemaManager {
+                public function alterTable(TableDiff $tableDiff)
+                {
+                    $hadForeignKeysEnabled = (bool) $this->_conn->executeQuery('PRAGMA foreign_keys')->fetchOne();
+                    if ($hadForeignKeysEnabled) {
+                        $this->_execSql('PRAGMA foreign_keys = 0');
+                    }
+
+                    parent::alterTable($tableDiff);
+
+                    if ($hadForeignKeysEnabled) {
+                        $this->_execSql('PRAGMA foreign_keys = 1');
+
+                        $rows = $this->_conn->executeQuery('PRAGMA foreign_key_check')->fetchAllAssociative();
+                        if (count($rows) > 0) {
+                            throw (new Exception('Foreign key constraints are violated'))
+                                ->addMoreInfo('data', $rows);
+                        }
+                    }
+                }
+            };
+        }
+
+        return $dbalConnection->createSchemaManager();
     }
 }

src/Persistence/Sql/Query.php

@@ -787,48 +787,36 @@ public function setMulti($fields)
 
     protected function _render_set(): ?string
     {
-        // will be joined for output
         $ret = [];
+        foreach ($this->args['set'] as [$field, $value]) {
+            $field = $this->consume($field, self::ESCAPE_IDENTIFIER);
+            $value = $this->consume($value, self::ESCAPE_PARAM);
 
-        if (isset($this->args['set']) && $this->args['set']) {
-            foreach ($this->args['set'] as [$field, $value]) {
-                $field = $this->consume($field, self::ESCAPE_IDENTIFIER);
-                $value = $this->consume($value, self::ESCAPE_PARAM);
-
-                $ret[] = $field . '=' . $value;
-            }
+            $ret[] = $field . '=' . $value;
         }
 
         return implode(', ', $ret);
     }
 
     protected function _render_set_fields(): ?string
     {
-        // will be joined for output
         $ret = [];
+        foreach ($this->args['set'] as $pair) {
+            $field = $this->consume($pair[0], self::ESCAPE_IDENTIFIER);
 
-        if ($this->args['set']) {
-            foreach ($this->args['set'] as $pair) {
-                $field = $this->consume($pair[0], self::ESCAPE_IDENTIFIER);
-
-                $ret[] = $field;
-            }
+            $ret[] = $field;
         }
 
         return implode(', ', $ret);
     }
 
     protected function _render_set_values(): ?string
     {
-        // will be joined for output
         $ret = [];
+        foreach ($this->args['set'] as $pair) {
+            $value = $this->consume($pair[1], self::ESCAPE_PARAM);
 
-        if ($this->args['set']) {
-            foreach ($this->args['set'] as $pair) {
-                $value = $this->consume($pair[1], self::ESCAPE_PARAM);
-
-                $ret[] = $value;
-            }
+            $ret[] = $value;
         }
 
         return implode(', ', $ret);

src/Schema/Migrator.php

@@ -123,11 +123,30 @@ public function create(): self
         return $this;
     }
 
-    public function drop(): self
+    public function drop(bool $dropForeignKeysFirst = false): self
     {
+        $schemaManager = $this->createSchemaManager();
+
+        if ($dropForeignKeysFirst) {
+            // TODO https://github.com/doctrine/dbal/issues/5488 implement all foreign keys fetch in one query
+            $foreignKeysByTableToDrop = [];
+            foreach ($schemaManager->listTableNames() as $tableName) {
+                $foreignKeys = $schemaManager->listTableForeignKeys($tableName);
+                foreach ($foreignKeys as $foreignKey) {
+                    if ($foreignKey->getForeignTableName() === preg_replace('~^.+\.~s', '', $this->table->getName())) {
+                        $foreignKeysByTableToDrop[$tableName][] = $foreignKey;
+                    }
+                }
+            }
+            foreach ($foreignKeysByTableToDrop as $tableName => $foreignKeys) {
+                foreach ($foreignKeys as $foreignKey) {
+                    $schemaManager->dropForeignKey($foreignKey, $this->getDatabasePlatform()->quoteIdentifier($tableName));
+                }
+            }
+        }
+
         try {
-            $this->createSchemaManager()
-                ->dropTable($this->table->getQuotedName($this->getDatabasePlatform()));
+            $schemaManager->dropTable($this->table->getQuotedName($this->getDatabasePlatform()));
         } catch (DatabaseObjectNotFoundException $e) {
             // fix exception not converted to TableNotFoundException for MSSQL
             // https://github.com/doctrine/dbal/pull/5492
@@ -144,10 +163,10 @@ public function drop(): self
         return $this;
     }
 
-    public function dropIfExists(): self
+    public function dropIfExists(bool $dropForeignKeysFirst = false): self
     {
         try {
-            $this->drop();
+            $this->drop($dropForeignKeysFirst);
         } catch (TableNotFoundException $e) {
         }
 

src/Schema/TestCase.php

@@ -89,12 +89,12 @@ public function stopQuery(): void
 
     protected function tearDown(): void
     {
-        foreach ($this->createdMigrators as $migrator) {
+        while (count($this->createdMigrators) > 0) {
+            $migrator = array_pop($this->createdMigrators);
             foreach ($migrator->getCreatedTableNames() as $t) {
-                (clone $migrator)->table($t)->dropIfExists();
+                (clone $migrator)->table($t)->dropIfExists(true);
             }
         }
-        $this->createdMigrators = [];
 
         parent::tearDown();
     }

tests/Schema/MigratorFkTest.php

@@ -0,0 +1,117 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Atk4\Data\Tests\Schema;
+
+use Atk4\Data\Exception;
+use Atk4\Data\Model;
+use Atk4\Data\Schema\TestCase;
+use Doctrine\DBAL\Driver\Exception as DbalDriverException;
+use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
+use Doctrine\DBAL\Schema\ForeignKeyConstraint;
+use Doctrine\DBAL\Schema\Identifier;
+
+class MigratorFkTest extends TestCase
+{
+    /**
+     * @param array<string> $localColumns
+     * @param array<string> $targetColumns
+     */
+    protected function createForeignKey(string $localTable, array $localColumns, string $targetTable, array $targetColumns): void
+    {
+        $platform = $this->getDatabasePlatform();
+        $this->getConnection()->createSchemaManager()->createForeignKey(
+            new ForeignKeyConstraint(
+                array_map(fn ($v) => $platform->quoteIdentifier($v), $localColumns),
+                $platform->quoteIdentifier($targetTable),
+                array_map(fn ($v) => $platform->quoteIdentifier($v), $targetColumns)
+            ),
+            $platform->quoteIdentifier($localTable)
+        );
+    }
+
+    protected function selectTableForeignKeys(string $localTable): array
+    {
+        $foreignKeys = $this->getConnection()->createSchemaManager()->listTableForeignKeys($localTable);
+
+        $unquoteIdentifierFx = fn (string $name): string => (new Identifier($name))->getName();
+
+        $res = array_map(function (ForeignKeyConstraint $v) use ($unquoteIdentifierFx) {
+            return [
+                array_map($unquoteIdentifierFx, $v->getLocalColumns()),
+                $unquoteIdentifierFx($v->getForeignTableName()),
+                array_map($unquoteIdentifierFx, $v->getForeignColumns()),
+            ];
+        }, $foreignKeys);
+        sort($res);
+
+        return $res;
+    }
+
+    public function testForeignKeyViolation(): void
+    {
+        $country = new Model($this->db, ['table' => 'country']);
+        $country->addField('name');
+
+        $client = new Model($this->db, ['table' => 'client']);
+        $client->addField('name');
+        $client->hasOne('country_id', ['model' => $country]);
+        $client->hasOne('created_by_client_id', ['model' => $client]);
+
+        $invoice = new Model($this->db, ['table' => 'invoice']);
+        $invoice->hasOne('client_id', ['model' => $client]);
+
+        $this->createMigrator($client)->create();
+        $this->createMigrator($invoice)->create();
+        $this->createMigrator($country)->create();
+
+        // https://github.com/doctrine/dbal/issues/5485
+        // TODO submit a PR to DBAL
+        $isBrokenFkSqlite = $this->getDatabasePlatform() instanceof SqlitePlatform;
+
+        $this->createForeignKey('client', ['country_id'], 'country', ['id']);
+        if (!$isBrokenFkSqlite) {
+            $this->createForeignKey('client', ['created_by_client_id'], 'client', ['id']);
+        }
+        $this->createForeignKey('invoice', ['client_id'], 'client', ['id']);
+
+        // make sure FK client-country was not removed during FK invoice-client setup
+        $this->assertSame([
+            [],
+            $isBrokenFkSqlite ?
+                [[['country_id'], 'country', ['id']]]
+                : [[['country_id'], 'country', ['id']], [['created_by_client_id'], 'client', ['id']]],
+            [[['client_id'], 'client', ['id']]],
+        ], [
+            $this->selectTableForeignKeys('country'),
+            $this->selectTableForeignKeys('client'),
+            $this->selectTableForeignKeys('invoice'),
+        ]);
+
+        $clientId = $client->insert(['name' => 'Leos']);
+        $invoice->insert(['client_id' => $clientId]);
+
+        // same table FK
+        $client->insert(['name' => 'Ewa', 'created_by_client_id' => $clientId]);
+
+        $this->expectException(Exception::class);
+        try {
+            $invoice->insert(['client_id' => 50]);
+        } catch (Exception $e) {
+            $dbalException = $e->getPrevious()->getPrevious();
+            if ($this->getDatabasePlatform() instanceof SqlitePlatform) {
+                // FK violation exception is not properly converted by ExceptionConverter
+                // https://github.com/doctrine/dbal/blob/3.3.7/src/Driver/API/SQLite/ExceptionConverter.php
+                // https://github.com/doctrine/dbal/issues/5496
+                // TODO submit a PR to DBAL
+                $this->assertInstanceOf(DbalDriverException::class, $dbalException);
+            } else {
+                $this->assertInstanceOf(ForeignKeyConstraintViolationException::class, $dbalException);
+            }
+
+            throw $e;
+        }
+    }
+}