Bug 121069: Remove $dbh->bz_(un)lock_tables from process_bug.cgi and Error.pm in favor of DB transactions. These methods are no longer used and are completely removed now - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat

4 files changed, 8 insertions, 166 deletions

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index ee064bb85..31427bc82 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -91,7 +91,6 @@ use File::Basename;
     CMT_POPULAR_VOTES
     CMT_MOVED_TO
 
-    UNLOCK_ABORT
     THROW_ERROR
     
     RELATIONSHIPS
@@ -269,10 +268,6 @@ use constant CMT_HAS_DUPE => 2;
 use constant CMT_POPULAR_VOTES => 3;
 use constant CMT_MOVED_TO => 4;
 
-# used by Bugzilla::DB to indicate that tables are being unlocked
-# because of error
-use constant UNLOCK_ABORT => 1;
-
 # Determine whether a validation routine should return 0 or throw
 # an error when the validation fails.
 use constant THROW_ERROR => 1;
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 2a71bcd75..4a0303287 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -273,8 +273,7 @@ EOT
 # List of abstract methods we are checking the derived class implements
 our @_abstract_methods = qw(REQUIRED_VERSION PROGRAM_NAME DBD_VERSION
                             new sql_regexp sql_not_regexp sql_limit sql_to_days
-                            sql_date_format sql_interval
-                            bz_lock_tables bz_unlock_tables);
+                            sql_date_format sql_interval);
 
 # This overridden import method will check implementation of inherited classes
 # for missing implementation of abstract methods
@@ -301,62 +300,6 @@ sub import {
     $Exporter::ExportLevel-- if $is_exporter;
 }
 
