Add custom DBAL driver middleware (#1029)

src/Persistence/Sql/Connection.php

@@ -6,17 +6,16 @@
 
 use Atk4\Core\DiContainerTrait;
 use Doctrine\Common\EventManager;
+use Doctrine\DBAL\Configuration as DbalConfiguration;
 use Doctrine\DBAL\Connection as DbalConnection;
+use Doctrine\DBAL\ConnectionException as DbalConnectionException;
+use Doctrine\DBAL\Driver as DbalDriver;
 use Doctrine\DBAL\Driver\Connection as DbalDriverConnection;
+use Doctrine\DBAL\Driver\Middleware as DbalMiddleware;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
-use Doctrine\DBAL\Platforms\OraclePlatform;
-use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
-use Doctrine\DBAL\Platforms\SqlitePlatform;
-use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Result as DbalResult;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
-use Doctrine\DBAL\Schema\SqliteSchemaManager;
 
 /**
  * Class for establishing and maintaining connection with your database.
@@ -224,6 +223,21 @@ private static function getDriverNameFromDbalDriverConnection(DbalDriverConnecti
         return null; // @phpstan-ignore-line
     }
 
+    protected static function createDbalConfiguration(): DbalConfiguration
+    {
+        $dbalConfiguration = new DbalConfiguration();
+        $dbalConfiguration->setMiddlewares([
+            new class() implements DbalMiddleware {
+                public function wrap(DbalDriver $driver): DbalDriver
+                {
+                    return new DbalDriverMiddleware($driver);
+                }
+            },
+        ]);
+
+        return $dbalConfiguration;
+    }
+
     protected static function createDbalEventManager(): EventManager
     {
         return new EventManager();
@@ -240,7 +254,7 @@ protected static function connectFromDsn(array $dsn): DbalDriverConnection
 
         $dbalConnection = DriverManager::getConnection(
             $dsn,
-            null,
+            (static::class)::createDbalConfiguration(),
             (static::class)::createDbalEventManager()
         );
 
@@ -249,39 +263,15 @@ protected static function connectFromDsn(array $dsn): DbalDriverConnection
 
     protected static function connectFromDbalDriverConnection(DbalDriverConnection $dbalDriverConnection): DbalConnection
     {
-        $dbalConnection = DriverManager::getConnection([
-            'driver' => self::getDriverNameFromDbalDriverConnection($dbalDriverConnection),
-        ], null, (static::class)::createDbalEventManager());
+        $dbalConnection = DriverManager::getConnection(
+            ['driver' => self::getDriverNameFromDbalDriverConnection($dbalDriverConnection)],
+            (static::class)::createDbalConfiguration(),
+            (static::class)::createDbalEventManager()
+        );
         \Closure::bind(function () use ($dbalConnection, $dbalDriverConnection): void {
             $dbalConnection->_conn = $dbalDriverConnection;
         }, null, \Doctrine\DBAL\Connection::class)();
 
-        if ($dbalConnection->getDatabasePlatform() instanceof SqlitePlatform) {
-            \Closure::bind(function () use ($dbalConnection) {
-                $dbalConnection->platform = new class() extends SqlitePlatform {
-                    use Sqlite\PlatformTrait;
-                };
-            }, null, DbalConnection::class)();
-        } elseif ($dbalConnection->getDatabasePlatform() instanceof PostgreSQLPlatform) {
-            \Closure::bind(function () use ($dbalConnection) {
-                $dbalConnection->platform = new class() extends \Doctrine\DBAL\Platforms\PostgreSQL94Platform { // @phpstan-ignore-line
-                    use Postgresql\PlatformTrait;
-                };
-            }, null, DbalConnection::class)();
-        } elseif ($dbalConnection->getDatabasePlatform() instanceof SQLServerPlatform) {
-            \Closure::bind(function () use ($dbalConnection) {
-                $dbalConnection->platform = new class() extends \Doctrine\DBAL\Platforms\SQLServer2012Platform { // @phpstan-ignore-line
-                    use Mssql\PlatformTrait;
-                };
-            }, null, DbalConnection::class)();
-        } elseif ($dbalConnection->getDatabasePlatform() instanceof OraclePlatform) {
-            \Closure::bind(function () use ($dbalConnection) {
-                $dbalConnection->platform = new class() extends OraclePlatform {
-                    use Oracle\PlatformTrait;
-                };
-            }, null, DbalConnection::class)();
-        }
-
         return $dbalConnection;
     }
 
@@ -383,7 +373,7 @@ public function beginTransaction(): void
     {
         try {
             $this->getConnection()->beginTransaction();
-        } catch (\Doctrine\DBAL\ConnectionException $e) {
+        } catch (DbalConnectionException $e) {
             throw new Exception('Begin transaction failed', 0, $e);
         }
     }
@@ -410,7 +400,7 @@ public function commit(): void
     {
         try {
             $this->getConnection()->commit();
-        } catch (\Doctrine\DBAL\ConnectionException $e) {
+        } catch (DbalConnectionException $e) {
             throw new Exception('Commit failed', 0, $e);
         }
     }
@@ -422,7 +412,7 @@ public function rollBack(): void
     {
         try {
             $this->getConnection()->rollBack();
-        } catch (\Doctrine\DBAL\ConnectionException $e) {
+        } catch (DbalConnectionException $e) {
             throw new Exception('Rollback failed', 0, $e);
         }
     }
@@ -449,15 +439,6 @@ public function getDatabasePlatform(): AbstractPlatform
      */
     public function createSchemaManager(): AbstractSchemaManager
     {
-        $dbalConnection = $this->getConnection();
-        $platform = $this->getDatabasePlatform();
-        if ($platform instanceof SqlitePlatform) {
-            // @phpstan-ignore-next-line
-            return new class($dbalConnection, $platform) extends SqliteSchemaManager {
-                use Sqlite\SchemaManagerTrait;
-            };
-        }
-
-        return $dbalConnection->createSchemaManager();
+        return $this->getConnection()->createSchemaManager();
     }
 }

