Skip to content

Commit 52190e3

Browse files
thePanzthePanz
committed
Fix(migrations) Handle transaction within migrations
This fixes #98
1 parent f47a790 commit 52190e3

File tree

7 files changed

+138
-24
lines changed

7 files changed

+138
-24
lines changed

‎.github/workflows/continuous-integration.yml

+12
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ jobs:
3535
- "8.1"
3636
- "8.2"
3737
- "8.3"
38+
services:
39+
mysql:
40+
image: mysql:5.7-debian
41+
env:
42+
MYSQL_DATABASE: 'doctrine1_test'
43+
MYSQL_USER: 'doctrine1'
44+
MYSQL_PASSWORD: 'd0ctr1n3'
45+
MYSQL_ALLOW_EMPTY_PASSWORD: true
46+
ports:
47+
- 3306:3306
3848

3949
steps:
4050
- name: Checkout
@@ -62,4 +72,6 @@ jobs:
6272
run: composer install --prefer-dist
6373

6474
- name: Run Tests
75+
env:
76+
MYSQL_TEST_DSN: 'mysql:host=127.0.0.1;dbname=doctrine1_test;user=doctrine1;password=d0ctr1n3'
6577
run: cd tests && php run.php

‎lib/Doctrine/Export.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -1221,7 +1221,7 @@ public function exportClasses(array $classes)
12211221
}
12221222
}
12231223

1224-
$connection->commit();
1224+
Doctrine_TransactionHelper::commitIfInTransaction($connection);
12251225
}
12261226
}
12271227

‎lib/Doctrine/Migration.php

+19-23
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ public function loadMigrationClass($name, $path = null)
187187
}
188188

189189
if ($class === false) {
190-
return false;
190+
return;
191191
}
192192

193193
if (empty($this->_migrationClasses)) {
@@ -307,15 +307,13 @@ public function getNextMigrationClassVersion()
307307
*
308308
* @param integer $to Version to migrate to
309309
* @param boolean $dryRun Whether or not to run the migrate process as a dry run
310-
* @return integer $to Version number migrated to
310+
* @return bool True if the migration succeeded, false otherwise
311311
* @throws Doctrine_Exception
312312
*/
313313
public function migrate($to = null, $dryRun = false)
314314
{
315315
$this->clearErrors();
316-
317316
$this->_createMigrationTable();
318-
319317
$this->_connection->beginTransaction();
320318

321319
try {
@@ -335,24 +333,21 @@ public function migrate($to = null, $dryRun = false)
335333

336334
if ($dryRun) {
337335
return false;
338-
} else {
339-
$this->_throwErrorsException();
340-
}
341-
} else {
342-
if ($dryRun) {
343-
$this->_connection->rollback();
344-
if ($this->hasErrors()) {
345-
return false;
346-
} else {
347-
return $to;
348-
}
349-
} else {
350-
$this->_connection->commit();
351-
$this->setCurrentVersion($to);
352-
return $to;
353336
}
337+
338+
$this->_throwErrorsException();
354339
}
355-
return false;
340+
341+
if ($dryRun) {
342+
$this->_connection->rollback();
343+
344+
return !$this->hasErrors();
345+
}
346+
347+
Doctrine_TransactionHelper::commitIfInTransaction($this->_connection);
348+
$this->setCurrentVersion($to);
349+
350+
return true;
356351
}
357352

358353
/**
@@ -437,8 +432,9 @@ public function getMigrationClass($num)
437432
/**
438433
* Throw an exception with all the errors trigged during the migration
439434
*
440-
* @return void
441-
* @throws Doctrine_Migration_Exception $e
435+
* @throws Doctrine_Migration_Exception
436+
*
437+
* @return never
442438
*/
443439
protected function _throwErrorsException()
444440
{
@@ -559,4 +555,4 @@ protected function _createMigrationTable()
559555
return false;
560556
}
561557
}
562-
}
558+
}

‎lib/Doctrine/TransactionHelper.php

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/**
4+
* Helper for transactions handling.
5+
*
6+
* @author Kyle McGrogan <[email protected]>
7+
*/
8+
final class Doctrine_TransactionHelper
9+
{
10+
/**
11+
* Execute a commit on the given connection, only if a transaction already started.
12+
*/
13+
public static function commitIfInTransaction(Doctrine_Connection $connection): void
14+
{
15+
$handler = $connection->getDbh();
16+
17+
// Attempt to commit while no transaction is running results in exception since PHP 8 + pdo_mysql combination
18+
if ($handler instanceof PDO && !$handler->inTransaction()) {
19+
return;
20+
}
21+
22+
$connection->commit();
23+
}
24+
}

