Repository: doctrine/migrations (php)

Issue:
Exception on wrong transactional logic hides underlying exception
### Bug Report

<!-- Fill in the relevant information below to help triage your issue. -->

|    Q        |   A
|------------ | ------
| BC Break    | 
| Version     | 3.3.0

#### Summary

Flawed transaction logic throws an Exception and leads to loss of the underlying Exception thus making it hard to debug the issue.


#### Details

https://github.com/doctrine/migrations/blob/36205d5af1d00fa2348ab91fddee5f4f45511732/lib/Doctrine/Migrations/Tools/TransactionHelper.php#L60-L64

This does not work anymore, because `wrappedConnection` returns `PDO\Connection` since one of the last updates. Line 60 should be replaced by `$connection->getWrappedConnection()->getWrappedConnection();` or something like that.

This method is executed deep down in this block (line 115):
https://github.com/doctrine/migrations/blob/36205d5af1d00fa2348ab91fddee5f4f45511732/lib/Doctrine/Migrations/Version/DbalExecutor.php#L112-L118
thus resulting in trying to rollback a transaction already rolled back, therefore throwing an exception and then leads to the loss of the underlying Exception `$e`.

#### How to reproduce

Write a failing migration playing with transactions?


Pull request description:
Look harder for a native connection
doctrine/dbal 3 switches from inheritance to composition but still
provides an accessor for PDO, and it is named like the method in
Connection.
Unwrapping until that method no longer exists should give the innermost
connection regardless of the DBAL version

I hade to adapt some tests to ensure `getWrappedConnection` returns an object.

Fixes #1209

# How can I test this?

```shell
composer config repositories.greg0ire vcs https://github.com/greg0ire/migrations
composer require doctrine/migrations "dev-fix-compat-with-dbal-3 as 3.3.0"
```


Patch:
diff --git a/lib/Doctrine/Migrations/Tools/TransactionHelper.php b/lib/Doctrine/Migrations/Tools/TransactionHelper.php
index bd675245..ebd9007d 100644
--- a/lib/Doctrine/Migrations/Tools/TransactionHelper.php
+++ b/lib/Doctrine/Migrations/Tools/TransactionHelper.php
@@ -8,6 +8,8 @@ use Doctrine\DBAL\Connection;
 use Doctrine\Deprecations\Deprecation;
 use PDO;
 
+use function method_exists;
+
 /**
  * @internal
  */
@@ -57,10 +59,13 @@ DEPRECATION
 
     private static function inTransaction(Connection $connection): bool
     {
-        $wrappedConnection = $connection->getWrappedConnection();
+        $innermostConnection = $connection;
+        while (method_exists($innermostConnection, 'getWrappedConnection')) {
+            $innermostConnection = $innermostConnection->getWrappedConnection();
+        }
 
         /* Attempt to commit or rollback while no transaction is running
            results in an exception since PHP 8 + pdo_mysql combination */
-        return ! $wrappedConnection instanceof PDO || $wrappedConnection->inTransaction();
+        return ! $innermostConnection instanceof PDO || $innermostConnection->inTransaction();
     }
 }
diff --git a/phpstan-common.neon.dist b/phpstan-common.neon.dist
index 7a41be19..92380af7 100644
--- a/phpstan-common.neon.dist
+++ b/phpstan-common.neon.dist
@@ -1,30 +1,35 @@
 parameters:
     level: 7
     paths:
-        - %currentWorkingDirectory%/lib
-        - %currentWorkingDirectory%/tests
+        - lib
+        - tests
     excludePaths:
-        - %currentWorkingDirectory%/tests/Doctrine/Migrations/Tests/Configuration/ConfigurationTestSource/Migrations/Version123.php
+        - tests/Doctrine/Migrations/Tests/Configuration/ConfigurationTestSource/Migrations/Version123.php
     ignoreErrors:
         - '~Variable method call on Doctrine\\Migrations\\AbstractMigration~'
         -
             message: '~^Call to function in_array\(\) requires parameter #3 to be true\.$~'
-            path: %currentWorkingDirectory%/lib/Doctrine/Migrations/Version/SortedMigrationPlanCalculator.php
+            path: lib/Doctrine/Migrations/Version/SortedMigrationPlanCalculator.php
         -
             message: '~^Variable property access on SimpleXMLElement\.$~'
-            path: %currentWorkingDirectory%/lib/Doctrine/Migrations/Configuration/Migration/XmlFile.php
+            path: lib/Doctrine/Migrations/Configuration/Migration/XmlFile.php
 
         -
             message: '~^Call to function is_bool\(\) with bool will always evaluate to true\.$~'
-            path: %currentWorkingDirectory%/lib/Doctrine/Migrations/InlineParameterFormatter.php
+            path: lib/Doctrine/Migrations/InlineParameterFormatter.php
         -
             message: '~^Call to an undefined method Symfony\\Component\\Console\\Output\\OutputInterface\:\:getErrorOutput\(\)\.$~'
