Merge pull request #4109 from morozov/oci8-finalization-backports

Preparation to marking OCI8 driver classes final

psalm.xml

@@ -29,7 +29,7 @@
                     This one is just too convoluted for Psalm to figure out, by
                     its author's own admission
                 -->
-                <file name="src/Driver/OCI8/OCI8Statement.php"/>
+                <file name="src/Driver/OCI8/ConvertPositionalToNamedPlaceholders.php"/>
             </errorLevel>
         </ConflictingReferenceConstraint>
         <FalsableReturnStatement>

src/Driver/OCI8/ConvertPositionalToNamedPlaceholders.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\OCI8;
+
+use Doctrine\DBAL\Driver\OCI8\Exception\NonTerminatedStringLiteral;
+
+use function count;
+use function implode;
+use function preg_match;
+use function preg_quote;
+use function substr;
+
+use const PREG_OFFSET_CAPTURE;
+
+/**
+ * Converts positional (?) into named placeholders (:param<num>).
+ *
+ * Oracle does not support positional parameters, hence this method converts all
+ * positional parameters into artificially named parameters. Note that this conversion
+ * is not perfect. All question marks (?) in the original statement are treated as
+ * placeholders and converted to a named parameter.
+ *
+ * @internal This class is not covered by the backward compatibility promise
+ */
+final class ConvertPositionalToNamedPlaceholders
+{
+    /**
+     * @param string $statement The SQL statement to convert.
+     *
+     * @return mixed[] [0] => the statement value (string), [1] => the paramMap value (array).
+     *
+     * @throws OCI8Exception
+     */
+    public function __invoke(string $statement): array
+    {
+        $fragmentOffset          = $tokenOffset = 0;
+        $fragments               = $paramMap = [];
+        $currentLiteralDelimiter = null;
+
+        do {
+            if ($currentLiteralDelimiter === null) {
+                $result = $this->findPlaceholderOrOpeningQuote(
+                    $statement,
+                    $tokenOffset,
+                    $fragmentOffset,
+                    $fragments,
+                    $currentLiteralDelimiter,
+                    $paramMap
+                );
+            } else {
+                $result = $this->findClosingQuote($statement, $tokenOffset, $currentLiteralDelimiter);
+            }
+        } while ($result);
+
+        if ($currentLiteralDelimiter !== null) {
+            throw NonTerminatedStringLiteral::new($tokenOffset - 1);
+        }
+
+        $fragments[] = substr($statement, $fragmentOffset);
+        $statement   = implode('', $fragments);
+
+        return [$statement, $paramMap];
+    }
+
+    /**
+     * Finds next placeholder or opening quote.
+     *
+     * @param string      $statement               The SQL statement to parse
+     * @param int         $tokenOffset             The offset to start searching from
+     * @param int         $fragmentOffset          The offset to build the next fragment from
+     * @param string[]    $fragments               Fragments of the original statement not containing placeholders
+     * @param string|null $currentLiteralDelimiter The delimiter of the current string literal
+     *                                             or NULL if not currently in a literal
+     * @param string[]    $paramMap                Mapping of the original parameter positions to their named replacements
+     *
+     * @return bool Whether the token was found
+     */
+    private function findPlaceholderOrOpeningQuote(
+        string $statement,
+        int &$tokenOffset,
+        int &$fragmentOffset,
+        array &$fragments,
+        ?string &$currentLiteralDelimiter,
+        array &$paramMap
+    ): bool {
+        $token = $this->findToken($statement, $tokenOffset, '/[?\'"]/');
+
+        if ($token === null) {
+            return false;
+        }
+
+        if ($token === '?') {
+            $position            = count($paramMap) + 1;
+            $param               = ':param' . $position;
+            $fragments[]         = substr($statement, $fragmentOffset, $tokenOffset - $fragmentOffset);
+            $fragments[]         = $param;
+            $paramMap[$position] = $param;
+            $tokenOffset        += 1;
+            $fragmentOffset      = $tokenOffset;
+
+            return true;
+        }
+
+        $currentLiteralDelimiter = $token;
+        ++$tokenOffset;
+
+        return true;
+    }
+
+    /**
+     * Finds closing quote
+     *
+     * @param string $statement               The SQL statement to parse
+     * @param int    $tokenOffset             The offset to start searching from
+     * @param string $currentLiteralDelimiter The delimiter of the current string literal
+     *
+     * @return bool Whether the token was found
+     */
+    private function findClosingQuote(
+        string $statement,
+        int &$tokenOffset,
+        string &$currentLiteralDelimiter
+    ): bool {
+        $token = $this->findToken(
+            $statement,
+            $tokenOffset,
+            '/' . preg_quote($currentLiteralDelimiter, '/') . '/'
+        );
+
+        if ($token === null) {
+            return false;
+        }
+
+        $currentLiteralDelimiter = null;
+        ++$tokenOffset;
+
+        return true;
+    }
+
+    /**
+     * Finds the token described by regex starting from the given offset. Updates the offset with the position
+     * where the token was found.
+     *
+     * @param string $statement The SQL statement to parse
+     * @param int    $offset    The offset to start searching from
+     * @param string $regex     The regex containing token pattern
+     *
+     * @return string|null Token or NULL if not found
+     */
+    private function findToken(string $statement, int &$offset, string $regex): ?string
+    {
+        if (preg_match($regex, $statement, $matches, PREG_OFFSET_CAPTURE, $offset) === 1) {
+            $offset = $matches[0][1];
+
+            return $matches[0][0];
+        }
+
+        return null;
+    }
+}

