=== modified file 'includes/bootstrap.inc' --- includes/bootstrap.inc 2009-10-09 16:33:13 +0000 +++ includes/bootstrap.inc 2009-10-16 01:26:42 +0000 @@ -1198,6 +1198,8 @@ function request_uri() { * * @see watchdog_severity_levels() * @see hook_watchdog() + * @see DatabaseConnection::rollback() + * @see DatabaseTransaction::rollback() */ function watchdog($type, $message, $variables = array(), $severity = WATCHDOG_NOTICE, $link = NULL) { global $user, $base_root; @@ -1497,6 +1499,10 @@ function _drupal_bootstrap($phase) { // Initialize the database system. Note that the connection // won't be initialized until it is actually requested. require_once DRUPAL_ROOT . '/includes/database/database.inc'; + + // Set Drupal's watchdog as the logging callback. + Database::setLoggingCallback('watchdog', WATCHDOG_NOTICE, WATCHDOG_ERROR); + // Register autoload functions so that we can access classes and interfaces. spl_autoload_register('drupal_autoload_class'); spl_autoload_register('drupal_autoload_interface'); === modified file 'includes/database/database.inc' --- includes/database/database.inc 2009-10-12 02:00:04 +0000 +++ includes/database/database.inc 2009-10-16 02:33:17 +0000 @@ -229,6 +229,13 @@ abstract class DatabaseConnection extend protected $willRollback; /** + * Array of argument arrays for logging post-rollback. + * + * @var array + */ + protected $rollbackLogs = array(); + + /** * The name of the Select class for this connection. * * Normally this and the following class names would be static variables, @@ -849,12 +856,53 @@ abstract class DatabaseConnection extend * Schedule the current transaction for rollback. * * This method throws an exception if no transaction is active. + * + * @param $type + * The category to which the rollback message belongs. + * @param $message + * The message to store in the log. Keep $message translatable + * by not concatenating dynamic values into it! Variables in the + * message should be added by using placeholder strings alongside + * the variables argument to declare the value of the placeholders. + * @param $variables + * Array of variables to replace in the message on display or + * NULL if message is already translated or not possible to + * translate. + * @param $severity + * The severity of the message, as per RFC 3164. + * @param $link + * A link to associate with the message. + * + * @see DatabaseTransaction::rollback() + * @see watchdog() */ - public function rollback() { + public function rollback($type = NULL, $message = NULL, $variables = array(), $severity = NULL, $link = NULL) { if ($this->transactionLayers == 0) { throw new NoActiveTransactionException(); } + // Set the severity to the configured default if not specified. + if (!isset($severity)) { + $logging = Database::getLoggingCallback(); + if (is_array($logging)) { + $severity = $logging['default_severity']; + } + } + + // Record in an array to send to the log after transaction rollback. Messages written + // directly to a log (with a database back-end) will roll back during the following + // transaction rollback. This is an array because rollback could be requested multiple + // times during a transaction, and all such errors ought to be logged. + if (isset($message)) { + $this->rollbackLogs[] = array( + 'type' => $type, + 'message' => $message, + 'variables' => $variables, + 'severity' => $severity, + 'link' => $link, + ); + } + $this->willRollback = TRUE; } @@ -890,9 +938,6 @@ abstract class DatabaseConnection extend if ($this->supportsTransactions()) { parent::beginTransaction(); } - - // Reset any scheduled rollback - $this->willRollback = FALSE; } } @@ -912,11 +957,41 @@ abstract class DatabaseConnection extend --$this->transactionLayers; - if ($this->transactionLayers == 0 && $this->supportsTransactions()) { + if ($this->transactionLayers == 0) { if ($this->willRollback) { + $logging = Database::getLoggingCallback(); + $logging_callback = NULL; + if (is_array($logging)) { + $logging_callback = $logging['callback']; + } + + if ($this->supportsTransactions()) { parent::rollBack(); } else { + if (isset($logging_callback)) { + // Log the failed rollback. + $logging_callback('database', 'Explicit rollback failed: not supported on active connection.', array(), $logging['error_severity']); + } + + // It would be nice to throw an exception here if logging failed, + // but throwing exceptions in destructors is not supported. + } + + if (isset($logging_callback)) { + // Play back the logged errors to the specified logging callback post-rollback. + foreach ($this->rollbackLogs as $log_item) { + $logging_callback($log_item['type'], $log_item['message'], $log_item['variables'], $log_item['severity'], $log_item['link']); + } + } + + // Reset any scheduled rollback. + $this->willRollback = FALSE; + + // Reset the error logs. + $this->rollbackLogs = array(); + } + elseif ($this->supportsTransactions()) { parent::commit(); } } @@ -1126,6 +1201,17 @@ abstract class Database { static protected $logs = array(); /** + * A logging function callback array. + * + * The function must accept the same function signature as Drupal's watchdog(). + * The array containst key/value pairs for callback (string), default_severity (int), + * and error_severity (int). + * + * @var string + */ + static protected $logging_callback = NULL; + + /** * Start logging a given logging key on the specified connection. * * @see DatabaseLog @@ -1156,6 +1242,37 @@ abstract class Database { } /** + * Set a logging callback for notices and errors. + * + * @see watchdog() + * @param $logging_callback + * The function to use as the logging callback. + * @param $logging_default_severity + * The default severity level to use for logged messages. + * @param $logging_error_severity + * The severity level to use for logging error messages. + */ + final public static function setLoggingCallback($callback, $default_severity, $error_severity) { + self::$logging_callback = array( + 'callback' => $callback, + 'default_severity' => $default_severity, + 'error_severity' => $error_severity, + ); + } + + /** + * Get the logging callback for notices and errors. + * + * @return + * An array with the logging callback and severity levels. + * + * @see watchdog() + */ + final public static function getLoggingCallback() { + return self::$logging_callback; + } + + /** * Retrieve the queries logged on for given logging key. * * This method also ends logging for the specified key. To get the query log @@ -1504,9 +1621,34 @@ class DatabaseTransaction { * * This is just a wrapper method to rollback whatever transaction stack we * are currently in, which is managed by the connection object itself. - */ - public function rollback() { - $this->connection->rollback(); + * + * @param $type + * The category to which the rollback message belongs. + * @param $message + * The message to store in the log. Keep $message translatable + * by not concatenating dynamic values into it! Variables in the + * message should be added by using placeholder strings alongside + * the variables argument to declare the value of the placeholders. + * @param $variables + * Array of variables to replace in the message on display or + * NULL if message is already translated or not possible to + * translate. + * @param $severity + * The severity of the message, as per RFC 3164. + * @param $link + * A link to associate with the message. + * + * @see DatabaseConnection::rollback() + * @see watchdog() + */ + public function rollback($type = NULL, $message = NULL, $variables = array(), $severity = NULL, $link = NULL) { + if (!isset($severity)) { + $logging = Database::getLoggingCallback(); + if (is_array($logging)) { + $severity = $logging['default_severity']; + } + } + $this->connection->rollback($type, $message, $variables, $severity, $link); } /** === modified file 'modules/block/block.admin.inc' --- modules/block/block.admin.inc 2009-10-14 02:13:14 +0000 +++ modules/block/block.admin.inc 2009-10-16 01:04:47 +0000 @@ -112,6 +112,8 @@ function block_admin_display_form($form, * Process main blocks administration form submissions. */ function block_admin_display_form_submit($form, &$form_state) { + $txn = db_transaction(); + foreach ($form_state['values'] as $block) { $block['status'] = (int) ($block['region'] != BLOCK_REGION_NONE); $block['region'] = $block['status'] ? $block['region'] : ''; @@ -364,6 +366,8 @@ function block_admin_configure_validate( function block_admin_configure_submit($form, &$form_state) { if (!form_get_errors()) { + $txn = db_transaction(); + db_update('block') ->fields(array( 'visibility' => (int) $form_state['values']['visibility'], === modified file 'modules/comment/comment.module' --- modules/comment/comment.module 2009-10-15 16:18:44 +0000 +++ modules/comment/comment.module 2009-10-16 01:04:47 +0000 @@ -1248,6 +1248,9 @@ function comment_access($op, $comment) { * A comment object. */ function comment_save($comment) { + $transaction = db_transaction(); + + try { global $user; $defaults = array( @@ -1386,6 +1389,10 @@ function comment_save($comment) { if ($comment->status == COMMENT_PUBLISHED) { module_invoke_all('comment_publish', $comment); } + } + catch (Exception $e) { + $transaction->rollback('comment', $e->getMessage(), array(), WATCHDOG_ERROR); + } } /** === modified file 'modules/node/node.module' --- modules/node/node.module 2009-10-15 12:44:34 +0000 +++ modules/node/node.module 2009-10-16 01:04:48 +0000 @@ -888,6 +888,9 @@ function node_submit($node) { * omitted (or $node->is_new is TRUE), a new node will be added. */ function node_save($node) { + $transaction = db_transaction(); + + try { field_attach_presave('node', $node); // Let modules modify the node before it is saved to the database. module_invoke_all('node_presave', $node); @@ -990,6 +993,10 @@ function node_save($node) { // Ignore slave server temporarily to give time for the // saved node to be propagated to the slave. db_ignore_slave(); + } + catch (Exception $e) { + $transaction->rollback('node', $e->getMessage(), array(), WATCHDOG_ERROR); + } } /** === modified file 'modules/node/node.test' --- modules/node/node.test 2009-10-15 12:44:34 +0000 +++ modules/node/node.test 2009-10-16 01:04:48 +0000 @@ -327,7 +327,8 @@ class PageCreationTestCase extends Drupa } function setUp() { - parent::setUp(); + // Enable dummy module that implements hook_node_post_save for exceptions. + parent::setUp('node_test_exception'); $web_user = $this->drupalCreateUser(array('create page content', 'edit own page content')); $this->drupalLogin($web_user); @@ -351,6 +352,37 @@ class PageCreationTestCase extends Drupa $node = $this->drupalGetNodeByTitle($edit["title[$langcode][0][value]"]); $this->assertTrue($node, t('Node found in database.')); } + + /** + * Create a page node and verify that a transaction rolls back the failed creation + */ + function testFailedPageCreation() { + // Create a node. + $edit = array(); + $langcode = FIELD_LANGUAGE_NONE; + $edit["title[$langcode][0][value]"] = 'testing_transaction_exception'; + $edit["body[$langcode][0][value]"] = $this->randomName(16); + $this->drupalPost('node/add/page', $edit, t('Save')); + + if (Database::getConnection()->supportsTransactions()) { + // Check that the node does not exist in the database. + $node = $this->drupalGetNodeByTitle($edit["title[$langcode][0][value]"]); + $this->assertFalse($node, t('Transactions supported, and node not found in database.')); + } + else { + // Check that the node exists in the database. + $node = $this->drupalGetNodeByTitle($edit["title[$langcode][0][value]"]); + $this->assertTrue($node, t('Transactions not supported, and node found in database.')); + + // Check that the failed rollback was logged. + $records = db_query("SELECT wid FROM {watchdog} WHERE message LIKE 'Explicit rollback failed%'")->fetchAll(); + $this->assertTrue(count($records) > 0, t('Transactions not supported, and rollback error logged to watchdog.')); + } + + // Check that the rollback error was logged. + $records = db_query("SELECT wid FROM {watchdog} WHERE message LIKE 'Test exception for rollback.'")->fetchAll(); + $this->assertTrue(count($records) > 0, t('Rollback explanatory error logged to watchdog.')); + } } class PageViewTestCase extends DrupalWebTestCase { === added file 'modules/node/tests/node_test_exception.info' --- modules/node/tests/node_test_exception.info 1970-01-01 00:00:00 +0000 +++ modules/node/tests/node_test_exception.info 2009-10-14 02:20:13 +0000 @@ -0,0 +1,8 @@ +; $Id: node_test.info,v 1.1 2009/04/02 20:47:54 dries Exp $ +name = "Node module exception tests" +description = "Support module for node related exception testing." +package = Testing +version = VERSION +core = 7.x +files[] = node_test_exception.module +hidden = TRUE === added file 'modules/node/tests/node_test_exception.module' --- modules/node/tests/node_test_exception.module 1970-01-01 00:00:00 +0000 +++ modules/node/tests/node_test_exception.module 2009-10-14 04:07:52 +0000 @@ -0,0 +1,17 @@ +title['zxx'][0]['value'] == 'testing_transaction_exception') { + throw new Exception('Test exception for rollback.'); + } +} === modified file 'modules/user/user.module' --- modules/user/user.module 2009-10-15 11:47:25 +0000 +++ modules/user/user.module 2009-10-16 02:37:21 +0000 @@ -304,6 +304,9 @@ function user_load_by_name($name) { * A fully-loaded $user object upon successful save or FALSE if the save failed. */ function user_save($account, $edit = array(), $category = 'account') { + $transaction = db_transaction(); + + try { $table = drupal_get_schema('users'); $user_fields = $table['fields']; @@ -512,6 +515,10 @@ function user_save($account, $edit = arr } return $user; + } + catch (Exception $e) { + $transaction->rollback('user', $e->getMessage(), array(), WATCHDOG_ERROR); + } } /**