From 8dd7376d5a15f2485fdd83c8db402a244314e30b Mon Sep 17 00:00:00 2001 From: Angie Byron Date: Sun, 14 Nov 2010 20:42:45 +0000 Subject: [PATCH] #923826 by catch, carlos8f, moshe weitzman: Fixed entity delete operations should use transactions. --- modules/comment/comment.module | 33 +++++---- modules/node/node.module | 63 +++++++++-------- modules/taxonomy/taxonomy.admin.inc | 2 + modules/taxonomy/taxonomy.module | 104 ++++++++++++++++------------ modules/user/user.module | 39 ++++++----- 5 files changed, 139 insertions(+), 102 deletions(-) diff --git a/modules/comment/comment.module b/modules/comment/comment.module index 622433e2403..5334c272802 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) { + $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 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); + // 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 252d72278ce..337c4098dfe 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); + 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'); + // 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(); + } + catch (Exception $e) { + $transaction->rollback(); + watchdog_exception('node', $e); + throw $e; } - // 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(); - // 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 84ac05bb36f..52cecb29815 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 10393eea999..5097da2884f 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 a9f7462dabb..d7a3be12320 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); + $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; } - - 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(); - entity_get_controller('user')->resetCache(); } }