Bug 398976: edit*.cgi scripts should use transactions for database interaction - Patch by Emmanuel Seyman <eseyman@linagora.com> r/a=mkanat

7 files changed, 36 insertions, 97 deletions

diff --git a/editclassifications.cgi b/editclassifications.cgi
index 0ebfb97fa..cacc47fb6 100755
--- a/editclassifications.cgi
+++ b/editclassifications.cgi
@@ -172,7 +172,7 @@ if ($action eq 'delete') {
     }
 
     # lock the tables before we start to change everything:
-    $dbh->bz_lock_tables('classifications WRITE', 'products WRITE');
+    $dbh->bz_start_transaction();
 
     # delete
     $dbh->do("DELETE FROM classifications WHERE id = ?", undef,
@@ -182,7 +182,7 @@ if ($action eq 'delete') {
     $dbh->do("UPDATE products SET classification_id = 1
               WHERE classification_id = ?", undef, $classification->id);
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     $vars->{'classification'} = $classification;
 
@@ -229,7 +229,7 @@ if ($action eq 'update') {
       || ThrowUserError('classification_invalid_sortkey', {'name' => $class_old->name,
                                                            'sortkey' => $stored_sortkey});
 
-    $dbh->bz_lock_tables('classifications WRITE');
+    $dbh->bz_start_transaction();
 
     if ($class_name ne $class_old->name) {
 
@@ -262,7 +262,7 @@ if ($action eq 'update') {
         $vars->{'updated_sortkey'} = 1;
     }
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     delete_token($token);
     LoadTemplate($action);
diff --git a/editcomponents.cgi b/editcomponents.cgi
index 09acc0c51..8628f7762 100755
--- a/editcomponents.cgi
+++ b/editcomponents.cgi
@@ -191,7 +191,7 @@ if ($action eq 'new') {
     trick_taint($comp_name);
     trick_taint($description);
 
-    $dbh->bz_lock_tables('components WRITE', 'component_cc WRITE');
+    $dbh->bz_start_transaction();
 
     $dbh->do("INSERT INTO components
                 (product_id, name, description, initialowner,
@@ -209,7 +209,7 @@ if ($action eq 'new') {
         $sth->execute($user_id, $component->id);
     }
 
-    $dbh->bz_unlock_tables;
+    $dbh->bz_commit_transaction();
 
     # Insert default charting queries for this product.
     # If they aren't using charting, this won't do any harm.
@@ -301,8 +301,7 @@ if ($action eq 'delete') {
         }
     }
     
-    $dbh->bz_lock_tables('components WRITE', 'component_cc WRITE',
-                         'flaginclusions WRITE', 'flagexclusions WRITE');
+    $dbh->bz_start_transaction();
 
     $dbh->do("DELETE FROM flaginclusions WHERE component_id = ?",
              undef, $component->id);
@@ -313,7 +312,7 @@ if ($action eq 'delete') {
     $dbh->do("DELETE FROM components WHERE id = ?",
              undef, $component->id);
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     $vars->{'comp'} = $component;
     $vars->{'product'} = $product;
@@ -402,8 +401,7 @@ if ($action eq 'update') {
 
     my $initial_cc_ids = check_initial_cc(\@initial_cc);
 
-    $dbh->bz_lock_tables('components WRITE', 'component_cc WRITE', 
-                         'profiles READ');
+    $dbh->bz_start_transaction();
 
     if ($comp_name ne $component_old->name) {
 
@@ -457,7 +455,7 @@ if ($action eq 'update') {
         $vars->{'updated_initialcc'} = 1;
     }
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     my $component = new Bugzilla::Component($component_old->id);
     
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 0aec0385d..02e043c5d 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -309,9 +309,7 @@ sub insert {
 
     my $target_type = $cgi->param('target_type') eq "bug" ? "b" : "a";
 
-    $dbh->bz_lock_tables('flagtypes WRITE', 'products READ',
-                         'components READ', 'flaginclusions WRITE',
-                         'flagexclusions WRITE');
+    $dbh->bz_start_transaction();
 
     # Insert a record for the new flag type into the database.
     $dbh->do('INSERT INTO flagtypes
@@ -332,7 +330,7 @@ sub insert {
     # Populate the list of inclusions/exclusions for this flag type.
     validateAndSubmit($id);
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     $vars->{'name'} = $cgi->param('name');
     $vars->{'message'} = "flag_type_created";
@@ -365,9 +363,7 @@ sub update {
 
     my $dbh = Bugzilla->dbh;
     my $user = Bugzilla->user;
-    $dbh->bz_lock_tables('flagtypes WRITE', 'products READ',
-                         'components READ', 'flaginclusions WRITE',
-                         'flagexclusions WRITE');
+    $dbh->bz_start_transaction();
     $dbh->do('UPDATE flagtypes
                  SET name = ?, description = ?, cc_list = ?,
                      sortkey = ?, is_active = ?, is_requestable = ?,
@@ -383,7 +379,7 @@ sub update {
     # Update the list of inclusions/exclusions for this flag type.
     validateAndSubmit($id);
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     # Clear existing flags for bugs/attachments in categories no longer on 
     # the list of inclusions or that have been added to the list of exclusions.
@@ -473,8 +469,7 @@ sub deleteType {
     my $id = $flag_type->id;
     my $dbh = Bugzilla->dbh;
 
-    $dbh->bz_lock_tables('flagtypes WRITE', 'flags WRITE',
-                         'flaginclusions WRITE', 'flagexclusions WRITE');
+    $dbh->bz_start_transaction();
 
     # Get the name of the flag type so we can tell users
     # what was deleted.
@@ -484,7 +479,7 @@ sub deleteType {
     $dbh->do('DELETE FROM flaginclusions WHERE type_id = ?', undef, $id);
     $dbh->do('DELETE FROM flagexclusions WHERE type_id = ?', undef, $id);
     $dbh->do('DELETE FROM flagtypes WHERE id = ?', undef, $id);
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     $vars->{'message'} = "flag_type_deleted";
     delete_token($token);
@@ -506,9 +501,9 @@ sub deactivate {
 
     my $dbh = Bugzilla->dbh;
 
-    $dbh->bz_lock_tables('flagtypes WRITE');
+    $dbh->bz_start_transaction();
     $dbh->do('UPDATE flagtypes SET is_active = 0 WHERE id = ?', undef, $flag_type->id);
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     $vars->{'message'} = "flag_type_deactivated";
     $vars->{'flag_type'} = $flag_type;
diff --git a/editgroups.cgi b/editgroups.cgi
index 43875da51..19db870ec 100755
--- a/editgroups.cgi
+++ b/editgroups.cgi
@@ -502,8 +502,7 @@ if ($action eq 'remove_regexp') {
     my $group  = new Bugzilla::Group(CheckGroupID($cgi->param('group_id')));
     my $regexp = CheckGroupRegexp($cgi->param('regexp'));
 
-    $dbh->bz_lock_tables('groups WRITE', 'profiles READ',
-                         'user_group_map WRITE');
+    $dbh->bz_start_transaction();
 
     my $users = $group->members_direct();
     my $sth_delete = $dbh->prepare(
@@ -517,7 +516,7 @@ if ($action eq 'remove_regexp') {
             push(@deleted, $member);
         }
     }
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     $vars->{'users'}  = \@deleted;
     $vars->{'regexp'} = $regexp;
@@ -543,13 +542,7 @@ sub doGroupChanges {
     my $cgi = Bugzilla->cgi;
     my $dbh = Bugzilla->dbh;
 
-    $dbh->bz_lock_tables('groups WRITE', 'group_group_map WRITE',
-                         'bug_group_map WRITE', 'user_group_map WRITE',
-                         'group_control_map READ', 'bugs READ', 'profiles READ',
-                         # Due to the way Bugzilla::Config::BugFields::get_param_list()
-                         # works, we need to lock these tables too.
-                         'priority READ', 'bug_severity READ', 'rep_platform READ',
-                         'op_sys READ');
+    $dbh->bz_start_transaction();
 
     # Check that the given group ID is valid and make a Group.
     my $group = new Bugzilla::Group(CheckGroupID($cgi->param('group_id')));
@@ -603,7 +596,7 @@ sub doGroupChanges {
                    $data->[0], $data->[1]);
     }
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
     return $changes;
 }
 
diff --git a/editproducts.cgi b/editproducts.cgi
index c3b29747c..5a937db9d 100755
--- a/editproducts.cgi
+++ b/editproducts.cgi
@@ -407,10 +407,7 @@ if ($action eq 'delete') {
         }
     }
 
-    $dbh->bz_lock_tables('products WRITE', 'components WRITE',
-                         'versions WRITE', 'milestones WRITE',
-                         'group_control_map WRITE', 'component_cc WRITE',
-                         'flaginclusions WRITE', 'flagexclusions WRITE');
+    $dbh->bz_start_transaction();
 
     my $comp_ids = $dbh->selectcol_arrayref('SELECT id FROM components
                                              WHERE product_id = ?',
@@ -440,7 +437,7 @@ if ($action eq 'delete') {
     $dbh->do("DELETE FROM products WHERE id = ?",
              undef, $product->id);
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     delete_token($token);
 
@@ -587,12 +584,7 @@ if ($action eq 'updategroupcontrols') {
                             {groupname => $groupname});
         }
     }
-    $dbh->bz_lock_tables('groups READ',
-                         'group_control_map WRITE',
-                         'bugs WRITE',
-                         'bugs_activity WRITE',
-                         'bug_group_map WRITE',
-                         'fielddefs READ');
+    $dbh->bz_start_transaction();
 
     my $sth_Insert = $dbh->prepare('INSERT INTO group_control_map
                                     (group_id, product_id, entry, membercontrol,
@@ -771,7 +763,7 @@ if ($action eq 'updategroupcontrols') {
 
         push(@added_mandatory, \%group);
     }
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     delete_token($token);
 
@@ -847,7 +839,7 @@ if ($action eq 'update') {
                        {votestoconfirm => $stored_votestoconfirm});
     }
 
-    $dbh->bz_lock_tables('products WRITE', 'milestones READ');
+    $dbh->bz_start_transaction();
 
     my $testproduct = 
         new Bugzilla::Product({name => $product_name});
@@ -917,7 +909,7 @@ if ($action eq 'update') {
                  undef, ($product_name, $product_old->id));
     }
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     my $product = new Bugzilla::Product({name => $product_name});
 
diff --git a/editusers.cgi b/editusers.cgi
index 076a2de98..e452eea01 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -227,18 +227,7 @@ if ($action eq 'search') {
     $otherUserID = $otherUser->id;
 
     # Lock tables during the check+update session.
-    $dbh->bz_lock_tables('profiles WRITE',
-                         'profiles_activity WRITE',
-                         'fielddefs READ',
-                         'tokens WRITE',
-                         'logincookies WRITE',
-                         'groups READ',
-                         'user_group_map WRITE',
-                         'group_group_map READ',
-                         'group_group_map AS ggm READ',
-                         'user_group_map AS directmember READ',
-                         'user_group_map AS regexpmember READ',
-                         'user_group_map AS directbless READ');
+    $dbh->bz_start_transaction();
  
     $editusers || $user->can_see_user($otherUser)
         || ThrowUserError('auth_failure', {reason => "not_visible",
@@ -338,7 +327,7 @@ if ($action eq 'search') {
     }
     # XXX: should create profiles_activity entries for blesser changes.
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     # XXX: userDataToVars may be off when editing ourselves.
     userDataToVars($otherUserID);
@@ -454,33 +443,7 @@ if ($action eq 'search') {
     # XXX: if there was some change on these tables after the deletion
     #      confirmation checks, we may do something here we haven't warned
     #      about.
-    $dbh->bz_lock_tables('bugs WRITE',
-                         'bugs_activity WRITE',
-                         'attachments READ',
-                         'fielddefs READ',
-                         'products READ',
-                         'components READ',
-                         'logincookies WRITE',
-                         'profiles WRITE',
-                         'profiles_activity WRITE',
-                         'email_setting WRITE',
-                         'profile_setting WRITE',
-                         'bug_group_map READ',
-                         'user_group_map WRITE',
-                         'flags WRITE',
-                         'flagtypes READ',
-                         'cc WRITE',
-                         'namedqueries WRITE',
-                         'namedqueries_link_in_footer WRITE',
-                         'namedquery_group_map WRITE',
-                         'tokens WRITE',
-                         'votes WRITE',
-                         'watch WRITE',
-                         'series WRITE',
-                         'series_data WRITE',
-                         'whine_schedules WRITE',
-                         'whine_queries WRITE',
-                         'whine_events WRITE');
+    $dbh->bz_start_transaction();
 
     Bugzilla->params->{'allowuserdeletion'}
         || ThrowUserError('users_deletion_disabled');
@@ -664,7 +627,7 @@ if ($action eq 'search') {
     # Finally, remove the user account itself.
     $dbh->do('DELETE FROM profiles WHERE userid = ?', undef, $otherUserID);
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
     delete_token($token);
 
     $vars->{'message'} = 'account_deleted';
diff --git a/editvalues.cgi b/editvalues.cgi
index a9d5878c0..520c0797b 100755
--- a/editvalues.cgi
+++ b/editvalues.cgi
@@ -312,9 +312,7 @@ if ($action eq 'delete') {
 
     trick_taint($value);
 
-    my @lock_tables = ('bugs READ', "$field WRITE");
-    push(@lock_tables, 'status_workflow WRITE') if ($field eq 'bug_status');
-    $dbh->bz_lock_tables(@lock_tables);
+    $dbh->bz_start_transaction();
 
     # Check if there are any bugs that still have this value.
     my $bug_ids = $dbh->selectcol_arrayref(
@@ -338,7 +336,7 @@ if ($action eq 'delete') {
 
     $dbh->do("DELETE FROM $field WHERE value = ?", undef, $value);
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
     delete_token($token);
 
     $template->process("admin/fieldvalues/deleted.html.tmpl",
@@ -396,7 +394,7 @@ if ($action eq 'update') {
         ThrowUserError('fieldvalue_name_too_long', $vars);
     }
 
-    $dbh->bz_lock_tables('bugs WRITE', "$field WRITE");
+    $dbh->bz_start_transaction();
 
     # Need to store because detaint_natural() will delete this if
     # invalid
@@ -442,7 +440,7 @@ if ($action eq 'update') {
         $vars->{'updated_value'} = 1;
     }
 
-    $dbh->bz_unlock_tables();
+    $dbh->bz_commit_transaction();
 
     # If the old value was the default value for the field,
     # update data/params accordingly.