Merge pull request #745 from cakephp/migration-transactions

Add hook method for migrations to skip creating a transaction
cakephp · Sep 16, 2024 · 8bc6285 · 8bc6285
2 parents 2fb93e5 + 62be947
commit 8bc6285
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 14 deletions.
diff --git a/src/AbstractMigration.php b/src/AbstractMigration.php
@@ -29,6 +29,19 @@ class AbstractMigration extends BaseAbstractMigration
      */
     public bool $autoId = true;
 
+    /**
+     * Hook method to decide if this migration should use transactions
+     *
+     * By default if your driver supports transactions, a transaction will be opened
+     * before the migration begins, and commit when the migration completes.
+     *
+     * @return bool
+     */
+    public function useTransactions(): bool
+    {
+        return $this->getAdapter()->hasTransactions();
+    }
+
     /**
      * Returns an instance of the Table class.
      *

diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php
@@ -1046,15 +1046,7 @@ protected function copyAndDropTmpTable(AlterInstructions $instructions, string $
                 $state['selectColumns']
             );
 
-            $result = $this->fetchRow('PRAGMA foreign_keys');
-            $foreignKeysEnabled = $result ? (bool)$result['foreign_keys'] : false;
-            if ($foreignKeysEnabled) {
-                $this->execute('PRAGMA foreign_keys = OFF');
-            }
             $this->execute(sprintf('DROP TABLE %s', $this->quoteTableName($tableName)));
-            if ($foreignKeysEnabled) {
-                $this->execute('PRAGMA foreign_keys = ON');
-            }
             $this->execute(sprintf(
                 'ALTER TABLE %s RENAME TO %s',
                 $this->quoteTableName($state['tmpTableName']),

diff --git a/src/Migration/Environment.php b/src/Migration/Environment.php
@@ -85,8 +85,12 @@ public function executeMigration(MigrationInterface $migration, string $directio
             $migration->{MigrationInterface::INIT}();
         }
 
+        $atomic = $adapter->hasTransactions();
+        if (method_exists($migration, 'useTransactions')) {
+            $atomic = $migration->useTransactions();
+        }
         // begin the transaction if the adapter supports it
-        if ($adapter->hasTransactions()) {
+        if ($atomic) {
             $adapter->beginTransaction();
         }
 
@@ -121,7 +125,7 @@ public function executeMigration(MigrationInterface $migration, string $directio
         $adapter->migrated($migration, $direction, date('Y-m-d H:i:s', $startTime), date('Y-m-d H:i:s', time()));
 
         // commit the transaction if the adapter supports it
-        if ($adapter->hasTransactions()) {
+        if ($atomic) {
             $adapter->commitTransaction();
         }
 
@@ -143,9 +147,9 @@ public function executeSeed(SeedInterface $seed): void
         if (method_exists($seed, SeedInterface::INIT)) {
             $seed->{SeedInterface::INIT}();
         }
-
         // begin the transaction if the adapter supports it
-        if ($adapter->hasTransactions()) {
+        $atomic = $adapter->hasTransactions();
+        if ($atomic) {
             $adapter->beginTransaction();
         }
 
@@ -155,7 +159,7 @@ public function executeSeed(SeedInterface $seed): void
         }
 
         // commit the transaction if the adapter supports it
-        if ($adapter->hasTransactions()) {
+        if ($atomic) {
             $adapter->commitTransaction();
         }
     }

diff --git a/tests/TestCase/Migration/EnvironmentTest.php b/tests/TestCase/Migration/EnvironmentTest.php
@@ -170,7 +170,7 @@ public function testExecutingAMigrationWithTransactions()
         $adapterStub->expects($this->once())
                     ->method('commitTransaction');
 
-        $adapterStub->expects($this->exactly(2))
+        $adapterStub->expects($this->exactly(1))
                     ->method('hasTransactions')
                     ->willReturn(true);
 
@@ -189,6 +189,43 @@ public function up(): void
         $this->assertTrue($migration->executed);
     }
 
+    public function testExecutingAMigrationWithUseTransactions()
+    {
+        // stub adapter
+        $adapterStub = $this->getMockBuilder(PdoAdapter::class)
+            ->setConstructorArgs([[]])
+            ->getMock();
+        $adapterStub->expects($this->never())
+                    ->method('beginTransaction');
+
+        $adapterStub->expects($this->never())
+                    ->method('commitTransaction');
+
+        $adapterStub->expects($this->exactly(1))
+                    ->method('hasTransactions')
+                    ->willReturn(true);
+
+        $this->environment->setAdapter($adapterStub);
+
+        // migrate
+        $migration = new class ('mockenv', 20110301080000) extends AbstractMigration {
+            public bool $executed = false;
+
+            public function useTransactions(): bool
+            {
+                return false;
+            }
+
+            public function up(): void
+            {
+                $this->executed = true;
+            }
+        };
+
+        $this->environment->executeMigration($migration, MigrationInterface::UP);
+        $this->assertTrue($migration->executed);
+    }
+
     public function testExecutingAChangeMigrationUp()
     {
         // stub adapter