From 2804b4fb56cf830eae6af0a63c0549c1d0a7de04 Mon Sep 17 00:00:00 2001 From: thePanz Date: Thu, 27 Jun 2024 15:11:52 +0200 Subject: [PATCH] Fix(migrations) Handle transaction within migrations This fixes #98 --- lib/Doctrine/Export.php | 2 +- lib/Doctrine/Migration.php | 43 ++++++------- lib/Doctrine/TransactionHelper.php | 47 ++++++++++++++ tests/DoctrineTest/Doctrine_UnitTestCase.php | 32 +++++++-- tests/MigrationMysqlTestCase.php | 68 ++++++++++++++++++++ tests/run.php | 1 + 6 files changed, 163 insertions(+), 30 deletions(-) create mode 100644 lib/Doctrine/TransactionHelper.php create mode 100644 tests/MigrationMysqlTestCase.php diff --git a/lib/Doctrine/Export.php b/lib/Doctrine/Export.php index ab56d6620..f09610451 100644 --- 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 index 0208ce540..5d0571af9 100644 --- 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; } } -} \ No newline at end of file +} diff --git a/lib/Doctrine/TransactionHelper.php b/lib/Doctrine/TransactionHelper.php new file mode 100644 index 000000000..ace8f7b7a --- /dev/null +++ b/lib/Doctrine/TransactionHelper.php @@ -0,0 +1,47 @@ +. + */ + +/** + * 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 + */ +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 index 3fa7bcbcd..c0e1723be 100644 --- 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 new file mode 100644 index 000000000..5e753ac01 --- /dev/null +++ b/tests/MigrationMysqlTestCase.php @@ -0,0 +1,68 @@ +. + */ + +/** + * Doctrine_Migration_TestCase + * + * @package Doctrine + * @author Konsta Vesterinen + * @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/run.php b/tests/run.php index d1dd2c7ee..ddb4619ba 100644 --- 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);