Fix DBAL Sqlite foreign key 5427, 5485, 5497 and 5501 issues (#1028)

atk4 · Jul 13, 2022 · 9ba23bf · 9ba23bf
1 parent eb1be25
commit 9ba23bf
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 49 deletions.
diff --git a/src/Persistence/Sql/Connection.php b/src/Persistence/Sql/Connection.php
@@ -17,7 +17,6 @@
 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.
@@ -455,25 +454,7 @@ public function createSchemaManager(): AbstractSchemaManager
         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);
-                        }
-                    }
-                }
+                use Sqlite\SchemaManagerTrait;
             };
         }
 

diff --git a/src/Persistence/Sql/Sqlite/PlatformTrait.php b/src/Persistence/Sql/Sqlite/PlatformTrait.php
@@ -4,10 +4,24 @@
 
 namespace Atk4\Data\Persistence\Sql\Sqlite;
 
+use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Identifier;
+use Doctrine\DBAL\Schema\TableDiff;
 
 trait PlatformTrait
 {
+    public function supportsForeignKeyConstraints(): bool
+    {
+        // backport https://github.com/doctrine/dbal/pull/5427, remove once DBAL 3.3.x support is dropped
+        return true;
+    }
+
+    protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array
+    {
+        // https://github.com/doctrine/dbal/pull/5486
+        return [];
+    }
+
     // fix quoted table name support
     // TODO submit a PR with fixed SqlitePlatform to DBAL
 
@@ -35,4 +49,18 @@ public function getListTableForeignKeysSQL($table, $database = null)
     {
         return parent::getListTableForeignKeysSQL($this->unquoteTableIdentifier($table), $database);
     }
+
+    public function getAlterTableSQL(TableDiff $diff): array
+    {
+        // fix https://github.com/doctrine/dbal/pull/5501
+        $diff = clone $diff;
+        $diff->fromTable = clone $diff->fromTable;
+        foreach ($diff->fromTable->getForeignKeys() as $foreignKey) {
+            \Closure::bind(function () use ($foreignKey) {
+                $foreignKey->_localColumnNames = $foreignKey->createIdentifierMap($foreignKey->getUnquotedLocalColumns());
+            }, null, ForeignKeyConstraint::class)();
+        }
+
+        return parent::getAlterTableSQL($diff);
+    }
 }
diff --git a/src/Persistence/Sql/Sqlite/SchemaManagerTrait.php b/src/Persistence/Sql/Sqlite/SchemaManagerTrait.php
@@ -0,0 +1,73 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Atk4\Data\Persistence\Sql\Sqlite;
+
+use Atk4\Data\Persistence\Sql\Exception;
+use Doctrine\DBAL\Schema\Identifier;
+use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Schema\TableDiff;
+
+trait SchemaManagerTrait
+{
+    public function alterTable(TableDiff $tableDiff): void
+    {
+        $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);
+            }
+        }
+    }
+
+    protected function _getPortableTableForeignKeysList($tableForeignKeys): array
+    {
+        $foreignKeys = parent::_getPortableTableForeignKeysList($tableForeignKeys);
+
+        // fix https://github.com/doctrine/dbal/pull/5486/files#r920239919
+        foreach ($foreignKeys as $foreignKey) {
+            \Closure::bind(function () use ($foreignKey) {
+                if (ctype_digit((string) $foreignKey->_name)) {
+                    $foreignKey->_name = '';
+                }
+            }, null, Identifier::class)();
+        }
+
+        return $foreignKeys;
+    }
+
+    // fix quoted table name support for private SqliteSchemaManager::getCreateTableSQL() method
+    // https://github.com/doctrine/dbal/blob/3.3.7/src/Schema/SqliteSchemaManager.php#L539
+    // TODO submit a PR with fixed SqliteSchemaManager to DBAL
+
+    private function unquoteTableIdentifier(string $tableName): string
+    {
+        return (new Identifier($tableName))->getName();
+    }
+
+    public function listTableDetails($name): Table
+    {
+        return parent::listTableDetails($this->unquoteTableIdentifier($name));
+    }
+
+    public function listTableIndexes($table): array
+    {
+        return parent::listTableIndexes($this->unquoteTableIdentifier($table));
+    }
+
+    public function listTableForeignKeys($table, $database = null): array
+    {
+        return parent::listTableForeignKeys($this->unquoteTableIdentifier($table), $database);
+    }
+}
diff --git a/src/Schema/TestCase.php b/src/Schema/TestCase.php
@@ -61,23 +61,24 @@ public function startQuery($sql, array $params = null, array $types = null): voi
                         return;
                     }
 
-                    echo "\n" . $sql . "\n" . (is_array($params) && count($params) > 0 ? substr(print_r(array_map(function ($v) {
-                        if ($v === null) {
-                            $v = 'null';
-                        } elseif (is_bool($v)) {
-                            $v = $v ? 'true' : 'false';
-                        } elseif (is_float($v) && (string) $v === (string) (int) $v) {
-                            $v = $v . '.0';
-                        } elseif (is_string($v)) {
-                            if (strlen($v) > 4096) {
-                                $v = '*long string* (length: ' . strlen($v) . ' bytes, sha256: ' . hash('sha256', $v) . ')';
-                            } else {
-                                $v = '\'' . $v . '\'';
+                    echo "\n" . $sql . (substr($sql, -1) !== ';' ? ';' : '') . "\n"
+                        . (is_array($params) && count($params) > 0 ? substr(print_r(array_map(function ($v) {
+                            if ($v === null) {
+                                $v = 'null';
+                            } elseif (is_bool($v)) {
+                                $v = $v ? 'true' : 'false';
+                            } elseif (is_float($v) && (string) $v === (string) (int) $v) {
+                                $v = $v . '.0';
+                            } elseif (is_string($v)) {
+                                if (strlen($v) > 4096) {
+                                    $v = '*long string* (length: ' . strlen($v) . ' bytes, sha256: ' . hash('sha256', $v) . ')';
+                                } else {
+                                    $v = '\'' . $v . '\'';
+                                }
                             }
-                        }
 
-                        return $v;
-                    }, $params), true), 6) : '') . "\n\n";
+                            return $v;
+                        }, $params), true), 6) : '') . "\n";
                 }
 
                 public function stopQuery(): void
@@ -89,11 +90,17 @@ public function stopQuery(): void
 
     protected function tearDown(): void
     {
-        while (count($this->createdMigrators) > 0) {
-            $migrator = array_pop($this->createdMigrators);
-            foreach ($migrator->getCreatedTableNames() as $t) {
-                (clone $migrator)->table($t)->dropIfExists(true);
+        $debugOrig = $this->debug;
+        try {
+            $this->debug = false;
+            while (count($this->createdMigrators) > 0) {
+                $migrator = array_pop($this->createdMigrators);
+                foreach ($migrator->getCreatedTableNames() as $t) {
+                    (clone $migrator)->table($t)->dropIfExists(true);
+                }
             }
+        } finally {
+            $this->debug = $debugOrig;
         }
 
         parent::tearDown();

diff --git a/tests/Schema/MigratorFkTest.php b/tests/Schema/MigratorFkTest.php
@@ -67,22 +67,14 @@ public function testForeignKeyViolation(): void
         $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('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']]],
+            [[['country_id'], 'country', ['id']], [['created_by_client_id'], 'client', ['id']]],
             [[['client_id'], 'client', ['id']]],
         ], [
             $this->selectTableForeignKeys('country'),