Issue #3407080 by mondrake, alexpott: Leaving the savepoint in the transaction stack upon rollback is incorrect

merge-requests/4690/merge
catch 2023-12-11 09:59:38 +00:00
parent fa46ee8ff4
commit 64f1c8543f
2 changed files with 108 additions and 5 deletions

View File

@ -318,9 +318,13 @@ abstract class TransactionManagerBase implements TransactionManagerInterface {
if ($this->getConnectionTransactionState() === ClientConnectionTransactionState::Active) {
if ($this->stackDepth() > 1 && $this->stack()[$id]->type === StackItemType::Savepoint) {
// Rollback the client transaction to the savepoint when the Drupal
// transaction is not a root one. The savepoint and therefore the
// client connection remain active.
// transaction is not a root one. Then, release the savepoint too. The
// client connection remains active.
$this->rollbackClientSavepoint($name);
$this->releaseClientSavepoint($name);
// The Transaction object remains open, and when it will get destructed
// no commit should happen. Void the stack item.
$this->voidStackItem($id);
}
elseif ($this->stackDepth() === 1 && $this->stack()[$id]->type === StackItemType::Root) {
// If this was the root Drupal transaction, we can rollback the client

View File

@ -175,6 +175,50 @@ class DriverSpecificTransactionTestBase extends DriverSpecificDatabaseTestBase {
$this->assertSame(0, $this->connection->transactionManager()->stackDepth());
}
/**
* Tests root transaction rollback after savepoint rollback.
*/
public function testRollbackRootAfterSavepointRollback() {
$this->assertFalse($this->connection->inTransaction());
$this->assertSame(0, $this->connection->transactionManager()->stackDepth());
// Start root transaction. Corresponds to 'BEGIN TRANSACTION' on the
// database.
$transaction = $this->connection->startTransaction();
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(1, $this->connection->transactionManager()->stackDepth());
// Insert a single row into the testing table.
$this->insertRow('David');
$this->assertRowPresent('David');
// Starts a savepoint transaction. Corresponds to 'SAVEPOINT savepoint_1'
// on the database.
$savepoint = $this->connection->startTransaction();
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(2, $this->connection->transactionManager()->stackDepth());
// Insert a single row into the testing table.
$this->insertRow('Roger');
$this->assertRowPresent('David');
$this->assertRowPresent('Roger');
// Rollback savepoint. It should get released too. Corresponds to 'ROLLBACK
// TO savepoint_1' plus 'RELEASE savepoint_1' on the database.
$savepoint->rollBack();
$this->assertRowPresent('David');
$this->assertRowAbsent('Roger');
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(1, $this->connection->transactionManager()->stackDepth());
// Try to rollback root. No savepoint is active, this should succeed.
$transaction->rollBack();
$this->assertRowAbsent('David');
$this->assertRowAbsent('Roger');
$this->assertFalse($this->connection->inTransaction());
$this->assertSame(0, $this->connection->transactionManager()->stackDepth());
}
/**
* Tests root transaction rollback failure when savepoint is open.
*/
@ -232,13 +276,13 @@ class DriverSpecificTransactionTestBase extends DriverSpecificDatabaseTestBase {
$this->assertRowPresent('David');
$this->assertRowPresent('Roger');
// Rollback to savepoint. It should remain open. Corresponds to 'ROLLBACK
// TO savepoint_1' on the database.
// Rollback savepoint. It should get released too. Corresponds to 'ROLLBACK
// TO savepoint_1' plus 'RELEASE savepoint_1' on the database.
$savepoint->rollBack();
$this->assertRowPresent('David');
$this->assertRowAbsent('Roger');
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(2, $this->connection->transactionManager()->stackDepth());
$this->assertSame(1, $this->connection->transactionManager()->stackDepth());
// Insert a row.
$this->insertRow('Syd');
@ -252,6 +296,61 @@ class DriverSpecificTransactionTestBase extends DriverSpecificDatabaseTestBase {
$this->assertSame(0, $this->connection->transactionManager()->stackDepth());
}
/**
* Tests savepoint transaction duplicated rollback.
*/
public function testRollbackTwiceSameSavepoint() {
$this->assertFalse($this->connection->inTransaction());
$this->assertSame(0, $this->connection->transactionManager()->stackDepth());
// Start root transaction. Corresponds to 'BEGIN TRANSACTION' on the
// database.
$transaction = $this->connection->startTransaction();
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(1, $this->connection->transactionManager()->stackDepth());
// Insert a row.
$this->insertRow('David');
$this->assertRowPresent('David');
// Starts a savepoint transaction. Corresponds to 'SAVEPOINT savepoint_1'
// on the database.
$savepoint = $this->connection->startTransaction();
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(2, $this->connection->transactionManager()->stackDepth());
// Insert a row.
$this->insertRow('Roger');
$this->assertRowPresent('David');
$this->assertRowPresent('Roger');
// Rollback savepoint. It should get released too. Corresponds to 'ROLLBACK
// TO savepoint_1' plus 'RELEASE savepoint_1' on the database.
$savepoint->rollBack();
$this->assertRowPresent('David');
$this->assertRowAbsent('Roger');
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(1, $this->connection->transactionManager()->stackDepth());
// Insert a row.
$this->insertRow('Syd');
// Rollback savepoint again. Should fail since it was released already.
try {
$savepoint->rollBack();
$this->fail('Expected TransactionOutOfOrderException was not thrown');
}
catch (\Exception $e) {
$this->assertInstanceOf(TransactionOutOfOrderException::class, $e);
$this->assertMatchesRegularExpression("/^Error attempting rollback of .*\\\\savepoint_1\\. Active stack: .*\\\\drupal_transaction/", $e->getMessage());
}
$this->assertRowPresent('David');
$this->assertRowAbsent('Roger');
$this->assertRowPresent('Syd');
$this->assertTrue($this->connection->inTransaction());
$this->assertSame(1, $this->connection->transactionManager()->stackDepth());
}
/**
* Tests savepoint transaction rollback failure when later savepoints exist.
*/