From a2eca825a00b33912ec60f797d1112115772ec30 Mon Sep 17 00:00:00 2001 From: "mkanat%kerio.com" <> Date: Thu, 9 Mar 2006 08:08:57 +0000 Subject: Bug 328638: Remove @::legal_keywords and %::keywordsbyname Patch By Max Kanat-Alexander r=LpSolit, a=justdave --- Bugzilla/Bug.pm | 9 +- Bugzilla/Keyword.pm | 213 +++++++++++++++++++++++++++++++++ Bugzilla/Search.pm | 7 +- Bugzilla/Search/Quicksearch.pm | 3 +- buglist.cgi | 4 +- colchange.cgi | 7 +- config.cgi | 4 +- enter_bug.cgi | 4 +- globals.pl | 20 ---- importxml.pl | 11 +- post_bug.cgi | 12 +- process_bug.cgi | 19 +-- query.cgi | 4 +- show_bug.cgi | 3 +- template/en/default/bug/edit.html.tmpl | 2 +- 15 files changed, 259 insertions(+), 63 deletions(-) create mode 100644 Bugzilla/Keyword.pm diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 43f5a1285..c314b8ee1 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -30,7 +30,7 @@ package Bugzilla::Bug; use strict; -use vars qw($legal_keywords @legal_platform +use vars qw(@legal_platform @legal_priority @legal_severity @legal_opsys @legal_bug_status @settable_resolution %components %target_milestone @enterable_products %milestoneurl %prodmaxvotes); @@ -523,11 +523,6 @@ sub show_attachment_flags { return $self->{'show_attachment_flags'}; } - -sub use_keywords { - return @::legal_keywords; -} - sub use_votes { my ($self) = @_; return 0 if $self->{'error'}; @@ -1337,7 +1332,7 @@ sub _validate_attribute { longdescs milestoneurl attachments isopened isunconfirmed flag_types num_attachment_flag_types - show_attachment_flags use_keywords any_flags_requesteeble), + show_attachment_flags any_flags_requesteeble), # Bug fields. Bugzilla::Bug->fields diff --git a/Bugzilla/Keyword.pm b/Bugzilla/Keyword.pm new file mode 100644 index 000000000..b35827288 --- /dev/null +++ b/Bugzilla/Keyword.pm @@ -0,0 +1,213 @@ +# -*- Mode: perl; indent-tabs-mode: nil -*- +# +# The contents of this file are subject to the Mozilla Public +# License Version 1.1 (the "License"); you may not use this file +# except in compliance with the License. You may obtain a copy of +# the License at http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS +# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or +# implied. See the License for the specific language governing +# rights and limitations under the License. +# +# The Original Code is the Bugzilla Bug Tracking System. +# +# Contributor(s): Max Kanat-Alexander + +use strict; + +package Bugzilla::Keyword; + +use Bugzilla::Util; +use Bugzilla::Error; + +############################### +#### Initialization #### +############################### + +use constant DB_COLUMNS => qw( + keyworddefs.id + keyworddefs.name + keyworddefs.description +); + +my $columns = join(", ", DB_COLUMNS); + +sub new { + my $invocant = shift; + my $class = ref($invocant) || $invocant; + my $self = {}; + bless($self, $class); + return $self->_init(@_); +} + +sub _init { + my $self = shift; + my ($param) = @_; + my $dbh = Bugzilla->dbh; + + my $id = $param unless (ref $param eq 'HASH'); + my $keyword; + + if (defined $id) { + detaint_natural($id) + || ThrowCodeError('param_must_be_numeric', + {function => 'Bugzilla::Keyword::_init'}); + + $keyword = $dbh->selectrow_hashref(qq{ + SELECT $columns FROM keyworddefs + WHERE id = ?}, undef, $id); + } elsif (defined $param->{'name'}) { + trick_taint($param->{'name'}); + $keyword = $dbh->selectrow_hashref(qq{ + SELECT $columns FROM keyworddefs + WHERE name = ?}, undef, $param->{'name'}); + } else { + ThrowCodeError('bad_arg', + {argument => 'param', + function => 'Bugzilla::Keyword::_init'}); + } + + return undef unless (defined $keyword); + + foreach my $field (keys %$keyword) { + $self->{$field} = $keyword->{$field}; + } + return $self; +} + +sub new_from_list { + my $class = shift; + my ($id_list) = @_; + my $dbh = Bugzilla->dbh; + + my $keywords; + if ($id_list) { + my @detainted_ids; + foreach my $id (@$id_list) { + detaint_natural($id) || + ThrowCodeError('param_must_be_numeric', + {function => 'Bugzilla::Keyword::new_from_list'}); + push(@detainted_ids, $id); + } + $keywords = $dbh->selectall_arrayref( + "SELECT $columns FROM keyworddefs WHERE id IN (" + . join(',', @detainted_ids) . ")", {Slice=>{}}) || []; + } else { + ThrowCodeError('bad_arg', + {argument => 'id_list', + function => 'Bugzilla::Keyword::new_from_list'}); + } + + foreach my $keyword (@$keywords) { + bless($keyword, $class); + } + return $keywords; +} + +############################### +#### Accessors ###### +############################### + +sub id { return $_[0]->{'id'}; } +sub name { return $_[0]->{'name'}; } +sub description { return $_[0]->{'description'}; } + +############################### +#### Subroutines ###### +############################### + +sub get_all_keywords { + my $dbh = Bugzilla->dbh; + + my $ids = $dbh->selectcol_arrayref(q{ + SELECT id FROM keyworddefs ORDER BY name}); + + my $keywords = Bugzilla::Keyword->new_from_list($ids); + return @$keywords; +} + +sub keyword_count { + my ($count) = + Bugzilla->dbh->selectrow_array('SELECT COUNT(*) FROM keyworddefs'); + return $count; +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::Keyword - A Keyword that can be added to a bug. + +=head1 SYNOPSIS + + use Bugzilla::Keyword; + + my $keyword = new Bugzilla::Keyword(1); + my $keyword = new Bugzilla::Keyword({name => 'perf'}); + + my $id = $keyword->id; + my $name = $keyword->name; + my $description = $keyword->description; + +=head1 DESCRIPTION + +Bugzilla::Keyword represents a keyword that can be added to a bug. + +=head1 METHODS + +=over + +=item C + + Description: The constructor is used to load an existing keyword + by passing a keyword id or a hash. + + Params: $param - If you pass an integer, the integer is the + keyword id from the database that we want to + read in. If you pass in a hash with 'name' key, + then the value of the name key is the name of a + keyword from the DB. + + Returns: A Bugzilla::Keyword object. + +=item C + + Description: Creates an array of Keyword objects, given an + array of ids. + + Params: \@id_list - A reference to an array of numbers, keyword ids. + If any of these are not numeric, the function + will throw an error. If any of these are not + valid keyword ids, they will simply be skipped. + + Returns: A reference to an array of C objects. + +=back + +=head1 SUBROUTINES + +=over + +=item C + + Description: Returns all keywords from the database. + + Params: none. + + Returns: A list of C objects, + or an empty list if there are none. + +=item C + + Description: A utility function to get the total number + of keywords defined. Mostly used to see + if there are any keywords defined at all. + Params: none + Returns: An integer, the count of keywords. + +=back + +=cut diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 0e1f7057d..181f49b19 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -42,6 +42,7 @@ use Bugzilla::Group; use Bugzilla::User; use Bugzilla::Field; use Bugzilla::Bug; +use Bugzilla::Keyword; use Date::Format; use Date::Parse; @@ -933,9 +934,9 @@ sub init { if ($value eq '') { next; } - my $id = &::GetKeywordIdFromName($value); - if ($id) { - push(@list, "$table.keywordid = $id"); + my $keyword = new Bugzilla::Keyword({name => $value}); + if ($keyword) { + push(@list, "$table.keywordid = " . $keyword->id); } else { ThrowUserError("unknown_keyword", diff --git a/Bugzilla/Search/Quicksearch.pm b/Bugzilla/Search/Quicksearch.pm index 4d9958c63..e4224b959 100644 --- a/Bugzilla/Search/Quicksearch.pm +++ b/Bugzilla/Search/Quicksearch.pm @@ -27,6 +27,7 @@ use Bugzilla; use Bugzilla::Config; use Bugzilla::Error; use Bugzilla::Constants; +use Bugzilla::Keyword; use Bugzilla::Bug; use base qw(Exporter); @@ -325,7 +326,7 @@ sub quicksearch { $word, $negate); } if (grep({lc($word) eq $_} - @::legal_keywords)) { + map($_->name, Bugzilla::Keyword::get_all_keywords()))) { addChart('keywords', 'substring', $word, $negate); if (length($word)>2) { diff --git a/buglist.cgi b/buglist.cgi index c778c5f10..0e6fb2ba5 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -41,12 +41,12 @@ use Bugzilla::Constants; use Bugzilla::User; use Bugzilla::Bug; use Bugzilla::Product; +use Bugzilla::Keyword; # Include the Bugzilla CGI and general utility library. require "globals.pl"; use vars qw(@components - @legal_keywords @legal_platform @legal_priority @legal_product @@ -1042,7 +1042,7 @@ $vars->{'currenttime'} = time(); # The following variables are used when the user is making changes to multiple bugs. if ($dotweak) { $vars->{'dotweak'} = 1; - $vars->{'use_keywords'} = 1 if @::legal_keywords; + $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); $vars->{'products'} = Bugzilla->user->get_enterable_products; $vars->{'platforms'} = \@::legal_platform; diff --git a/colchange.cgi b/colchange.cgi index 43bfd86ee..72ce01f44 100755 --- a/colchange.cgi +++ b/colchange.cgi @@ -25,13 +25,10 @@ use strict; use lib qw(.); -use vars qw( - @legal_keywords -); - use Bugzilla; use Bugzilla::Constants; use Bugzilla::User; +use Bugzilla::Keyword; require "globals.pl"; Bugzilla->login(); @@ -71,7 +68,7 @@ if (Param("useqacontact")) { if (Param("usestatuswhiteboard")) { push(@masterlist, "status_whiteboard"); } -if (@::legal_keywords) { +if (Bugzilla::Keyword::keyword_count()) { push(@masterlist, "keywords"); } diff --git a/config.cgi b/config.cgi index 371383e1c..f3471e14f 100755 --- a/config.cgi +++ b/config.cgi @@ -33,6 +33,7 @@ use lib qw(.); require "globals.pl"; use Bugzilla; use Bugzilla::Constants; +use Bugzilla::Keyword; use Bugzilla::Bug; # Suppress "used only once" warnings. @@ -46,7 +47,6 @@ use vars @legal_components @legal_target_milestone - @legal_keywords ); # Use the global template variables defined in globals.pl @@ -69,7 +69,7 @@ $vars->{'priority'} = \@::legal_priority; $vars->{'severity'} = \@::legal_severity; $vars->{'platform'} = \@::legal_platform; $vars->{'op_sys'} = \@::legal_opsys; -$vars->{'keyword'} = \@::legal_keywords; +$vars->{'keyword'} = [map($_->name, Bugzilla::Keyword::get_all_keywords())]; $vars->{'resolution'} = \@::legal_resolution; $vars->{'status'} = \@::legal_bug_status; diff --git a/enter_bug.cgi b/enter_bug.cgi index 3e87bf405..d1d070148 100755 --- a/enter_bug.cgi +++ b/enter_bug.cgi @@ -43,6 +43,7 @@ use Bugzilla::Bug; use Bugzilla::User; use Bugzilla::Hook; use Bugzilla::Product; +use Bugzilla::Keyword; require "globals.pl"; use vars qw( @@ -51,7 +52,6 @@ use vars qw( @legal_platform @legal_priority @legal_severity - @legal_keywords %target_milestone ); @@ -375,7 +375,7 @@ $vars->{'bug_severity'} = \@legal_severity; $vars->{'rep_platform'} = \@legal_platform; $vars->{'op_sys'} = \@legal_opsys; -$vars->{'use_keywords'} = 1 if (@::legal_keywords); +$vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); $vars->{'assigned_to'} = formvalue('assigned_to'); $vars->{'assigned_to_disabled'} = !UserInGroup('editbugs'); diff --git a/globals.pl b/globals.pl index be3df7d75..556ea3f3e 100644 --- a/globals.pl +++ b/globals.pl @@ -45,10 +45,8 @@ use Bugzilla::Error; sub globals_pl_sillyness { my $zz; $zz = @main::enterable_products; - $zz = %main::keywordsbyname; $zz = @main::legal_bug_status; $zz = @main::legal_components; - $zz = @main::legal_keywords; $zz = @main::legal_opsys; $zz = @main::legal_platform; $zz = @main::legal_priority; @@ -258,17 +256,6 @@ sub GenerateVersionTable { '*::milestoneurl'])); } - SendSQL("SELECT id, name FROM keyworddefs ORDER BY name"); - while (MoreSQLData()) { - my ($id, $name) = FetchSQLData(); - push(@::legal_keywords, $name); - $name = lc($name); - $::keywordsbyname{$name} = $id; - } - - print $fh (Data::Dumper->Dump([\@::legal_keywords, \%::keywordsbyname], - ['*::legal_keywords', '*::keywordsbyname'])); - print $fh "1;\n"; close $fh; @@ -278,13 +265,6 @@ sub GenerateVersionTable { } -sub GetKeywordIdFromName { - my ($name) = (@_); - $name = lc($name); - return $::keywordsbyname{$name}; -} - - $::VersionTableLoaded = 0; sub GetVersionTable { return if $::VersionTableLoaded; diff --git a/importxml.pl b/importxml.pl index 7b96cb59e..494a00eca 100755 --- a/importxml.pl +++ b/importxml.pl @@ -84,6 +84,7 @@ use Bugzilla::BugMail; use Bugzilla::User; use Bugzilla::Util; use Bugzilla::Constants; +use Bugzilla::Keyword; use MIME::Base64; use MIME::Parser; @@ -1070,14 +1071,14 @@ sub process_bug { ); foreach my $keyword ( split( /[\s,]+/, $bug_fields{'keywords'} )) { next unless $keyword; - my $i = GetKeywordIdFromName($keyword); - if ( !$i ) { + my $keyword_obj = new Bugzilla::Keyword({name => $keyword}); + if (!$keyword_obj) { $err .= "Skipping unknown keyword: $keyword.\n"; next; } - if ( !$keywordseen{$i} ) { - $key_sth->execute( $id, $i ); - $keywordseen{$i} = 1; + if (!$keywordseen{$keyword_obj->id}) { + $key_sth->execute($id, $keyword_obj->id); + $keywordseen{$keyword_obj->id} = 1; } } my ($keywordarray) = $dbh->selectcol_arrayref( diff --git a/post_bug.cgi b/post_bug.cgi index 3e589ebe8..de8063543 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -34,6 +34,7 @@ use Bugzilla::Bug; use Bugzilla::User; use Bugzilla::Field; use Bugzilla::Product; +use Bugzilla::Keyword; # Shut up misguided -w warnings about "used only once". For some reason, # "use vars" chokes on me when I try it here. @@ -273,14 +274,14 @@ if ($cgi->param('keywords') && UserInGroup("editbugs")) { if ($keyword eq '') { next; } - my $i = GetKeywordIdFromName($keyword); - if (!$i) { + my $keyword_obj = new Bugzilla::Keyword({name => $keyword}); + if (!$keyword_obj) { ThrowUserError("unknown_keyword", { keyword => $keyword }); } - if (!$keywordseen{$i}) { - push(@keywordlist, $i); - $keywordseen{$i} = 1; + if (!$keywordseen{$keyword_obj->id}) { + push(@keywordlist, $keyword_obj->id); + $keywordseen{$keyword_obj->id} = 1; } } } @@ -518,6 +519,7 @@ if ($cgi->cookie("BUGLIST")) { @bug_list = split(/:/, $cgi->cookie("BUGLIST")); } $vars->{'bug_list'} = \@bug_list; +$vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); print $cgi->header(); $template->process("bug/create/created.html.tmpl", $vars) diff --git a/process_bug.cgi b/process_bug.cgi index c2605afc2..d1efd3825 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -57,6 +57,7 @@ use Bugzilla::User; use Bugzilla::Util; use Bugzilla::Field; use Bugzilla::Product; +use Bugzilla::Keyword; # Use the Flag module to modify flag data if the user set flags. use Bugzilla::Flag; @@ -82,6 +83,7 @@ my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; my $template = Bugzilla->template; my $vars = {}; +$vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); my $requiremilestone = 0; @@ -1278,14 +1280,14 @@ if (defined $cgi->param('keywords')) { if ($keyword eq '') { next; } - my $i = GetKeywordIdFromName($keyword); - if (!$i) { + my $keyword_obj = new Bugzilla::Keyword({name => $keyword}); + if (!$keyword_obj) { ThrowUserError("unknown_keyword", { keyword => $keyword }); } - if (!$keywordseen{$i}) { - push(@keywordlist, $i); - $keywordseen{$i} = 1; + if (!$keywordseen{$keyword_obj->id}) { + push(@keywordlist, $keyword_obj->id); + $keywordseen{$keyword_obj->id} = 1; } } } @@ -1297,7 +1299,8 @@ if (!grep($keywordaction eq $_, qw(add delete makeexact))) { if ($::comma eq "" && (! @groupAdd) && (! @groupDel) - && (! @::legal_keywords || (0 == @keywordlist && $keywordaction ne "makeexact")) + && (!Bugzilla::Keyword::keyword_count() + || (0 == @keywordlist && $keywordaction ne "makeexact")) && defined $cgi->param('masscc') && ! $cgi->param('masscc') ) { if (!defined $cgi->param('comment') || $cgi->param('comment') =~ /^\s*$/) { @@ -1651,7 +1654,9 @@ foreach my $id (@idlist) { $bug_changed = 1; } - if (@::legal_keywords && defined $cgi->param('keywords')) { + if (Bugzilla::Keyword::keyword_count() + && defined $cgi->param('keywords')) + { # There are three kinds of "keywordsaction": makeexact, add, delete. # For makeexact, we delete everything, and then add our things. # For add, we delete things we're adding (to make sure we don't diff --git a/query.cgi b/query.cgi index 33f2d19fa..9bb90e782 100755 --- a/query.cgi +++ b/query.cgi @@ -36,12 +36,12 @@ use Bugzilla::User; use Bugzilla::Util; use Bugzilla::Product; use Bugzilla::Version; +use Bugzilla::Keyword; use vars qw( @legal_resolution @legal_bug_status @legal_components - @legal_keywords @legal_opsys @legal_platform @legal_priority @@ -275,7 +275,7 @@ if (Param('usetargetmilestone')) { $vars->{'target_milestone'} = \@milestones; } -$vars->{'have_keywords'} = scalar(@::legal_keywords); +$vars->{'have_keywords'} = Bugzilla::Keyword::keyword_count(); push @::legal_resolution, "---"; # Oy, what a hack. shift @::legal_resolution; diff --git a/show_bug.cgi b/show_bug.cgi index 19aa88573..7f846efea 100755 --- a/show_bug.cgi +++ b/show_bug.cgi @@ -27,6 +27,7 @@ use lib qw(.); use Bugzilla; use Bugzilla::Constants; use Bugzilla::User; +use Bugzilla::Keyword; require "globals.pl"; @@ -94,7 +95,7 @@ eval { $vars->{'bugs'} = \@bugs; $vars->{'marks'} = \%marks; -$vars->{'use_keywords'} = 1 if (@::legal_keywords); +$vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); # Next bug in list (if there is one) my @bug_list; diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl index 02fb20d61..b08ce6432 100644 --- a/template/en/default/bug/edit.html.tmpl +++ b/template/en/default/bug/edit.html.tmpl @@ -394,7 +394,7 @@ [% END %] - [% IF bug.use_keywords %] + [% IF use_keywords %] -- cgit v1.2.3-24-g4f1b