Rework sqlite fk pragma handling

Signed-off-by: Matthew Peveler <[email protected]>

Author: MasterOdin

src/Phinx/Db/Adapter/AdapterInterface.php

@@ -275,6 +275,13 @@ public function rollbackTransaction(): void;
      */
     public function execute(string $sql, array $params = []): int;
 
+    /**
+     * Function to be called before executing any migration actions.
+     *
+     * @return array
+     */
+    public function preExecuteActions(): array;
+
     /**
      * Executes a list of migration actions for the given table
      *
@@ -284,6 +291,15 @@ public function execute(string $sql, array $params = []): int;
      */
     public function executeActions(Table $table, array $actions): void;
 
+    /**
+     * Function to be called after executing any migration actions.
+     *
+     * @param array $tableNames List of table names that were affected by the actions
+     * @param array $preOptions Options that were set before executing the actions
+     * @return void
+     */
+    public function postExecuteActions(array $tableNames, array $preOptions): void;
+
     /**
      * Returns a new Query object
      *

src/Phinx/Db/Adapter/PdoAdapter.php

@@ -1001,6 +1001,14 @@ public function changeComment(Table $table, $newComment): void
      */
     abstract protected function getChangeCommentInstructions(Table $table, ?string $newComment): AlterInstructions;
 
+    /**
+     * {@inheritDoc}
+     */
+    public function preExecuteActions(): array
+    {
+        return [];
+    }
+
     /**
      * {@inheritDoc}
      *
@@ -1126,4 +1134,12 @@ public function executeActions(Table $table, array $actions): void
 
         $this->executeAlterSteps($table->getName(), $instructions);
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function postExecuteActions(array $tableNames, array $preOptions): void
+    {
+
+    }
 }

src/Phinx/Db/Adapter/SQLiteAdapter.php

@@ -1034,57 +1034,52 @@ protected function recreateIndicesAndTriggers(AlterInstructions $instructions):
      * the given table, and of those tables whose constraints are
      * targeting it.
      *
-     * @param \Phinx\Db\Util\AlterInstructions $instructions The instructions to process
-     * @param string $tableName The name of the table for which to check constraints.
+     * @param string|array<string> $tableName The name of the table for which to check constraints.
      * @return \Phinx\Db\Util\AlterInstructions
      */
