aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kanat-Alexander <mkanat@bugzilla.org>2010-07-15 19:55:10 -0700
committerMax Kanat-Alexander <mkanat@bugzilla.org>2010-07-15 19:55:10 -0700
commitdbaf1c3aaf975ed78d8e8538b2df18ab9f47654b (patch)
treedb913e59441783297ad780e89cd372897a6ec184
parent6a2a01fbc44c5a777b3a612540a9ced23401c2bc (diff)
downloadbugs-dbaf1c3aaf975ed78d8e8538b2df18ab9f47654b.tar
bugs-dbaf1c3aaf975ed78d8e8538b2df18ab9f47654b.tar.gz
bugs-dbaf1c3aaf975ed78d8e8538b2df18ab9f47654b.tar.bz2
bugs-dbaf1c3aaf975ed78d8e8538b2df18ab9f47654b.tar.xz
bugs-dbaf1c3aaf975ed78d8e8538b2df18ab9f47654b.zip
Bug 398308: Make Search.pm take a hashref for its "params" argument
instead of taking a CGI object. r=mkanat, a=mkanat (module owner)
-rw-r--r--Bugzilla/CGI.pm12
-rw-r--r--Bugzilla/Search.pm134
-rwxr-xr-xbuglist.cgi2
-rwxr-xr-xcollectstats.pl2
-rwxr-xr-xreport.cgi2
-rwxr-xr-xwhine.pl2
-rw-r--r--xt/lib/Bugzilla/Test/Search/AndTest.pm11
-rw-r--r--xt/lib/Bugzilla/Test/Search/FakeCGI.pm61
-rw-r--r--xt/lib/Bugzilla/Test/Search/FieldTest.pm18
-rw-r--r--xt/lib/Bugzilla/Test/Search/OrTest.pm11
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';
diff --git a/whine.pl b/whine.pl
index 789cea79e..3932f854c 100755
--- a/whine.pl
+++ b/whine.pl
@@ -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