summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kanat-Alexander <mkanat@bugzilla.org>2011-04-02 18:49:04 +0200
committerMax Kanat-Alexander <mkanat@bugzilla.org>2011-04-02 18:49:04 +0200
commitd5c5177f2ef698aefa8aeffaa458016583c20f79 (patch)
treea4cc7750fd3febf12d13cea89a3b1677e38559c6
parent14151ea2f8fb33d4336afa161a0e8b8bed6e5daa (diff)
downloadbugzilla-d5c5177f2ef698aefa8aeffaa458016583c20f79.tar.gz
bugzilla-d5c5177f2ef698aefa8aeffaa458016583c20f79.tar.xz
Bug 640045: Convert Search.pm to use the new AND/OR system internally.
This includes creating new Search::Clause and Search::Condition objects. r=mkanat, a=mkanat (module owner)
-rw-r--r--Bugzilla/Search.pm240
-rw-r--r--Bugzilla/Search/Clause.pm113
-rw-r--r--Bugzilla/Search/Condition.pm66
3 files changed, 272 insertions, 147 deletions
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 5aef38c05..ab0bdae89 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -49,6 +49,7 @@ use Bugzilla::Constants;
use Bugzilla::Group;
use Bugzilla::User;
use Bugzilla::Field;
+use Bugzilla::Search::Clause;
use Bugzilla::Status;
use Bugzilla::Keyword;
@@ -125,7 +126,6 @@ use Storable qw(dclone);
# bar@blah.org
# --------------------------------------------------------------
-
#############
# Constants #
#############
@@ -400,10 +400,6 @@ use constant FIELD_MAP => {
# <none> for the X, Y, or Z axis in report.cgi.
use constant EMPTY_COLUMN => '-1';
-# A special value that is pushed into charts during _params_to_charts to
-# represent that the particular chart we're dealing with should be negated.
-use constant NEGATE => 'NOT';
-
# Some fields are not sorted on themselves, but on other fields.
# We need to have a list of these fields and what they map to.
use constant SPECIAL_ORDER => {
@@ -694,14 +690,12 @@ sub sql {
return $self->{sql} if $self->{sql};
my $dbh = Bugzilla->dbh;
- my ($joins, $having_terms, $where_terms) = $self->_charts_to_conditions();
+ my ($joins, $clause) = $self->_charts_to_conditions();
my $select = join(', ', $self->_sql_select);
my $from = $self->_sql_from($joins);
- my $where = $self->_sql_where($where_terms);
+ my $where = $self->_sql_where($clause);
my $group_by = $dbh->sql_group_by($self->_sql_group_by);
- my $having = @$having_terms
- ? "\nHAVING " . join(' AND ', @$having_terms) : '';
my $order_by = $self->_sql_order_by
? "\nORDER BY " . join(', ', $self->_sql_order_by) : '';
my $limit = $self->_sql_limit;
@@ -711,7 +705,7 @@ sub sql {
SELECT $select
FROM $from
WHERE $where
-$group_by$having$order_by$limit
+$group_by$order_by$limit
END
$self->{sql} = $query;
return $self->{sql};
@@ -1135,13 +1129,15 @@ sub _standard_where {
my $user = $self->_user;
if ($user->id) {
my $userid = $user->id;
+ # This indentation makes the resulting SQL more readable.
$security_term .= <<END;
- OR (bugs.reporter_accessible = 1 AND bugs.reporter = $userid)
- OR (bugs.cclist_accessible = 1 AND security_cc.who IS NOT NULL)
- OR bugs.assigned_to = $userid
+
+ OR (bugs.reporter_accessible = 1 AND bugs.reporter = $userid)
+ OR (bugs.cclist_accessible = 1 AND security_cc.who IS NOT NULL)
+ OR bugs.assigned_to = $userid
END
if (Bugzilla->params->{'useqacontact'}) {
- $security_term.= " OR bugs.qa_contact = $userid";
+ $security_term.= " OR bugs.qa_contact = $userid";
}
$security_term = "($security_term)";
}
@@ -1152,10 +1148,15 @@ END
}
sub _sql_where {
- my ($self, $where_terms) = @_;
+ my ($self, $main_clause) = @_;
# The newline and this particular spacing makes the resulting
# SQL a bit more readable for debugging.
- return join("\n AND ", $self->_standard_where, @$where_terms);
+ my $where = join("\n AND ", $self->_standard_where);
+ my $clause_sql = $main_clause->as_string;
+ if ($clause_sql) {
+ $where .= "\n AND " . $clause_sql;
+ }
+ return $where;
}
################################
@@ -1224,35 +1225,6 @@ sub _convert_old_params {
}
}
-sub _convert_special_params_to_chart_params {
- my ($self) = @_;
- my $params = $self->_params;
-
- my @special_charts = $self->_special_charts();
-
- # First we delete any sign of "Chart #-1" from the input parameters,
- # because we want to guarantee the user didn't hide something there.
- my @badcharts = grep { /^(field|type|value)-1-/ } keys %$params;
- foreach my $field (@badcharts) {
- delete $params->{$field};
- }
-
- # now we take our special chart and stuff it into the form hash
- my $chart = -1;
- my $and = 0;
- foreach my $or_array (@special_charts) {
- my $or = 0;
- while (@$or_array) {
- my $identifier = "$chart-$and-$or";
- $params->{"field$identifier"} = shift @$or_array;
- $params->{"type$identifier"} = shift @$or_array;
- $params->{"value$identifier"} = shift @$or_array;
- $or++;
- }
- $and++;
- }
-}
-
# This parses all the standard search parameters except for the boolean
# charts.
sub _special_charts {
@@ -1260,11 +1232,12 @@ sub _special_charts {
$self->_convert_old_params();
$self->_special_parse_bug_status();
$self->_special_parse_resolution();
- my @charts = $self->_parse_basic_fields();
- push(@charts, $self->_special_parse_email());
- push(@charts, $self->_special_parse_chfield());
- push(@charts, $self->_special_parse_deadline());
- return @charts;
+ my $clause = new Bugzilla::Search::Clause();
+ $clause->add( $self->_parse_basic_fields() );
+ $clause->add( $self->_special_parse_email() );
+ $clause->add( $self->_special_parse_chfield() );
+ $clause->add( $self->_special_parse_deadline() );
+ return $clause;
}
sub _parse_basic_fields {
@@ -1272,7 +1245,7 @@ sub _parse_basic_fields {
my $params = $self->_params;
my $chart_fields = $self->_chart_fields;
- my @charts;
+ my $clause = new Bugzilla::Search::Clause();
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.
@@ -1296,9 +1269,9 @@ sub _parse_basic_fields {
else {
$pass_value = join(',', @values);
}
- push(@charts, [$field_name, $operator, $pass_value]);
+ $clause->add($field_name, $operator, $pass_value);
}
- return @charts;
+ return $clause;
}
sub _special_parse_bug_status {
@@ -1358,7 +1331,8 @@ sub _special_parse_chfield {
@fields = map { $_ eq '[Bug creation]' ? 'creation_ts' : $_ } @fields;
- my @charts;
+ my $clause = new Bugzilla::Search::Clause();
+
# It is always safe and useful to push delta_ts into the charts
# if there is a "from" date specified. It doesn't conflict with
# searching [Bug creation], because a bug's delta_ts is set to
@@ -1366,7 +1340,7 @@ sub _special_parse_chfield {
# database an additional index to possibly choose, on a table that
# is smaller than bugs_activity.
if ($date_from ne '') {
- push(@charts, ['delta_ts', 'greaterthaneq', $date_from]);
+ $clause->add('delta_ts', 'greaterthaneq', $date_from);
}
# It's not normally safe to do it for "to" dates, though--"chfieldto" means
# "a field that changed before this date", and delta_ts could be either
@@ -1375,7 +1349,7 @@ sub _special_parse_chfield {
# chfield, means "just search delta_ts", and so we still want that to
# work.
if ($date_to ne '' and !@fields and $value_to eq '') {
- push(@charts, ['delta_ts', 'lessthaneq', $date_to]);
+ $clause->add('delta_ts', 'lessthaneq', $date_to);
}
# Basically, we construct the chart like:
@@ -1390,29 +1364,29 @@ sub _special_parse_chfield {
# change date of the fields.
if ($value_to ne '') {
- my @value_chart;
+ my $value_clause = new Bugzilla::Search::Clause('OR');
foreach my $field (@fields) {
- push(@value_chart, $field, 'changedto', $value_to);
+ $value_clause->add($field, 'changedto', $value_to);
}
- push(@charts, \@value_chart) if @value_chart;
+ $clause->add($value_clause);
}
if ($date_from ne '') {
- my @date_from_chart;
+ my $from_clause = new Bugzilla::Search::Clause('OR');
foreach my $field (@fields) {
- push(@date_from_chart, $field, 'changedafter', $date_from);
+ $from_clause->add($field, 'changedafter', $date_from);
}
- push(@charts, \@date_from_chart) if @date_from_chart;
+ $clause->add($from_clause);
}
if ($date_to ne '') {
- my @date_to_chart;
+ my $to_clause = new Bugzilla::Search::Clause('OR');
foreach my $field (@fields) {
- push(@date_to_chart, $field, 'changedbefore', $date_to);
+ $to_clause->add($field, 'changedbefore', $date_to);
}
- push(@charts, \@date_to_chart) if @date_to_chart;
+ $clause->add($to_clause);
}
- return @charts;
+ return $clause;
}
sub _special_parse_deadline {
@@ -1420,15 +1394,15 @@ sub _special_parse_deadline {
return if !$self->_user->is_timetracker;
my $params = $self->_params;
- my @charts;
+ my $clause = new Bugzilla::Search::Clause();
if (my $from = $params->{'deadlinefrom'}) {
- push(@charts, ['deadline', 'greaterthaneq', $from]);
+ $clause->add('deadline', 'greaterthaneq', $from);
}
if (my $to = $params->{'deadlineto'}) {
- push(@charts, ['deadline', 'lessthaneq', $to]);
+ $clause->add('deadline', 'lessthaneq', $to);
}
- return @charts;
+ return $clause;
}
sub _special_parse_email {
@@ -1437,7 +1411,7 @@ sub _special_parse_email {
my @email_params = grep { $_ =~ /^email\d+$/ } keys %$params;
- my @charts;
+ my $clause = new Bugzilla::Search::Clause();
foreach my $param (@email_params) {
$param =~ /(\d+)$/;
my $id = $1;
@@ -1446,20 +1420,20 @@ sub _special_parse_email {
my $type = $params->{"emailtype$id"} || 'anyexact';
$type = "anyexact" if $type eq "exact";
- my @or_charts;
+ my $or_clause = new Bugzilla::Search::Clause('OR');
foreach my $field (qw(assigned_to reporter cc qa_contact)) {
if ($params->{"email$field$id"}) {
- push(@or_charts, $field, $type, $email);
+ $or_clause->add($field, $type, $email);
}
}
if ($params->{"emaillongdesc$id"}) {
- push(@or_charts, "commenter", $type, $email);
+ $or_clause->add("commenter", $type, $email);
}
-
- push(@charts, \@or_charts);
+
+ $clause->add($or_clause);
}
- return @charts;
+ return $clause;
}
sub _special_parse_resolution {
@@ -1496,52 +1470,36 @@ sub _valid_values {
sub _charts_to_conditions {
my ($self) = @_;
- my @charts = $self->_params_to_charts();
-
- my (@joins, @having, @where_terms);
- foreach my $chart (@charts) {
- my @and_terms;
- my $negate;
- foreach my $and_item (@$chart) {
- if (!ref $and_item and $and_item eq NEGATE) {
- $negate = 1;
- next;
- }
- my @or_terms;
- foreach my $or_item (@$and_item) {
- if ($or_item->{term} ne '') {
- push(@or_terms, $or_item->{term});
- }
- push(@joins, @{ $or_item->{joins} });
- push(@having, @{ $or_item->{having} });
- }
-
- if (@or_terms) {
- # If a term contains ANDs, we need to put parens around the
- # condition. This is a pretty weak test, but it's actually OK
- # to put parens around too many things.
- @or_terms = map { $_ =~ /\bAND\b/i ? "($_)" : $_ } @or_terms;
- my $or_sql = join(' OR ', @or_terms);
- push(@and_terms, $or_sql);
- }
- }
- # And here we need to paren terms that contain ORs.
- @and_terms = map { $_ =~ /\bOR\b/i ? "($_)" : $_ } @and_terms;
- my $and_sql = join(' AND ', @and_terms);
- if ($negate and $and_sql ne '') {
- $and_sql = "NOT ($and_sql)";
- }
- push(@where_terms, $and_sql) if $and_sql ne '';
- }
+ my $clause = $self->_charts;
+ my @joins;
+ $clause->walk_conditions(sub {
+ my ($condition) = @_;
+ return if !$condition->translated;
+ push(@joins, @{ $condition->translated->{joins} });
+ });
+ return (\@joins, $clause);
+}
- return (\@joins, \@having, \@where_terms);
+sub _charts {
+ my ($self) = @_;
+
+ my $clause = $self->_params_to_data_structure();
+ my $chart_id = 0;
+ $clause->walk_conditions(sub { $self->_handle_chart($chart_id++, @_) });
+ return $clause;
}
-sub _params_to_charts {
+sub _params_to_data_structure {
my ($self) = @_;
+
+ # First we get the "special" charts, representing all the normal
+ # field son the search page. This may modify _params, so it needs to
+ # happen first.
+ my $clause = $self->_special_charts;
+
+ # Then we process the old Boolean Charts input format.
my $params = $self->_params;
- $self->_convert_special_params_to_chart_params();
my @param_list = keys %$params;
my @all_field_params = grep { /^field-?\d+/ } @param_list;
@@ -1549,54 +1507,43 @@ sub _params_to_charts {
@chart_ids = sort { $a <=> $b } uniq @chart_ids;
my $sequence = 0;
- my @charts;
foreach my $chart_id (@chart_ids) {
my @all_and = grep { /^field$chart_id-\d+/ } @param_list;
my @and_ids = map { /^field$chart_id-(\d+)/; $1 } @all_and;
@and_ids = sort { $a <=> $b } uniq @and_ids;
- my @and_charts;
+ my $and_clause = new Bugzilla::Search::Clause();
foreach my $and_id (@and_ids) {
my @all_or = grep { /^field$chart_id-$and_id-\d+/ } @param_list;
my @or_ids = map { /^field$chart_id-$and_id-(\d+)/; $1 } @all_or;
@or_ids = sort { $a <=> $b } uniq @or_ids;
- my @or_charts;
+ my $or_clause = new Bugzilla::Search::Clause('OR');
foreach my $or_id (@or_ids) {
- my $info = $self->_handle_chart($chart_id, $and_id, $or_id);
- # $info will be undefined if _handle_chart returned early,
- # meaning that the field, value, or operator were empty.
- push(@or_charts, $info) if defined $info;
- }
- if ($params->{"negate$chart_id"}) {
- push(@and_charts, NEGATE);
+ my $identifier = "$chart_id-$and_id-$or_id";
+ my $field = $params->{"field$identifier"};
+ my $operator = $params->{"type$identifier"};
+ my $value = $params->{"value$identifier"};
+ $or_clause->add($field, $operator, $value);
}
- push(@and_charts, \@or_charts);
+ $and_clause->add($or_clause);
+
+ $and_clause->negate(1) if $params->{"negate$chart_id"};
}
- push(@charts, \@and_charts);
+ $clause->add($and_clause);
}
- return @charts;
+ return $clause;
}
sub _handle_chart {
- my ($self, $chart_id, $and_id, $or_id) = @_;
+ my ($self, $chart_id, $condition) = @_;
my $dbh = Bugzilla->dbh;
my $params = $self->_params;
-
- my $sql_chart_id = $chart_id;
- if ($chart_id < 0) {
- $sql_chart_id = "default_" . abs($chart_id);
- }
-
- my $identifier = "$chart_id-$and_id-$or_id";
-
- my $field = $params->{"field$identifier"};
- my $operator = $params->{"type$identifier"};
- my $value = $params->{"value$identifier"};
+ my ($field, $operator, $value) = $condition->fov;
return if (!defined $field or !defined $operator or !defined $value);
-
+
my $string_value;
if (ref $value eq 'ARRAY') {
# Trim input and ignore blank values.
@@ -1626,15 +1573,14 @@ sub _handle_chart {
# on multiple values, like anyexact.
my %search_args = (
- chart_id => $sql_chart_id,
- sequence => $or_id,
+ chart_id => $chart_id,
+ sequence => $chart_id,
field => $field,
full_field => $full_field,
operator => $operator,
value => $string_value,
all_values => $value,
joins => [],
- having => [],
);
$search_args{quoted} = $self->_quote_unless_numeric(\%search_args);
# This should add a "term" selement to %search_args.
@@ -1648,7 +1594,7 @@ sub _handle_chart {
value => $string_value, term => $search_args{term},
});
- return \%search_args;
+ $condition->translated(\%search_args);
}
##################################
diff --git a/Bugzilla/Search/Clause.pm b/Bugzilla/Search/Clause.pm
new file mode 100644
index 000000000..e31bfdd40
--- /dev/null
+++ b/Bugzilla/Search/Clause.pm
@@ -0,0 +1,113 @@
+# -*- 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 BugzillaSource, Inc.
+# Portions created by the Initial Developer are Copyright (C) 2011 the
+# Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+# Max Kanat-Alexander <mkanat@bugzilla.org>
+
+package Bugzilla::Search::Clause;
+use strict;
+use Bugzilla::Search::Condition qw(condition);
+
+sub new {
+ my ($class, $joiner) = @_;
+ bless { joiner => $joiner || 'AND' }, $class;
+}
+
+sub children {
+ my ($self) = @_;
+ $self->{children} ||= [];
+ return $self->{children};
+}
+
+sub joiner { return $_[0]->{joiner} }
+
+sub has_children {
+ my ($self) = @_;
+ return scalar(@{ $self->children }) > 0 ? 1 : 0;
+}
+
+sub has_conditions {
+ my ($self) = @_;
+ my $children = $self->children;
+ return 1 if grep { $_->isa('Bugzilla::Search::Condition') } @$children;
+ foreach my $child (@$children) {
+ return 1 if $child->has_conditions;
+ }
+ return 0;
+}
+
+sub add {
+ my $self = shift;
+ my $children = $self->children;
+ if (@_ == 3) {
+ push(@$children, condition(@_));
+ return;
+ }
+
+ my ($child) = @_;
+ return if !defined $child;
+ $child->isa(__PACKAGE__) || $child->isa('Bugzilla::Search::Condition')
+ || die 'child not the right type: ' . $child;
+ push(@{ $self->children }, $child);
+}
+
+sub negate {
+ my ($self, $value) = @_;
+ if (@_ == 2) {
+ $self->{negate} = $value;
+ }
+ return $self->{negate};
+}
+
+sub walk_conditions {
+ my ($self, $callback) = @_;
+ foreach my $child (@{ $self->children }) {
+ if ($child->isa('Bugzilla::Search::Condition')) {
+ $callback->($child);
+ }
+ else {
+ $child->walk_conditions($callback);
+ }
+ }
+}
+
+sub as_string {
+ my ($self) = @_;
+ my @strings;
+ foreach my $child (@{ $self->children }) {
+ next if $child->isa(__PACKAGE__) && !$child->has_conditions;
+ next if $child->isa('Bugzilla::Search::Condition')
+ && !$child->translated;
+
+ my $string = $child->as_string;
+ if ($self->joiner eq 'AND') {
+ $string = "( $string )" if $string =~ /OR/;
+ }
+ else {
+ $string = "( $string )" if $string =~ /AND/;
+ }
+ push(@strings, $string);
+ }
+
+ my $sql = join(' ' . $self->joiner . ' ', @strings);
+ $sql = "NOT( $sql )" if $sql && $self->negate;
+ return $sql;
+}
+
+
+1; \ No newline at end of file
diff --git a/Bugzilla/Search/Condition.pm b/Bugzilla/Search/Condition.pm
new file mode 100644
index 000000000..db20e7f3b
--- /dev/null
+++ b/Bugzilla/Search/Condition.pm
@@ -0,0 +1,66 @@
+# -*- 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 BugzillaSource, Inc.
+# Portions created by the Initial Developer are Copyright (C) 2011 the
+# Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+# Max Kanat-Alexander <mkanat@bugzilla.org>
+
+package Bugzilla::Search::Condition;
+use strict;
+use base qw(Exporter);
+our @EXPORT_OK = qw(condition);
+
+sub new {
+ my ($class, $params) = @_;
+ my %self = %$params;
+ bless \%self, $class;
+ return \%self;
+}
+
+sub field { return $_[0]->{field} }
+sub operator { return $_[0]->{operator} }
+sub value { return $_[0]->{value} }
+
+sub fov {
+ my ($self) = @_;
+ return ($self->field, $self->operator, $self->value);
+}
+
+sub translated {
+ my ($self, $params) = @_;
+ if (@_ == 2) {
+ $self->{translated} = $params;
+ }
+ return $self->{translated};
+}
+
+sub as_string {
+ my ($self) = @_;
+ return $self->translated->{term};
+}
+
+###########################
+# Convenience Subroutines #
+###########################
+
+sub condition {
+ my ($field, $operator, $value) = @_;
+ return __PACKAGE__->new({ field => $field, operator => $operator,
+ value => $value });
+}
+
+1; \ No newline at end of file