From 424bf11478bcdc42d3f9dadb471cc980078334dd Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 2 Apr 2021 20:04:52 +0200 Subject: [PATCH 1/9] Add Statement::executeQuery and Statement::executeStatement --- lib/Doctrine/DBAL/Statement.php | 29 ++++++++++++++ .../Tests/DBAL/Functional/StatementTest.php | 38 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/lib/Doctrine/DBAL/Statement.php b/lib/Doctrine/DBAL/Statement.php index 79e984e30a0..4d07511b2d2 100644 --- a/lib/Doctrine/DBAL/Statement.php +++ b/lib/Doctrine/DBAL/Statement.php @@ -7,6 +7,7 @@ use Doctrine\DBAL\Driver\Statement as DriverStatement; use Doctrine\DBAL\Exception\NoKeyValue; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Result as BaseResult; use Doctrine\DBAL\Types\Type; use Doctrine\Deprecations\Deprecation; use IteratorAggregate; @@ -181,6 +182,34 @@ public function execute($params = null) return $stmt; } + /** + * Executes the statement with the currently bound parameters and return result. + * + * @param mixed[]|null $params + + * @throws Exception + */ + public function executeQuery(?array $params = null): BaseResult + { + $this->execute($params); + + return new ForwardCompatibility\Result($this); + } + + /** + * Executes the statement with the currently bound parameters and return affected rows. + * + * @param mixed[]|null $params + + * @throws Exception + */ + public function executeStatement(?array $params = null): int + { + $this->execute($params); + + return $this->rowCount(); + } + /** * Closes the cursor, freeing the database resources used by this statement. * diff --git a/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php b/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php index 1be228045b7..01c30c98300 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php @@ -326,4 +326,42 @@ public function testFetchInColumnMode(): void self::assertEquals(1, $result); } + + public function testExecuteQuery(): void + { + $platform = $this->connection->getDatabasePlatform(); + $query = $platform->getDummySelectSQL(); + $result = $this->connection->prepare($query)->executeQuery()->fetchOne(); + + self::assertEquals(1, $result); + } + + public function testExecuteQueryWithParams(): void + { + $this->connection->insert('stmt_test', ['id' => 1]); + + $query = 'SELECT id FROM stmt_test WHERE id = ?'; + $result = $this->connection->prepare($query)->executeQuery([1])->fetchOne(); + + self::assertEquals(1, $result); + } + + public function testExecuteStatement(): void + { + $this->connection->insert('stmt_test', ['id' => 1]); + + $query = 'UPDATE stmt_test SET name = ? WHERE id = 1'; + $stmt = $this->connection->prepare($query); + + $stmt->bindValue(1, 'bar'); + + $result = $stmt->executeStatement(); + + $this->assertEquals(1, $result); + + $query = 'UPDATE stmt_test SET name = ? WHERE id = ?'; + $result = $this->connection->prepare($query)->executeStatement(['foo', 1]); + + $this->assertEquals(1, $result); + } } From 8a502f32e640da1471454a8630396aa15fde4b06 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 2 Apr 2021 23:49:39 +0200 Subject: [PATCH 2/9] Deprecate Statement::execute() --- lib/Doctrine/DBAL/Statement.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/Doctrine/DBAL/Statement.php b/lib/Doctrine/DBAL/Statement.php index 4d07511b2d2..bcc907dfa2b 100644 --- a/lib/Doctrine/DBAL/Statement.php +++ b/lib/Doctrine/DBAL/Statement.php @@ -148,6 +148,8 @@ public function bindParam($param, &$variable, $type = ParameterType::STRING, $le /** * Executes the statement with the currently bound parameters. * + * @deprecated Statement::execute() is deprecated, use Statement::executeQuery() or executeStatement() instead + * * @param mixed[]|null $params * * @return bool TRUE on success, FALSE on failure. @@ -156,6 +158,12 @@ public function bindParam($param, &$variable, $type = ParameterType::STRING, $le */ public function execute($params = null) { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4580', + 'Statement::execute() is deprecated, use Statement::executeQuery() or Statement::executeStatement() instead' + ); + if (is_array($params)) { $this->params = $params; } From 499199f56d61ac5235aabcf2c54fb733026c06ef Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 5 Apr 2021 11:36:02 +0200 Subject: [PATCH 3/9] Improve API by removing nullability --- lib/Doctrine/DBAL/Statement.php | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/DBAL/Statement.php b/lib/Doctrine/DBAL/Statement.php index bcc907dfa2b..60309343a5e 100644 --- a/lib/Doctrine/DBAL/Statement.php +++ b/lib/Doctrine/DBAL/Statement.php @@ -193,12 +193,16 @@ public function execute($params = null) /** * Executes the statement with the currently bound parameters and return result. * - * @param mixed[]|null $params - + * @param mixed[] $params + * * @throws Exception */ - public function executeQuery(?array $params = null): BaseResult + public function executeQuery(array $params = []): BaseResult { + if ($params === []) { + $params = null; // Workaround as long execute() exists and used internally. + } + $this->execute($params); return new ForwardCompatibility\Result($this); @@ -207,12 +211,16 @@ public function executeQuery(?array $params = null): BaseResult /** * Executes the statement with the currently bound parameters and return affected rows. * - * @param mixed[]|null $params - + * @param mixed[] $params + * * @throws Exception */ - public function executeStatement(?array $params = null): int + public function executeStatement(array $params = []): int { + if ($params === []) { + $params = null; // Workaround as long execute() exists and used internally. + } + $this->execute($params); return $this->rowCount(); From 5bd5b4b1f44dcb1c263275f7ede50287d0f4a1e4 Mon Sep 17 00:00:00 2001 From: Andrew Menning Date: Fri, 2 Apr 2021 17:43:15 -0400 Subject: [PATCH 4/9] Add error handler around oci_fetch_array to screen for oracle warnings The error handler screens for oracle warnings from php oci8 driver and converts them to a truncated query result exception. This also adds a test that demonstrates the issue where a result set larger than the oci8 default prefetch value is queried, but a truncated row set is returned due to the dataset being invalidated during the fetch sequenece. --- src/Driver/OCI8/Exception/PHPError.php | 37 +++++ src/Driver/OCI8/Result.php | 53 +++++- tests/Functional/Driver/OCI8/ResultTest.php | 174 ++++++++++++++++++++ 3 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 src/Driver/OCI8/Exception/PHPError.php create mode 100644 tests/Functional/Driver/OCI8/ResultTest.php diff --git a/src/Driver/OCI8/Exception/PHPError.php b/src/Driver/OCI8/Exception/PHPError.php new file mode 100644 index 00000000000..599f3a4ce97 --- /dev/null +++ b/src/Driver/OCI8/Exception/PHPError.php @@ -0,0 +1,37 @@ +statement, - $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS + set_error_handler( + static function (int $errno, string $errstr, string $errfile, int $errline): bool { + /* + * Screen PHP error messages for any error that the OCI8 extension + * doesn't report via oci_error() + * + * @link http://www.dba-oracle.com/t_error_code_list.htm + */ + if (preg_match_all('/ORA-(\d+)/', $errstr, $matches) === 0) { + // If no ORA error codes are present in error message, + // skip this error and bubble this error up to + // the normal error handler + return false; + } + + foreach ($matches[1] as $oraErrorCode) { + switch ($oraErrorCode) { + case '04061': + case '04065': + case '04068': + throw PHPError::new( + 'There was an error before all rows could be fetched.', + $errno, + $errstr, + $errfile, + $errline + ); + } + } + + // If ORA error codes in message do not match specified blocklist + // ORA error codes, skip this error and bubble this error up to + // the normal error handler + return false; + }, + E_WARNING ); + + try { + return oci_fetch_array( + $this->statement, + $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS + ); + } finally { + restore_error_handler(); + } } /** diff --git a/tests/Functional/Driver/OCI8/ResultTest.php b/tests/Functional/Driver/OCI8/ResultTest.php new file mode 100644 index 00000000000..8af13872657 --- /dev/null +++ b/tests/Functional/Driver/OCI8/ResultTest.php @@ -0,0 +1,174 @@ + + */ + private $connectionParams; + + protected function setUp(): void + { + parent::setUp(); + $this->connectionParams = TestUtil::getConnectionParams(); + + if ($this->connection->getDriver() instanceof Driver) { + return; + } + + self::markTestSkipped('oci8 only test.'); + } + + protected function tearDown(): void + { + $this->connection->executeQuery(sprintf( + 'DROP FUNCTION test_oracle_fetch_failure', + $this->connectionParams['user'] + )); + $this->connection->executeQuery(sprintf( + 'DROP TYPE return_numbers', + $this->connectionParams['user'] + )); + + parent::tearDown(); + } + + /** + * This test will recreate the case where a data set that is larger than the + * oci8 default prefetch is invalidated on the database after a fetch has begun, + * but before the fetch has completed. + * + * Note that this test requires 2 separate user connections so that the + * pipelined function can be changed mid fetch. + * + * @dataProvider dataProviderForTestTruncatedFetch + */ + public function testTruncatedFetch( + bool $invalidateDataMidFetch + ): void { + if ($invalidateDataMidFetch) { + $this->expectException(DriverException::class); + $this->expectErrorMessageMatches( + '/^An exception occurred in the driver: ' + . 'There was an error before all rows could be fetched. ' + . 'Error number: 2, Error String: oci_fetch_array.*/' + ); + } + + // Create a pipelined funtion that returns 10 rows more than the + // oci8 default prefetch + $this->createReturnTypeNeededForPipelinedFunction(); + $expectedTotalRowCount = (int) ini_get('oci8.default_prefetch') + 10; + $this->createOrReplacePipelinedFunction($expectedTotalRowCount); + + // Create a separate connection from that used to create/update the function + // This must be a different user with permissions to change the given function + $separateConnection = TestUtil::getPrivilegedConnection(); + + // Query the pipelined function to get initial dataset + $statement = $separateConnection->prepare(sprintf( + 'SELECT * FROM TABLE(%s.test_oracle_fetch_failure())', + $this->connectionParams['user'] + )); + $result = $statement->execute(); + + // Access the first result to cause the first X rows to be prefetched + // as defined by oci8.default_prefetch (often 100 rows) + $firstNumber = $result->fetchOne(); + + if ($invalidateDataMidFetch) { + // Invalidate the original dataset by changing the pipelined function + // after the initial prefetch that caches locally the first X results + $this->createOrReplacePipelinedFunction($expectedTotalRowCount + 10); + } + + while ($number = $result->fetchOne()) { + // Attempt to access all remaining rows from the original fetch + // The rows locally cached from the default prefetch will first be used + // but when the result attempts to get the remaining 10 rows beyond + // the first prefetch, nothing will be returned + // + // PHP oci8 oci_fetch_array will issue a PHP E_WARNING when the 2nd prefetch occurs + // oci_fetch_array(): ORA-04068: existing state of packages has been discarded + // ORA-04061: existing state of function "ROOT.TEST_ORACLE_FETCH_FAILURE" has been invalidated + // ORA-04065: not executed, altered or dropped function "ROOT.TEST_ORACLE_FETCH_FAILURE" + // + // If there was no issue, this should have returned rows totalling 10 + // higher than the oci8 default prefetch + continue; + } + + $this->assertEquals( + $expectedTotalRowCount, + $result->rowCount(), + sprintf( + 'Expected to have %s total rows fetched but only found %s rows fetched', + $expectedTotalRowCount, + $result->rowCount() + ) + ); + } + + public function dataProviderForTestTruncatedFetch(): Generator + { + yield 'it should return all rows if no data invalidation occurs' + => [false]; + + yield 'it should convert oci8 data invalidation error to DriverException' + => [true]; + } + + private function createReturnTypeNeededForPipelinedFunction(): void + { + $this->connection->executeQuery( + 'CREATE TYPE return_numbers AS TABLE OF NUMBER(11)' + ); + } + + /** + * This will create a pipelined function that returns X rows with + * each row returning a single column_value of that row's row number. + * The total number of rows returned is equal to $totalRowCount. + */ + private function createOrReplacePipelinedFunction(int $totalRowCount): void + { + $this->connection->executeQuery(sprintf( + 'CREATE OR REPLACE FUNCTION test_oracle_fetch_failure + RETURN return_numbers PIPELINED + AS + v_number_list return_numbers; + BEGIN + SELECT ROWNUM r + BULK COLLECT INTO v_number_list + FROM DUAL + CONNECT BY ROWNUM <= %d; + + FOR i IN 1 .. v_number_list.COUNT + LOOP + PIPE ROW (v_number_list(i)); + END LOOP; + + RETURN; + END;', + $totalRowCount + )); + } +} From 2d232c177836fa84692e3f2cd5aaa96cf4dc8685 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sat, 10 Apr 2021 11:12:53 -0700 Subject: [PATCH 5/9] [GH-4553] Simplify error handling in the OCI8 driver --- src/Driver/OCI8/Exception/PHPError.php | 37 ------------- src/Driver/OCI8/Result.php | 59 ++++----------------- tests/Functional/Driver/OCI8/ResultTest.php | 16 +++--- 3 files changed, 19 insertions(+), 93 deletions(-) delete mode 100644 src/Driver/OCI8/Exception/PHPError.php diff --git a/src/Driver/OCI8/Exception/PHPError.php b/src/Driver/OCI8/Exception/PHPError.php deleted file mode 100644 index 599f3a4ce97..00000000000 --- a/src/Driver/OCI8/Exception/PHPError.php +++ /dev/null @@ -1,37 +0,0 @@ -statement, $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS); - try { - return oci_fetch_array( - $this->statement, - $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS - ); - } finally { - restore_error_handler(); + if ($result === false && oci_error($this->statement) !== false) { + throw Error::new($this->statement); } + + return $result; } /** diff --git a/tests/Functional/Driver/OCI8/ResultTest.php b/tests/Functional/Driver/OCI8/ResultTest.php index 8af13872657..b5ad9925568 100644 --- a/tests/Functional/Driver/OCI8/ResultTest.php +++ b/tests/Functional/Driver/OCI8/ResultTest.php @@ -13,6 +13,9 @@ use function ini_get; use function sprintf; +use const E_ALL; +use const E_WARNING; + /** * @requires extension oci8 */ @@ -65,12 +68,11 @@ public function testTruncatedFetch( bool $invalidateDataMidFetch ): void { if ($invalidateDataMidFetch) { + // prevent the PHPUnit error handler from handling the warnings that oci_*() functions may trigger + $this->iniSet('error_reporting', (string) (E_ALL & ~E_WARNING)); + $this->expectException(DriverException::class); - $this->expectErrorMessageMatches( - '/^An exception occurred in the driver: ' - . 'There was an error before all rows could be fetched. ' - . 'Error number: 2, Error String: oci_fetch_array.*/' - ); + $this->expectExceptionCode(4068); } // Create a pipelined funtion that returns 10 rows more than the @@ -92,7 +94,7 @@ public function testTruncatedFetch( // Access the first result to cause the first X rows to be prefetched // as defined by oci8.default_prefetch (often 100 rows) - $firstNumber = $result->fetchOne(); + $result->fetchOne(); if ($invalidateDataMidFetch) { // Invalidate the original dataset by changing the pipelined function @@ -100,7 +102,7 @@ public function testTruncatedFetch( $this->createOrReplacePipelinedFunction($expectedTotalRowCount + 10); } - while ($number = $result->fetchOne()) { + while ($result->fetchOne()) { // Attempt to access all remaining rows from the original fetch // The rows locally cached from the default prefetch will first be used // but when the result attempts to get the remaining 10 rows beyond From 1372a42083dedc56016051d61526507fb5521f9c Mon Sep 17 00:00:00 2001 From: Martin Dumoulin Date: Fri, 9 Apr 2021 22:18:03 +0200 Subject: [PATCH 6/9] Restore backward compatibility for QueryBuilder::execute() Following methods are returning Doctrine\DBAL\Driver\Statement as before : - Doctrine\DBAL\Connection::executeQuery() - Doctrine\DBAL\Query\QueryBuilder::execute() --- lib/Doctrine/DBAL/Connection.php | 15 +- .../DriverResultStatement.php | 9 ++ .../ForwardCompatibility/DriverStatement.php | 9 ++ .../DBAL/ForwardCompatibility/Result.php | 121 ++++++++++++++- lib/Doctrine/DBAL/Query/QueryBuilder.php | 8 +- .../DBAL/ForwardCompatibility/ResultTest.php | 138 +++++++++++++++++- .../Tests/DBAL/Functional/ConnectionTest.php | 53 ++++++- .../Tests/DBAL/Query/QueryBuilderTest.php | 23 ++- 8 files changed, 352 insertions(+), 24 deletions(-) create mode 100644 lib/Doctrine/DBAL/ForwardCompatibility/DriverResultStatement.php create mode 100644 lib/Doctrine/DBAL/ForwardCompatibility/DriverStatement.php diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index 573b60c963f..67ffe820341 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -18,7 +18,6 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Query\Expression\ExpressionBuilder; use Doctrine\DBAL\Query\QueryBuilder; -use Doctrine\DBAL\Result as BaseResult; use Doctrine\DBAL\Schema\AbstractSchemaManager; use Doctrine\DBAL\Types\Type; use Doctrine\Deprecations\Deprecation; @@ -1264,7 +1263,9 @@ public function prepare($sql) * @param array|array $params Query parameters * @param array|array $types Parameter types * - * @return ResultStatement&BaseResult The executed statement. + * @return ForwardCompatibility\DriverStatement|ForwardCompatibility\DriverResultStatement + * + * The executed statement or the cached result statement if a query cache profile is used * * @throws Exception */ @@ -1320,7 +1321,7 @@ public function executeQuery($sql, array $params = [], $types = [], ?QueryCacheP * @param array|array $params Query parameters * @param array|array $types Parameter types * - * @return ResultStatement&BaseResult + * @return ForwardCompatibility\DriverResultStatement * * @throws CacheException */ @@ -1365,15 +1366,11 @@ public function executeCacheQuery($sql, $params, $types, QueryCacheProfile $qcp) } /** - * @return ResultStatement&BaseResult + * @return ForwardCompatibility\Result */ private function ensureForwardCompatibilityStatement(ResultStatement $stmt) { - if ($stmt instanceof BaseResult) { - return $stmt; - } - - return new ForwardCompatibility\Result($stmt); + return ForwardCompatibility\Result::ensure($stmt); } /** diff --git a/lib/Doctrine/DBAL/ForwardCompatibility/DriverResultStatement.php b/lib/Doctrine/DBAL/ForwardCompatibility/DriverResultStatement.php new file mode 100644 index 00000000000..0cf9aa3dda3 --- /dev/null +++ b/lib/Doctrine/DBAL/ForwardCompatibility/DriverResultStatement.php @@ -0,0 +1,9 @@ +stmt = $stmt; } /** - * @return DriverResultStatement + * @return Driver\ResultStatement */ public function getIterator() { @@ -301,4 +310,104 @@ private function ensureHasKeyValue(): void throw NoKeyValue::fromColumnCount($columnCount); } } + + /** + * {@inheritDoc} + * + * @deprecated This feature will no longer be available on Result object in 3.0.x version. + */ + public function bindValue($param, $value, $type = ParameterType::STRING) + { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4019', + 'Result::bindValue() is deprecated, no replacement.' + ); + + if ($this->stmt instanceof Driver\Statement) { + return $this->stmt->bindValue($param, $value, $type); + } + + throw Exception::notSupported('bindValue'); + } + + /** + * {@inheritDoc} + * + * @deprecated This feature will no longer be available on Result object in 3.0.x version. + */ + public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null) + { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4019', + 'Result::bindParam() is deprecated, no replacement.' + ); + + if ($this->stmt instanceof Driver\Statement) { + return $this->stmt->bindParam($param, $variable, $type, $length); + } + + throw Exception::notSupported('bindParam'); + } + + /** + * {@inheritDoc} + * + * @deprecated The error information is available via exceptions. + */ + public function errorCode() + { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4019', + 'Result::errorCode() is deprecated, the error information is available via exceptions.' + ); + + if ($this->stmt instanceof Driver\Statement) { + return $this->stmt->errorCode(); + } + + throw Exception::notSupported('errorCode'); + } + + /** + * {@inheritDoc} + * + * @deprecated The error information is available via exceptions. + */ + public function errorInfo() + { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4019', + 'Result::errorInfo() is deprecated, the error information is available via exceptions.' + ); + + if ($this->stmt instanceof Driver\Statement) { + return $this->stmt->errorInfo(); + } + + throw Exception::notSupported('errorInfo'); + } + + /** + * {@inheritDoc} + * + * @deprecated This feature will no longer be available on Result object in 3.0.x version. + */ + public function execute($params = null) + { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4019', + 'Result::execute() is deprecated, no replacement.' + ); + + if ($this->stmt instanceof Driver\Statement) { + return $this->stmt->execute($params); + } + + throw Exception::notSupported('execute'); + } } diff --git a/lib/Doctrine/DBAL/Query/QueryBuilder.php b/lib/Doctrine/DBAL/Query/QueryBuilder.php index 4d27ce25dd2..419937d3586 100644 --- a/lib/Doctrine/DBAL/Query/QueryBuilder.php +++ b/lib/Doctrine/DBAL/Query/QueryBuilder.php @@ -3,8 +3,8 @@ namespace Doctrine\DBAL\Query; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Driver\ResultStatement; use Doctrine\DBAL\Exception; +use Doctrine\DBAL\ForwardCompatibility; use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Query\Expression\CompositeExpression; use Doctrine\DBAL\Query\Expression\ExpressionBuilder; @@ -201,14 +201,16 @@ public function getState() /** * Executes this query using the bound parameters and their types. * - * @return ResultStatement|int + * @return ForwardCompatibility\DriverStatement|int * * @throws Exception */ public function execute() { if ($this->type === self::SELECT) { - return $this->connection->executeQuery($this->getSQL(), $this->params, $this->paramTypes); + return ForwardCompatibility\Result::ensure( + $this->connection->executeQuery($this->getSQL(), $this->params, $this->paramTypes) + ); } return $this->connection->executeStatement($this->getSQL(), $this->params, $this->paramTypes); diff --git a/tests/Doctrine/Tests/DBAL/ForwardCompatibility/ResultTest.php b/tests/Doctrine/Tests/DBAL/ForwardCompatibility/ResultTest.php index a519e33c8c4..0798c7dbd3d 100644 --- a/tests/Doctrine/Tests/DBAL/ForwardCompatibility/ResultTest.php +++ b/tests/Doctrine/Tests/DBAL/ForwardCompatibility/ResultTest.php @@ -3,9 +3,10 @@ namespace Doctrine\Tests\DBAL\ForwardCompatibility; use Doctrine\DBAL\Cache\ArrayStatement; -use Doctrine\DBAL\Driver\ResultStatement as DriverResultStatement; +use Doctrine\DBAL\Driver; use Doctrine\DBAL\Exception; use Doctrine\DBAL\ForwardCompatibility\Result; +use Doctrine\DBAL\ParameterType; use PDO; use PHPUnit\Framework\TestCase; use Traversable; @@ -40,6 +41,24 @@ public function setUp(): void ); } + public function testEnsureWithResult(): void + { + $instance = Result::ensure($this->instance); + $this->assertSame($this->instance, $instance, 'Result is not wrapped twice'); + } + + public function testEnsureWithResultStatement(): void + { + $instance = Result::ensure($this->createMock(Driver\ResultStatement::class)); + $this->assertInstanceOf(Result::class, $instance); + } + + public function testEnsureWithStatement(): void + { + $instance = Result::ensure($this->createMock(Driver\Statement::class)); + $this->assertInstanceOf(Result::class, $instance); + } + public function testIsTraversable(): void { $this->instance->setFetchMode(PDO::FETCH_ASSOC); @@ -335,7 +354,7 @@ public function testIterateColumn(): void ); } - public function testRowCountIsSupportedByWrappedStatement(): void + public function testRowCountIsSupportedByWrappedArrayStatement(): void { $this->assertSame(3, $this->instance->rowCount()); } @@ -343,7 +362,120 @@ public function testRowCountIsSupportedByWrappedStatement(): void public function testRowCountIsNotSupportedByWrappedStatement(): void { $this->expectExceptionObject(Exception::notSupported('rowCount')); - $instance = new Result($this->createMock(DriverResultStatement::class)); + $instance = new Result($this->createMock(Driver\ResultStatement::class)); $instance->rowCount(); } + + public function testBindValueIsSupportedByWrappedStatement(): void + { + $param = ':key'; + $value = 'value'; + + $statement = $this->createMock(Driver\Statement::class); + $statement + ->expects($this->once()) + ->method('bindValue') + ->with($param, $value, ParameterType::STRING) + ->willReturn(true); + + $instance = new Result($statement); + + $this->assertTrue($instance->bindValue($param, $value)); + } + + public function testBindValueIsNotSupportedByWrappedResultStatement(): void + { + $this->expectExceptionObject(Exception::notSupported('bindValue')); + $this->instance->bindValue(':key', 'value'); + } + + public function testBindParamIsSupportedByWrappedStatement(): void + { + $param = ':key'; + $value = 'value'; + + $statement = $this->createMock(Driver\Statement::class); + $statement + ->expects($this->once()) + ->method('bindParam') + ->with($param, $value, ParameterType::STRING) + ->willReturn(true); + + $instance = new Result($statement); + + $this->assertTrue($instance->bindParam($param, $value)); + } + + public function testBindParamIsNotSupportedByWrappedResultStatement(): void + { + $param = ':key'; + $value = 'value'; + + $this->expectExceptionObject(Exception::notSupported('bindParam')); + $this->instance->bindParam($param, $value); + } + + public function testErrorCodeIsSupportedByWrappedStatement(): void + { + $errorCode = 32; + + $statement = $this->createMock(Driver\Statement::class); + $statement + ->expects($this->once()) + ->method('errorCode') + ->willReturn($errorCode); + + $instance = new Result($statement); + + $this->assertSame($errorCode, $instance->errorCode()); + } + + public function testErrorCodeIsNotSupportedByWrappedResultStatement(): void + { + $this->expectExceptionObject(Exception::notSupported('errorCode')); + $this->instance->errorCode(); + } + + public function testErrorInfoIsSupportedByWrappedStatement(): void + { + $errorInfo = ['Some info']; + + $statement = $this->createMock(Driver\Statement::class); + $statement + ->expects($this->once()) + ->method('errorInfo') + ->willReturn($errorInfo); + + $instance = new Result($statement); + + $this->assertSame($errorInfo, $instance->errorInfo()); + } + + public function testErrorInfoIsNotSupportedByWrappedResultStatement(): void + { + $this->expectExceptionObject(Exception::notSupported('errorInfo')); + $this->instance->errorInfo(); + } + + public function testExecuteIsSupportedByWrappedStatement(): void + { + $params = [':key' => 'value']; + + $statement = $this->createMock(Driver\Statement::class); + $statement + ->expects($this->once()) + ->method('execute') + ->with($params) + ->willReturn(true); + + $instance = new Result($statement); + + $this->assertTrue($instance->execute($params)); + } + + public function testExecuteIsNotSupportedByWrappedResultStatement(): void + { + $this->expectExceptionObject(Exception::notSupported('execute')); + $this->instance->execute([':key' => 'value']); + } } diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php index b43ade0ae27..61128bdf6bd 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php @@ -2,11 +2,14 @@ namespace Doctrine\Tests\DBAL\Functional; +use Doctrine\Common\Cache\ArrayCache; +use Doctrine\DBAL\Cache\QueryCacheProfile; use Doctrine\DBAL\Connection; use Doctrine\DBAL\ConnectionException; +use Doctrine\DBAL\Driver; use Doctrine\DBAL\Driver\Connection as DriverConnection; -use Doctrine\DBAL\Driver\ResultStatement; use Doctrine\DBAL\DriverManager; +use Doctrine\DBAL\ForwardCompatibility; use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Result; @@ -356,7 +359,53 @@ public function testUserProvidedPDOConnection(): void $result = $connection->executeQuery('SELECT 1'); - self::assertInstanceOf(ResultStatement::class, $result); + self::assertInstanceOf(ForwardCompatibility\Result::class, $result); + } + + public function testResultCompatibilityWhenExecutingQueryWithoutParam(): void + { + $result = $this->connection->executeQuery( + $this->connection->getDatabasePlatform()->getDummySelectSQL() + ); + + self::assertInstanceOf(Result::class, $result); + self::assertInstanceOf(Driver\Statement::class, $result); + } + + public function testResultCompatibilityWhenExecutingQueryWithParams(): void + { + $result = $this->connection->executeQuery( + $this->connection->getDatabasePlatform()->getDummySelectSQL(), + ['param1' => 'value'] + ); + + self::assertInstanceOf(Result::class, $result); + self::assertInstanceOf(Driver\Statement::class, $result); + } + + public function testResultCompatibilityWhenExecutingQueryWithQueryCacheParam(): void + { + $result = $this->connection->executeQuery( + $this->connection->getDatabasePlatform()->getDummySelectSQL(), + [], + [], + new QueryCacheProfile(1, 'cacheKey', new ArrayCache()) + ); + + self::assertInstanceOf(Result::class, $result); + self::assertInstanceOf(Driver\ResultStatement::class, $result); + } + + public function testResultCompatibilityWhenExecutingCacheQuery(): void + { + $result = $this->connection->executeCacheQuery( + $this->connection->getDatabasePlatform()->getDummySelectSQL(), + [], + [], + new QueryCacheProfile(1, 'cacheKey', new ArrayCache()) + ); + self::assertInstanceOf(Result::class, $result); + self::assertInstanceOf(Driver\ResultStatement::class, $result); } } diff --git a/tests/Doctrine/Tests/DBAL/Query/QueryBuilderTest.php b/tests/Doctrine/Tests/DBAL/Query/QueryBuilderTest.php index a83362ed588..63969855009 100644 --- a/tests/Doctrine/Tests/DBAL/Query/QueryBuilderTest.php +++ b/tests/Doctrine/Tests/DBAL/Query/QueryBuilderTest.php @@ -3,15 +3,18 @@ namespace Doctrine\Tests\DBAL\Query; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Driver; use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Query\Expression\ExpressionBuilder; use Doctrine\DBAL\Query\QueryBuilder; use Doctrine\DBAL\Query\QueryException; +use Doctrine\DBAL\Result; use Doctrine\Tests\DbalTestCase; +use PHPUnit\Framework\MockObject\MockObject; class QueryBuilderTest extends DbalTestCase { - /** @var Connection */ + /** @var Connection&MockObject */ protected $conn; protected function setUp(): void @@ -1075,4 +1078,22 @@ public function testOrHavingEmptyStringStartingWithNonEmptyExpression(): void self::assertSame('SELECT id FROM foo HAVING (a = b) OR (c = d)', $qb->getSQL()); } + + public function testExecuteSelect(): void + { + $qb = new QueryBuilder($this->conn); + + $this->conn + ->expects($this->any()) + ->method('executeQuery') + ->willReturn($this->createMock(Driver\Statement::class)); + + $result = $qb + ->select('id') + ->from('foo') + ->execute(); + + self::assertInstanceOf(Driver\Statement::class, $result); + self::assertInstanceOf(Result::class, $result); + } } From ef629cfd332e866b51914e48d0be1de6fdfb7c67 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 9 Apr 2021 07:48:41 -0700 Subject: [PATCH 7/9] Collect type coverage metrics, add badge to README --- .github/workflows/static-analysis.yml | 1 + README.md | 3 +++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 35f1d6645a0..d049b95c7b2 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -52,3 +52,4 @@ jobs: uses: docker://vimeo/psalm-github-actions:4.6.4 with: composer_require_dev: true + args: --shepherd diff --git a/README.md b/README.md index ae4467a5f12..c197719de3d 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ | [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.0 image]][GA 3.0] | [![GitHub Actions][GA 2.12 image]][GA 2.12] | | [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.0 image]][AppVeyor 3.0] | [![AppVeyor][AppVeyor 2.12 image]][AppVeyor 2.12] | | [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.0 image]][CodeCov 3.0] | [![Code Coverage][Coverage 2.12 image]][CodeCov 2.12] | +| N/A | N/A | [![Code Coverage][TypeCov 2.12 image]][TypeCov 2.12] | Powerful database abstraction layer with many features for database schema introspection, schema management and PDO abstraction. @@ -37,3 +38,5 @@ Powerful database abstraction layer with many features for database schema intro [AppVeyor 2.12 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/2.12.x?svg=true [GA 2.12]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A2.12.x [GA 2.12 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=2.12.x + [TypeCov 2.12]: https://shepherd.dev/github/doctrine/dbal + [TypeCov 2.12 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg From a25bc6af1fd792eb91e2600dc6a54715aa9e5b30 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Thu, 15 Apr 2021 16:28:58 -0700 Subject: [PATCH 8/9] Replace 2.12 with 2.13 in README --- README.md | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index c197719de3d..48b9477da11 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,11 @@ # Doctrine DBAL -| [4.0-dev][4.0] | [3.0][3.0] | [2.12][2.12] | +| [4.0-dev][4.0] | [3.0][3.0] | [2.13][2.13] | |:----------------:|:----------:|:----------:| -| [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.0 image]][GA 3.0] | [![GitHub Actions][GA 2.12 image]][GA 2.12] | -| [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.0 image]][AppVeyor 3.0] | [![AppVeyor][AppVeyor 2.12 image]][AppVeyor 2.12] | -| [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.0 image]][CodeCov 3.0] | [![Code Coverage][Coverage 2.12 image]][CodeCov 2.12] | -| N/A | N/A | [![Code Coverage][TypeCov 2.12 image]][TypeCov 2.12] | +| [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.0 image]][GA 3.0] | [![GitHub Actions][GA 2.13 image]][GA 2.13] | +| [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.0 image]][AppVeyor 3.0] | [![AppVeyor][AppVeyor 2.13 image]][AppVeyor 2.13] | +| [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.0 image]][CodeCov 3.0] | [![Code Coverage][Coverage 2.13 image]][CodeCov 2.13] | +| N/A | N/A | [![Code Coverage][TypeCov 2.13 image]][TypeCov 2.13] | Powerful database abstraction layer with many features for database schema introspection, schema management and PDO abstraction. @@ -31,12 +31,12 @@ Powerful database abstraction layer with many features for database schema intro [GA 3.0]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A3.0.x [GA 3.0 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=3.0.x - [Coverage 2.12 image]: https://codecov.io/gh/doctrine/dbal/branch/2.12.x/graph/badge.svg - [2.12]: https://github.com/doctrine/dbal/tree/2.12.x - [CodeCov 2.12]: https://codecov.io/gh/doctrine/dbal/branch/2.12.x - [AppVeyor 2.12]: https://ci.appveyor.com/project/doctrine/dbal/branch/2.12.x - [AppVeyor 2.12 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/2.12.x?svg=true - [GA 2.12]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A2.12.x - [GA 2.12 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=2.12.x - [TypeCov 2.12]: https://shepherd.dev/github/doctrine/dbal - [TypeCov 2.12 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg + [Coverage 2.13 image]: https://codecov.io/gh/doctrine/dbal/branch/2.13.x/graph/badge.svg + [2.13]: https://github.com/doctrine/dbal/tree/2.13.x + [CodeCov 2.13]: https://codecov.io/gh/doctrine/dbal/branch/2.13.x + [AppVeyor 2.13]: https://ci.appveyor.com/project/doctrine/dbal/branch/2.13.x + [AppVeyor 2.13 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/2.13.x?svg=true + [GA 2.13]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A2.13.x + [GA 2.13 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=2.13.x + [TypeCov 2.13]: https://shepherd.dev/github/doctrine/dbal + [TypeCov 2.13 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg From 05b7797f327d871b36e4990a5aef1b26b2fb32c5 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sat, 17 Apr 2021 10:13:47 -0700 Subject: [PATCH 9/9] Attribute type coverage metric to 3.1.x instead of 2.13.x --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 48b9477da11..f936180712e 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ | [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.0 image]][GA 3.0] | [![GitHub Actions][GA 2.13 image]][GA 2.13] | | [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.0 image]][AppVeyor 3.0] | [![AppVeyor][AppVeyor 2.13 image]][AppVeyor 2.13] | | [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.0 image]][CodeCov 3.0] | [![Code Coverage][Coverage 2.13 image]][CodeCov 2.13] | -| N/A | N/A | [![Code Coverage][TypeCov 2.13 image]][TypeCov 2.13] | +| N/A | [![Code Coverage][TypeCov 3.1 image]][TypeCov 3.1] | N/A | Powerful database abstraction layer with many features for database schema introspection, schema management and PDO abstraction. @@ -38,5 +38,5 @@ Powerful database abstraction layer with many features for database schema intro [AppVeyor 2.13 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/2.13.x?svg=true [GA 2.13]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A2.13.x [GA 2.13 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=2.13.x - [TypeCov 2.13]: https://shepherd.dev/github/doctrine/dbal - [TypeCov 2.13 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg + [TypeCov 3.1]: https://shepherd.dev/github/doctrine/dbal + [TypeCov 3.1 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg