Merge pull request #4157 from morozov/issues/4156

Rework the portability layer to act as a middleware

UPGRADE.md

@@ -158,9 +158,11 @@ The method no longer accepts the `$username`, `$password` and `$driverOptions` a
 
 This class was deprecated in favor of `PrimaryReadReplicaConnection`
 
-## Removed `Portability\Connection::PORTABILITY_{PLATFORM}` constants`
+## BC BREAK: Changes in the portability layer
 
-The platform-specific portability constants were internal implementation details which are longer relevant.
+1. The platform-specific portability constants (`Portability\Connection::PORTABILITY_{PLATFORM}`) were internal implementation details which are no longer relevant.
+2. The `Portability\Connection` class no longer extends the DBAL `Connection`.
+3. The `Portability\Class` class has been made final.
 
 ## BC BREAK changes in fetching statement results
 

phpstan.neon.dist

@@ -98,10 +98,5 @@ parameters:
             paths:
                 - %currentWorkingDirectory%/src/Id/TableGenerator.php
                 - %currentWorkingDirectory%/src/Schema/SqliteSchemaManager.php
-
-        -
-            message: '~Return type \(Doctrine\\DBAL\\Portability\\Statement\) of method Doctrine\\DBAL\\Portability\\Connection::prepare\(\) should be compatible with return type \(Doctrine\\DBAL\\Statement\) of method Doctrine\\DBAL\\Connection::prepare\(\)~'
-            paths:
-                - %currentWorkingDirectory%/src/Portability/Connection.php
 includes:
     - vendor/phpstan/phpstan-strict-rules/rules.neon

src/Configuration.php

@@ -3,16 +3,19 @@
 namespace Doctrine\DBAL;
 
 use Doctrine\Common\Cache\Cache;
+use Doctrine\DBAL\Driver\Middleware;
 use Doctrine\DBAL\Logging\SQLLogger;
 
 /**
  * Configuration container for the Doctrine DBAL.
  *
- * @internal When adding a new configuration option just write a getter/setter
- *           pair and add the option to the _attributes array with a proper default value.
+ * @internal
  */
 class Configuration
 {
+    /** @var Middleware[] */
+    private $middlewares = [];
+
     /**
      * The attributes that are contained in the configuration.
      * Values are default values.
@@ -108,4 +111,24 @@ class Configuration
     {
         return $this->_attributes['autoCommit'] ?? true;
     }
+
+    /**
+     * @param Middleware[] $middlewares
+     *
+     * @return $this
+     */
+    public function setMiddlewares(array $middlewares): self
+    {
+        $this->middlewares = $middlewares;
+
+        return $this;
+    }
+
+    /**
+     * @return Middleware[]
+     */
+    public function getMiddlewares(): array
+    {
+        return $this->middlewares;
+    }
 }

src/Driver/Middleware.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver;
+
+use Doctrine\DBAL\Driver;
+
+interface Middleware
+{
+    public function wrap(Driver $driver): Driver;
+}

src/DriverManager.php

@@ -146,6 +146,10 @@ final class DriverManager
 
         $driver = new $className();
 
