Bug 304075: Eliminate use of $::userid from Bugzilla - Patch by Frédéric Buclin…

Bugzilla.pm

@@ -145,7 +145,6 @@ sub sudo_request {
 
     $_user = $new_user;
     $_sudoer = $new_sudoer;
-    $::userid = $new_user->id;
 
     # NOTE: If you want to log the start of an sudo session, do it here.
 
@@ -177,7 +176,6 @@ sub login {
     {
         $_user = $sudo_target;
         $_sudoer = $authenticated_user;
-        $::userid = $sudo_target->id;
 
         # NOTE: If you want to do any special logging, do it here.
     }
@@ -216,8 +214,6 @@ sub logout_user_by_id {
 sub logout_request {
     undef $_user;
     undef $_sudoer;
-    # XXX clean this up eventually
-    $::userid = 0;
     # We can't delete from $cgi->cookie, so logincookie data will remain
     # there. Don't rely on it: use Bugzilla->user->login instead!
 }

Bugzilla/Auth/Login/WWW.pm

@@ -75,11 +75,7 @@ sub login {
         if (Param('sslbase') ne '' and Param('ssl') ne 'never') {
             Bugzilla->cgi->require_https(Param('sslbase'));
         }
-
         $user->set_flags('can_logout' => $class->can_logout);
-
-        # Compat stuff
-        $::userid = $userid;
     } else {
         Bugzilla->logout_request();
     }

Bugzilla/Flag.pm

@@ -492,6 +492,7 @@ sub process {
 sub update_activity {
     my ($bug_id, $attach_id, $timestamp, $old_summaries, $new_summaries) = @_;
     my $dbh = Bugzilla->dbh;
+    my $user_id = Bugzilla->user->id;
 
     $attach_id ||= 'NULL';
     $old_summaries = join(", ", @$old_summaries);
@@ -503,7 +504,7 @@ sub update_activity {
         my $field_id = get_field_id('flagtypes.name');
         $dbh->do("INSERT INTO bugs_activity
                   (bug_id, attach_id, who, bug_when, fieldid, removed, added)
-                  VALUES ($bug_id, $attach_id, $::userid, $timestamp,
+                  VALUES ($bug_id, $attach_id, $user_id, $timestamp,
                   $field_id, $sql_removed, $sql_added)");
 
         $dbh->do("UPDATE bugs SET delta_ts = $timestamp WHERE bug_id = ?",
@@ -603,6 +604,7 @@ attachment.cgi midairs. See bug 223878 for details.
 
 sub modify {
     my ($cgi, $timestamp) = @_;
+    my $setter = Bugzilla->user;
 
     # Use the date/time we were given if possible (allowing calling code
     # to synchronize the comment's timestamp with those of other records).
@@ -621,7 +623,6 @@ sub modify {
 
         my $status = $cgi->param("flag-$id");
 
-        
         # If the user entered more than one name into the requestee field
         # (i.e. they want more than one person to set the flag) we can reuse
         # the existing flag for the first person (who may well be the existing
@@ -639,7 +640,7 @@ sub modify {
             foreach my $login (@requestees) {
                 create({ type      => $flag->{type} ,
                          target    => $flag->{target} , 
-                         setter    => new Bugzilla::User($::userid), 
+                         setter    => $setter, 
                          status    => "?",
                          requestee => new Bugzilla::User(login_to_id($login)) },
                        $timestamp);
@@ -678,7 +679,7 @@ sub modify {
         
         if ($status eq '+' || $status eq '-') {
             &::SendSQL("UPDATE flags 
-                        SET    setter_id = $::userid , 
+                        SET    setter_id = " . $setter->id . ", 
                                requestee_id = NULL , 
                                status = '$status' , 
                                modification_date = $sql_timestamp ,
@@ -703,7 +704,7 @@ sub modify {
 
             # Update the database with the changes.
             &::SendSQL("UPDATE flags 
-                        SET    setter_id = $::userid , 
+                        SET    setter_id = " . $setter->id . ", 
                                requestee_id = $requestee_id , 
                                status = '$status' , 
                                modification_date = $sql_timestamp ,
@@ -778,8 +779,8 @@ array of flag objects. This array is then passed to Flag::create().
 
 sub FormToNewFlags {
     my ($target, $cgi) = @_;
-
     my $dbh = Bugzilla->dbh;
+    my $setter = Bugzilla->user;
     
     # Extract a list of flag type IDs from field names.
     my @type_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
@@ -787,9 +788,6 @@ sub FormToNewFlags {
 
     return () unless scalar(@type_ids);
 
-    # Get information about the setter to add to each flag.
-    my $setter = new Bugzilla::User($::userid);
-
     # Get a list of active flag types available for this target.
     my $flag_types = Bugzilla::FlagType::match(
         { 'target_type'  => $target->{'type'},

Bugzilla/Search.pm

@@ -30,10 +30,6 @@
 
 use strict;
 
-# The caller MUST require globals.pl before using this module.
-
-use vars qw($userid);
-
 package Bugzilla::Search;
 use base qw(Exporter);
 @Bugzilla::Search::EXPORT = qw(IsValidQueryType);

buglist.cgi

@@ -44,8 +44,7 @@ use Bugzilla::Bug;
 # Include the Bugzilla CGI and general utility library.
 require "globals.pl";
 
-use vars qw($db_name
-            @components
+use vars qw(@components
             @legal_keywords
             @legal_platform
             @legal_priority
@@ -53,7 +52,6 @@ use vars qw($db_name
             @legal_severity
             @settable_resolution
             @target_milestone
-            $userid
             @versions);
 
 my $cgi = Bugzilla->cgi;
@@ -204,7 +202,7 @@ sub DiffDate {
 
 sub LookupNamedQuery {
     my ($name) = @_;
-    Bugzilla->login(LOGIN_REQUIRED);
+    my $user = Bugzilla->login(LOGIN_REQUIRED);
     my $dbh = Bugzilla->dbh;
     # $name is safe -- we only use it below in a SELECT placeholder and then 
     # in error messages (which are always HTML-filtered).
@@ -212,7 +210,7 @@ sub LookupNamedQuery {
     trick_taint($name);
     my $result = $dbh->selectrow_array("SELECT query FROM namedqueries" 
                           . " WHERE userid = ? AND name = ?"
-                          , undef, (Bugzilla->user->id, $name));
+                          , undef, ($user->id, $name));
     
     defined($result) || ThrowUserError("missing_query", {'queryname' => $name});
     $result
@@ -316,11 +314,12 @@ sub GetQuip {
 
 sub GetGroups {
     my $dbh = Bugzilla->dbh;
+    my $user = Bugzilla->user;
 
     # Create an array where each item is a hash. The hash contains 
     # as keys the name of the columns, which point to the value of 
     # the columns for that row.
-    my $grouplist = Bugzilla->user->groups_as_string;
+    my $grouplist = $user->groups_as_string;
     my $groups = $dbh->selectall_arrayref(
                 "SELECT  id, name, description, isactive
                    FROM  groups
@@ -387,7 +386,7 @@ if ($cgi->param('cmdtype') eq "dorem") {
         $order = $params->param('order') || $order;
     }
     elsif ($cgi->param('remaction') eq "forget") {
-        Bugzilla->login(LOGIN_REQUIRED);
+        my $user = Bugzilla->login(LOGIN_REQUIRED);
         # Copy the name into a variable, so that we can trick_taint it for
         # the DB. We know it's safe, because we're using placeholders in 
         # the SQL, and the SQL is only a DELETE.
@@ -405,7 +404,7 @@ if ($cgi->param('cmdtype') eq "dorem") {
                                                       = ?
                                                   AND whine_queries.query_name
                                                       = ?
-                                      ', undef, Bugzilla->user->id, $qname);
+                                      ', undef, $user->id, $qname);
         if (scalar(@$whines_in_use)) {
             ThrowUserError('saved_search_used_by_whines', 
                            { subjects    => join(',', @$whines_in_use),
@@ -416,10 +415,10 @@ if ($cgi->param('cmdtype') eq "dorem") {
         # If we are here, then we can safely remove the saved search
         $dbh->do("DELETE FROM namedqueries"
             . " WHERE userid = ? AND name = ?"
-            , undef, ($userid, $qname));
+            , undef, ($user->id, $qname));
 
         # Now reset the cached queries
-        Bugzilla->user->flush_queries_cache();
+        $user->flush_queries_cache();
 
         print $cgi->header();
         # Generate and return the UI (HTML page) from the appropriate template.
@@ -433,13 +432,12 @@ if ($cgi->param('cmdtype') eq "dorem") {
 }
 elsif (($cgi->param('cmdtype') eq "doit") && defined $cgi->param('remtype')) {
     if ($cgi->param('remtype') eq "asdefault") {
-        Bugzilla->login(LOGIN_REQUIRED);
-        InsertNamedQuery(Bugzilla->user->id, DEFAULT_QUERY_NAME, $buffer);
+        my $user = Bugzilla->login(LOGIN_REQUIRED);
+        InsertNamedQuery($user->id, DEFAULT_QUERY_NAME, $buffer);
         $vars->{'message'} = "buglist_new_default_query";
     }
     elsif ($cgi->param('remtype') eq "asnamed") {
-        Bugzilla->login(LOGIN_REQUIRED);
-        my $userid = Bugzilla->user->id;
+        my $user = Bugzilla->login(LOGIN_REQUIRED);
         my $query_name = $cgi->param('newqueryname');
         my $new_query = $cgi->param('newquery');
         my $query_type = QUERY_LIST;
@@ -468,7 +466,7 @@ elsif (($cgi->param('cmdtype') eq "doit") && defined $cgi->param('remtype')) {
             $query_type = LIST_OF_BUGS;
         }
         my $tofooter = 1;
-        my $existed_before = InsertNamedQuery($userid, $query_name, $new_query,
+        my $existed_before = InsertNamedQuery($user->id, $query_name, $new_query,
                                               $tofooter, $query_type);
         if ($existed_before) {
             $vars->{'message'} = "buglist_updated_named_query";
@@ -479,7 +477,7 @@ elsif (($cgi->param('cmdtype') eq "doit") && defined $cgi->param('remtype')) {
 
         # Make sure to invalidate any cached query data, so that the footer is
         # correctly displayed
-        Bugzilla->user->flush_queries_cache();
+        $user->flush_queries_cache();
 
         $vars->{'queryname'} = $query_name;
         

chart.cgi

@@ -86,7 +86,7 @@ if ($action eq "search") {
     exit;
 }
 
-Bugzilla->login(LOGIN_REQUIRED);
+my $user = Bugzilla->login(LOGIN_REQUIRED);
 
 UserInGroup(Param("chartgroup"))
   || ThrowUserError("auth_failure", {group  => Param("chartgroup"),
@@ -103,7 +103,7 @@ if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) {
     if ($action =~ /^subscribe|unsubscribe$/) {
         detaint_natural($series_id) || ThrowCodeError("invalid_series_id");
         my $series = new Bugzilla::Series($series_id);
-        $series->$action($::userid);
+        $series->$action($user->id);
     }
 
     my $chart = new Bugzilla::Chart($cgi);
@@ -203,14 +203,15 @@ sub getSelectedLines {
 # Check if the user is the owner of series_id or is an admin. 
 sub assertCanEdit {
     my ($series_id) = @_;
-    
-    return if UserInGroup("admin");
+    my $user = Bugzilla->user;
+
+    return if $user->in_group('admin');
 
     my $dbh = Bugzilla->dbh;
     my $iscreator = $dbh->selectrow_array("SELECT CASE WHEN creator = ? " .
                                           "THEN 1 ELSE 0 END FROM series " .
                                           "WHERE series_id = ?", undef,
-                                          $::userid, $series_id);
+                                          $user->id, $series_id);
     $iscreator || ThrowUserError("illegal_series_edit");
 }
 

checksetup.pl

@@ -3427,10 +3427,6 @@ if (!$series_exists) {
     my $all_name = "-All-";
     my $open_name = "All Open";
         
-    # We can't give the Series we create a meaningful owner; that's not a big 
-    # problem. But we do need to set this global, otherwise Series.pm objects.
-    $::userid = 0;
-    
     my $products = $dbh->selectall_arrayref("SELECT name FROM products");
      
     foreach my $product ((map { $_->[0] } @$products), "-All-") {
@@ -3448,9 +3444,10 @@ if (!$series_exists) {
         $queries{$_} = ($query_prod . "resolution=$_") foreach (@resolutions);
         
         foreach my $field (@fields) {            
-            # Create a Series for each field in this product
+            # Create a Series for each field in this product.
+            # user ID = 0 is used.
             my $series = new Bugzilla::Series(undef, $product, $all_name,
-                                              $field, $::userid, 1,
+                                              $field, 0, 1,
                                               $queries{$field}, 1);
             $series->writeToDatabase();
             $seriesids{$field} = $series->{'series_id'};
@@ -3461,7 +3458,7 @@ if (!$series_exists) {
         my @openedstatuses = ("UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED");
         my $query = join("&", map { "bug_status=$_" } @openedstatuses);
         my $series = new Bugzilla::Series(undef, $product, $all_name,
-                                          $open_name, $::userid, 1, 
+                                          $open_name, 0, 1, 
                                           $query_prod . $query, 1);
         $series->writeToDatabase();
         $seriesids{$open_name} = $series->{'series_id'};

duplicates.cgi

@@ -68,7 +68,7 @@ else {
 
 Bugzilla->switch_to_shadow_db();
 
-use vars qw ($userid @legal_product);
+use vars qw (@legal_product);
 
 my %dbmcount;
 my %count;

editcomponents.cgi

@@ -204,7 +204,7 @@ if ($action eq 'new') {
     foreach my $sdata (@series) {
         my $series = new Bugzilla::Series(undef, $product->name,
                                           $comp_name, $sdata->[0],
-                                          $::userid, 1, $sdata->[1], 1);
+                                          $whoid, 1, $sdata->[1], 1);
         $series->writeToDatabase();
     }
 

editproducts.cgi

@@ -304,7 +304,7 @@ if ($action eq 'new') {
         foreach my $sdata (@series) {
             my $series = new Bugzilla::Series(undef, $product->name, 
                             scalar $cgi->param('subcategory'),
-                            $sdata->[0], $::userid, 1,
+                            $sdata->[0], $whoid, 1,
                             $sdata->[1] . "&product=" .
                             url_quote($product->name), 1);
             $series->writeToDatabase();
@@ -652,7 +652,7 @@ if ($action eq 'updategroupcontrols') {
                 $dbh->selectrow_array($sth_Select2, undef, $groupid);
 
             LogActivityEntry($bugid, "bug_group", $removed, "",
-                             $::userid, $timestamp);
+                             $whoid, $timestamp);
 
             if ($mailiscurrent) {
                 $sth_Update2->execute($timestamp, $timestamp, $bugid);
@@ -695,7 +695,7 @@ if ($action eq 'updategroupcontrols') {
                 $dbh->selectrow_array($sth_Select2, undef, $groupid);
 
             LogActivityEntry($bugid, "bug_group", "", $added,
-                             $::userid, $timestamp);
+                             $whoid, $timestamp);
 
             if ($mailiscurrent) {
                 $sth_Update2->execute($timestamp, $timestamp, $bugid);

enter_bug.cgi

@@ -50,7 +50,6 @@ use vars qw(
   @legal_priority
   @legal_severity
   @legal_keywords
-  $userid
   %versions
   %target_milestone
   $proddesc
@@ -323,7 +322,7 @@ $cloned_bug_id = $cgi->param('cloned_bug_id');
 
 if ($cloned_bug_id) {
     ValidateBugID($cloned_bug_id);
-    $cloned_bug = new Bugzilla::Bug($cloned_bug_id, $userid);
+    $cloned_bug = new Bugzilla::Bug($cloned_bug_id, Bugzilla->user->id);
 }
 
 # We need to check and make sure

globals.pl

@@ -44,7 +44,6 @@ use Bugzilla::Error;
 
 sub globals_pl_sillyness {
     my $zz;
-    $zz = @main::default_column_list;
     $zz = @main::enterable_products;
     $zz = %main::keywordsbyname;
     $zz = @main::legal_bug_status;
@@ -60,7 +59,6 @@ sub globals_pl_sillyness {
     $zz = @main::milestoneurl;
     $zz = %main::proddesc;
     $zz = @main::prodmaxvotes;
-    $zz = $main::userid;
 }
 
 #

index.cgi

@@ -32,7 +32,7 @@ use strict;
 use lib ".";
 require "globals.pl";
 
-# Check whether or not the user is logged in and, if so, set the $::userid 
+# Check whether or not the user is logged in
 use Bugzilla::Constants;
 Bugzilla->login(LOGIN_OPTIONAL);
 

post_bug.cgi

@@ -298,7 +298,7 @@ $comment = trim($comment);
 # OK except for the fact that it causes e-mail to be suppressed.
 $comment = $comment ? $comment : " ";
 
-$sql .= "$::userid, $sql_timestamp, ";
+$sql .= $user->id . ", $sql_timestamp, ";
 
 # Time Tracking
 if (UserInGroup(Param("timetrackinggroup")) &&
@@ -443,10 +443,10 @@ $dbh->do("UPDATE bugs SET creation_ts = ? WHERE bug_id = ?",
 $dbh->bz_unlock_tables();
 
 # Email everyone the details of the new bug 
-$vars->{'mailrecipients'} = {'changer' => Bugzilla->user->login};
+$vars->{'mailrecipients'} = {'changer' => $user->login};
 
 $vars->{'id'} = $id;
-my $bug = new Bugzilla::Bug($id, $::userid);
+my $bug = new Bugzilla::Bug($id, $user->id);
 $vars->{'bug'} = $bug;
 
 ThrowCodeError("bug_error", { bug => $bug }) if $bug->error;

quips.cgi

@@ -25,15 +25,13 @@
 
 use strict;
 
-use vars qw($userid);
-
 use lib qw(.);
 
 require "globals.pl";
 
 use Bugzilla::Constants;
 
-Bugzilla->login(LOGIN_REQUIRED);
+my $user = Bugzilla->login(LOGIN_REQUIRED);
 
 my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
@@ -82,7 +80,7 @@ if ($action eq "add") {
     trick_taint($comment); # Used in a placeholder below
 
     $dbh->do("INSERT INTO quips (userid, quip, approved) VALUES (?, ?, ?)",
-             undef, ($userid, $comment, $approved));
+             undef, ($user->id, $comment, $approved));
 
     $vars->{'added_quip'} = $comment;
 }

show_bug.cgi

@@ -30,8 +30,6 @@ use Bugzilla::User;
 
 require "globals.pl";
 
-use vars qw($userid);
-
 use Bugzilla::Bug;
 
 my $cgi = Bugzilla->cgi;
@@ -69,7 +67,7 @@ if ($single) {
     # Its a bit silly to do the validation twice - that functionality should
     # probably move into Bug.pm at some point
     ValidateBugID($id);
-    push @bugs, new Bugzilla::Bug($id, $userid);
+    push @bugs, new Bugzilla::Bug($id, Bugzilla->user->id);
     if (defined $cgi->param('mark')) {
         foreach my $range (split ',', $cgi->param('mark')) {
             if ($range =~ /^(\d+)-(\d+)$/) {
@@ -83,7 +81,7 @@ if ($single) {
     }
 } else {
     foreach my $id ($cgi->param('id')) {
-        my $bug = new Bugzilla::Bug($id, $userid);
+        my $bug = new Bugzilla::Bug($id, Bugzilla->user->id);
         push @bugs, $bug;
     }
 }

showdependencygraph.cgi

@@ -44,8 +44,6 @@ my $vars = {};
 # performance.
 Bugzilla->switch_to_shadow_db();
 
-use vars qw($userid);
-
 my %seen;
 my %edgesdone;
 my %bugtitles; # html title attributes for imagemap areas

showdependencytree.cgi

@@ -41,9 +41,6 @@ my $vars = {};
 # performance.
 Bugzilla->switch_to_shadow_db();
 
-# More warning suppression silliness.
-$::userid = $::userid;
-
 ################################################################################
 # Data/Security Validation                                                     #
 ################################################################################