#108818 by David Strauss, chx, Crell: Add transactions to key X_save() routines.

4 files changed, 155 insertions, 91 deletions

diff --git a/modules/node/node.module b/modules/node/node.module
index 4b036225b..e3775ca64 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -933,108 +933,115 @@ function node_submit($node) {
  *   omitted (or $node->is_new is TRUE), a new node will be added.
  */
 function node_save($node) {
-  field_attach_presave('node', $node);
-  // Let modules modify the node before it is saved to the database.
-  module_invoke_all('node_presave', $node);
-  global $user;
+  $transaction = db_transaction();
 
-  if (!isset($node->is_new)) {
-    $node->is_new = empty($node->nid);
-  }
+  try {
+    field_attach_presave('node', $node);
+    // Let modules modify the node before it is saved to the database.
+    module_invoke_all('node_presave', $node);
+    global $user;
 
-  // Apply filters to some default node fields:
-  if ($node->is_new) {
-    // Insert a new node.
-    $node->is_new = TRUE;
+    if (!isset($node->is_new)) {
+      $node->is_new = empty($node->nid);
+    }
 
-    // When inserting a node, $node->log must be set because
-    // {node_revision}.log does not (and cannot) have a default
-    // value. If the user does not have permission to create
-    // revisions, however, the form will not contain an element for
-    // log so $node->log will be unset at this point.
-    if (!isset($node->log)) {
-      $node->log = '';
+    // Apply filters to some default node fields:
+    if ($node->is_new) {
+      // Insert a new node.
+      $node->is_new = TRUE;
+
+      // When inserting a node, $node->log must be set because
+      // {node_revision}.log does not (and cannot) have a default
+      // value. If the user does not have permission to create
+      // revisions, however, the form will not contain an element for
+      // log so $node->log will be unset at this point.
+      if (!isset($node->log)) {
+        $node->log = '';
+      }
     }
-  }
-  elseif (!empty($node->revision)) {
-    $node->old_vid = $node->vid;
-    unset($node->vid);
-  }
-  else {
-    // When updating a node, avoid clobbering an existing log entry with an empty one.
-    if (empty($node->log)) {
-      unset($node->log);
+    elseif (!empty($node->revision)) {
+      $node->old_vid = $node->vid;
+      unset($node->vid);
+    }
+    else {
+      // When updating a node, avoid clobbering an existing log entry with an empty one.
+      if (empty($node->log)) {
+        unset($node->log);
+      }
     }
-  }
-
-  // Set some required fields:
-  if (empty($node->created)) {
-    $node->created = REQUEST_TIME;
-  }
-  // The changed timestamp is always updated for bookkeeping purposes (revisions, searching, ...)
-  $node->changed = REQUEST_TIME;
-
-  $node->timestamp = REQUEST_TIME;
-  $update_node = TRUE;
-
-  // When converting the title property to fields we preserved the {node}.title
-  // db column for performance, setting the default language value into this
-  // column. After this we restore the field data structure to the previous node
-  // title field.
-  $title_field = $node->title;
-  $langcode = FIELD_LANGUAGE_NONE;
-  $node->title = $title_field[$langcode][0]['value'];
 
-  // Generate the node table query and the node_revisions table query.
-  if ($node->is_new) {
-    drupal_write_record('node', $node);
-    _node_save_revision($node, $user->uid);
-    $op = 'insert';
-  }
-  else {
-    drupal_write_record('node', $node, 'nid');
-    if (!empty($node->revision)) {
+    // Set some required fields:
+    if (empty($node->created)) {
+      $node->created = REQUEST_TIME;
+    }
+    // The changed timestamp is always updated for bookkeeping purposes (revisions, searching, ...)
+    $node->changed = REQUEST_TIME;
+  
+    $node->timestamp = REQUEST_TIME;
+    $update_node = TRUE;
+
+    // When converting the title property to fields we preserved the {node}.title
+    // db column for performance, setting the default language value into this
+    // column. After this we restore the field data structure to the previous node
+    // title field.
+    $title_field = $node->title;
+    $langcode = FIELD_LANGUAGE_NONE;
+    $node->title = $title_field[$langcode][0]['value'];
+
+    // Generate the node table query and the node_revisions table query.
+    if ($node->is_new) {
+      drupal_write_record('node', $node);
       _node_save_revision($node, $user->uid);
+      $op = 'insert';
     }
     else {
-      _node_save_revision($node, $user->uid, 'vid');
-      $update_node = FALSE;
+      drupal_write_record('node', $node, 'nid');
+      if (!empty($node->revision)) {
+        _node_save_revision($node, $user->uid);
+      }
+      else {
+        _node_save_revision($node, $user->uid, 'vid');
+        $update_node = FALSE;
+      }
+      $op = 'update';
     }
-    $op = 'update';
+    if ($update_node) {
+      db_update('node')
+        ->fields(array('vid' => $node->vid))
+        ->condition('nid', $node->nid)
+        ->execute();
+    }
+  
+    // Restore the title field data structure after db storage.
+    $node->title = $title_field;
+  
+    // Call the node specific callback (if any). This can be
+    // node_invoke($node, 'insert') or
+    // node_invoke($node, 'update').
+    node_invoke($node, $op);
+  
+    // Save fields.
+    $function = "field_attach_$op";
+    $function('node', $node);
+  
+    module_invoke_all('node_' . $op, $node);
+  
+    // Update the node access table for this node.
+    node_access_acquire_grants($node);
+  
+    // Clear internal properties.
+    unset($node->is_new);
+  
+    // Clear the page and block caches.
+    cache_clear_all();
+  
+    // Ignore slave server temporarily to give time for the
+    // saved node to be propagated to the slave.
+    db_ignore_slave();
   }
-  if ($update_node) {
-    db_update('node')
-      ->fields(array('vid' => $node->vid))
-      ->condition('nid', $node->nid)
-      ->execute();
+  catch (Exception $e) {
+    $transaction->rollback('node', $e->getMessage(), array(), WATCHDOG_ERROR);
   }
-
-  // Restore the title field data structure after db storage.
-  $node->title = $title_field;
-
-  // Call the node specific callback (if any). This can be
-  // node_invoke($node, 'insert') or
-  // node_invoke($node, 'update').
-  node_invoke($node, $op);
-
-  // Save fields.
-  $function = "field_attach_$op";
-  $function('node', $node);
-
-  module_invoke_all('node_' . $op, $node);
-
-  // Update the node access table for this node.
-  node_access_acquire_grants($node);
-
-  // Clear internal properties.
-  unset($node->is_new);
-
-  // Clear the page and block caches.
-  cache_clear_all();
-
-  // Ignore slave server temporarily to give time for the
-  // saved node to be propagated to the slave.
-  db_ignore_slave();
 }
 
 /**
diff --git a/modules/node/node.test b/modules/node/node.test
index 807dc5f65..2fd36a97c 100644
--- a/modules/node/node.test
+++ b/modules/node/node.test
@@ -329,7 +329,8 @@ class PageCreationTestCase extends DrupalWebTestCase {
   }
 
   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);
@@ -353,6 +354,37 @@ class PageCreationTestCase extends DrupalWebTestCase {
     $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 {
diff --git a/modules/node/tests/node_test_exception.info b/modules/node/tests/node_test_exception.info
new file mode 100644
index 000000000..afe5b3719
--- /dev/null
+++ b/modules/node/tests/node_test_exception.info
@@ -0,0 +1,8 @@
+; $Id$
+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
diff --git a/modules/node/tests/node_test_exception.module b/modules/node/tests/node_test_exception.module
new file mode 100644
index 000000000..7b09c77c7
--- /dev/null
+++ b/modules/node/tests/node_test_exception.module
@@ -0,0 +1,17 @@
+<?php
+// $Id$
+
+/**
+ * @file
+ * Dummy module implementing node related hooks to test API interaction with
+ * the Node module.
+ */
+
+/**
+ * Implement hook_node_insert().
+ */
+function node_test_exception_node_insert($node) {
+  if ($node->title['zxx'][0]['value'] == 'testing_transaction_exception') {
+    throw new Exception('Test exception for rollback.');
+  }
+}