src/Driver/OCI8/ExecutionMode.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\OCI8;
+
+/**
+ * Encapsulates the execution mode that is shared between the connection and its statements.
+ *
+ * @internal This class is not covered by the backward compatibility promise
+ */
+final class ExecutionMode
+{
+    /** @var bool */
+    private $isAutoCommitEnabled = true;
+
+    public function enableAutoCommit(): void
+    {
+        $this->isAutoCommitEnabled = true;
+    }
+
+    public function disableAutoCommit(): void
+    {
+        $this->isAutoCommitEnabled = false;
+    }
+
+    public function isAutoCommitEnabled(): bool
+    {
+        return $this->isAutoCommitEnabled;
+    }
+}

src/Driver/OCI8/OCI8Connection.php

@@ -23,7 +23,6 @@ use function preg_match;
 use function sprintf;
 use function str_replace;
 
-use const OCI_COMMIT_ON_SUCCESS;
 use const OCI_NO_AUTO_COMMIT;
 
 /**
@@ -36,8 +35,8 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
     /** @var resource */
     protected $dbh;
 
-    /** @var int */
-    protected $executeMode = OCI_COMMIT_ON_SUCCESS;
+    /** @var ExecutionMode */
+    private $executionMode;
 
     /**
      * Creates a Connection to an Oracle Database using oci8 extension.
@@ -67,7 +66,8 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
             throw OCI8Exception::fromErrorInfo(oci_error());
         }
 
-        $this->dbh = $dbh;
+        $this->dbh           = $dbh;
+        $this->executionMode = new ExecutionMode();
     }
 
     /**
@@ -107,7 +107,7 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
 
     public function prepare(string $sql): DriverStatement
     {
-        return new Statement($this->dbh, $sql, $this);
+        return new Statement($this->dbh, $sql, $this->executionMode);
     }
 
     public function query(string $sql): ResultInterface
@@ -154,22 +154,12 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
         return (int) $result;
     }
 
-    /**
-     * Returns the current execution mode.
-     *
-     * @return int
-     */
-    public function getExecuteMode()
-    {
-        return $this->executeMode;
-    }
-
     /**
      * {@inheritdoc}
      */
     public function beginTransaction()
     {
-        $this->executeMode = OCI_NO_AUTO_COMMIT;
+        $this->executionMode->disableAutoCommit();
 
         return true;
     }
@@ -183,7 +173,7 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
             throw OCI8Exception::fromErrorInfo(oci_error($this->dbh));
         }
 
-        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
+        $this->executionMode->enableAutoCommit();
 
         return true;
     }
@@ -197,7 +187,7 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
             throw OCI8Exception::fromErrorInfo(oci_error($this->dbh));
         }
 
-        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
+        $this->executionMode->enableAutoCommit();
 
         return true;
     }

src/Driver/OCI8/OCI8Statement.php

@@ -2,15 +2,12 @@
 
 namespace Doctrine\DBAL\Driver\OCI8;
 
-use Doctrine\DBAL\Driver\OCI8\Exception\NonTerminatedStringLiteral;
 use Doctrine\DBAL\Driver\OCI8\Exception\UnknownParameterIndex;
 use Doctrine\DBAL\Driver\Result as ResultInterface;
 use Doctrine\DBAL\Driver\Statement as StatementInterface;
 use Doctrine\DBAL\ParameterType;
 
 use function assert;
