From d902c2f214e88ad53cdc2985f533c85aa98db39b Mon Sep 17 00:00:00 2001
From: Dries Buytaert <dries@buytaert.net>
Date: Mon, 1 Aug 2011 20:04:11 -0400
Subject: [PATCH] - Patch #1185780 by Damien Tournoud: make transactions more
flexible and useful.
---
includes/database/database.inc | 43 +++++-
includes/database/mysql/database.inc | 22 +--
modules/simpletest/tests/database_test.test | 159 +++++++++++++++++++-
3 files changed, 202 insertions(+), 22 deletions(-)
diff --git a/includes/database/database.inc b/includes/database/database.inc
index e08f9074f396..610861429d1b 100644
--- a/includes/database/database.inc
+++ b/includes/database/database.inc
@@ -1022,7 +1022,9 @@ public function rollback($savepoint_name = 'drupal_transaction') {
}
// We need to find the point we're rolling back to, all other savepoints
- // before are no longer needed.
+ // before are no longer needed. If we rolled back other active savepoints,
+ // we need to throw an exception.
+ $rolled_back_other_active_savepoints = FALSE;
while ($savepoint = array_pop($this->transactionLayers)) {
if ($savepoint == $savepoint_name) {
// If it is the last the transaction in the stack, then it is not a
@@ -1032,10 +1034,20 @@ public function rollback($savepoint_name = 'drupal_transaction') {
break;
}
$this->query('ROLLBACK TO SAVEPOINT ' . $savepoint);
+ $this->popCommittableTransactions();
+ if ($rolled_back_other_active_savepoints) {
+ throw new DatabaseTransactionOutOfOrderException();
+ }
return;
}
+ else {
+ $rolled_back_other_active_savepoints = TRUE;
+ }
}
parent::rollBack();
+ if ($rolled_back_other_active_savepoints) {
+ throw new DatabaseTransactionOutOfOrderException();
+ }
}
/**
@@ -1084,15 +1096,28 @@ public function popTransaction($name) {
if (!$this->supportsTransactions()) {
return;
}
- if (!$this->inTransaction()) {
+ if (!isset($this->transactionLayers[$name])) {
throw new DatabaseTransactionNoActiveException();
}
- // Commit everything since SAVEPOINT $name.
- while($savepoint = array_pop($this->transactionLayers)) {
- if ($savepoint != $name) continue;
+ // Mark this layer as committable.
+ $this->transactionLayers[$name] = FALSE;
+ $this->popCommittableTransactions();
+ }
+
+ /**
+ * Internal function: commit all the transaction layers that can commit.
+ */
+ protected function popCommittableTransactions() {
+ // Commit all the committable layers.
+ foreach (array_reverse($this->transactionLayers) as $name => $active) {
+ // Stop once we found an active transaction.
+ if ($active) {
+ break;
+ }
// If there are no more layers left then we should commit.
+ unset($this->transactionLayers[$name]);
if (empty($this->transactionLayers)) {
if (!parent::commit()) {
throw new DatabaseTransactionCommitFailedException();
@@ -1100,7 +1125,6 @@ public function popTransaction($name) {
}
else {
$this->query('RELEASE SAVEPOINT ' . $name);
- break;
}
}
}
@@ -1744,6 +1768,11 @@ class DatabaseTransactionCommitFailedException extends Exception { }
*/
class DatabaseTransactionExplicitCommitNotAllowedException extends Exception { }
+/**
+ * Exception thrown when a rollback() resulted in other active transactions being rolled-back.
+ */
+class DatabaseTransactionOutOfOrderException extends Exception { }
+
/**
* Exception thrown for merge queries that do not make semantic sense.
*
@@ -1839,7 +1868,7 @@ public function __construct(DatabaseConnection &$connection, $name = NULL) {
public function __destruct() {
// If we rolled back then the transaction would have already been popped.
- if ($this->connection->inTransaction() && !$this->rolledBack) {
+ if (!$this->rolledBack) {
$this->connection->popTransaction($this->name);
}
}
diff --git a/includes/database/mysql/database.inc b/includes/database/mysql/database.inc
index 157cbfa567eb..0d9158789bba 100644
--- a/includes/database/mysql/database.inc
+++ b/includes/database/mysql/database.inc
@@ -137,21 +137,16 @@ public function nextIdDelete() {
/**
* Overridden to work around issues to MySQL not supporting transactional DDL.
*/
- public function popTransaction($name) {
- if (!$this->supportsTransactions()) {
- return;
- }
- if (!$this->inTransaction()) {
- throw new DatabaseTransactionNoActiveException();
- }
-
- // Commit everything since SAVEPOINT $name.
- while ($savepoint = array_pop($this->transactionLayers)) {
- if ($savepoint != $name) {
- continue;
+ protected function popCommittableTransactions() {
+ // Commit all the committable layers.
+ foreach (array_reverse($this->transactionLayers) as $name => $active) {
+ // Stop once we found an active transaction.
+ if ($active) {
+ break;
}
// If there are no more layers left then we should commit.
+ unset($this->transactionLayers[$name]);
if (empty($this->transactionLayers)) {
if (!PDO::commit()) {
throw new DatabaseTransactionCommitFailedException();
@@ -173,13 +168,12 @@ public function popTransaction($name) {
if ($e->errorInfo[1] == '1305') {
// If one SAVEPOINT was released automatically, then all were.
// Therefore, we keep just the topmost transaction.
- $this->transactionLayers = array('drupal_transaction');
+ $this->transactionLayers = array('drupal_transaction' => 'drupal_transaction');
}
else {
throw $e;
}
}
- break;
}
}
}
diff --git a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test
index 143640d60b39..e27693c97539 100644
--- a/modules/simpletest/tests/database_test.test
+++ b/modules/simpletest/tests/database_test.test
@@ -3433,7 +3433,7 @@ class DatabaseTransactionTestCase extends DatabaseTestCase {
$this->assertIdentical($count, '0', t('Table was successfully created inside a transaction.'));
}
catch (Exception $e) {
- $this->fail($e->getMessage());
+ $this->fail((string) $e);
}
// If we rollback the transaction, an exception might be thrown.
@@ -3451,6 +3451,163 @@ class DatabaseTransactionTestCase extends DatabaseTestCase {
$this->assertTrue(true, t('Exception thrown on rollback after a DDL statement was executed.'));
}
}
+
+ /**
+ * Insert a single row into the testing table.
+ */
+ protected function insertRow($name) {
+ db_insert('test')
+ ->fields(array(
+ 'name' => $name,
+ ))
+ ->execute();
+ }
+
+ /**
+ * Start over for a new test.
+ */
+ protected function cleanUp() {
+ db_truncate('test')
+ ->execute();
+ }
+
+ /**
+ * Assert that a given row is present in the test table.
+ *
+ * @param $name
+ * The name of the row.
+ * @param $message
+ * The message to log for the assertion.
+ */
+ function assertRowPresent($name, $message = NULL) {
+ if (!isset($message)) {
+ $message = t('Row %name is present.', array('%name' => $name));
+ }
+ $present = (boolean) db_query('SELECT 1 FROM {test} WHERE name = :name', array(':name' => $name))->fetchField();
+ return $this->assertTrue($present, $message);
+ }
+
+ /**
+ * Assert that a given row is absent from the test table.
+ *
+ * @param $name
+ * The name of the row.
+ * @param $message
+ * The message to log for the assertion.
+ */
+ function assertRowAbsent($name, $message = NULL) {
+ if (!isset($message)) {
+ $message = t('Row %name is absent.', array('%name' => $name));
+ }
+ $present = (boolean) db_query('SELECT 1 FROM {test} WHERE name = :name', array(':name' => $name))->fetchField();
+ return $this->assertFalse($present, $message);
+ }
+
+ /**
+ * Test transaction stacking and commit / rollback.
+ */
+ function testTransactionStacking() {
+ // This test won't work right if transactions are supported.
+ if (Database::getConnection()->supportsTransactions()) {
+ return;
+ }
+
+ $database = Database::getConnection();
+
+ // Standard case: pop the inner transaction before the outer transaction.
+ $transaction = db_transaction();
+ $this->insertRow('outer');
+ $transaction2 = db_transaction();
+ $this->insertRow('inner');
+ // Pop the inner transaction.
+ unset($transaction2);
+ $this->assertTrue($database->inTransaction(), t('Still in a transaction after popping the inner transaction'));
+ // Pop the outer transaction.
+ unset($transaction);
+ $this->assertFalse($database->inTransaction(), t('Transaction closed after popping the outer transaction'));
+ $this->assertRowPresent('outer');
+ $this->assertRowPresent('inner');
+
+ // Pop the transaction in a different order they have been pushed.
+ $this->cleanUp();
+ $transaction = db_transaction();
+ $this->insertRow('outer');
+ $transaction2 = db_transaction();
+ $this->insertRow('inner');
+ // Pop the outer transaction, nothing should happen.
+ unset($transaction);
+ $this->insertRow('inner-after-outer-commit');
+ $this->assertTrue($database->inTransaction(), t('Still in a transaction after popping the outer transaction'));
+ // Pop the inner transaction, the whole transaction should commit.
+ unset($transaction2);
+ $this->assertFalse($database->inTransaction(), t('Transaction closed after popping the inner transaction'));
+ $this->assertRowPresent('outer');
+ $this->assertRowPresent('inner');
+ $this->assertRowPresent('inner-after-outer-commit');
+
+ // Rollback the inner transaction.
+ $this->cleanUp();
+ $transaction = db_transaction();
+ $this->insertRow('outer');
+ $transaction2 = db_transaction();
+ $this->insertRow('inner');
+ // Now rollback the inner transaction.
+ $transaction2->rollback();
+ unset($transaction2);
+ $this->assertTrue($database->inTransaction(), t('Still in a transaction after popping the outer transaction'));
+ // Pop the outer transaction, it should commit.
+ $this->insertRow('outer-after-inner-rollback');
+ unset($transaction);
+ $this->assertFalse($database->inTransaction(), t('Transaction closed after popping the inner transaction'));
+ $this->assertRowPresent('outer');
+ $this->assertRowAbsent('inner');
+ $this->assertRowPresent('outer-after-inner-rollback');
+
+ // Rollback the inner transaction after committing the outer one.
+ $this->cleanUp();
+ $transaction = db_transaction();
+ $this->insertRow('outer');
+ $transaction2 = db_transaction();
+ $this->insertRow('inner');
+ // Pop the outer transaction, nothing should happen.
+ unset($transaction);
+ $this->assertTrue($database->inTransaction(), t('Still in a transaction after popping the outer transaction'));
+ // Now rollback the inner transaction, it should rollback.
+ $transaction2->rollback();
+ unset($transaction2);
+ $this->assertFalse($database->inTransaction(), t('Transaction closed after popping the inner transaction'));
+ $this->assertRowPresent('outer');
+ $this->assertRowAbsent('inner');
+
+ // Rollback the outer transaction while the inner transaction is active.
+ // In that case, an exception will be triggered because we cannot
+ // ensure that the final result will have any meaning.
+ $this->cleanUp();
+ $transaction = db_transaction();
+ $this->insertRow('outer');
+ $transaction2 = db_transaction();
+ $this->insertRow('inner');
+ // Rollback the outer transaction.
+ try {
+ $transaction->rollback();
+ unset($transaction);
+ $this->fail(t('Rolling back the outer transaction while the inner transaction is active resulted in an exception.'));
+ }
+ catch (Exception $e) {
+ $this->pass(t('Rolling back the outer transaction while the inner transaction is active resulted in an exception.'));
+ }
+ $this->assertFalse($database->inTransaction(), t('No more in a transaction after rolling back the outer transaction'));
+ // Try to commit the inner transaction.
+ try {
+ unset($transaction2);
+ $this->fail(t('Trying to commit the inner transaction resulted in an exception.'));
+ }
+ catch (Exception $e) {
+ $this->pass(t('Trying to commit the inner transaction resulted in an exception.'));
+ }
+ $this->assertRowAbsent('outer');
+ $this->assertRowAbsent('inner');
+ }
}
--
GitLab