src/Persistence/Sql/DbalDriverMiddleware.php

@@ -0,0 +1,146 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Atk4\Data\Persistence\Sql;
+
+use Doctrine\DBAL\Connection as DbalConnection;
+use Doctrine\DBAL\Driver\API\ExceptionConverter;
+use Doctrine\DBAL\Driver\API\SQLite\ExceptionConverter as SqliteExceptionConverter;
+use Doctrine\DBAL\Driver\API\SQLSrv\ExceptionConverter as SQLServerExceptionConverter;
+use Doctrine\DBAL\Driver\Exception as DbalDriverException;
+use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
+use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException;
+use Doctrine\DBAL\Exception\DriverException as DbalDriverConvertedException;
+use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
+use Doctrine\DBAL\Exception\TableNotFoundException;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\OraclePlatform;
+use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
+use Doctrine\DBAL\Platforms\SQLServerPlatform;
+use Doctrine\DBAL\Query as DbalQuery;
+use Doctrine\DBAL\Schema\AbstractSchemaManager;
+use Doctrine\DBAL\Schema\SqliteSchemaManager;
+
+class DbalDriverMiddleware extends AbstractDriverMiddleware
+{
+    protected function replaceDatabasePlatform(AbstractPlatform $platform): AbstractPlatform
+    {
+        if ($platform instanceof SqlitePlatform) {
+            $platform = new class() extends SqlitePlatform {
+                use Sqlite\PlatformTrait;
+            };
+        } elseif ($platform instanceof PostgreSQLPlatform) {
+            $platform = new class() extends \Doctrine\DBAL\Platforms\PostgreSQL94Platform { // @phpstan-ignore-line
+                use Postgresql\PlatformTrait;
+            };
+        } elseif ($platform instanceof SQLServerPlatform) {
+            $platform = new class() extends \Doctrine\DBAL\Platforms\SQLServer2012Platform { // @phpstan-ignore-line
+                use Mssql\PlatformTrait;
+            };
+        } elseif ($platform instanceof OraclePlatform) {
+            $platform = new class() extends OraclePlatform {
+                use Oracle\PlatformTrait;
+            };
+        }
+
+        return $platform;
+    }
+
+    public function getDatabasePlatform(): AbstractPlatform
+    {
+        return $this->replaceDatabasePlatform(parent::getDatabasePlatform());
+    }
+
+    public function createDatabasePlatformForVersion($version): AbstractPlatform
+    {
+        return $this->replaceDatabasePlatform(parent::createDatabasePlatformForVersion($version));
+    }
+
+    /**
+     * @phpstan-return AbstractSchemaManager<AbstractPlatform>
+     */
+    public function getSchemaManager(DbalConnection $connection, AbstractPlatform $platform): AbstractSchemaManager
+    {
+        if ($platform instanceof SqlitePlatform) {
+            return new class($connection, $platform) extends SqliteSchemaManager { // @phpstan-ignore-line
+                use Sqlite\SchemaManagerTrait;
+            };
+        }
+
+        return parent::getSchemaManager($connection, $platform);
+    }
+
+    /**
+     * @param \Closure(DbalDriverConvertedException, ?DbalQuery): DbalDriverConvertedException $convertFx
+     */
+    protected function createExceptionConvertorMiddleware(ExceptionConverter $wrappedExceptionConverter, \Closure $convertFx): ExceptionConverter
+    {
+        return new class($wrappedExceptionConverter, $convertFx) implements ExceptionConverter {
+            private ExceptionConverter $wrappedExceptionConverter;
+            private \Closure $convertFx;
+
+            /**
+             * @param \Closure(DbalDriverConvertedException, ?DbalQuery): DbalDriverConvertedException $convertFx
+             */
+            public function __construct(ExceptionConverter $wrappedExceptionConverter, \Closure $convertFx)
+            {
+                $this->wrappedExceptionConverter = $wrappedExceptionConverter;
+                $this->convertFx = $convertFx;
+            }
+
+            public function convert(DbalDriverException $exception, ?DbalQuery $query): DbalDriverConvertedException
+            {
+                $convertedException = $this->wrappedExceptionConverter->convert($exception, $query);
+
+                return ($this->convertFx)($convertedException, $query);
+            }
+        };
+    }
+
+    final protected static function getUnconvertedException(DbalDriverConvertedException $convertedException): DbalDriverException
+    {
+        return $convertedException->getPrevious(); // @phpstan-ignore-line
+    }
+
+    public function getExceptionConverter(): ExceptionConverter
+    {
+        $exceptionConverter = parent::getExceptionConverter();
+        if ($exceptionConverter instanceof SqliteExceptionConverter) {
+            $exceptionConverter = $this->createExceptionConvertorMiddleware(
+                $exceptionConverter,
+                function (DbalDriverConvertedException $convertedException, ?DbalQuery $query): DbalDriverConvertedException {
+                    // fix FK violation exception conversion
+                    // https://github.com/doctrine/dbal/issues/5496
+                    $exception = self::getUnconvertedException($convertedException);
+                    $exceptionMessageLc = strtolower($exception->getMessage());
+                    if (str_contains($exceptionMessageLc, 'integrity constraint violation')) {
+                        return new ForeignKeyConstraintViolationException($exception, $query);
+                    }
+
+                    return $convertedException;
+                }
+            );
+        } elseif ($exceptionConverter instanceof SQLServerExceptionConverter) {
+            $exceptionConverter = $this->createExceptionConvertorMiddleware(
+                $exceptionConverter,
+                function (DbalDriverConvertedException $convertedException, ?DbalQuery $query): DbalDriverConvertedException {
+                    // fix table not found exception conversion
+                    // https://github.com/doctrine/dbal/pull/5492
+                    if ($convertedException instanceof DatabaseObjectNotFoundException) {
+                        $exception = self::getUnconvertedException($convertedException);
+                        $exceptionMessageLc = strtolower($exception->getMessage());
+                        if (str_contains($exceptionMessageLc, 'cannot drop the table')) {
+                            return new TableNotFoundException($exception, $query);
+                        }
+                    }
+
+                    return $convertedException;
+                }
+            );
+        }
+
+        return $exceptionConverter;
+    }
+}

