diff options
| -rw-r--r-- | Bugzilla/CGI.pm | 12 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 134 | ||||
| -rwxr-xr-x | buglist.cgi | 2 | ||||
| -rwxr-xr-x | collectstats.pl | 2 | ||||
| -rwxr-xr-x | report.cgi | 2 | ||||
| -rwxr-xr-x | whine.pl | 2 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/AndTest.pm | 11 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/FakeCGI.pm | 61 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/FieldTest.pm | 18 | ||||
| -rw-r--r-- | xt/lib/Bugzilla/Test/Search/OrTest.pm | 11 |
10 files changed, 99 insertions, 156 deletions
diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index e2d238f5a..295c57bb2 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -110,7 +110,6 @@ sub new { sub canonicalise_query { my ($self, @exclude) = @_; - $self->convert_old_params(); # Reconstruct the URL by concatenating the sorted param=value pairs my @parameters; foreach my $key (sort($self->param())) { @@ -135,17 +134,6 @@ sub canonicalise_query { return join("&", @parameters); } -sub convert_old_params { - my $self = shift; - - # bugidtype is now bug_id_type. - if ($self->param('bugidtype')) { - my $value = $self->param('bugidtype') eq 'exclude' ? 'nowords' : 'anyexact'; - $self->param('bug_id_type', $value); - $self->delete('bugidtype'); - } -} - sub clean_search_url { my $self = shift; # Delete any empty URL parameter. diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index e2c868dd8..63dfe6d4c 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -332,8 +332,10 @@ use constant SPECIAL_PARSING => { delta_ts => \&_timestamp_translate, }; -# Backwards compatibility for times that we changed the names of fields. +# Backwards compatibility for times that we changed the names of fields +# or URL parameters. use constant FIELD_MAP => { + bugidtype => 'bug_id_type', changedin => 'days_elapsed', long_desc => 'longdesc', }; @@ -680,6 +682,23 @@ sub _multi_select_fields { return $self->{multi_select_fields}; } +# $self->{params} contains values that could be undef, could be a string, +# or could be an arrayref. Sometimes we want that value as an array, +# always. +sub _param_array { + my ($self, $name) = @_; + my $value = $self->_params->{$name}; + if (!defined $value) { + return (); + } + if (ref($value) eq 'ARRAY') { + return @$value; + } + return ($value); +} + +sub _params { $_[0]->{params} } + sub _user { return $_[0]->{user} } ############################## @@ -1068,7 +1087,24 @@ sub _skip_group_by { # Internal Accessors: Special Params Parsing # ############################################## -sub _params { $_[0]->{params} } +# Backwards compatibility for old field names. +sub _convert_old_params { + my ($self) = @_; + my $params = $self->_params; + + # bugidtype has different values in modern Search.pm. + if (defined $params->{'bugidtype'}) { + my $value = $params->{'bugidtype'}; + $params->{'bugidtype'} = $value eq 'exclude' ? 'nowords' : 'anyexact'; + } + + foreach my $old_name (keys %{ FIELD_MAP() }) { + if (defined $params->{$old_name}) { + my $new_name = FIELD_MAP->{$old_name}; + $params->{$new_name} = delete $params->{$old_name}; + } + } +} sub _convert_special_params_to_chart_params { my ($self) = @_; @@ -1078,9 +1114,9 @@ sub _convert_special_params_to_chart_params { # first we delete any sign of "Chart #-1" from the HTML form hash # since we want to guarantee the user didn't hide something here - my @badcharts = grep { /^(field|type|value)-1-/ } $params->param(); + my @badcharts = grep { /^(field|type|value)-1-/ } keys %$params; foreach my $field (@badcharts) { - $params->delete($field); + delete $params->{$field}; } # now we take our special chart and stuff it into the form hash @@ -1088,10 +1124,11 @@ sub _convert_special_params_to_chart_params { my $and = 0; foreach my $or_array (@special_charts) { my $or = 0; + my $identifier = "$chart-$and-$or"; while (@$or_array) { - $params->param("field$chart-$and-$or", shift @$or_array); - $params->param("type$chart-$and-$or", shift @$or_array); - $params->param("value$chart-$and-$or", shift @$or_array); + $params->{"field$identifier"} = shift @$or_array; + $params->{"type$identifier"} = shift @$or_array; + $params->{"value$identifier"} = shift @$or_array; $or++; } $and++; @@ -1102,6 +1139,7 @@ sub _convert_special_params_to_chart_params { # charts. sub _special_charts { my ($self) = @_; + $self->_convert_old_params(); $self->_special_parse_bug_status(); $self->_special_parse_resolution(); my @charts = $self->_parse_basic_fields(); @@ -1116,27 +1154,17 @@ sub _parse_basic_fields { my $params = $self->_params; my $chart_fields = $self->_chart_fields; - foreach my $old_name (keys %{ FIELD_MAP() }) { - if (defined $params->param($old_name)) { - my @value = $params->param($old_name); - $params->delete($old_name); - my $new_name = FIELD_MAP->{$old_name}; - $params->param($new_name, @value); - } - } - my @charts; foreach my $field_name (keys %$chart_fields) { # CGI params shouldn't have periods in them, so we only accept # period-separated fields with underscores where the periods go. my $param_name = $field_name; $param_name =~ s/\./_/g; - next if !defined $params->param($param_name); - my $operator = $params->param("${param_name}_type"); - $operator = 'anyexact' if !$operator; + my @values = $self->_param_array($param_name); + next if !@values; + my $operator = $params->{"${param_name}_type"} || 'anyexact'; $operator = 'matches' if $operator eq 'content'; - my $string_value = join(',', $params->param($param_name)); - push(@charts, [$field_name, $operator, $string_value]); + push(@charts, [$field_name, $operator, \@values]); } return @charts; } @@ -1144,9 +1172,9 @@ sub _parse_basic_fields { sub _special_parse_bug_status { my ($self) = @_; my $params = $self->_params; - return if !defined $params->param('bug_status'); + return if !defined $params->{'bug_status'}; - my @bug_status = $params->param('bug_status'); + my @bug_status = $self->_param_array('bug_status'); # Also include inactive bug statuses, as you can query them. my $legal_statuses = $self->_chart_fields->{'bug_status'}->legal_values; @@ -1172,10 +1200,10 @@ sub _special_parse_bug_status { # If the user has selected every status, change to selecting none. # This is functionally equivalent, but quite a lot faster. if ($all or scalar(@bug_status) == scalar(@$legal_statuses)) { - $params->delete('bug_status'); + delete $params->{'bug_status'}; } else { - $params->param('bug_status', @bug_status); + $params->{'bug_status'} = \@bug_status; } } @@ -1183,14 +1211,12 @@ sub _special_parse_chfield { my ($self) = @_; my $params = $self->_params; - my $date_from = trim(lc($params->param('chfieldfrom'))); - $date_from = '' if !defined $date_from; - my $date_to = trim(lc($params->param('chfieldto'))); - $date_to = '' if !defined $date_to; + my $date_from = trim(lc($params->{'chfieldfrom'} || '')); + my $date_to = trim(lc($params->{'chfieldto'} || '')); $date_from = '' if $date_from eq 'now'; $date_to = '' if $date_to eq 'now'; - my @fields = $params->param('chfield'); - my $value_to = trim($params->param('chfieldvalue')); + my @fields = $self->_param_array('chfield'); + my $value_to = $params->{'chfieldvalue'}; $value_to = '' if !defined $value_to; my @charts; @@ -1260,38 +1286,38 @@ sub _special_parse_deadline { my $params = $self->_params; my @charts; - if (my $from = $params->param('deadlinefrom')) { + if (my $from = $params->{'deadlinefrom'}) { push(@charts, ['deadline', 'greaterthaneq', $from]); } - if (my $to = $params->param('deadlineto')) { + if (my $to = $params->{'deadlineto'}) { push(@charts, ['deadline', 'lessthaneq', $to]); } - - return @charts; + + return @charts; } sub _special_parse_email { my ($self) = @_; my $params = $self->_params; - my @email_params = grep { $_ =~ /^email\d+$/ } $params->param(); + my @email_params = grep { $_ =~ /^email\d+$/ } keys %$params; my @charts; foreach my $param (@email_params) { $param =~ /(\d+)$/; my $id = $1; - my $email = trim($params->param("email$id")); - next if $email eq ""; - my $type = $params->param("emailtype$id"); + my $email = trim($params->{"email$id"}); + next if !$email; + my $type = $params->{"emailtype$id"} || 'anyexact'; $type = "anyexact" if $type eq "exact"; my @or_charts; foreach my $field qw(assigned_to reporter cc qa_contact) { - if ($params->param("email$field$id")) { + if ($params->{"email$field$id"}) { push(@or_charts, $field, $type, $email); } } - if ($params->param("emaillongdesc$id")) { + if ($params->{"emaillongdesc$id"}) { push(@or_charts, "commenter", $type, $email); } @@ -1304,17 +1330,16 @@ sub _special_parse_email { sub _special_parse_resolution { my ($self) = @_; my $params = $self->_params; - return if !defined $params->param('resolution'); + return if !defined $params->{'resolution'}; - my @resolution = $params->param('resolution'); + my @resolution = $self->_param_array('resolution'); my $legal_resolutions = $self->_chart_fields->{resolution}->legal_values; @resolution = _valid_values(\@resolution, $legal_resolutions, '---'); if (scalar(@resolution) == scalar(@$legal_resolutions)) { - $params->delete('resolution'); + delete $params->{'resolution'}; } } - sub _valid_values { my ($input, $valid, $extra_value) = @_; my @result; @@ -1380,11 +1405,8 @@ sub _charts_to_conditions { sub _params_to_charts { my ($self) = @_; my $params = $self->_params; - # XXX This should probably just be moved into using FIELD_MAP here - # in Search.pm. - $params->convert_old_params(); $self->_convert_special_params_to_chart_params(); - my @param_list = $params->param(); + my @param_list = keys %$params; my @all_field_params = grep { /^field-?\d+/ } @param_list; my @chart_ids = map { /^field(-?\d+)/; $1 } @all_field_params; @@ -1410,7 +1432,7 @@ sub _params_to_charts { # meaning that the field, value, or operator were empty. push(@or_charts, $info) if defined $info; } - if ($params->param("negate$chart_id")) { + if ($params->{"negate$chart_id"}) { push(@and_charts, NEGATE); } push(@and_charts, \@or_charts); @@ -1433,12 +1455,12 @@ sub _handle_chart { my $identifier = "$chart_id-$and_id-$or_id"; - my $field = $params->param("field$identifier"); - my $operator = $params->param("type$identifier"); - my $value = $params->param("value$identifier"); + my $field = $params->{"field$identifier"}; + my $operator = $params->{"type$identifier"}; + my @values = $self->_param_array("value$identifier"); - return if (!defined $field or !defined $operator or !defined $value); - $value = trim($value); + return if (!defined $field or !defined $operator or !@values); + my $value = trim(join(',', @values)); return if $value eq ''; $self->_chart_fields->{$field} or ThrowCodeError("invalid_field_name", { field => $field }); @@ -1474,7 +1496,7 @@ sub _handle_chart { return \%search_args; } - + ################################## # do_search_function And Helpers # ################################## diff --git a/buglist.cgi b/buglist.cgi index 878d13d71..0c1615d6f 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -849,7 +849,7 @@ my @orderstrings = split(/,\s*/, $order); # Generate the basic SQL query that will be used to generate the bug list. my $search = new Bugzilla::Search('fields' => \@selectcolumns, - 'params' => $params, + 'params' => scalar $params->Vars, 'order' => \@orderstrings); my $query = $search->sql; $vars->{'search_description'} = $search->search_description; diff --git a/collectstats.pl b/collectstats.pl index f090ba5fc..f5ba2ddab 100755 --- a/collectstats.pl +++ b/collectstats.pl @@ -509,7 +509,7 @@ sub CollectSeriesData { # Do not die if Search->new() detects invalid data, such as an obsolete # login name or a renamed product or component, etc. eval { - my $search = new Bugzilla::Search('params' => $cgi, + my $search = new Bugzilla::Search('params' => scalar $cgi->Vars, 'fields' => ["bug_id"], 'user' => $user); my $sql = $search->sql; diff --git a/report.cgi b/report.cgi index 89f5ff674..dd5a0bb15 100755 --- a/report.cgi +++ b/report.cgi @@ -127,7 +127,7 @@ my @axis_fields = ($row_field || EMPTY_COLUMN, # Clone the params, so that Bugzilla::Search can modify them my $params = new Bugzilla::CGI($cgi); my $search = new Bugzilla::Search('fields' => \@axis_fields, - 'params' => $params); + 'params' => scalar $params->Vars); my $query = $search->sql; $::SIG{TERM} = 'DEFAULT'; @@ -446,7 +446,7 @@ sub run_queries { my $searchparams = new Bugzilla::CGI($savedquery); my $search = new Bugzilla::Search( 'fields' => \@searchfields, - 'params' => $searchparams, + 'params' => scalar $searchparams->Vars, 'user' => $args->{'recipient'}, # the search runs as the recipient ); my $sqlquery = $search->sql; diff --git a/xt/lib/Bugzilla/Test/Search/AndTest.pm b/xt/lib/Bugzilla/Test/Search/AndTest.pm index d7b21af48..b4554584b 100644 --- a/xt/lib/Bugzilla/Test/Search/AndTest.pm +++ b/xt/lib/Bugzilla/Test/Search/AndTest.pm @@ -25,7 +25,6 @@ package Bugzilla::Test::Search::AndTest; use base qw(Bugzilla::Test::Search::OrTest); use Bugzilla::Test::Search::Constants; -use Bugzilla::Test::Search::FakeCGI; use List::MoreUtils qw(all); use constant type => 'AND'; @@ -55,15 +54,15 @@ sub _join_broken_constant { {} } sub search_params { my ($self) = @_; my @all_params = map { $_->search_params } $self->field_tests; - my $params = new Bugzilla::Test::Search::FakeCGI; + my %params; my $chart = 0; foreach my $item (@all_params) { - $params->param("field0-$chart-0", $item->param('field0-0-0')); - $params->param("type0-$chart-0", $item->param('type0-0-0')); - $params->param("value0-$chart-0", $item->param('value0-0-0')); + $params{"field0-$chart-0"} = $item->{'field0-0-0'}; + $params{"type0-$chart-0"} = $item->{'type0-0-0'}; + $params{"value0-$chart-0"} = $item->{'value0-0-0'}; $chart++; } - return $params; + return \%params; } 1;
\ No newline at end of file diff --git a/xt/lib/Bugzilla/Test/Search/FakeCGI.pm b/xt/lib/Bugzilla/Test/Search/FakeCGI.pm deleted file mode 100644 index e20a57daf..000000000 --- a/xt/lib/Bugzilla/Test/Search/FakeCGI.pm +++ /dev/null @@ -1,61 +0,0 @@ -# -*- 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. -# -# The Initial Developer of the Original Code is Everything Solved, Inc. -# Portions created by the Initial Developer are Copyright (C) 2010 the -# Initial Developer. All Rights Reserved. -# -# Contributor(s): -# Max Kanat-Alexander <mkanat@bugzilla.org> - -# Calling CGI::param over and over turned out to be one of the slowest -# parts of search.t. So we create a simpler thing here that just supports -# "param" in a fast way. -package Bugzilla::Test::Search::FakeCGI; - -sub new { - my ($class) = @_; - return bless {}, $class; -} - -sub param { - my ($self, $name, @values) = @_; - if (!defined $name) { - return keys %$self; - } - - if (@values) { - if (ref $values[0] eq 'ARRAY') { - $self->{$name} = $values[0]; - } - else { - $self->{$name} = \@values; - } - } - - return () if !exists $self->{$name}; - - my $item = $self->{$name}; - return wantarray ? @{ $item || [] } : $item->[0]; -} - -sub delete { - my ($self, $name) = @_; - delete $self->{$name}; -} - -# We don't need to do this, because we don't use old params in search.t. -sub convert_old_params {} - -1;
\ No newline at end of file diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm index 4d24c5bd3..558742f71 100644 --- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm +++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm @@ -26,7 +26,6 @@ package Bugzilla::Test::Search::FieldTest; use strict; use warnings; -use Bugzilla::Test::Search::FakeCGI; use Bugzilla::Search; use Bugzilla::Test::Search::Constants; @@ -278,19 +277,16 @@ sub join_broken { # The CGI object that will get passed to Bugzilla::Search as its arguments. sub search_params { - my $self = shift; + my ($self) = @_; return $self->{search_params} if $self->{search_params}; - my $field = $self->field; - my $operator = $self->operator; - my $value = $self->translated_value; - - my $cgi = new Bugzilla::Test::Search::FakeCGI; - $cgi->param("field0-0-0", $field); - $cgi->param('type0-0-0', $operator); - $cgi->param('value0-0-0', $value); + my %params = ( + "field0-0-0" => $self->field, + "type0-0-0" => $self->operator, + "value0-0-0" => $self->translated_value, + ); - $self->{search_params} = $cgi; + $self->{search_params} = \%params; return $self->{search_params}; } diff --git a/xt/lib/Bugzilla/Test/Search/OrTest.pm b/xt/lib/Bugzilla/Test/Search/OrTest.pm index 101e19fd5..35653d0f0 100644 --- a/xt/lib/Bugzilla/Test/Search/OrTest.pm +++ b/xt/lib/Bugzilla/Test/Search/OrTest.pm @@ -25,7 +25,6 @@ package Bugzilla::Test::Search::OrTest; use base qw(Bugzilla::Test::Search::FieldTest); use Bugzilla::Test::Search::Constants; -use Bugzilla::Test::Search::FakeCGI; use List::MoreUtils qw(any uniq); use constant type => 'OR'; @@ -172,15 +171,15 @@ sub search_columns { sub search_params { my ($self) = @_; my @all_params = map { $_->search_params } $self->field_tests; - my $params = new Bugzilla::Test::Search::FakeCGI; + my %params; my $chart = 0; foreach my $item (@all_params) { - $params->param("field0-0-$chart", $item->param('field0-0-0')); - $params->param("type0-0-$chart", $item->param('type0-0-0')); - $params->param("value0-0-$chart", $item->param('value0-0-0')); + $params{"field0-0-$chart"} = $item->{'field0-0-0'}; + $params{"type0-0-$chart"} = $item->{'type0-0-0'}; + $params{"value0-0-$chart"} = $item->{'value0-0-0'}; $chart++; } - return $params; + return \%params; } 1;
\ No newline at end of file |