‎tests/DoctrineTest/Doctrine_UnitTestCase.php

+13
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ public function init()
216216
}
217217
}
218218
}
219+
219220
public function prepareTables() {
220221
foreach($this->tables as $name) {
221222
$name = ucwords($name);
@@ -230,6 +231,7 @@ public function prepareTables() {
230231
$this->conn->export->exportClasses($this->tables);
231232
$this->objTable = $this->connection->getTable('User');
232233
}
234+
233235
public function prepareData()
234236
{
235237
$groups = new Doctrine_Collection($this->connection->getTable('Group'));
@@ -307,6 +309,17 @@ public function getDeclaration($type)
307309
return $this->dataDict->getPortableDeclaration(array('type' => $type, 'name' => 'colname', 'length' => 1, 'fixed' => true));
308310
}
309311

312+
/**
313+
* @param string $pdoDsn
314+
* @return Doctrine_Connection
315+
*/
316+
protected function openAdditionalPdoConnection(string $pdoDsn)
317+
{
318+
$pdoFromDsn = new PDO($pdoDsn);
319+
320+
return $this->openAdditionalConnection($pdoFromDsn);
321+
}
322+
310323
protected function openAdditionalConnection($adapter = null, $name = null)
311324
{
312325
$connection = $this->manager->openConnection($adapter, $name);

‎tests/MigrationMysqlTestCase.php

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
/*
3+
* $Id$
4+
*
5+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
6+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
7+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
8+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
9+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
10+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
11+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
12+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
13+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
14+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
15+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
16+
*
17+
* This software consists of voluntary contributions made by many individuals
18+
* and is licensed under the LGPL. For more information, see
19+
* <http://www.doctrine-project.org>.
20+
*/
21+
22+
/**
23+
* Doctrine_Migration_TestCase
24+
*
25+
* @package Doctrine
26+
* @author Konsta Vesterinen <[email protected]>
27+
* @license http://www.opensource.org/licenses/lgpl-license.php LGPL
28+
* @category Object Relational Mapping
29+
* @link www.doctrine-project.org
30+
* @since 1.0
31+
* @version $Revision$
32+
*/
33+
class Doctrine_MigrationMysql_TestCase extends Doctrine_UnitTestCase
34+
{
35+
public function setUp()
36+
{
37+
parent::setUp();
38+
$dsn = getenv('MYSQL_TEST_DSN') ?? '';
39+
40+
$this->connection = $this->openAdditionalPdoConnection($dsn);
41+
$this->connection->setOption('dsn', $dsn);
42+
$this->connection->dropDatabase();
43+
$this->connection->createDatabase();
44+
}
45+
46+
public function testAfterSuccessfullMigrationItWillSetMigratedVersionAsCurrentVersionInMysqlDB()
47+
{
48+
$migration = new Doctrine_Migration(__DIR__.'/migration_classes', $this->connection);
49+
$this->assertFalse($migration->hasMigrated());
50+
$migration->setCurrentVersion(0);
51+
$this->assertFalse($this->connection->import->tableExists('migration_phonenumber'));
52+
$this->assertFalse($this->connection->import->tableExists('migration_user'));
53+
$this->assertFalse($this->connection->import->tableExists('migration_profile'));
54+
55+
$migration->migrate(3);
56+
$this->assertTrue($migration->hasMigrated());
57+
$this->assertEqual($migration->getCurrentVersion(), 3);
58+
$this->assertTrue($this->connection->import->tableExists('migration_phonenumber'));
59+
$this->assertTrue($this->connection->import->tableExists('migration_user'));
60+
$this->assertTrue($this->connection->import->tableExists('migration_profile'));
61+
62+
$migration->migrate(4);
63+
$this->assertEqual($migration->getCurrentVersion(), 4);
64+
$this->assertTrue($this->connection->import->tableExists('migration_phonenumber'));
65+
$this->assertTrue($this->connection->import->tableExists('migration_user'));
66+
$this->assertFalse($this->connection->import->tableExists('migration_profile'));
67+
}
68+
}

‎tests/run.php

+1
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@
279279
// Migration Tests
280280
$migration = new GroupTest('Migration Tests', 'migration');
281281
$migration->addTestCase(new Doctrine_Migration_TestCase());
282+
$migration->addTestCase(new Doctrine_MigrationMysql_TestCase());
282283
$migration->addTestCase(new Doctrine_Migration_Base_TestCase());
283284
$migration->addTestCase(new Doctrine_Migration_Diff_TestCase());
284285
$test->addTestCase($migration);

0 commit comments

Comments
 (0)