+        foreach ($config->getMiddlewares() as $middleware) {
+            $driver = $middleware->wrap($driver);
+        }
+
         $wrapperClass = Connection::class;
         if (isset($params['wrapperClass'])) {
             if (! is_subclass_of($params['wrapperClass'], $wrapperClass)) {

src/Portability/Connection.php

@@ -2,26 +2,15 @@
 
 namespace Doctrine\DBAL\Portability;
 
-use Doctrine\Common\EventManager;
-use Doctrine\DBAL\Abstraction\Result as AbstractionResult;
-use Doctrine\DBAL\Cache\QueryCacheProfile;
-use Doctrine\DBAL\ColumnCase;
-use Doctrine\DBAL\Configuration;
-use Doctrine\DBAL\Connection as BaseConnection;
-use Doctrine\DBAL\Driver;
-use Doctrine\DBAL\Driver\PDO\Connection as PDOConnection;
+use Doctrine\DBAL\Driver\Connection as ConnectionInterface;
 use Doctrine\DBAL\Driver\Result as DriverResult;
 use Doctrine\DBAL\Driver\Statement as DriverStatement;
-use Doctrine\DBAL\Result as DBALResult;
-use PDO;
-
-use const CASE_LOWER;
-use const CASE_UPPER;
+use Doctrine\DBAL\ParameterType;
 
 /**
  * Portability wrapper for a Connection.
  */
-class Connection extends BaseConnection
+final class Connection implements ConnectionInterface
 {
     public const PORTABILITY_ALL           = 255;
     public const PORTABILITY_NONE          = 0;
@@ -29,98 +18,79 @@ class Connection extends BaseConnection
     public const PORTABILITY_EMPTY_TO_NULL = 4;
     public const PORTABILITY_FIX_CASE      = 8;
 
-    /** @var int */
-    private $portability = self::PORTABILITY_NONE;
-
-    /** @var int */
-    private $case = 0;
+    /** @var ConnectionInterface */
+    private $connection;
 
     /** @var Converter */
     private $converter;
 
-    /** {@inheritDoc} */
-    public function __construct(
-        array $params,
-        Driver $driver,
-        ?Configuration $config = null,
-        ?EventManager $eventManager = null
-    ) {
-        if (isset($params['portability'])) {
-            $this->portability = $params['portability'];
-        }
-
-        if (isset($params['fetch_case'])) {
-            $this->case = $params['fetch_case'];
-        }
-
-        unset($params['portability'], $params['fetch_case']);
-
-        parent::__construct($params, $driver, $config, $eventManager);
+    public function __construct(ConnectionInterface $connection, Converter $converter)
+    {
+        $this->connection = $connection;
+        $this->converter  = $converter;
     }
 
     /**
-     * {@inheritdoc}
+     * @return Statement
      */
-    public function connect()
+    public function prepare(string $sql): DriverStatement
     {
-        $ret = parent::connect();
-        if ($ret) {
-            $portability = (new OptimizeFlags())(
-                $this->getDatabasePlatform(),
-                $this->portability
-            );
-
-            $case = 0;
+        return new Statement(
+            $this->connection->prepare($sql),
+            $this->converter
+        );
+    }
 
-            if ($this->case !== 0 && ($portability & self::PORTABILITY_FIX_CASE) !== 0) {
-                if ($this->_conn instanceof PDOConnection) {
-                    // make use of c-level support for case handling
-                    $this->_conn->getWrappedConnection()->setAttribute(PDO::ATTR_CASE, $this->case);
-                } else {
-                    $case = $this->case === ColumnCase::LOWER ? CASE_LOWER : CASE_UPPER;
-                }
-            }
+    public function query(string $sql): DriverResult
+    {
+        return new Result(
+            $this->connection->query($sql),
+            $this->converter
+        );
+    }
 
-            $this->converter = new Converter(
-                ($portability & self::PORTABILITY_EMPTY_TO_NULL) !== 0,
-                ($portability & self::PORTABILITY_RTRIM) !== 0,
-                $case
-            );
-        }
+    /**
+     * {@inheritDoc}
+     */
+    public function quote($input, $type = ParameterType::STRING)
+    {
+        return $this->connection->quote($input, $type);
+    }
 
-        return $ret;
+    public function exec(string $statement): int
+    {
+        return $this->connection->exec($statement);
     }
 
     /**
-     * {@inheritdoc}
+     * {@inheritDoc}
      */
-    public function executeQuery(string $query, array $params = [], $types = [], ?QueryCacheProfile $qcp = null): AbstractionResult
+    public function lastInsertId($name = null)
     {
-        return $this->wrapResult(
-            parent::executeQuery($query, $params, $types, $qcp)
-        );
+        return $this->connection->lastInsertId($name);
     }
 
     /**
-     * @return Statement
+     * {@inheritDoc}
      */
-    public function prepare(string $sql): DriverStatement
+    public function beginTransaction()
     {
-        return new Statement(parent::prepare($sql), $this->converter);
+        return $this->connection->beginTransaction();
     }
 
-    public function query(string $sql): DriverResult
+    /**
+     * {@inheritDoc}
+     */
+    public function commit()
     {
-        return $this->wrapResult(
-            parent::query($sql)
-        );
+        return $this->connection->commit();
     }
 
-    private function wrapResult(DriverResult $result): AbstractionResult
+    /**
+     * {@inheritDoc}
+     */
+    public function rollBack()
     {
-        return new DBALResult(
-            new Result($result, $this->converter),
-            $this
-        );
+        return $this->connection->rollBack();
     }
 }

src/Portability/Driver.php

+<?php
+
+namespace Doctrine\DBAL\Portability;
+
+use Doctrine\DBAL\ColumnCase;
+use Doctrine\DBAL\Connection as DBALConnection;
+use Doctrine\DBAL\Driver as DriverInterface;
+use Doctrine\DBAL\Driver\API\ExceptionConverter;
+use Doctrine\DBAL\Driver\PDO;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+
+use const CASE_LOWER;
+use const CASE_UPPER;
+
+final class Driver implements DriverInterface
+{
+    /** @var DriverInterface */
+    private $driver;
+
+    /** @var int */
+    private $mode;
+
+    /** @var int */
+    private $case;
+
+    public function __construct(DriverInterface $driver, int $mode, int $case)
+    {
+        $this->driver = $driver;
+        $this->mode   = $mode;
+        $this->case   = $case;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function connect(array $params)
+    {
+        $connection = $this->driver->connect($params);
+
+        $portability = (new OptimizeFlags())(
+            $this->getDatabasePlatform(),
+            $this->mode
+        );
+
+        $case = 0;
+
+        if ($this->case !== 0 && ($portability & Connection::PORTABILITY_FIX_CASE) !== 0) {
+            if ($connection instanceof PDO\Connection) {
+                // make use of c-level support for case handling
+                $portability &= ~Connection::PORTABILITY_FIX_CASE;
+                $connection->getWrappedConnection()->setAttribute(\PDO::ATTR_CASE, $this->case);
+            } else {
+                $case = $this->case === ColumnCase::LOWER ? CASE_LOWER : CASE_UPPER;
+            }
+        }
+
+        $convertEmptyStringToNull = ($portability & Connection::PORTABILITY_EMPTY_TO_NULL) !== 0;
+        $rightTrimString          = ($portability & Connection::PORTABILITY_RTRIM) !== 0;
+
+        if (! $convertEmptyStringToNull && ! $rightTrimString && $case === 0) {
+            return $connection;
+        }
+
+        return new Connection(
+            $connection,
+            new Converter($convertEmptyStringToNull, $rightTrimString, $case)
+        );
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getDatabasePlatform()
+    {
+        return $this->driver->getDatabasePlatform();
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getSchemaManager(DBALConnection $conn, AbstractPlatform $platform)
+    {
+        return $this->driver->getSchemaManager($conn, $platform);
+    }
+
+    public function getExceptionConverter(): ExceptionConverter
+    {
+        return $this->driver->getExceptionConverter();
+    }
+}

src/Portability/Middleware.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Portability;
+
+use Doctrine\DBAL\Driver as DriverInterface;
+use Doctrine\DBAL\Driver\Middleware as MiddlewareInterface;
+
+final class Middleware implements MiddlewareInterface
+{
+    /** @var int */
+    private $mode;
+
+    /** @var int */
+    private $case;
+
+    public function __construct(int $mode, int $case)
+    {
+        $this->mode = $mode;
+        $this->case = $case;
+    }
+
+    public function wrap(DriverInterface $driver): DriverInterface
+    {
+        if ($this->mode !== 0) {
+            return new Driver($driver, $this->mode, $this->case);
+        }
+
+        return $driver;
+    }
+}

src/Portability/Statement.php

@@ -9,7 +9,7 @@ use Doctrine\DBAL\ParameterType;
 /**
  * Portability wrapper for a Statement.
  */
-class Statement implements DriverStatement
+final class Statement implements DriverStatement
 {
     /** @var DriverStatement */
     private $stmt;

tests/Functional/PortabilityTest.php

@@ -3,9 +3,9 @@
 namespace Doctrine\DBAL\Tests\Functional;
 
 use Doctrine\DBAL\ColumnCase;
-use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\DriverManager;
-use Doctrine\DBAL\Portability\Connection as ConnectionPortability;
+use Doctrine\DBAL\Portability\Connection;
+use Doctrine\DBAL\Portability\Middleware;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Tests\FunctionalTestCase;
 use Throwable;
@@ -17,62 +17,49 @@ use function strlen;
  */
 class PortabilityTest extends FunctionalTestCase
 {
-    /** @var Connection */
-    private $portableConnection;
-
-    protected function tearDown(): void
+    protected function setUp(): void
     {
-        if ($this->portableConnection) {
-            $this->portableConnection->close();
+        parent::setUp();
+
+        $this->connection = DriverManager::getConnection(
+            $this->connection->getParams(),
+            $this->connection->getConfiguration()
+                ->setMiddlewares([new Middleware(Connection::PORTABILITY_ALL, ColumnCase::LOWER)])
+        );
+
+        try {
+            $table = new Table('portability_table');
+            $table->addColumn('Test_Int', 'integer');
+            $table->addColumn('Test_String', 'string', ['fixed' => true, 'length' => 32]);
+            $table->addColumn('Test_Null', 'string', ['notnull' => false]);
+            $table->setPrimaryKey(['Test_Int']);
+
+            $sm = $this->connection->getSchemaManager();
+            $sm->createTable($table);
+
+            $this->connection->insert('portability_table', ['Test_Int' => 1, 'Test_String' => 'foo', 'Test_Null' => '']);
+            $this->connection->insert('portability_table', ['Test_Int' => 2, 'Test_String' => 'foo  ', 'Test_Null' => null]);
+        } catch (Throwable $e) {
         }
-
-        parent::tearDown();
     }
 
-    private function getPortableConnection(
-        int $portabilityMode = ConnectionPortability::PORTABILITY_ALL,
-        int $case = ColumnCase::LOWER
-    ): Connection {
-        if (! $this->portableConnection) {
-            $params = $this->connection->getParams();
-
-            $params['wrapperClass'] = ConnectionPortability::class;
-            $params['portability']  = $portabilityMode;
-            $params['fetch_case']   = $case;
-
-            $this->portableConnection = DriverManager::getConnection($params, $this->connection->getConfiguration(), $this->connection->getEventManager());
-
-            try {
-                $table = new Table('portability_table');
-                $table->addColumn('Test_Int', 'integer');
-                $table->addColumn('Test_String', 'string', ['fixed' => true, 'length' => 32]);
-                $table->addColumn('Test_Null', 'string', ['notnull' => false]);
-                $table->setPrimaryKey(['Test_Int']);
-
-                $sm = $this->portableConnection->getSchemaManager();
-                $sm->createTable($table);
-
-                $this->portableConnection->insert('portability_table', ['Test_Int' => 1, 'Test_String' => 'foo', 'Test_Null' => '']);
-                $this->portableConnection->insert('portability_table', ['Test_Int' => 2, 'Test_String' => 'foo  ', 'Test_Null' => null]);
-            } catch (Throwable $e) {
-            }
-        }
-
-        return $this->portableConnection;
+    public function tearDown(): void
+    {
+        self::resetSharedConn();
     }
 
     public function testFullFetchMode(): void
     {
-        $rows = $this->getPortableConnection()->fetchAllAssociative('SELECT * FROM portability_table');
+        $rows = $this->connection->fetchAllAssociative('SELECT * FROM portability_table');
         $this->assertFetchResultRows($rows);
 
-        $result = $this->getPortableConnection()->query('SELECT * FROM portability_table');
+        $result = $this->connection->query('SELECT * FROM portability_table');
 
         while (($row = $result->fetchAssociative())) {
             $this->assertFetchResultRow($row);
         }
 
-        $result = $this->getPortableConnection()
+        $result = $this->connection
             ->prepare('SELECT * FROM portability_table')
             ->execute();
 
@@ -83,17 +70,15 @@ class PortabilityTest extends FunctionalTestCase
 
     public function testConnFetchMode(): void
     {
-        $conn = $this->getPortableConnection();
-
-        $rows = $conn->fetchAllAssociative('SELECT * FROM portability_table');
+        $rows = $this->connection->fetchAllAssociative('SELECT * FROM portability_table');
         $this->assertFetchResultRows($rows);
 
-        $result = $conn->query('SELECT * FROM portability_table');
+        $result = $this->connection->query('SELECT * FROM portability_table');
         while (($row = $result->fetchAssociative())) {
             $this->assertFetchResultRow($row);
         }
 
-        $result = $conn->prepare('SELECT * FROM portability_table')
+        $result = $this->connection->prepare('SELECT * FROM portability_table')
             ->execute();
 
         while (($row = $result->fetchAssociative())) {
@@ -133,10 +118,9 @@ class PortabilityTest extends FunctionalTestCase
      *
      * @dataProvider fetchColumnProvider
      */
-    public function testfetchColumn(string $field, array $expected): void
+    public function testFetchColumn(string $field, array $expected): void
     {
-        $conn   = $this->getPortableConnection();
-        $result = $conn->query('SELECT ' . $field . ' FROM portability_table');
+        $result = $this->connection->query('SELECT ' . $field . ' FROM portability_table');
 
         $column = $result->fetchFirstColumn();
         self::assertEquals($expected, $column);
@@ -161,8 +145,7 @@ class PortabilityTest extends FunctionalTestCase
 
     public function testFetchAllNullColumn(): void
     {
-        $conn   = $this->getPortableConnection();
-        $result = $conn->query('SELECT Test_Null FROM portability_table');
+        $result = $this->connection->query('SELECT Test_Null FROM portability_table');
 
         $column = $result->fetchFirstColumn();
         self::assertSame([null, null], $column);