src/Schema/Migrator.php

@@ -11,14 +11,12 @@
 use Atk4\Data\Persistence;
 use Atk4\Data\Persistence\Sql\Connection;
 use Atk4\Data\Reference\HasOne;
-use Doctrine\DBAL\Driver\Exception as DbalDriverException;
 use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException;
 use Doctrine\DBAL\Exception\TableNotFoundException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
-use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\Table;
 
@@ -145,18 +143,7 @@ public function drop(bool $dropForeignKeysFirst = false): self
             }
         }
 
-        try {
-            $schemaManager->dropTable($this->table->getQuotedName($this->getDatabasePlatform()));
-        } catch (DatabaseObjectNotFoundException $e) {
-            // fix exception not converted to TableNotFoundException for MSSQL
-            // https://github.com/doctrine/dbal/pull/5492
-            if ($this->getDatabasePlatform() instanceof SQLServerPlatform && $e->getPrevious() instanceof DbalDriverException
-                && preg_match('~[cC]annot drop the table \'.*\', because it does not exist or you do not have permission\.~', $e->getMessage())) {
-                throw new TableNotFoundException($e->getPrevious(), $e->getQuery());
-            }
-
-            throw $e;
-        }
+        $schemaManager->dropTable($this->table->getQuotedName($this->getDatabasePlatform()));
 
         $this->createdTableNames = array_diff($this->createdTableNames, [$this->table->getName()]);
 

