Merge pull request #4039 from morozov/refactor-portability-statement

Refactor portability statement into a functional composition

src/Portability/Connection.php

@@ -30,11 +30,8 @@ class Connection extends \Doctrine\DBAL\Connection
     public const PORTABILITY_SQLANYWHERE  = 13;
     public const PORTABILITY_SQLSRV       = 13;
 
-    /** @var int */
-    private $portability = self::PORTABILITY_NONE;
-
-    /** @var int */
-    private $case;
+    /** @var Converter */
+    private $converter;
 
     /**
      * {@inheritdoc}
@@ -43,75 +40,63 @@ class Connection extends \Doctrine\DBAL\Connection
     {
         $ret = parent::connect();
         if ($ret) {
-            $params = $this->getParams();
+            $params      = $this->getParams();
+            $portability = self::PORTABILITY_NONE;
+
             if (isset($params['portability'])) {
                 if ($this->getDatabasePlatform()->getName() === 'oracle') {
-                    $params['portability'] &= self::PORTABILITY_ORACLE;
+                    $portability = $params['portability'] & self::PORTABILITY_ORACLE;
                 } elseif ($this->getDatabasePlatform()->getName() === 'postgresql') {
-                    $params['portability'] &= self::PORTABILITY_POSTGRESQL;
+                    $portability = $params['portability'] & self::PORTABILITY_POSTGRESQL;
                 } elseif ($this->getDatabasePlatform()->getName() === 'sqlite') {
-                    $params['portability'] &= self::PORTABILITY_SQLITE;
+                    $portability = $params['portability'] & self::PORTABILITY_SQLITE;
                 } elseif ($this->getDatabasePlatform()->getName() === 'sqlanywhere') {
-                    $params['portability'] &= self::PORTABILITY_SQLANYWHERE;
+                    $portability = $params['portability'] & self::PORTABILITY_SQLANYWHERE;
                 } elseif ($this->getDatabasePlatform()->getName() === 'db2') {
-                    $params['portability'] &= self::PORTABILITY_DB2;
+                    $portability = $params['portability'] & self::PORTABILITY_DB2;
                 } elseif ($this->getDatabasePlatform()->getName() === 'mssql') {
-                    $params['portability'] &= self::PORTABILITY_SQLSRV;
+                    $portability = $params['portability'] & self::PORTABILITY_SQLSRV;
                 } else {
-                    $params['portability'] &= self::PORTABILITY_OTHERVENDORS;
+                    $portability = $params['portability'] & self::PORTABILITY_OTHERVENDORS;
                 }
-
-                $this->portability = $params['portability'];
             }
 
-            if (isset($params['fetch_case']) && ($this->portability & self::PORTABILITY_FIX_CASE) !== 0) {
+            $case = null;
+
+            if (isset($params['fetch_case']) && ($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, $params['fetch_case']);
                 } else {
-                    $this->case = $params['fetch_case'] === ColumnCase::LOWER ? CASE_LOWER : CASE_UPPER;
+                    $case = $params['fetch_case'] === ColumnCase::LOWER ? CASE_LOWER : CASE_UPPER;
                 }
             }
+
+            $this->converter = new Converter(
+                ($portability & self::PORTABILITY_EMPTY_TO_NULL) !== 0,
+                ($portability & self::PORTABILITY_RTRIM) !== 0,
+                $case
+            );
         }
 
         return $ret;
     }
 
-    /**
-     * @return int
-     */
-    public function getPortability()
-    {
-        return $this->portability;
-    }
-
-    /**
-     * @return int
-     */
-    public function getFetchCase()
-    {
-        return $this->case;
-    }
-
     /**
      * {@inheritdoc}
      */
     public function executeQuery(string $query, array $params = [], $types = [], ?QueryCacheProfile $qcp = null) : ResultStatement
     {
-        return new Statement(parent::executeQuery($query, $params, $types, $qcp), $this);
+        return new Statement(parent::executeQuery($query, $params, $types, $qcp), $this->converter);
     }
 
     public function prepare(string $sql) : DriverStatement
     {
-        return new Statement(parent::prepare($sql), $this);
+        return new Statement(parent::prepare($sql), $this->converter);
     }
 
     public function query(string $sql) : ResultStatement
     {
-        return new Statement(
-            $this->getWrappedConnection()
-                ->query($sql),
-            $this
-        );
+        return new Statement(parent::query($sql), $this->converter);
     }
 }

