diff --git a/src/Connection.php b/src/Connection.php index 184b01b3a94..ffb45efeca0 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -919,14 +919,15 @@ public function transactional(Closure $func): mixed $this->beginTransaction(); try { $res = $func($this); - $this->commit(); - - return $res; } catch (Throwable $e) { $this->rollBack(); throw $e; } + + $this->commit(); + + return $res; } /** @@ -1005,17 +1006,26 @@ public function commit(): void $connection = $this->connect(); - if ($this->transactionNestingLevel === 1) { - try { - $connection->commit(); - } catch (Driver\Exception $e) { - throw $this->convertException($e); + try { + if ($this->transactionNestingLevel === 1) { + try { + $connection->commit(); + } catch (Driver\Exception $e) { + throw $this->convertException($e); + } + } else { + $this->releaseSavepoint($this->_getNestedTransactionSavePointName()); } - } else { - $this->releaseSavepoint($this->_getNestedTransactionSavePointName()); + } finally { + $this->updateTransactionStateAfterCommit(); } + } - --$this->transactionNestingLevel; + private function updateTransactionStateAfterCommit(): void + { + if ($this->transactionNestingLevel !== 0) { + --$this->transactionNestingLevel; + } if ($this->autoCommit !== false || $this->transactionNestingLevel !== 0) { return; diff --git a/src/Driver/OCI8/Connection.php b/src/Driver/OCI8/Connection.php index 3652ca0eb85..8334cb94171 100644 --- a/src/Driver/OCI8/Connection.php +++ b/src/Driver/OCI8/Connection.php @@ -95,7 +95,7 @@ public function beginTransaction(): void public function commit(): void { - if (! oci_commit($this->connection)) { + if (! @oci_commit($this->connection)) { throw Error::new($this->connection); } diff --git a/src/Driver/OCI8/Exception/Error.php b/src/Driver/OCI8/Exception/Error.php index 6abdf233c11..29ab677e02b 100644 --- a/src/Driver/OCI8/Exception/Error.php +++ b/src/Driver/OCI8/Exception/Error.php @@ -7,7 +7,9 @@ use Doctrine\DBAL\Driver\AbstractException; use function assert; +use function explode; use function oci_error; +use function str_replace; /** * @internal @@ -16,12 +18,26 @@ */ final class Error extends AbstractException { + private const CODE_TRANSACTION_ROLLED_BACK = 2091; + /** @param resource $resource */ public static function new($resource): self { $error = oci_error($resource); assert($error !== false); - return new self($error['message'], null, $error['code']); + $code = $error['code']; + $message = $error['message']; + if ($code === self::CODE_TRANSACTION_ROLLED_BACK) { + // There's no way this can be unit-tested as it's impossible to mock $resource + //ORA-02091: transaction rolled back + //ORA-00001: unique constraint (DOCTRINE.GH3423_UNIQUE) violated + [$firstMessage, $secondMessage] = explode("\n", $message, 2); + + [$code, $message] = explode(': ', $secondMessage, 2); + $code = (int) str_replace('ORA-', '', $code); + } + + return new self($message, null, $code); } } diff --git a/src/Driver/PDO/Exception.php b/src/Driver/PDO/Exception.php index fbb81253bf2..faac1a1176d 100644 --- a/src/Driver/PDO/Exception.php +++ b/src/Driver/PDO/Exception.php @@ -7,6 +7,9 @@ use Doctrine\DBAL\Driver\AbstractException; use PDOException; +use function explode; +use function str_replace; + /** * @internal * @@ -14,8 +17,12 @@ */ final class Exception extends AbstractException { + private const CODE_TRANSACTION_ROLLED_BACK = 2091; + public static function new(PDOException $exception): self { + $message = $exception->getMessage(); + if ($exception->errorInfo !== null) { [$sqlState, $code] = $exception->errorInfo; @@ -25,6 +32,15 @@ public static function new(PDOException $exception): self $sqlState = null; } - return new self($exception->getMessage(), $sqlState, $code, $exception); + if ($code === self::CODE_TRANSACTION_ROLLED_BACK) { + //ORA-02091: transaction rolled back + //ORA-00001: unique constraint (DOCTRINE.GH3423_UNIQUE) violated + [$firstMessage, $secondMessage] = explode("\n", $message, 2); + + [$code, $message] = explode(': ', $secondMessage, 2); + $code = (int) str_replace('ORA-', '', $code); + } + + return new self($message, $sqlState, $code, $exception); } } diff --git a/tests/Driver/PDO/ExceptionTest.php b/tests/Driver/PDO/ExceptionTest.php index 44f4c288f3f..2666220a0c7 100644 --- a/tests/Driver/PDO/ExceptionTest.php +++ b/tests/Driver/PDO/ExceptionTest.php @@ -56,4 +56,25 @@ public function testOriginalExceptionIsInChain(): void { self::assertSame($this->wrappedException, $this->exception->getPrevious()); } + + public function testExposesUnderlyingErrorOnOracle(): void + { + $pdoException = new PDOException(<<<'TEXT' +OCITransCommit: ORA-02091: transaction rolled back +ORA-00001: unique constraint (DOCTRINE.C1_UNIQUE) violated + (/private/tmp/php-20211003-35441-1sggrmq/php-8.0.11/ext/pdo_oci/oci_driver.c:410) +TEXT); + + $pdoException->errorInfo = [self::SQLSTATE, 2091, + + ]; + + $exception = Exception::new($pdoException); + + self::assertSame(1, $exception->getCode()); + self::assertStringContainsString( + 'unique constraint (DOCTRINE.C1_UNIQUE) violated', + $exception->getMessage(), + ); + } } diff --git a/tests/Functional/UniqueConstraintViolationsTest.php b/tests/Functional/UniqueConstraintViolationsTest.php new file mode 100644 index 00000000000..8777b06d549 --- /dev/null +++ b/tests/Functional/UniqueConstraintViolationsTest.php @@ -0,0 +1,249 @@ +connection->getDatabasePlatform(); + + if ($platform instanceof OraclePlatform) { + $constraintName = 'C1_UNIQUE'; + } elseif ($platform instanceof PostgreSQLPlatform) { + $constraintName = 'c1_unique'; + } else { + $constraintName = 'c1_unique'; + } + + $this->constraintName = $constraintName; + + $schemaManager = $this->connection->createSchemaManager(); + + $table = new Table('unique_constraint_violations'); + $table->addColumn('unique_field', 'integer', ['notnull' => true]); + $schemaManager->createTable($table); + + if ($platform instanceof OraclePlatform) { + $createConstraint = sprintf( + 'ALTER TABLE unique_constraint_violations ' . + 'ADD CONSTRAINT %s UNIQUE (unique_field) DEFERRABLE INITIALLY IMMEDIATE', + $constraintName, + ); + } elseif ($platform instanceof PostgreSQLPlatform) { + $createConstraint = sprintf( + 'ALTER TABLE unique_constraint_violations ' . + 'ADD CONSTRAINT %s UNIQUE (unique_field) DEFERRABLE INITIALLY IMMEDIATE', + $constraintName, + ); + } elseif ($platform instanceof SQLitePlatform) { + $createConstraint = sprintf( + 'CREATE UNIQUE INDEX %s ON unique_constraint_violations(unique_field)', + $constraintName, + ); + } else { + $createConstraint = new UniqueConstraint($constraintName, ['unique_field']); + } + + if ($createConstraint instanceof UniqueConstraint) { + $schemaManager->createUniqueConstraint($createConstraint, 'unique_constraint_violations'); + } else { + $this->connection->executeStatement($createConstraint); + } + + $this->connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + } + + public function testTransactionalViolatesDeferredConstraint(): void + { + $this->skipIfDeferrableIsNotSupported(); + + $this->connection->transactional(function (Connection $connection): void { + $connection->executeStatement(sprintf('SET CONSTRAINTS "%s" DEFERRED', $this->constraintName)); + + $connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + + $this->expectUniqueConstraintViolation(); + }); + } + + public function testTransactionalViolatesConstraint(): void + { + $this->connection->transactional(function (Connection $connection): void { + $this->expectUniqueConstraintViolation(); + $connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + }); + } + + public function testTransactionalViolatesDeferredConstraintWhileUsingTransactionNesting(): void + { + if (! $this->connection->getDatabasePlatform()->supportsSavepoints()) { + self::markTestSkipped('This test requires the platform to support savepoints.'); + } + + $this->skipIfDeferrableIsNotSupported(); + + $this->connection->transactional(function (Connection $connection): void { + $connection->executeStatement(sprintf('SET CONSTRAINTS "%s" DEFERRED', $this->constraintName)); + $connection->beginTransaction(); + $connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + $connection->commit(); + + $this->expectUniqueConstraintViolation(); + }); + } + + public function testTransactionalViolatesConstraintWhileUsingTransactionNesting(): void + { + if (! $this->connection->getDatabasePlatform()->supportsSavepoints()) { + self::markTestSkipped('This test requires the platform to support savepoints.'); + } + + $this->connection->transactional(function (Connection $connection): void { + $connection->beginTransaction(); + + try { + $this->connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + } catch (Throwable $t) { + $this->connection->rollBack(); + + $this->expectUniqueConstraintViolation(); + + throw $t; + } + }); + } + + public function testCommitViolatesDeferredConstraint(): void + { + $this->skipIfDeferrableIsNotSupported(); + + $this->connection->beginTransaction(); + $this->connection->executeStatement(sprintf('SET CONSTRAINTS "%s" DEFERRED', $this->constraintName)); + $this->connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + + $this->expectUniqueConstraintViolation(); + $this->connection->commit(); + } + + public function testInsertViolatesConstraint(): void + { + $this->connection->beginTransaction(); + + try { + $this->connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + } catch (Throwable $t) { + $this->connection->rollBack(); + + $this->expectUniqueConstraintViolation(); + + throw $t; + } + } + + public function testCommitViolatesDeferredConstraintWhileUsingTransactionNesting(): void + { + if (! $this->connection->getDatabasePlatform()->supportsSavepoints()) { + self::markTestSkipped('This test requires the platform to support savepoints.'); + } + + $this->skipIfDeferrableIsNotSupported(); + + $this->connection->beginTransaction(); + $this->connection->executeStatement(sprintf('SET CONSTRAINTS "%s" DEFERRED', $this->constraintName)); + $this->connection->beginTransaction(); + $this->connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + $this->connection->commit(); + + $this->expectUniqueConstraintViolation(); + + $this->connection->commit(); + } + + public function testCommitViolatesConstraintWhileUsingTransactionNesting(): void + { + if (! $this->connection->getDatabasePlatform()->supportsSavepoints()) { + self::markTestSkipped('This test requires the platform to support savepoints.'); + } + + $this->skipIfDeferrableIsNotSupported(); + + $this->connection->beginTransaction(); + $this->connection->beginTransaction(); + + try { + $this->connection->executeStatement('INSERT INTO unique_constraint_violations VALUES (1)'); + } catch (Throwable $t) { + $this->connection->rollBack(); + + $this->expectUniqueConstraintViolation(); + + throw $t; + } + } + + private function supportsDeferrableConstraints(): bool + { + $platform = $this->connection->getDatabasePlatform(); + + return $platform instanceof OraclePlatform || $platform instanceof PostgreSQLPlatform; + } + + private function skipIfDeferrableIsNotSupported(): void + { + if ($this->supportsDeferrableConstraints()) { + return; + } + + self::markTestSkipped('Only databases supporting deferrable constraints are eligible for this test.'); + } + + private function expectUniqueConstraintViolation(): void + { + if ($this->connection->getDatabasePlatform() instanceof SQLServerPlatform) { + $this->expectExceptionMessage(sprintf("Violation of UNIQUE KEY constraint '%s'", $this->constraintName)); + + return; + } + + if ($this->connection->getDatabasePlatform() instanceof DB2Platform) { + // No concrete message is provided + $this->expectException(DriverException::class); + + return; + } + + $this->expectException(UniqueConstraintViolationException::class); + } + + protected function tearDown(): void + { + $schemaManager = $this->connection->createSchemaManager(); + $schemaManager->dropTable('unique_constraint_violations'); + + $this->markConnectionNotReusable(); + + parent::tearDown(); + } +}