Fix(migrations) Handle transaction within migrations

This fixes #98
FriendsOfSymfony1 · Jun 27, 2024 · c43d678 · c43d678
1 parent f47a790
commit c43d678
Show file tree

Hide file tree

Showing 7 changed files with 170 additions and 37 deletions.
diff --git a/lib/Doctrine/Export.php b/lib/Doctrine/Export.php
@@ -1221,7 +1221,7 @@ public function exportClasses(array $classes)
                  }
              }
 
-             $connection->commit();
+             Doctrine_TransactionHelper::commitIfInTransaction($connection);
          }
      }
 

diff --git a/lib/Doctrine/Migration.php b/lib/Doctrine/Migration.php
@@ -187,7 +187,7 @@ public function loadMigrationClass($name, $path = null)
         }
 
         if ($class === false) {
-            return false;
+            return;
         }
 
         if (empty($this->_migrationClasses)) {
@@ -307,15 +307,13 @@ public function getNextMigrationClassVersion()
      *
      * @param  integer $to       Version to migrate to
      * @param  boolean $dryRun   Whether or not to run the migrate process as a dry run
-     * @return integer $to       Version number migrated to
+     * @return bool              True if the migration succeeded, false otherwise
      * @throws Doctrine_Exception
      */
     public function migrate($to = null, $dryRun = false)
     {
         $this->clearErrors();
-
         $this->_createMigrationTable();
-
         $this->_connection->beginTransaction();
 
         try {
@@ -335,24 +333,22 @@ public function migrate($to = null, $dryRun = false)
 
             if ($dryRun) {
                 return false;
-            } else {
-                $this->_throwErrorsException();
-            }
-        } else {
-            if ($dryRun) {
-                $this->_connection->rollback();
-                if ($this->hasErrors()) {
-                    return false;
-                } else {
-                    return $to;
-                }
-            } else {
-                $this->_connection->commit();
-                $this->setCurrentVersion($to);
-                return $to;
             }
+
+            $this->_throwErrorsException();
         }
-        return false;
+
+
+        if ($dryRun) {
+            $this->_connection->rollback();
+
+            return !$this->hasErrors();
+        }
+
+        Doctrine_TransactionHelper::commitIfInTransaction($this->_connection);
+        $this->setCurrentVersion($to);
+
+        return true;
     }
 
     /**
@@ -437,8 +433,9 @@ public function getMigrationClass($num)
     /**
      * Throw an exception with all the errors trigged during the migration
      *
-     * @return void
-     * @throws Doctrine_Migration_Exception $e
+     * @throws Doctrine_Migration_Exception
+     *
+     * @return never
      */
     protected function _throwErrorsException()
     {
@@ -559,4 +556,4 @@ protected function _createMigrationTable()
             return false;
         }
     }
-}
+}
diff --git a/lib/Doctrine/TransactionHelper.php b/lib/Doctrine/TransactionHelper.php
@@ -0,0 +1,47 @@
+<?php
+
+/*
+ *  $Id$
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the LGPL. For more information, see
+ * <http://www.doctrine-project.org>.
+ */
+
+/**
+ * Doctrine_Transaction_Helper
+ *
+ * @package     Doctrine
+ * @subpackage  Transaction
+ * @license     http://www.opensource.org/licenses/lgpl-license.php LGPL
+ * @link        www.doctrine-project.org
+ * @since       1.4
+ * @version     $Revision$
+ * @author      Kyle McGrogan <[email protected]>
+ */
+final class Doctrine_TransactionHelper
+{
+    public static function commitIfInTransaction(Doctrine_Connection $connection): void
+    {
+        $handler = $connection->getDbh();
+
+        // Attempt to commit while no transaction is running results in exception since PHP 8 + pdo_mysql combination
+        if ($handler instanceof PDO && !$handler->inTransaction()) {
+            return;
+        }
+
+        $connection->commit();
+    }
+}
diff --git a/tests/DoctrineTest/Doctrine_UnitTestCase.php b/tests/DoctrineTest/Doctrine_UnitTestCase.php
@@ -216,20 +216,29 @@ public function init()
             }
         }
     }