src/Portability/Converter.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Portability;
+
+use function array_change_key_case;
+use function array_map;
+use function array_reduce;
+use function is_string;
+use function rtrim;
+
+final class Converter
+{
+    /** @var callable */
+    private $convertNumeric;
+
+    /** @var callable */
+    private $convertAssociative;
+
+    /** @var callable */
+    private $convertOne;
+
+    /** @var callable */
+    private $convertAllNumeric;
+
+    /** @var callable */
+    private $convertAllAssociative;
+
+    /** @var callable */
+    private $convertFirstColumn;
+
+    /**
+     * @param bool     $convertEmptyStringToNull Whether each empty string should be converted to NULL
+     * @param bool     $rightTrimString          Whether each string should right-trimmed
+     * @param int|null $case                     Convert the case of the column names
+     *                                           (one of {@link CASE_LOWER} and {@link CASE_UPPER})
+     */
+    public function __construct(bool $convertEmptyStringToNull, bool $rightTrimString, ?int $case)
+    {
+        $id = static function ($value) {
+            return $value;
+        };
+
+        $convertValue       = $this->createConvertValue($convertEmptyStringToNull, $rightTrimString);
+        $convertNumeric     = $this->createConvertRow($convertValue, null);
+        $convertAssociative = $this->createConvertRow($convertValue, $case);
+
+        $this->convertNumeric     = $this->createConvert($convertNumeric, $id);
+        $this->convertAssociative = $this->createConvert($convertAssociative, $id);
+        $this->convertOne         = $this->createConvert($convertValue, $id);
+
+        $this->convertAllNumeric     = $this->createConvertAll($convertNumeric, $id);
+        $this->convertAllAssociative = $this->createConvertAll($convertAssociative, $id);
+        $this->convertFirstColumn    = $this->createConvertAll($convertValue, $id);
+    }
+
+    /**
+     * @param array<int,mixed>|false $row
+     *
+     * @return array<int,mixed>|false
+     */
+    public function convertNumeric($row)
+    {
+        return ($this->convertNumeric)($row);
+    }
+
+    /**
+     * @param array<string,mixed>|false $row
+     *
+     * @return array<string,mixed>|false
+     */
+    public function convertAssociative($row)
+    {
+        return ($this->convertAssociative)($row);
+    }
+
+    /**
+     * @param mixed|false $value
+     *
+     * @return mixed|false
+     */
+    public function convertOne($value)
+    {
+        return ($this->convertOne)($value);
+    }
+
+    /**
+     * @param array<int,array<int,mixed>> $data
+     *
+     * @return array<int,array<int,mixed>>
+     */
+    public function convertAllNumeric(array $data) : array
+    {
+        return ($this->convertAllNumeric)($data);
+    }
+
+    /**
+     * @param array<int,array<string,mixed>> $data
+     *
+     * @return array<int,array<string,mixed>>
+     */
+    public function convertAllAssociative(array $data) : array
+    {
+        return ($this->convertAllAssociative)($data);
+    }
+
+    /**
+     * @param array<int,mixed> $data
+     *
+     * @return array<int,mixed>
+     */
+    public function convertFirstColumn(array $data) : array
+    {
+        return ($this->convertFirstColumn)($data);
+    }
+
+    /**
+     * Creates a function that will convert each individual value retrieved from the database
+     *
+     * @param bool $convertEmptyStringToNull Whether each empty string should be converted to NULL
+     * @param bool $rightTrimString          Whether each string should right-trimmed
+     *
+     * @return callable|null The resulting function or NULL if no conversion is needed
+     */
+    private function createConvertValue(bool $convertEmptyStringToNull, bool $rightTrimString) : ?callable
+    {
+        $functions = [];
+
+        if ($convertEmptyStringToNull) {
+            $functions[] = static function ($value) {
+                if ($value === '') {
+                    return null;
+                }
+
+                return $value;
+            };
+        }
+
+        if ($rightTrimString) {
+            $functions[] = static function ($value) {
+                if (! is_string($value)) {
+                    return $value;
+                }
+
+                return rtrim($value);
+            };
+        }
+
+        return $this->compose(...$functions);
+    }
+
+    /**
+     * Creates a function that will convert each array-row retrieved from the database
+     *
+     * @param callable|null $function The function that will convert each value
+     * @param int|null      $case     Column name case
+     *
+     * @return callable|null The resulting function or NULL if no conversion is needed
+     */
+    private function createConvertRow(?callable $function, ?int $case) : ?callable
+    {
+        $functions = [];
+
+        if ($function !== null) {
+            $functions[] = $this->createMapper($function);
+        }
+
+        if ($case !== null) {
+            $functions[] = static function (array $row) use ($case) : array {
+                return array_change_key_case($row, $case);
+            };
+        }
+
+        return $this->compose(...$functions);
+    }
+
+    /**
+     * Creates a function that will be applied to the return value of Statement::fetch*()
+     * or an identity function if no conversion is needed
+     *
+     * @param callable|null $function The function that will convert each tow
+     * @param callable      $id       Identity function
+     */
+    private function createConvert(?callable $function, callable $id) : callable
+    {
+        if ($function === null) {
+            return $id;
+        }
+
+        return static function ($value) use ($function) {
+            if ($value === false) {
+                return false;
+            }
+
+            return $function($value);
+        };
+    }
+
+    /**
+     * Creates a function that will be applied to the return value of Statement::fetchAll*()
+     * or an identity function if no transformation is required
+     *
+     * @param callable|null $function The function that will transform each value
+     * @param callable      $id       Identity function
+     */
+    private function createConvertAll(?callable $function, callable $id) : callable
+    {
+        if ($function === null) {
+            return $id;
+        }
+
+        return $this->createMapper($function);
+    }
+
+    /**
+     * Creates a function that maps each value of the array using the given function
+     *
+     * @param callable $function The function that maps each value of the array
+     */
+    private function createMapper(callable $function) : callable
+    {
+        return static function (array $array) use ($function) : array {
+            return array_map($function, $array);
+        };
+    }
+
+    /**
+     * Creates a composition of the given set of functions
+     *
+     * @param callable ...$functions The functions to compose
+     *
+     * @return callable|null The composition or NULL if an empty set is provided
+     */
+    private function compose(callable ...$functions) : ?callable
+    {
+        return array_reduce($functions, static function (?callable $carry, callable $item) : callable {
+            if ($carry === null) {
+                return $item;
+            }
+
+            return static function ($value) use ($carry, $item) {
+                return $item($carry($value));
+            };
+        });
+    }
+}

