Bug 343338: Eliminate "my" variables from the root level of modules

Bugzilla/BugMail.pm

@@ -49,25 +49,28 @@ use constant BIT_WATCHING  => 2;
 
 # We need these strings for the X-Bugzilla-Reasons header
 # Note: this hash uses "," rather than "=>" to avoid auto-quoting of the LHS.
-my %rel_names = (REL_ASSIGNEE          , "AssignedTo", 
-                 REL_REPORTER          , "Reporter",
+use constant REL_NAMES => {
+    REL_ASSIGNEE, "AssignedTo", 
+    REL_REPORTER, "Reporter",
     REL_QA      , "QAcontact",
     REL_CC      , "CC",
-                 REL_VOTER             , "Voter");
+    REL_VOTER   , "Voter"
+};
 
-# This code is really ugly. It was a commandline interface, then it was moved.
-# This really needs to be cleaned at some point.
-
-my %nomail;
-
-# This is run when we load the package
-if (open(NOMAIL, '<', bz_locations->{'datadir'} . "/nomail")) {
+sub _read_nomail {
+    my $nomail = Bugzilla->request_cache->{bugmail_nomail};
+    return $nomail if $nomail;
+    if (open(NOMAIL, '<', bz_locations->{'datadir'} . "/nomail")) {
         while (<NOMAIL>) {
-        $nomail{trim($_)} = 1;
+            $nomail->{trim($_)} = 1;
         }
         close(NOMAIL);
+    }
+    Bugzilla->request_cache->{bugmail_nomail} = $nomail;
+    return $nomail;
 }
 
+
 sub FormatTriple {
     my ($a, $b, $c) = (@_);
     $^A = "";
@@ -462,7 +465,8 @@ sub ProcessOneBug {
 
             # Make sure the user isn't in the nomail list, and the insider and 
             # dep checks passed.
-            if ((!$nomail{$user->login}) &&
+            my $nomail = _read_nomail();
+            if ((!$nomail->{$user->login}) &&
                 $insider_ok &&
                 $dep_ok)
             {
@@ -625,8 +629,8 @@ sub sendMail {
     $substs{"summary"} = $values{'short_desc'};
     my (@headerrel, @watchingrel);
     while (my ($rel, $bits) = each %{$relRef}) {
-        push @headerrel, ($rel_names{$rel}) if ($bits & BIT_DIRECT);
-        push @watchingrel, ($rel_names{$rel}) if ($bits & BIT_WATCHING);
+        push @headerrel, (REL_NAMES->{$rel}) if ($bits & BIT_DIRECT);
+        push @watchingrel, (REL_NAMES->{$rel}) if ($bits & BIT_WATCHING);
     }
     push @headerrel, 'None' if !scalar(@headerrel);
     push @watchingrel, 'None' if !scalar(@watchingrel);

Bugzilla/Config.pm

@@ -60,9 +60,9 @@ use vars qw(@param_list);
 
 # INITIALISATION CODE
 # Perl throws a warning if we use bz_locations() directly after do.
-my $localconfig = bz_locations()->{'localconfig'};
+our $localconfig = bz_locations()->{'localconfig'};
 do $localconfig;
-my %params;
+our %params;
 # Load in the param definitions
 sub _load_params {
     foreach my $module (param_panels()) {

Bugzilla/FlagType.pm

@@ -70,7 +70,7 @@ use Bugzilla::Util;
 
 =over
 
-=item C<@base_columns>
+=item C<BASE_COLUMNS>
 
 basic sets of columns and tables for getting flag types from the
 database.  B<Used by get, match, sqlify_criteria and perlify_record>
@@ -79,22 +79,23 @@ database.  B<Used by get, match, sqlify_criteria and perlify_record>
 
 =cut
 
-my @base_columns = 
-  ("1", "flagtypes.id", "flagtypes.name", "flagtypes.description", 
+use constant BASE_COLUMNS => (
+    "1", "flagtypes.id", "flagtypes.name", "flagtypes.description", 
     "flagtypes.cc_list", "flagtypes.target_type", "flagtypes.sortkey", 
     "flagtypes.is_active", "flagtypes.is_requestable", 
     "flagtypes.is_requesteeble", "flagtypes.is_multiplicable", 
-   "flagtypes.grant_group_id", "flagtypes.request_group_id");
+    "flagtypes.grant_group_id", "flagtypes.request_group_id",
+);
 
 =pod
 
 =over
 
-=item C<@base_tables>
+=item C<BASE_TABLES>
 
 Which database(s) is the data coming from?
 
-Note: when adding tables to @base_tables, make sure to include the separator 
+Note: when adding tables to BASE_TABLES, make sure to include the separator 
 (i.e. words like "LEFT OUTER JOIN") before the table name, since tables take
 multiple separators based on the join type, and therefore it is not possible
 to join them later using a single known separator.
@@ -106,7 +107,7 @@ B<Used by get, match, sqlify_criteria and perlify_record>
 
 =cut
 
-my @base_tables = ("flagtypes");
+use constant BASE_TABLES => ("flagtypes");
 
 ######################################################################
 # Public Functions
@@ -128,7 +129,7 @@ sub get {
     my ($id) = @_;
     my $dbh = Bugzilla->dbh;
 
-    my $columns = join(", ", @base_columns);
+    my $columns = join(", ", BASE_COLUMNS);
 
     my @data = $dbh->selectrow_array("SELECT $columns FROM flagtypes
                                       WHERE id = ?", undef, $id);
@@ -227,8 +228,9 @@ and returns the set of matching types.
 sub match {
     my ($criteria, $include_count) = @_;
 
-    my @tables = @base_tables;
-    my @columns = @base_columns;
+    my @tables       = BASE_TABLES;
+    my @base_columns = BASE_COLUMNS;
+    my @columns      = BASE_COLUMNS;
     my $dbh = Bugzilla->dbh;
 
     # Include a count of the number of flags per type if requested.
@@ -279,7 +281,7 @@ sub count {
     my ($criteria) = @_;
     my $dbh = Bugzilla->dbh;
 
-    my @tables = @base_tables;
+    my @tables = BASE_TABLES;
     my @criteria = sqlify_criteria($criteria, \@tables);
     # The way tables are joined is already included in @tables.
     my $tables = join(' ', @tables);
@@ -511,7 +513,7 @@ sub sqlify_criteria {
 =item C<perlify_record()>
 
 Converts data retrieved from the database into a Perl record.  Depends on the
-formatting as described in @base_columns.
+formatting as described in C<BASE_COLUMNS>.
 
 =back
 

Bugzilla/Search/Quicksearch.pm

@@ -33,7 +33,8 @@ use base qw(Exporter);
 @Bugzilla::Search::Quicksearch::EXPORT = qw(quicksearch);
 
 # Word renamings
-my %mappings = (# Status, Resolution, Platform, OS, Priority, Severity
+use constant MAPPINGS => {
+                # Status, Resolution, Platform, OS, Priority, Severity
                 "status" => "bug_status",
                 "resolution" => "resolution",  # no change
                 "platform" => "rep_platform",
@@ -83,28 +84,33 @@ my %mappings = (# Status, Resolution, Platform, OS, Priority, Severity
                 "attachmentdata" => "attach_data.thedata",
                 "attachdata" => "attach_data.thedata",
                 "attachmentmimetype" => "attachments.mimetype",
-                "attachmimetype" => "attachments.mimetype");
+                "attachmimetype" => "attachments.mimetype"
+};
 
 # We might want to put this into localconfig or somewhere
-my @platforms = ('pc', 'sun', 'macintosh', 'mac');
-my @productExceptions =   ('row'    # [Browser]
+use constant PLATFORMS => ('pc', 'sun', 'macintosh', 'mac');
+use constant PRODUCT_EXCEPTIONS => (
+    'row',   # [Browser]
              #   ^^^
-                          ,'new'    # [MailNews]
+    'new',   # [MailNews]
              #      ^^^
-                          );
-my @componentExceptions = ('hang'   # [Bugzilla: Component/Keyword Changes]
+);
+use constant COMPONENT_EXCEPTIONS => (
+    'hang'   # [Bugzilla: Component/Keyword Changes]
              #                               ^^^^
-                          );
+);
 
 # Quicksearch-wide globals for boolean charts.
-my $chart = 0;
-my $and = 0;
-my $or = 0;
+our ($chart, $and, $or);
 
 sub quicksearch {
     my ($searchstring) = (@_);
     my $cgi = Bugzilla->cgi;
 
+    $chart = 0;
+    $and   = 0;
+    $or    = 0;
+
     # Remove leading and trailing commas and whitespace.
     $searchstring =~ s/(^[\s,]+|[\s,]+$)//g;
     ThrowUserError('buglist_parameters_required') unless ($searchstring);
@@ -268,8 +274,8 @@ sub quicksearch {
                         my @values = split(/,/, $2);
                         foreach my $field (@fields) {
                             # Be tolerant about unknown fields
-                            next unless defined($mappings{$field});
-                            $field = $mappings{$field};
+                            next unless defined(MAPPINGS->{$field});
+                            $field = MAPPINGS->{$field};
                             foreach (@values) {
                                 addChart($field, 'substring', $_, $negate);
                             }
@@ -282,7 +288,7 @@ sub quicksearch {
                         # by comma, which is another legal boolean OR indicator.
                         foreach my $word (split(/,/, $or_operand)) {
                             # Platform
-                            if (grep({lc($word) eq $_} @platforms)) {
+                            if (grep({lc($word) eq $_} PLATFORMS)) {
                                 addChart('rep_platform', 'substring',
                                          $word, $negate);
                             }
@@ -311,14 +317,14 @@ sub quicksearch {
                             else { # Default QuickSearch word
 
                                 if (!grep({lc($word) eq $_}
-                                          @productExceptions) &&
+                                          PRODUCT_EXCEPTIONS) &&
                                     length($word)>2
                                    ) {
                                     addChart('product', 'substring',
                                              $word, $negate);
                                 }
                                 if (!grep({lc($word) eq $_}
-                                          @componentExceptions) &&
+                                          COMPONENT_EXCEPTIONS) &&
                                     length($word)>2
                                    ) {
                                     addChart('component', 'substring',

Bugzilla/Template.pm

@@ -52,27 +52,28 @@ use base qw(Template);
 # traverse the arrays of exported and exportable symbols, pulling out functions
 # (which is how Perl implements constants) and ignoring the rest (which, if
 # Constants.pm exports only constants, as it should, will be nothing else).
-use Bugzilla::Constants ();
-my %constants;
-foreach my $constant (@Bugzilla::Constants::EXPORT,
+sub _load_constants {
+    use Bugzilla::Constants ();
+    my %constants;
+    foreach my $constant (@Bugzilla::Constants::EXPORT,
                           @Bugzilla::Constants::EXPORT_OK)
-{
+    {
         if (defined &{$Bugzilla::Constants::{$constant}}) {
-        # Constants can be lists, and we can't know whether we're getting
-        # a scalar or a list in advance, since they come to us as the return
-        # value of a function call, so we have to retrieve them all in list
-        # context into anonymous arrays, then extract the scalar ones (i.e.
-        # the ones whose arrays contain a single element) from their arrays.
+            # Constants can be lists, and we can't know whether we're
+            # getting a scalar or a list in advance, since they come to us
+            # as the return value of a function call, so we have to
+            # retrieve them all in list context into anonymous arrays,
+            # then extract the scalar ones (i.e. the ones whose arrays
+            # contain a single element) from their arrays.
             $constants{$constant} = [&{$Bugzilla::Constants::{$constant}}];
             if (scalar(@{$constants{$constant}}) == 1) {
                 $constants{$constant} = @{$constants{$constant}}[0];
             }
         }
+    }
+    return \%constants;
 }
 
-# XXX - mod_perl
-my $template_include_path;
-
 # Make an ordered list out of a HTTP Accept-Language header see RFC 2616, 14.4
 # We ignore '*' and <language-range>;q=0
 # For languages with the same priority q the order remains unchanged.
@@ -105,23 +106,22 @@ sub sortAcceptLanguage {
 sub getTemplateIncludePath {
     # Return cached value if available
 
-    # XXXX - mod_perl!
-    if ($template_include_path) {
-        return $template_include_path;
-    }
+    my $include_path = Bugzilla->request_cache->{template_include_path};
+    return $include_path if $include_path;
+
     my $templatedir = bz_locations()->{'templatedir'};
     my $project     = bz_locations()->{'project'};
 
     my $languages = trim(Bugzilla->params->{'languages'});
     if (not ($languages =~ /,/)) {
        if ($project) {
-           $template_include_path = [
+           $include_path = [
                "$templatedir/$languages/$project",
                "$templatedir/$languages/custom",
                "$templatedir/$languages/default"
            ];
        } else {
-           $template_include_path = [
+           $include_path = [
                "$templatedir/$languages/custom",
                "$templatedir/$languages/default"
            ];
@@ -142,7 +142,7 @@ sub getTemplateIncludePath {
     }
     push(@usedlanguages, Bugzilla->params->{'defaultlanguage'});
     if ($project) {
-        $template_include_path = [
+        $include_path = [
            map((
                "$templatedir/$_/$project",
                "$templatedir/$_/custom",
@@ -151,7 +151,7 @@ sub getTemplateIncludePath {
             )
         ];
     } else {
-        $template_include_path = [
+        $include_path = [
            map((
                "$templatedir/$_/custom",
                "$templatedir/$_/default"
@@ -165,7 +165,7 @@ sub getTemplateIncludePath {
     foreach my $extension (@extensions) {
         trick_taint($extension); # since this comes right from the filesystem
                                  # we have bigger issues if it is insecure
-        push(@$template_include_path,
+        push(@$include_path,
             map((
                 $extension."/template/".$_),
                @usedlanguages));
@@ -173,12 +173,12 @@ sub getTemplateIncludePath {
     
     # remove duplicates since they keep popping up:
     my @dirs;
-    foreach my $dir (@$template_include_path) {
+    foreach my $dir (@$include_path) {
         push(@dirs, $dir) unless grep ($dir eq $_, @dirs);
     }
-    $template_include_path = [@dirs];
+    Bugzilla->request_cache->{template_include_path} = \@dirs;
     
-    return $template_include_path;
+    return Bugzilla->request_cache->{template_include_path};
 }
 
 sub put_header {
@@ -520,7 +520,7 @@ sub create {
     # We need a possibility to reset the cache, so that no files from
     # the previous language pollute the action.
     if ($opts{'clean_cache'}) {
-        $template_include_path = undef;
+        delete Bugzilla->request_cache->{template_include_path};
     }
 
     # IMPORTANT - If you make any configuration changes here, make sure to
@@ -765,7 +765,7 @@ sub create {
 
         PLUGIN_BASE => 'Bugzilla::Template::Plugin',
 
-        CONSTANTS => \%constants,
+        CONSTANTS => _load_constants(),
 
         # Default variables for all templates
         VARIABLES => {

Bugzilla/Token.pm

@@ -41,7 +41,7 @@ use Date::Parse;
 ################################################################################
 
 # The maximum number of days a token will remain valid.
-my $maxtokenage = 3;
+use constant MAX_TOKEN_AGE => 3;
 
 ################################################################################
 # Public Functions
@@ -63,7 +63,7 @@ sub IssueEmailChangeToken {
     $vars->{'oldemailaddress'} = $old_email . $email_suffix;
     $vars->{'newemailaddress'} = $new_email . $email_suffix;
     
-    $vars->{'max_token_age'} = $maxtokenage;
+    $vars->{'max_token_age'} = MAX_TOKEN_AGE;
     $vars->{'token_ts'} = $token_ts;
 
     $vars->{'token'} = $token;
@@ -114,7 +114,7 @@ sub IssuePasswordToken {
     $vars->{'token'} = $token;
     $vars->{'emailaddress'} = $loginname . Bugzilla->params->{'emailsuffix'};
 
-    $vars->{'max_token_age'} = $maxtokenage;
+    $vars->{'max_token_age'} = MAX_TOKEN_AGE;
     $vars->{'token_ts'} = $token_ts;
 
     my $message = "";
@@ -139,7 +139,7 @@ sub CleanTokenTable {
     $dbh->do('DELETE FROM tokens
               WHERE ' . $dbh->sql_to_days('NOW()') . ' - ' .
                         $dbh->sql_to_days('issuedate') . ' >= ?',
-              undef, $maxtokenage);
+              undef, MAX_TOKEN_AGE);
     $dbh->bz_unlock_tables();
 }