-    protected function validateForeignKeys(AlterInstructions $instructions, string $tableName): AlterInstructions
+    protected function validateForeignKeys(string|array $tableNames): void
     {
-        $instructions->addPostStep(function ($state) use ($tableName) {
-            $tablesToCheck = [
-                $tableName,
-            ];
-
-            $otherTables = $this
-                ->query(
-                    "SELECT name FROM sqlite_master WHERE type = 'table' AND name != ?",
-                    [$tableName],
-                )
-                ->fetchAll();
-
-            foreach ($otherTables as $otherTable) {
-                $foreignKeyList = $this->getTableInfo($otherTable['name'], 'foreign_key_list');
-                foreach ($foreignKeyList as $foreignKey) {
-                    if (strcasecmp($foreignKey['table'], $tableName) === 0) {
-                        $tablesToCheck[] = $otherTable['name'];
-                        break;
-                    }
-                }
-            }
-
-            $tablesToCheck = array_unique(array_map('strtolower', $tablesToCheck));
+        if (!is_array($tableNames)) {
+            $tableNames = [$tableNames];
+        }
 
-            foreach ($tablesToCheck as $tableToCheck) {
-                $schema = $this->getSchemaName($tableToCheck, true)['schema'];
+        $tablesToCheck = $tableNames;
 
-                $stmt = $this->query(
-                    sprintf('PRAGMA %sforeign_key_check(%s)', $schema, $this->quoteTableName($tableToCheck)),
-                );
-                $row = $stmt->fetch();
-                $stmt->closeCursor();
+        $otherTables = $this
+            ->query(
+                "SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT IN (" . implode(',',array_fill(0, count($tableNames), '?')) . ')',
+                $tableNames
+            )
+            ->fetchAll();
 
-                if (is_array($row)) {
-                    throw new RuntimeException(sprintf(
-                        'Integrity constraint violation: FOREIGN KEY constraint on `%s` failed.',
-                        $tableToCheck,
-                    ));
+        foreach ($otherTables as $otherTable) {
+            $foreignKeyList = $this->getTableInfo($otherTable['name'], 'foreign_key_list');
+            foreach ($foreignKeyList as $foreignKey) {
+                if (in_array(strtolower($foreignKey['table']), $tableNames)) {
+                    $tablesToCheck[] = $otherTable['name'];
+                    break;
                 }
             }
+        }
 
-            return $state;
-        });
+        $tablesToCheck = array_unique(array_map('strtolower', $tablesToCheck));
 
-        return $instructions;
+        foreach ($tablesToCheck as $tableToCheck) {
+            $schema = $this->getSchemaName($tableToCheck, true)['schema'];
+
+            $stmt = $this->query(
+                sprintf('PRAGMA %sforeign_key_check(%s)', $schema, $this->quoteTableName($tableToCheck)),
+            );
+            $row = $stmt->fetch();
+            $stmt->closeCursor();
+
+            if (is_array($row)) {
+                throw new RuntimeException(sprintf(
+                    'Integrity constraint violation: FOREIGN KEY constraint on `%s` failed.',
+                    $tableToCheck,
+                ));
+            }
+        }
     }
 
     /**
@@ -1239,7 +1234,6 @@ protected function endAlterByCopyTable(
         string $tableName,
         ?string $renamedOrRemovedColumnName = null,
         ?string $newColumnName = null,
-        bool $validateForeignKeys = true,
     ): AlterInstructions {
         $instructions = $this->bufferIndicesAndTriggers($instructions, $tableName);
 
@@ -1251,26 +1245,9 @@ protected function endAlterByCopyTable(
             }
         }
 
-        $foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'];
-
-        if ($foreignKeysEnabled) {
-            $instructions->addPostStep('PRAGMA foreign_keys = OFF');
-        }
-
         $instructions = $this->copyAndDropTmpTable($instructions, $tableName);
         $instructions = $this->recreateIndicesAndTriggers($instructions);
 
-        if ($foreignKeysEnabled) {
-            $instructions->addPostStep('PRAGMA foreign_keys = ON');
-        }
-
-        if (
-            $foreignKeysEnabled &&
-            $validateForeignKeys
-        ) {
-            $instructions = $this->validateForeignKeys($instructions, $tableName);
-        }
-
         return $instructions;
     }
 
@@ -1661,7 +1638,7 @@ protected function getDropPrimaryKeyInstructions(Table $table, string $column):
             return $newState + $state;
         });
 
-        return $this->endAlterByCopyTable($instructions, $tableName, null, null, false);
+        return $this->endAlterByCopyTable($instructions, $tableName, null, null);
     }
 
     /**
@@ -2013,4 +1990,31 @@ public function getDecoratedConnection(): Connection
 
         return $this->decoratedConnection = $this->buildConnection(SqliteDriver::class, $options);
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function preExecuteActions(): array
+    {
+        $foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'];
+
+        if ($foreignKeysEnabled) {
+            $this->execute('PRAGMA foreign_keys = OFF');
+        }
+
+        return [
+            'foreignKeysEnabled' => $foreignKeysEnabled,
+        ];
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function postExecuteActions(array $tableNames, array $preOptions): void
+    {
+        if ($preOptions['foreignKeysEnabled']) {
+            $this->execute('PRAGMA foreign_keys = ON');
+            $this->validateForeignKeys($tableNames);
+        }
+    }
 }

src/Phinx/Db/Plan/Plan.php

@@ -143,15 +143,21 @@ protected function inverseUpdatesSequence(): array
      */
     public function execute(AdapterInterface $executor): void
     {
+        $preOptions = $executor->preExecuteActions();
+
         foreach ($this->tableCreates as $newTable) {
             $executor->createTable($newTable->getTable(), $newTable->getColumns(), $newTable->getIndexes());
         }
 
+        $tables = [];
         foreach ($this->updatesSequence() as $updates) {
             foreach ($updates as $update) {
+                $tables[] = $update->getTable()->getName();
                 $executor->executeActions($update->getTable(), $update->getActions());
             }
         }
+
+        $executor->postExecuteActions(array_unique($tables), $preOptions);
     }
 
     /**

tests/Phinx/Db/Adapter/SQLiteAdapterTest.php

@@ -2387,6 +2387,7 @@ public function testAlterTableDoesViolateForeignKeyConstraintOnTargetTableChange
      */
     public function testAlterTableDoesViolateForeignKeyConstraintOnSourceTableChange()
     {
+        /** @var \Phinx\Db\Adapter\AdapterInterface&\PHPUnit\Framework\MockObject\MockObject */
         $adapter = $this
             ->getMockBuilder(SQLiteAdapter::class)
             ->setConstructorArgs([SQLITE_DB_CONFIG, new ArrayInput([]), new NullOutput()])