+
     public function prepareTables() {
-        foreach($this->tables as $name) {
+        $this->dropTablesOnConnection($this->tables, $this->connection);
+
+        $this->objTable = $this->connection->getTable('User');
+    }
+
+    protected function dropTablesOnConnection(array $tables, Doctrine_Connection $connection)
+    {
+        foreach($tables as $name) {
             $name = ucwords($name);
-            $table = $this->connection->getTable($name);
+            $table = $connection->getTable($name);
             $query = 'DROP TABLE ' . $table->getTableName();
+
             try {
-                $this->conn->exec($query);
+                $connection->exec($query);
             } catch(Doctrine_Connection_Exception $e) {
-
             }
         }
-        $this->conn->export->exportClasses($this->tables);
-        $this->objTable = $this->connection->getTable('User');
+
+        $connection->export->exportClasses($tables);
     }
+
     public function prepareData()
     {
         $groups = new Doctrine_Collection($this->connection->getTable('Group'));
@@ -307,6 +316,17 @@ public function getDeclaration($type)
         return $this->dataDict->getPortableDeclaration(array('type' => $type, 'name' => 'colname', 'length' => 1, 'fixed' => true));
     }
 
+    /**
+     * @param string $pdoDsn
+     * @return Doctrine_Connection
+     */
+    protected function openAdditionalPdoConnection(string $pdoDsn)
+    {
+        $pdoFromDsn = new PDO($pdoDsn);
+
+        return $this->openAdditionalConnection($pdoFromDsn);
+    }
+
     protected function openAdditionalConnection($adapter = null, $name = null)
     {
         $connection = $this->manager->openConnection($adapter, $name);

diff --git a/tests/MigrationMysqlTestCase.php b/tests/MigrationMysqlTestCase.php
@@ -0,0 +1,68 @@
+<?php
+/*
+ *  $Id$
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the LGPL. For more information, see
+ * <http://www.doctrine-project.org>.
+ */
+
+/**
+ * Doctrine_Migration_TestCase
+ *
+ * @package     Doctrine
+ * @author      Konsta Vesterinen <[email protected]>
+ * @license     http://www.opensource.org/licenses/lgpl-license.php LGPL
+ * @category    Object Relational Mapping
+ * @link        www.doctrine-project.org
+ * @since       1.0
+ * @version     $Revision$
+ */
+class Doctrine_MigrationMysql_TestCase extends Doctrine_UnitTestCase
+{
+    public function setUp()
+    {
+        parent::setUp();
+        $dsn = getenv('MYSQL_TEST_DSN') ?? '';
+
+        $this->connection = $this->openAdditionalPdoConnection($dsn);
+        $this->connection->setOption('dsn', $dsn);
+        $this->connection->dropDatabase();
+        $this->connection->createDatabase();
+    }
+
+    public function testAfterSuccessfullMigrationItWillSetMigratedVersionAsCurrentVersionInMysqlDB()
+    {
+        $migration = new Doctrine_Migration(__DIR__.'/migration_classes', $this->connection);
+        $this->assertFalse($migration->hasMigrated());
+        $migration->setCurrentVersion(0);
+        $this->assertFalse($this->connection->import->tableExists('migration_phonenumber'));
+        $this->assertFalse($this->connection->import->tableExists('migration_user'));
+        $this->assertFalse($this->connection->import->tableExists('migration_profile'));
+
+        $migration->migrate(3);
+        $this->assertTrue($migration->hasMigrated());
+        $this->assertEqual($migration->getCurrentVersion(), 3);
+        $this->assertTrue($this->connection->import->tableExists('migration_phonenumber'));
+        $this->assertTrue($this->connection->import->tableExists('migration_user'));
+        $this->assertTrue($this->connection->import->tableExists('migration_profile'));
+
+        $migration->migrate(4);
+        $this->assertEqual($migration->getCurrentVersion(), 4);
+        $this->assertTrue($this->connection->import->tableExists('migration_phonenumber'));
+        $this->assertTrue($this->connection->import->tableExists('migration_user'));
+        $this->assertFalse($this->connection->import->tableExists('migration_profile'));
+    }
+}
diff --git a/tests/MigrationTestCase.php b/tests/MigrationTestCase.php
@@ -32,13 +32,13 @@
  */
 class Doctrine_Migration_TestCase extends Doctrine_UnitTestCase
 {
-    public function prepareTables()
-    {
-        $this->tables[] = 'MigrationPhonenumber';
-        $this->tables[] = 'MigrationUser';
-        $this->tables[] = 'MigrationProfile';
-        parent::prepareTables();
-    }
+    const TABLES = array(
+        'MigrationPhonenumber',
+        'MigrationUser',
+        'MigrationProfile',
+    );
+
+    protected $tables = self::TABLES;
 
     public function testMigration()
     {

diff --git a/tests/run.php b/tests/run.php
@@ -279,6 +279,7 @@
 // Migration Tests
 $migration = new GroupTest('Migration Tests', 'migration');
 $migration->addTestCase(new Doctrine_Migration_TestCase());
+$migration->addTestCase(new Doctrine_MigrationMysql_TestCase());
 $migration->addTestCase(new Doctrine_Migration_Base_TestCase());
 $migration->addTestCase(new Doctrine_Migration_Diff_TestCase());
 $test->addTestCase($migration);
-Original file line number
+Diff line change
@@ Expand Up / @@ -1221,7 +1221,7 @@ public function exportClasses(array $classes) @@
                      }
                  }
-                 $connection->commit();
+                 Doctrine_TransactionHelper::commitIfInTransaction($connection);
              }
          }
@@ Expand Down @@