-use function count;
-use function implode;
 use function is_int;
 use function is_resource;
 use function oci_bind_by_name;
@@ -18,15 +15,13 @@ use function oci_error;
 use function oci_execute;
 use function oci_new_descriptor;
 use function oci_parse;
-use function preg_match;
-use function preg_quote;
-use function substr;
 
 use const OCI_B_BIN;
 use const OCI_B_BLOB;
+use const OCI_COMMIT_ON_SUCCESS;
 use const OCI_D_LOB;
+use const OCI_NO_AUTO_COMMIT;
 use const OCI_TEMP_BLOB;
-use const PREG_OFFSET_CAPTURE;
 use const SQLT_CHR;
 
 /**
@@ -42,8 +37,8 @@ class OCI8Statement implements StatementInterface
     /** @var resource */
     protected $_sth;
 
-    /** @var OCI8Connection */
-    protected $_conn;
+    /** @var ExecutionMode */
+    private $executionMode;
 
     /** @var string[] */
     protected $_paramMap = [];
@@ -63,165 +58,17 @@ class OCI8Statement implements StatementInterface
      * @param resource $dbh   The connection handle.
      * @param string   $query The SQL query.
      */
-    public function __construct($dbh, $query, OCI8Connection $conn)
+    public function __construct($dbh, $query, ExecutionMode $executionMode)
     {
-        [$query, $paramMap] = self::convertPositionalToNamedPlaceholders($query);
+        [$query, $paramMap] = (new ConvertPositionalToNamedPlaceholders())($query);
 
         $stmt = oci_parse($dbh, $query);
         assert(is_resource($stmt));
 
-        $this->_sth      = $stmt;
-        $this->_dbh      = $dbh;
-        $this->_paramMap = $paramMap;
-        $this->_conn     = $conn;
-    }
-
-    /**
-     * Converts positional (?) into named placeholders (:param<num>).
-     *
-     * Oracle does not support positional parameters, hence this method converts all
-     * positional parameters into artificially named parameters. Note that this conversion
-     * is not perfect. All question marks (?) in the original statement are treated as
-     * placeholders and converted to a named parameter.
-     *
-     * The algorithm uses a state machine with two possible states: InLiteral and NotInLiteral.
-     * Question marks inside literal strings are therefore handled correctly by this method.
-     * This comes at a cost, the whole sql statement has to be looped over.
-     *
-     * @param string $statement The SQL statement to convert.
-     *
-     * @return mixed[] [0] => the statement value (string), [1] => the paramMap value (array).
-     *
-     * @throws OCI8Exception
-     *
-     * @todo extract into utility class in Doctrine\DBAL\Util namespace
-     * @todo review and test for lost spaces. we experienced missing spaces with oci8 in some sql statements.
-     */
-    public static function convertPositionalToNamedPlaceholders($statement)
-    {
-        $fragmentOffset          = $tokenOffset = 0;
-        $fragments               = $paramMap = [];
-        $currentLiteralDelimiter = null;
-
-        do {
-            if (! $currentLiteralDelimiter) {
-                $result = self::findPlaceholderOrOpeningQuote(
-                    $statement,
-                    $tokenOffset,
-                    $fragmentOffset,
-                    $fragments,
-                    $currentLiteralDelimiter,
-                    $paramMap
-                );
-            } else {
-                $result = self::findClosingQuote($statement, $tokenOffset, $currentLiteralDelimiter);
-            }
-        } while ($result);
-
-        if ($currentLiteralDelimiter) {
-            throw NonTerminatedStringLiteral::new($tokenOffset - 1);
-        }
-
-        $fragments[] = substr($statement, $fragmentOffset);
-        $statement   = implode('', $fragments);
-
-        return [$statement, $paramMap];
-    }
-
-    /**
-     * Finds next placeholder or opening quote.
-     *
-     * @param string             $statement               The SQL statement to parse
-     * @param string             $tokenOffset             The offset to start searching from
-     * @param int                $fragmentOffset          The offset to build the next fragment from
-     * @param string[]           $fragments               Fragments of the original statement not containing placeholders
-     * @param string|null        $currentLiteralDelimiter The delimiter of the current string literal
-     *                                                    or NULL if not currently in a literal
-     * @param array<int, string> $paramMap                Mapping of the original parameter positions to their named replacements
-     *
-     * @return bool Whether the token was found
-     */
-    private static function findPlaceholderOrOpeningQuote(
-        $statement,
-        &$tokenOffset,
-        &$fragmentOffset,
-        &$fragments,
-        &$currentLiteralDelimiter,
-        &$paramMap
-    ) {
-        $token = self::findToken($statement, $tokenOffset, '/[?\'"]/');
-
-        if ($token === null) {
-            return false;
-        }
-
-        if ($token === '?') {
-            $position            = count($paramMap) + 1;
-            $param               = ':param' . $position;
-            $fragments[]         = substr($statement, $fragmentOffset, $tokenOffset - $fragmentOffset);
-            $fragments[]         = $param;
-            $paramMap[$position] = $param;
-            $tokenOffset        += 1;
-            $fragmentOffset      = $tokenOffset;
-
-            return true;
-        }
-
-        $currentLiteralDelimiter = $token;
-        ++$tokenOffset;
-
-        return true;
-    }
-
-    /**
-     * Finds closing quote
-     *
-     * @param string $statement               The SQL statement to parse
-     * @param string $tokenOffset             The offset to start searching from
-     * @param string $currentLiteralDelimiter The delimiter of the current string literal
-     *
-     * @return bool Whether the token was found
-     */
-    private static function findClosingQuote(
-        $statement,
-        &$tokenOffset,
-        &$currentLiteralDelimiter
-    ) {
-        $token = self::findToken(
-            $statement,
-            $tokenOffset,
-            '/' . preg_quote($currentLiteralDelimiter, '/') . '/'
-        );
-
-        if ($token === null) {
-            return false;
-        }
-
-        $currentLiteralDelimiter = false;
-        ++$tokenOffset;
-
-        return true;
-    }
-
-    /**
-     * Finds the token described by regex starting from the given offset. Updates the offset with the position
-     * where the token was found.
-     *
-     * @param string $statement The SQL statement to parse
-     * @param int    $offset    The offset to start searching from
-     * @param string $regex     The regex containing token pattern
-     *
-     * @return string|null Token or NULL if not found
-     */
-    private static function findToken($statement, &$offset, $regex)
-    {
-        if (preg_match($regex, $statement, $matches, PREG_OFFSET_CAPTURE, $offset) === 1) {
-            $offset = $matches[0][1];
-
-            return $matches[0][0];
-        }
-
-        return null;
+        $this->_sth          = $stmt;
+        $this->_dbh          = $dbh;
+        $this->_paramMap     = $paramMap;
+        $this->executionMode = $executionMode;
     }
 
     /**
@@ -299,7 +146,13 @@ class OCI8Statement implements StatementInterface
             }
         }
 
-        $ret = @oci_execute($this->_sth, $this->_conn->getExecuteMode());
+        if ($this->executionMode->isAutoCommitEnabled()) {
+            $mode = OCI_COMMIT_ON_SUCCESS;
+        } else {
+            $mode = OCI_NO_AUTO_COMMIT;
+        }
+
+        $ret = @oci_execute($this->_sth, $mode);
         if (! $ret) {
             throw OCI8Exception::fromErrorInfo(oci_error($this->_sth));
         }

tests/UtilTest.php → tests/Driver/OCI8/ConvertPositionalToNamedPlaceholdersTest.php

 <?php
 
-namespace Doctrine\DBAL\Tests;
+declare(strict_types=1);
 
-use Doctrine\DBAL\Driver\OCI8\Statement;
+namespace Doctrine\Tests\DBAL\Driver\OCI8;
+
+use Doctrine\DBAL\Driver\OCI8\ConvertPositionalToNamedPlaceholders;
+use Doctrine\DBAL\Driver\OCI8\OCI8Exception;
 use PHPUnit\Framework\TestCase;
 
-class UtilTest extends TestCase
+class ConvertPositionalToNamedPlaceholdersTest extends TestCase
 {
+    /** @var ConvertPositionalToNamedPlaceholders */
+    private $convertPositionalToNamedPlaceholders;
+
+    protected function setUp(): void
+    {
+        $this->convertPositionalToNamedPlaceholders = new ConvertPositionalToNamedPlaceholders();
+    }
+
+    /**
+     * @param mixed[] $expectedOutputParamsMap
+     *
+     * @dataProvider positionalToNamedPlaceholdersProvider
+     */
+    public function testConvertPositionalToNamedParameters(string $inputSQL, string $expectedOutputSQL, array $expectedOutputParamsMap): void
+    {
+        [$statement, $params] = ($this->convertPositionalToNamedPlaceholders)($inputSQL);
+
+        self::assertEquals($expectedOutputSQL, $statement);
+        self::assertEquals($expectedOutputParamsMap, $params);
+    }
+
     /**
      * @return mixed[][]
      */
