diff options
| -rw-r--r-- | includes/database/database.inc | 43 | ||||
| -rw-r--r-- | includes/database/mysql/database.inc | 22 | ||||
| -rw-r--r-- | 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 e08f9074f..610861429 100644 --- a/includes/database/database.inc +++ b/includes/database/database.inc @@ -1022,7 +1022,9 @@ abstract class DatabaseConnection extends PDO { } // 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 @@ abstract class DatabaseConnection extends PDO { 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 @@ abstract class DatabaseConnection extends PDO { 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 @@ abstract class DatabaseConnection extends PDO { } else { $this->query('RELEASE SAVEPOINT ' . $name); - break; } } } @@ -1745,6 +1769,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. * * There are many ways that a merge query could be malformed. They should all @@ -1839,7 +1868,7 @@ class DatabaseTransaction { 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 157cbfa56..0d9158789 100644 --- a/includes/database/mysql/database.inc +++ b/includes/database/mysql/database.inc @@ -137,21 +137,16 @@ class DatabaseConnection_mysql extends DatabaseConnection { /** * 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 @@ class DatabaseConnection_mysql extends DatabaseConnection { 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 143640d60..e27693c97 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'); + } } |