-sub bz_lock_tables {
-    my ($self, @tables) = @_;
-
-    my $list = join(', ', @tables);
-    # Check first if there was no lock before
-    if ($self->{private_bz_tables_locked}) {
-        ThrowCodeError("already_locked", { current => $self->{private_bz_tables_locked},
-                                           new => $list });
-    } else {
-        my %read_tables;
-        my %write_tables;
-        foreach my $table (@tables) {
-            $table =~ /^([\d\w]+)([\s]+AS[\s]+[\d\w]+)?[\s]+(WRITE|READ)$/i;
-            my $table_name = $1;
-            if ($3 =~ /READ/i) {
-                if (!exists $read_tables{$table_name}) {
-                    $read_tables{$table_name} = undef;
-                }
-            }
-            else {
-                if (!exists $write_tables{$table_name}) {
-                    $write_tables{$table_name} = undef;
-                }
-            }
-        }
-
-        # Begin Transaction
-        $self->bz_start_transaction();
-
-        Bugzilla->dbh->do('LOCK TABLE ' . join(', ', keys %read_tables) .
-                          ' IN ROW SHARE MODE') if keys %read_tables;
-        Bugzilla->dbh->do('LOCK TABLE ' . join(', ', keys %write_tables) .
-                          ' IN ROW EXCLUSIVE MODE') if keys %write_tables;
-        $self->{private_bz_tables_locked} = $list;
-    }
-}
-
-sub bz_unlock_tables {
-    my ($self, $abort) = @_;
-
-    # Check first if there was previous matching lock
-    if (!$self->{private_bz_tables_locked}) {
-        # Abort is allowed even without previous lock for error handling
-        return if $abort;
-        ThrowCodeError("no_matching_lock");
-    } else {
-        $self->{private_bz_tables_locked} = "";
-        # End transaction, tables will be unlocked automatically
-        if ($abort) {
-            $self->bz_rollback_transaction();
-        } else {
-            $self->bz_commit_transaction();
-        }
-    }
-}
-
 sub sql_istrcmp {
     my ($self, $left, $right, $op) = @_;
     $op ||= "=";
@@ -1949,63 +1892,6 @@ Formatted SQL for the C<IN> operator.
 
 =back
 
-=item C<bz_lock_tables>
-
-=over
-
-=item B<Description>
-
-Performs a table lock operation on specified tables. If the underlying 
-database supports transactions, it should also implicitly start a new 
-transaction.
-
-Abstract method, should be overridden by database specific code.
-
-=item B<Params>
-
-=over
-
-=item C<@tables> - list of names of tables to lock in MySQL
-notation (ex. 'bugs AS bugs2 READ', 'logincookies WRITE')
-
-=back
-
-=item B<Returns> (nothing)
-
-=back
-
-=item C<bz_unlock_tables>
-
-=over
-
-=item B<Description>
-
-Performs a table unlock operation.
-
-If the underlying database supports transactions, it should also implicitly 
-commit or rollback the transaction.
-
-Also, this function should allow to be called with the abort flag
-set even without locking tables first without raising an error
-to simplify error handling.
-
-Abstract method, should be overridden by database specific code.
-
-=item B<Params>
-
-=over
-
-=item C<$abort> - C<UNLOCK_ABORT> if the operation on locked tables
-failed (if transactions are supported, the action will be rolled
-back). No param if the operation succeeded. This is only used by
-L<Bugzilla::Error/throw_error>.
-
-=back
-
-=item B<Returns> (none)
-
-=back
-
 =back
 
 
diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm
index 9e0d25277..720c8ff42 100644
--- a/Bugzilla/DB/Mysql.pm
+++ b/Bugzilla/DB/Mysql.pm
@@ -200,44 +200,6 @@ sub sql_group_by {
 }
 
 
-sub bz_lock_tables {
-    my ($self, @tables) = @_;
-
-    my $list = join(', ', @tables);
-    # Check first if there was no lock before
-    if ($self->{private_bz_tables_locked}) {
-        ThrowCodeError("already_locked", { current => $self->{private_bz_tables_locked},
-                                           new => $list });
-    } else {
-        $self->bz_start_transaction();
-        $self->do('LOCK TABLE ' . $list); 
-        $self->{private_bz_tables_locked} = $list;
-    }
-}
-
-sub bz_unlock_tables {
-    my ($self, $abort) = @_;
-
-    if ($self->bz_in_transaction) {
-        if ($abort) {
-            $self->bz_rollback_transaction();
-        }
-        else {
-            $self->bz_commit_transaction();
-        }
-    }
-    
-    # Check first if there was previous matching lock
-    if (!$self->{private_bz_tables_locked}) {
-        # Abort is allowed even without previous lock for error handling
-        return if $abort;
-        ThrowCodeError("no_matching_lock");
-    } else {
-        $self->do("UNLOCK TABLES");
-        $self->{private_bz_tables_locked} = "";
-    }
-}
-
 sub _bz_get_initial_schema {
     my ($self) = @_;
     return $self->_bz_build_schema_from_disk();
diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
index 18f9aae54..3e5688e2a 100644
--- a/Bugzilla/Error.pm
+++ b/Bugzilla/Error.pm
@@ -46,16 +46,15 @@ sub _in_eval {
 
 sub _throw_error {
     my ($name, $error, $vars) = @_;
-
+    my $dbh = Bugzilla->dbh;
     $vars ||= {};
 
     $vars->{error} = $error;
 
-    # Make sure any locked tables are unlocked
-    # and the transaction is rolled back (if supported)
-    # If we are within an eval(), do not unlock tables as we are
+    # Make sure any transaction is rolled back (if supported).
+    # If we are within an eval(), do not roll back transactions as we are
     # eval'uating some test on purpose.
-    Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT) unless _in_eval();
+    $dbh->bz_rollback_transaction() if ($dbh->bz_in_transaction() && !_in_eval());
 
     my $datadir = bz_locations()->{'datadir'};
     # If a writable $datadir/errorlog exists, log error details there.
@@ -124,10 +123,10 @@ sub ThrowCodeError {
 
 sub ThrowTemplateError {
     my ($template_err) = @_;
+    my $dbh = Bugzilla->dbh;
 
-    # Make sure any locked tables are unlocked
-    # and the transaction is rolled back (if supported)
-    Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT);
+    # Make sure the transaction is rolled back (if supported).
+    $dbh->bz_rollback_transaction() if $dbh->bz_in_transaction();
 
     my $vars = {};
     if (Bugzilla->error_mode == ERROR_MODE_DIE) {