-    public static function dataConvertPositionalToNamedParameters(): iterable
+    public static function positionalToNamedPlaceholdersProvider(): iterable
     {
         return [
             [
@@ -67,15 +91,33 @@ class UtilTest extends TestCase
     }
 
     /**
-     * @param mixed[] $expectedOutputParamsMap
-     *
-     * @dataProvider dataConvertPositionalToNamedParameters
+     * @dataProvider nonTerminatedLiteralProvider
      */
-    public function testConvertPositionalToNamedParameters(string $inputSQL, string $expectedOutputSQL, array $expectedOutputParamsMap): void
+    public function testConvertNonTerminatedLiteral(string $sql, string $expectedExceptionMessageRegExp): void
     {
-        [$statement, $params] = Statement::convertPositionalToNamedPlaceholders($inputSQL);
+        $this->expectException(OCI8Exception::class);
+        $this->expectExceptionMessageMatches($expectedExceptionMessageRegExp);
+        ($this->convertPositionalToNamedPlaceholders)($sql);
+    }
 
-        self::assertEquals($expectedOutputSQL, $statement);
-        self::assertEquals($expectedOutputParamsMap, $params);
+    /**
+     * @return array<string, array<int, mixed>>
+     */
+    public static function nonTerminatedLiteralProvider(): iterable
+    {
+        return [
+            'no-matching-quote' => [
+                "SELECT 'literal FROM DUAL",
+                '/offset 7./',
+            ],
+            'no-matching-double-quote' => [
+                'SELECT 1 "COL1 FROM DUAL',
+                '/offset 9./',
+            ],
+            'incorrect-escaping-syntax' => [
+                "SELECT 'quoted \\'string' FROM DUAL",
+                '/offset 23./',
+            ],
+        ];
     }
 }

tests/Driver/OCI8/ExecutionModeTest.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Tests\Driver\OCI8;
+
+use Doctrine\DBAL\Driver\OCI8\ExecutionMode;
+use PHPStan\Testing\TestCase;
+
+final class ExecutionModeTest extends TestCase
+{
+    /** @var ExecutionMode */
+    private $mode;
+
+    protected function setUp(): void
+    {
+        $this->mode = new ExecutionMode();
+    }
+
+    public function testDefaultAutoCommitStatus(): void
+    {
+        self::assertTrue($this->mode->isAutoCommitEnabled());
+    }
+
+    public function testChangeAutoCommitStatus(): void
+    {
+        $this->mode->disableAutoCommit();
+        self::assertFalse($this->mode->isAutoCommitEnabled());
+
+        $this->mode->enableAutoCommit();
+        self::assertTrue($this->mode->isAutoCommitEnabled());
+    }
+}

tests/Driver/OCI8/OCI8StatementTest.php

-<?php
-
-namespace Doctrine\DBAL\Tests\Driver\OCI8;
-
-use Doctrine\DBAL\Driver\OCI8\OCI8Exception;
-use Doctrine\DBAL\Driver\OCI8\OCI8Statement;
-use PHPUnit\Framework\TestCase;
-
-use function extension_loaded;
-
-class OCI8StatementTest extends TestCase
-{
-    protected function setUp(): void
-    {
-        if (! extension_loaded('oci8')) {
-            $this->markTestSkipped('oci8 is not installed.');
-        }
-
-        parent::setUp();
-    }
-
-    /**
-     * @dataProvider nonTerminatedLiteralProvider
-     */
-    public function testConvertNonTerminatedLiteral(string $sql, string $message): void
-    {
-        $this->expectException(OCI8Exception::class);
-        $this->expectExceptionMessageMatches($message);
-        OCI8Statement::convertPositionalToNamedPlaceholders($sql);
-    }
-
-    /**
-     * @return array<string, array<int, mixed>>
-     */
-    public static function nonTerminatedLiteralProvider(): iterable
-    {
-        return [
-            'no-matching-quote' => [
-                "SELECT 'literal FROM DUAL",
-                '/offset 7/',
-            ],
-            'no-matching-double-quote' => [
-                'SELECT 1 "COL1 FROM DUAL',
-                '/offset 9/',
-            ],
-            'incorrect-escaping-syntax' => [
-                "SELECT 'quoted \\'string' FROM DUAL",
-                '/offset 23/',
-            ],
-        ];
-    }
-}