#923826 by catch, carlos8f, moshe weitzman: Fixed entity delete operations should use transactions.

5 files changed, 141 insertions, 104 deletions

diff --git a/modules/comment/comment.module b/modules/comment/comment.module
index 622433e24..5334c2728 100644
--- a/modules/comment/comment.module
+++ b/modules/comment/comment.module
@@ -1583,20 +1583,27 @@ function comment_delete($cid) {
 function comment_delete_multiple($cids) {
   $comments = comment_load_multiple($cids);
   if ($comments) {
-
-    // Delete the comments.
-    db_delete('comment')
-      ->condition('cid', array_keys($comments), 'IN')
-      ->execute();
-    foreach ($comments as $comment) {
-      field_attach_delete('comment', $comment);
-      module_invoke_all('comment_delete', $comment);
-      module_invoke_all('entity_delete', $comment, 'comment');
-
-      // Delete the comment's replies.
-      $child_cids = db_query('SELECT cid FROM {comment} WHERE pid = :cid', array(':cid' => $comment->cid))->fetchCol();
-      comment_delete_multiple($child_cids);
-      _comment_update_node_statistics($comment->nid);
+    $transaction = db_transaction();
+    try {
+      // Delete the comments.
+      db_delete('comment')
+        ->condition('cid', array_keys($comments), 'IN')
+        ->execute();
+      foreach ($comments as $comment) {
+        field_attach_delete('comment', $comment);
+        module_invoke_all('comment_delete', $comment);
+        module_invoke_all('entity_delete', $comment, 'comment');
+
+        // Delete the comment's replies.
+        $child_cids = db_query('SELECT cid FROM {comment} WHERE pid = :cid', array(':cid' => $comment->cid))->fetchCol();
+        comment_delete_multiple($child_cids);
+        _comment_update_node_statistics($comment->nid);
+      }
+    }
+    catch (Exception $e) {
+      $transaction->rollback();
+      watchdog_exception('comment', $e);
+      throw $e;
     }
   }
 }
diff --git a/modules/node/node.module b/modules/node/node.module
index 252d72278..337c4098d 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -1123,7 +1123,7 @@ function node_save($node) {
     db_ignore_slave();
   }
   catch (Exception $e) {
-    $transaction->rollback('node');
+    $transaction->rollback();
     watchdog_exception('node', $e);
     throw $e;
   }
@@ -1164,41 +1164,48 @@ function node_delete($nid) {
  *   An array of node IDs.
  */
 function node_delete_multiple($nids) {
+  $transaction = db_transaction();
   if (!empty($nids)) {
     $nodes = node_load_multiple($nids, array());
 
-    foreach ($nodes as $nid => $node) {
-      // Call the node-specific callback (if any):
-      node_invoke($node, 'delete');
-      module_invoke_all('node_delete', $node);
-      module_invoke_all('entity_delete', $node, 'node');
-      field_attach_delete('node', $node);
-
-      // Remove this node from the search index if needed.
-      // This code is implemented in node module rather than in search module,
-      // because node module is implementing search module's API, not the other
-      // way around.
-      if (module_exists('search')) {
-        search_reindex($nid, 'node');
+    try {
+      foreach ($nodes as $nid => $node) {
+        // Call the node-specific callback (if any):
+        node_invoke($node, 'delete');
+        module_invoke_all('node_delete', $node);
+        module_invoke_all('entity_delete', $node, 'node');
+        field_attach_delete('node', $node);
+
+        // Remove this node from the search index if needed.
+        // This code is implemented in node module rather than in search module,
+        // because node module is implementing search module's API, not the other
+        // way around.
+        if (module_exists('search')) {
+          search_reindex($nid, 'node');
+        }
       }
-    }
 
-    // Delete after calling hooks so that they can query node tables as needed.
-    db_delete('node')
-      ->condition('nid', $nids, 'IN')
-      ->execute();
-    db_delete('node_revision')
-      ->condition('nid', $nids, 'IN')
-      ->execute();
-    db_delete('history')
-      ->condition('nid', $nids, 'IN')
-      ->execute();
-    db_delete('node_access')
-      ->condition('nid', $nids, 'IN')
-      ->execute();
+      // Delete after calling hooks so that they can query node tables as needed.
+      db_delete('node')
+        ->condition('nid', $nids, 'IN')
+        ->execute();
+      db_delete('node_revision')
+        ->condition('nid', $nids, 'IN')
+        ->execute();
+      db_delete('history')
+        ->condition('nid', $nids, 'IN')
+        ->execute();
+      db_delete('node_access')
+       ->condition('nid', $nids, 'IN')
+       ->execute();
+    }
+    catch (Exception $e) {
+      $transaction->rollback();
+      watchdog_exception('node', $e);
+      throw $e;
+    }
 
     // Clear the page and block and node_load_multiple caches.
-    cache_clear_all();
     entity_get_controller('node')->resetCache();
   }
 }
diff --git a/modules/taxonomy/taxonomy.admin.inc b/modules/taxonomy/taxonomy.admin.inc
index 84ac05bb3..52cecb298 100644
--- a/modules/taxonomy/taxonomy.admin.inc
+++ b/modules/taxonomy/taxonomy.admin.inc
@@ -902,6 +902,7 @@ function taxonomy_term_confirm_delete_submit($form, &$form_state) {
   drupal_set_message(t('Deleted term %name.', array('%name' => $form_state['values']['name'])));
   watchdog('taxonomy', 'Deleted term %name.', array('%name' => $form_state['values']['name']), WATCHDOG_NOTICE);
   $form_state['redirect'] = 'admin/structure/taxonomy';
+  cache_clear_all();
   return;
 }
 
@@ -941,6 +942,7 @@ function taxonomy_vocabulary_confirm_delete_submit($form, &$form_state) {
   drupal_set_message(t('Deleted vocabulary %name.', array('%name' => $form_state['values']['name'])));
   watchdog('taxonomy', 'Deleted vocabulary %name.', array('%name' => $form_state['values']['name']), WATCHDOG_NOTICE);
   $form_state['redirect'] = 'admin/structure/taxonomy';
+  cache_clear_all();
   return;
 }
 
diff --git a/modules/taxonomy/taxonomy.module b/modules/taxonomy/taxonomy.module
index 10393eea9..5097da288 100644
--- a/modules/taxonomy/taxonomy.module
+++ b/modules/taxonomy/taxonomy.module
@@ -421,23 +421,31 @@ function taxonomy_vocabulary_save($vocabulary) {
 function taxonomy_vocabulary_delete($vid) {
   $vocabulary = taxonomy_vocabulary_load($vid);
 
-  // Only load terms without a parent, child terms will get deleted too.
-  $result = db_query('SELECT t.tid FROM {taxonomy_term_data} t INNER JOIN {taxonomy_term_hierarchy} th ON th.tid = t.tid WHERE t.vid = :vid AND th.parent = 0', array(':vid' => $vid))->fetchCol();
-  foreach ($result as $tid) {
-    taxonomy_term_delete($tid);
+  $transaction = db_transaction();
+  try {
+    // Only load terms without a parent, child terms will get deleted too.
+    $result = db_query('SELECT t.tid FROM {taxonomy_term_data} t INNER JOIN {taxonomy_term_hierarchy} th ON th.tid = t.tid WHERE t.vid = :vid AND th.parent = 0', array(':vid' => $vid))->fetchCol();
+    foreach ($result as $tid) {
+      taxonomy_term_delete($tid);
+    }
+    db_delete('taxonomy_vocabulary')
+      ->condition('vid', $vid)
+      ->execute();
+  
+    field_attach_delete_bundle('taxonomy_term', $vocabulary->machine_name);
+    module_invoke_all('taxonomy_vocabulary_delete', $vocabulary);
+    module_invoke_all('entity_delete', $vocabulary, 'taxonomy_vocabulary');
+  
+    cache_clear_all();
+    entity_get_controller('taxonomy_vocabulary')->resetCache();
+  
+    return SAVED_DELETED;
+  }
+  catch (Exception $e) {
+    $transaction->rollback();
+    watchdog_exception('taxonomy', $e);
+    throw $e;
   }
-  db_delete('taxonomy_vocabulary')
-    ->condition('vid', $vid)
-    ->execute();
-
-  field_attach_delete_bundle('taxonomy_term', $vocabulary->machine_name);
-  module_invoke_all('taxonomy_vocabulary_delete', $vocabulary);
-  module_invoke_all('entity_delete', $vocabulary, 'taxonomy_vocabulary');
-
-  cache_clear_all();
-  entity_get_controller('taxonomy_vocabulary')->resetCache();
-
-  return SAVED_DELETED;
 }
 
 /**
@@ -588,41 +596,47 @@ function taxonomy_term_save($term) {
  *   Status constant indicating deletion.
  */
 function taxonomy_term_delete($tid) {
-  $tids = array($tid);
-  while ($tids) {
-    $children_tids = $orphans = array();
-    foreach ($tids as $tid) {
-      // See if any of the term's children are about to be become orphans:
-      if ($children = taxonomy_get_children($tid)) {
-        foreach ($children as $child) {
-          // If the term has multiple parents, we don't delete it.
-          $parents = taxonomy_get_parents($child->tid);
-          if (count($parents) == 1) {
-            $orphans[] = $child->tid;
+  $transaction = db_transaction();
+  try {
+    $tids = array($tid);
+    while ($tids) {
+      $children_tids = $orphans = array();
+      foreach ($tids as $tid) {
+        // See if any of the term's children are about to be become orphans:
+        if ($children = taxonomy_get_children($tid)) {
+          foreach ($children as $child) {
+            // If the term has multiple parents, we don't delete it.
+            $parents = taxonomy_get_parents($child->tid);
+            if (count($parents) == 1) {
+              $orphans[] = $child->tid;
+            }
           }
         }
-      }
 
-      if ($term = taxonomy_term_load($tid)) {
-        db_delete('taxonomy_term_data')
-          ->condition('tid', $tid)
-          ->execute();
-        db_delete('taxonomy_term_hierarchy')
-          ->condition('tid', $tid)
-          ->execute();
-
-        field_attach_delete('taxonomy_term', $term);
-        module_invoke_all('taxonomy_term_delete', $term);
-        module_invoke_all('entity_delete', $term, 'taxonomy_term');
-        taxonomy_terms_static_reset();
+        if ($term = taxonomy_term_load($tid)) {
+          db_delete('taxonomy_term_data')
+            ->condition('tid', $tid)
+            ->execute();
+          db_delete('taxonomy_term_hierarchy')
+            ->condition('tid', $tid)
+            ->execute();
+
+          field_attach_delete('taxonomy_term', $term);
+          module_invoke_all('taxonomy_term_delete', $term);
+          module_invoke_all('entity_delete', $term, 'taxonomy_term');
+          taxonomy_terms_static_reset();
+        }
       }
-    }
 
-    $tids = $orphans;
+      $tids = $orphans;
+    }
+    return SAVED_DELETED;
+  }
+  catch (Exception $e) {
+    $transaction->rollback();
+    watchdog_exception('taxonomy', $e);
+    throw $e;
   }
-
-  cache_clear_all();
-  return SAVED_DELETED;
 }
 
 /**
diff --git a/modules/user/user.module b/modules/user/user.module
index a9f7462da..d7a3be123 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -2361,23 +2361,30 @@ function user_delete_multiple(array $uids) {
   if (!empty($uids)) {
     $accounts = user_load_multiple($uids, array());
 
-    foreach ($accounts as $uid => $account) {
-      module_invoke_all('user_delete', $account);
-      module_invoke_all('entity_delete', $account, 'user');
-      field_attach_delete('user', $account);
-      drupal_session_destroy_uid($account->uid);
-    }
-
-    db_delete('users')
-      ->condition('uid', $uids, 'IN')
-      ->execute();
-    db_delete('users_roles')
-      ->condition('uid', $uids, 'IN')
-      ->execute();
-    db_delete('authmap')
-      ->condition('uid', $uids, 'IN')
-      ->execute();
+    $transaction = db_transaction();
+    try {
+      foreach ($accounts as $uid => $account) {
+        module_invoke_all('user_delete', $account);
+        module_invoke_all('entity_delete', $account, 'user');
+        field_attach_delete('user', $account);
+        drupal_session_destroy_uid($account->uid);
+      }
 
+      db_delete('users')
+        ->condition('uid', $uids, 'IN')
+        ->execute();
+      db_delete('users_roles')
+        ->condition('uid', $uids, 'IN')
+        ->execute();
+      db_delete('authmap')
+        ->condition('uid', $uids, 'IN')
+        ->execute();
+    }
+    catch (Exception $e) {
+      $transaction->rollback();
+      watchdog_exception('user', $e);
+      throw $e;
+    }
     entity_get_controller('user')->resetCache();
   }
 }