-            path: %currentWorkingDirectory%/lib/Doctrine/Migrations/Tools/Console/ConsoleLogger.php
+            path: lib/Doctrine/Migrations/Tools/Console/ConsoleLogger.php
         -
             message: '~^Method Doctrine\\Migrations\\Tests\\Stub\\DoctrineRegistry::getService\(\) should return Doctrine\\Persistence\\ObjectManager but returns Doctrine\\DBAL\\Connection\|Doctrine\\ORM\\EntityManager~'
-            path: %currentWorkingDirectory%/tests/Doctrine/Migrations/Tests/Stub/DoctrineRegistry.php
+            path: tests/Doctrine/Migrations/Tests/Stub/DoctrineRegistry.php
         - '~Call to method getVersion\(\) of deprecated class PackageVersions\\Versions\:.*~'
 
+	# https://github.com/phpstan/phpstan/issues/5982
+        -
+            message: '~^Cannot call method getWrappedConnection\(\) on class-string\|object\.~'
+            path: lib/Doctrine/Migrations/Tools/TransactionHelper.php
+
         # Requires PHPUnit 9
         -
             message: '~assert.*Reg~'


Test patch:
diff --git a/tests/Doctrine/Migrations/Tests/Event/Listeners/AutoCommitListenerTest.php b/tests/Doctrine/Migrations/Tests/Event/Listeners/AutoCommitListenerTest.php
index 1f285c0e..16907960 100644
--- a/tests/Doctrine/Migrations/Tests/Event/Listeners/AutoCommitListenerTest.php
+++ b/tests/Doctrine/Migrations/Tests/Event/Listeners/AutoCommitListenerTest.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Doctrine\Migrations\Tests\Event\Listeners;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver\Connection as DriverConnection;
 use Doctrine\Migrations\Event\Listeners\AutoCommitListener;
 use Doctrine\Migrations\Event\MigrationsEventArgs;
 use Doctrine\Migrations\Metadata\MigrationPlanList;
@@ -51,9 +52,9 @@ class AutoCommitListenerTest extends MigrationTestCase
 
     protected function setUp(): void
     {
-        $this->conn = $this->getMockBuilder(Connection::class)
-            ->disableOriginalConstructor()
-            ->getMock();
+        $this->conn       = $this->createStub(Connection::class);
+        $driverConnection = $this->createStub(DriverConnection::class);
+        $this->conn->method('getWrappedConnection')->willReturn($driverConnection);
 
         $this->listener = new AutoCommitListener();
     }
diff --git a/tests/Doctrine/Migrations/Tests/MigratorTest.php b/tests/Doctrine/Migrations/Tests/MigratorTest.php
index ce8a9f6c..f4d0685c 100644
--- a/tests/Doctrine/Migrations/Tests/MigratorTest.php
+++ b/tests/Doctrine/Migrations/Tests/MigratorTest.php
@@ -6,6 +6,7 @@ namespace Doctrine\Migrations\Tests;
 
 use Doctrine\Common\EventManager;
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver\Connection as DriverConnection;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\DbalMigrator;
 use Doctrine\Migrations\EventDispatcher;
@@ -54,7 +55,10 @@ class MigratorTest extends MigrationTestCase
 
     protected function setUp(): void
     {
-        $this->conn   = $this->createMock(Connection::class);
+        $this->conn       = $this->createMock(Connection::class);
+        $driverConnection = $this->createStub(DriverConnection::class);
+        $this->conn->method('getWrappedConnection')->willReturn($driverConnection);
+
         $this->config = new Configuration();
 
         $this->migratorConfiguration = new MigratorConfiguration();
diff --git a/tests/Doctrine/Migrations/Tests/Version/ExecutorTest.php b/tests/Doctrine/Migrations/Tests/Version/ExecutorTest.php
index b6715860..a35862f2 100644
--- a/tests/Doctrine/Migrations/Tests/Version/ExecutorTest.php
+++ b/tests/Doctrine/Migrations/Tests/Version/ExecutorTest.php
@@ -6,6 +6,7 @@ namespace Doctrine\Migrations\Tests\Version;
 
 use Doctrine\Common\EventManager;
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver\Connection as DriverConnection;
 use Doctrine\Migrations\EventDispatcher;
 use Doctrine\Migrations\Events;
 use Doctrine\Migrations\Metadata\MigrationPlan;
@@ -518,8 +519,10 @@ class ExecutorTest extends TestCase
 
     protected function setUp(): void
     {
-        $this->metadataStorage    = $this->createMock(MetadataStorage::class);
-        $this->connection         = $this->createMock(Connection::class);
+        $this->metadataStorage = $this->createMock(MetadataStorage::class);
+        $this->connection      = $this->createMock(Connection::class);
+        $driverConnection      = $this->createStub(DriverConnection::class);
+        $this->connection->method('getWrappedConnection')->willReturn($driverConnection);
         $this->schemaDiffProvider = $this->createMock(SchemaDiffProvider::class);
         $this->parameterFormatter = $this->createMock(ParameterFormatter::class);