diff --git a/psalm.xml b/psalm.xml index 7aafe8fe5d4..7628f4f0a65 100644 --- a/psalm.xml +++ b/psalm.xml @@ -29,7 +29,7 @@ This one is just too convoluted for Psalm to figure out, by its author's own admission --> - + diff --git a/src/Driver/OCI8/ConvertPositionalToNamedPlaceholders.php b/src/Driver/OCI8/ConvertPositionalToNamedPlaceholders.php new file mode 100644 index 00000000000..947c041a6c4 --- /dev/null +++ b/src/Driver/OCI8/ConvertPositionalToNamedPlaceholders.php @@ -0,0 +1,162 @@ +). + * + * 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; + } +} diff --git a/src/Driver/OCI8/ExecutionMode.php b/src/Driver/OCI8/ExecutionMode.php new file mode 100644 index 00000000000..88c00216fda --- /dev/null +++ b/src/Driver/OCI8/ExecutionMode.php @@ -0,0 +1,31 @@ +isAutoCommitEnabled = true; + } + + public function disableAutoCommit(): void + { + $this->isAutoCommitEnabled = false; + } + + public function isAutoCommitEnabled(): bool + { + return $this->isAutoCommitEnabled; + } +} diff --git a/src/Driver/OCI8/OCI8Connection.php b/src/Driver/OCI8/OCI8Connection.php index c3022e180e7..23140a39ee8 100644 --- a/src/Driver/OCI8/OCI8Connection.php +++ b/src/Driver/OCI8/OCI8Connection.php @@ -23,7 +23,6 @@ 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 @@ public function __construct( throw OCI8Exception::fromErrorInfo(oci_error()); } - $this->dbh = $dbh; + $this->dbh = $dbh; + $this->executionMode = new ExecutionMode(); } /** @@ -107,7 +107,7 @@ public function requiresQueryForServerVersion() 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 @@ public function lastInsertId($name = null) 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 @@ public function commit() throw OCI8Exception::fromErrorInfo(oci_error($this->dbh)); } - $this->executeMode = OCI_COMMIT_ON_SUCCESS; + $this->executionMode->enableAutoCommit(); return true; } @@ -197,7 +187,7 @@ public function rollBack() throw OCI8Exception::fromErrorInfo(oci_error($this->dbh)); } - $this->executeMode = OCI_COMMIT_ON_SUCCESS; + $this->executionMode->enableAutoCommit(); return true; } diff --git a/src/Driver/OCI8/OCI8Statement.php b/src/Driver/OCI8/OCI8Statement.php index 180629667e5..6ef87d448e7 100644 --- a/src/Driver/OCI8/OCI8Statement.php +++ b/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_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). - * - * 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 $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 @@ public function execute($params = null): ResultInterface } } - $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)); } diff --git a/tests/UtilTest.php b/tests/Driver/OCI8/ConvertPositionalToNamedPlaceholdersTest.php similarity index 60% rename from tests/UtilTest.php rename to tests/Driver/OCI8/ConvertPositionalToNamedPlaceholdersTest.php index 054caeada17..51323932d55 100644 --- a/tests/UtilTest.php +++ b/tests/Driver/OCI8/ConvertPositionalToNamedPlaceholdersTest.php @@ -1,16 +1,40 @@ 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 @@ public static function dataConvertPositionalToNamedParameters(): iterable } /** - * @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> + */ + 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./', + ], + ]; } } diff --git a/tests/Driver/OCI8/ExecutionModeTest.php b/tests/Driver/OCI8/ExecutionModeTest.php new file mode 100644 index 00000000000..ab041e0fb9b --- /dev/null +++ b/tests/Driver/OCI8/ExecutionModeTest.php @@ -0,0 +1,33 @@ +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()); + } +} diff --git a/tests/Driver/OCI8/OCI8StatementTest.php b/tests/Driver/OCI8/OCI8StatementTest.php deleted file mode 100644 index 77466f37f8b..00000000000 --- a/tests/Driver/OCI8/OCI8StatementTest.php +++ /dev/null @@ -1,52 +0,0 @@ -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> - */ - 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/', - ], - ]; - } -}