src/Portability/Statement.php

@@ -5,35 +5,28 @@ namespace Doctrine\DBAL\Portability;
 use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\Driver\Statement as DriverStatement;
 use Doctrine\DBAL\ParameterType;
-use function array_change_key_case;
 use function assert;
-use function is_string;
-use function rtrim;
 
 /**
  * Portability wrapper for a Statement.
  */
 class Statement implements DriverStatement
 {
-    /** @var int */
-    private $portability;
-
     /** @var DriverStatement|ResultStatement */
     private $stmt;
 
-    /** @var int */
-    private $case;
+    /** @var Converter */
+    private $converter;
 
     /**
      * Wraps <tt>Statement</tt> and applies portability measures.
      *
      * @param DriverStatement|ResultStatement $stmt
      */
-    public function __construct($stmt, Connection $conn)
+    public function __construct($stmt, Converter $converter)
     {
-        $this->stmt        = $stmt;
-        $this->portability = $conn->getPortability();
-        $this->case        = $conn->getFetchCase();
+        $this->stmt      = $stmt;
+        $this->converter = $converter;
     }
 
     /**
@@ -82,14 +75,20 @@ class Statement implements DriverStatement
         return $this->stmt->execute($params);
     }
 
+    public function rowCount() : int
+    {
+        assert($this->stmt instanceof DriverStatement);
+
+        return $this->stmt->rowCount();
+    }
+
     /**
      * {@inheritdoc}
      */
     public function fetchNumeric()
     {
-        return $this->fixResult(
-            $this->stmt->fetchAssociative(),
-            false
+        return $this->converter->convertNumeric(
+            $this->stmt->fetchNumeric()
         );
     }
 
@@ -98,9 +97,8 @@ class Statement implements DriverStatement
      */
     public function fetchAssociative()
     {
-        return $this->fixResult(
-            $this->stmt->fetchAssociative(),
-            true
+        return $this->converter->convertAssociative(
+            $this->stmt->fetchAssociative()
         );
     }
 
@@ -109,15 +107,9 @@ class Statement implements DriverStatement
      */
     public function fetchOne()
     {
-        $value = $this->stmt->fetchOne();
-
-        if (($this->portability & Connection::PORTABILITY_EMPTY_TO_NULL) !== 0 && $value === '') {
-            $value = null;
-        } elseif (($this->portability & Connection::PORTABILITY_RTRIM) !== 0 && is_string($value)) {
-            $value = rtrim($value);
-        }
-
-        return $value;
+        return $this->converter->convertOne(
+            $this->stmt->fetchOne()
+        );
     }
 
     /**
@@ -125,10 +117,8 @@ class Statement implements DriverStatement
      */
     public function fetchAllNumeric() : array
     {
-        return $this->fixResultSet(
-            $this->stmt->fetchAllNumeric(),
-            false,
-            true
+        return $this->converter->convertAllNumeric(
+            $this->stmt->fetchAllNumeric()
         );
     }
 
@@ -137,10 +127,8 @@ class Statement implements DriverStatement
      */
     public function fetchAllAssociative() : array
     {
-        return $this->fixResultSet(
-            $this->stmt->fetchAllAssociative(),
-            true,
-            true
+        return $this->converter->convertAllAssociative(
+            $this->stmt->fetchAllAssociative()
         );
     }
 
@@ -149,93 +137,8 @@ class Statement implements DriverStatement
      */
     public function fetchColumn() : array
     {
-        return $this->fixResultSet(
-            $this->stmt->fetchColumn(),
-            true,
-            false
+        return $this->converter->convertFirstColumn(
+            $this->stmt->fetchColumn()
         );
     }
-
-    /**
-     * @param mixed $result
-     *
-     * @return mixed
-     */
-    private function fixResult($result, bool $fixCase)
-    {
-        $iterateRow = ($this->portability & (Connection::PORTABILITY_EMPTY_TO_NULL|Connection::PORTABILITY_RTRIM)) !== 0;
-        $fixCase    = $fixCase && $this->case !== null && ($this->portability & Connection::PORTABILITY_FIX_CASE) !== 0;
-
-        return $this->fixRow($result, $iterateRow, $fixCase);
-    }
-
-    /**
-     * @param array<int,mixed> $resultSet
-     *
-     * @return array<int,mixed>
-     */
-    private function fixResultSet(array $resultSet, bool $fixCase, bool $isArray) : array
-    {
-        $iterateRow = ($this->portability & (Connection::PORTABILITY_EMPTY_TO_NULL|Connection::PORTABILITY_RTRIM)) !== 0;
-        $fixCase    = $fixCase && $this->case !== null && ($this->portability & Connection::PORTABILITY_FIX_CASE) !== 0;
-
-        if (! $iterateRow && ! $fixCase) {
-            return $resultSet;
-        }
-
-        if (! $isArray) {
-            foreach ($resultSet as $num => $value) {
-                $resultSet[$num] = [$value];
-            }
-        }
-
-        foreach ($resultSet as $num => $row) {
-            $resultSet[$num] = $this->fixRow($row, $iterateRow, $fixCase);
-        }
-
-        if (! $isArray) {
-            foreach ($resultSet as $num => $row) {
-                $resultSet[$num] = $row[0];
-            }
-        }
-
-        return $resultSet;
-    }
-
-    /**
-     * @param mixed $row
-     * @param bool  $iterateRow
-     * @param bool  $fixCase
-     *
-     * @return mixed
-     */
-    protected function fixRow($row, $iterateRow, $fixCase)
-    {
-        if ($row === false) {
-            return $row;
-        }
-
-        if ($fixCase) {
-            $row = array_change_key_case($row, $this->case);
-        }
-
-        if ($iterateRow) {
-            foreach ($row as $k => $v) {
-                if (($this->portability & Connection::PORTABILITY_EMPTY_TO_NULL) !== 0 && $v === '') {
-                    $row[$k] = null;
-                } elseif (($this->portability & Connection::PORTABILITY_RTRIM) !== 0 && is_string($v)) {
-                    $row[$k] = rtrim($v);
-                }
-            }
-        }
-
-        return $row;
-    }
-
-    public function rowCount() : int
-    {
-        assert($this->stmt instanceof DriverStatement);
-
-        return $this->stmt->rowCount();
-    }
 }

tests/Portability/StatementTest.php

@@ -5,15 +5,13 @@ namespace Doctrine\DBAL\Tests\Portability;
 use Doctrine\DBAL\Driver\Statement as DriverStatement;
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Portability\Connection;
+use Doctrine\DBAL\Portability\Converter;
 use Doctrine\DBAL\Portability\Statement;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 class StatementTest extends TestCase
 {
-    /** @var Connection|MockObject */
-    protected $conn;
-
     /** @var Statement */
     protected $stmt;
 
@@ -23,8 +21,8 @@ class StatementTest extends TestCase
     protected function setUp() : void
     {
         $this->wrappedStmt = $this->createMock(DriverStatement::class);
-        $this->conn        = $this->createConnection();
-        $this->stmt        = $this->createStatement($this->wrappedStmt, $this->conn);
+        $converter         = new Converter(false, false, null);
+        $this->stmt        = new Statement($this->wrappedStmt, $converter);
     }
 
     /**
@@ -114,9 +112,4 @@ class StatementTest extends TestCase
             ->disableOriginalConstructor()
             ->getMock();
     }
-
-    protected function createStatement(DriverStatement $wrappedStatement, Connection $connection) : Statement
-    {
-        return new Statement($wrappedStatement, $connection);
-    }
 }