tests/Schema/MigratorFkTest.php

@@ -7,9 +7,7 @@
 use Atk4\Data\Exception;
 use Atk4\Data\Model;
 use Atk4\Data\Schema\TestCase;
-use Doctrine\DBAL\Driver\Exception as DbalDriverException;
 use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
-use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Identifier;
 
@@ -93,15 +91,7 @@ public function testForeignKeyViolation(): void
             $invoice->insert(['client_id' => 50]);
         } catch (Exception $e) {
             $dbalException = $e->getPrevious()->getPrevious();
-            if ($this->getDatabasePlatform() instanceof SqlitePlatform) {
-                // FK violation exception is not properly converted by ExceptionConverter
-                // https://github.com/doctrine/dbal/blob/3.3.7/src/Driver/API/SQLite/ExceptionConverter.php
-                // https://github.com/doctrine/dbal/issues/5496
-                // TODO submit a PR to DBAL
-                $this->assertInstanceOf(DbalDriverException::class, $dbalException);
-            } else {
-                $this->assertInstanceOf(ForeignKeyConstraintViolationException::class, $dbalException);
-            }
+            $this->assertInstanceOf(ForeignKeyConstraintViolationException::class, $dbalException);
 
             throw $e;
         }

tests/Util/DeepCopyTest.php

@@ -192,7 +192,7 @@ public function testBasic(): void
         // now that invoice is mostly paid, due amount will reflect that
         $this->assertEquals(5, $invoice->get('due'));
 
-        // Next we copy invocie into simply a new record. Duplicate. However this time we will also duplicate payments,
+        // Next we copy invoice into simply a new record. Duplicate. However this time we will also duplicate payments,
         // and client. Because Payment references client too, we need to duplicate that one also, this way new record
         // structure will not be related to any existing records.
         $dc = new DeepCopy();