Bug 647466: Allow Search.pm to take the new URL syntax for custom search

r=mkanat, a=mkanat (module owner)

4 files changed, 89 insertions, 10 deletions

diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index ab0bdae89..fddb18194 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -50,6 +50,7 @@ use Bugzilla::Group;
 use Bugzilla::User;
 use Bugzilla::Field;
 use Bugzilla::Search::Clause;
+use Bugzilla::Search::Condition qw(condition);
 use Bugzilla::Status;
 use Bugzilla::Keyword;
 
@@ -691,7 +692,6 @@ sub sql {
     my $dbh = Bugzilla->dbh;
     
     my ($joins, $clause) = $self->_charts_to_conditions();
-
     my $select = join(', ', $self->_sql_select);
     my $from = $self->_sql_from($joins);
     my $where = $self->_sql_where($clause);
@@ -1498,7 +1498,18 @@ sub _params_to_data_structure {
     # happen first.
     my $clause = $self->_special_charts;
 
-    # Then we process the old Boolean Charts input format.    
+    # Then we process the old Boolean Charts input format.
+    $clause->add( $self->_boolean_charts );
+    
+    # And then process the modern "custom search" format.
+    $clause->add( $self->_custom_search );
+   
+    return $clause;
+}
+
+sub _boolean_charts {
+    my ($self) = @_;
+    
     my $params = $self->_params;
     my @param_list = keys %$params;
     
@@ -1506,7 +1517,7 @@ sub _params_to_data_structure {
     my @chart_ids = map { /^field(-?\d+)/; $1 } @all_field_params;
     @chart_ids = sort { $a <=> $b } uniq @chart_ids;
     
-    my $sequence = 0;
+    my $clause = new Bugzilla::Search::Clause();
     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;
@@ -1527,7 +1538,6 @@ sub _params_to_data_structure {
                 $or_clause->add($field, $operator, $value);
             }
             $and_clause->add($or_clause);
-
             $and_clause->negate(1) if $params->{"negate$chart_id"};
         }
         $clause->add($and_clause);
@@ -1536,6 +1546,48 @@ sub _params_to_data_structure {
     return $clause;
 }
 
+sub _custom_search {
+    my ($self) = @_;
+    my $params = $self->_params;
+    my @param_list = keys %$params;
+    
+    my @field_params = grep { /^f\d+$/ } @param_list;
+    my @field_ids = map { /(\d+)/; $1 } @field_params;
+    @field_ids = sort { $a <=> $b } @field_ids;
+    
+    my $current_clause = new Bugzilla::Search::Clause();
+    my @clause_stack;
+    foreach my $id (@field_ids) {
+        my $field = $params->{"f$id"};
+        if ($field eq 'OP') {
+            my $joiner = $params->{"j$id"};
+            my $new_clause = new Bugzilla::Search::Clause($joiner);
+            $new_clause->negate($params->{"n$id"});
+            $current_clause->add($new_clause);
+            push(@clause_stack, $current_clause);
+            $current_clause = $new_clause;
+            next;
+        }
+        if ($field eq 'CP') {
+            $current_clause = pop @clause_stack;
+            ThrowCodeError('search_cp_without_op', { id => $id })
+                if !$current_clause;
+            next;
+        }
+        
+        my $operator = $params->{"o$id"};
+        my $value = $params->{"v$id"};
+        my $condition = condition($field, $operator, $value);
+        $condition->negate($params->{"n$id"});
+        $current_clause->add($condition);
+    }
+    
+    # We allow people to specify more OPs than CPs, so at the end of the
+    # loop our top clause may be still in the stack instead of being
+    # $current_clause.
+    return $clause_stack[0] || $current_clause;
+}
+
 sub _handle_chart {
     my ($self, $chart_id, $condition) = @_;
     my $dbh = Bugzilla->dbh;
diff --git a/Bugzilla/Search/Clause.pm b/Bugzilla/Search/Clause.pm
index e31bfdd40..aa87842af 100644
--- a/Bugzilla/Search/Clause.pm
+++ b/Bugzilla/Search/Clause.pm
@@ -21,10 +21,18 @@
 
 package Bugzilla::Search::Clause;
 use strict;
+
+use Bugzilla::Error;
 use Bugzilla::Search::Condition qw(condition);
+use Bugzilla::Util qw(trick_taint);
 
 sub new {
     my ($class, $joiner) = @_;
+    if ($joiner and $joiner ne 'OR' and $joiner ne 'AND') {
+        ThrowCodeError('search_invalid_joiner', { joiner => $joiner });
+    }
+    # This will go into SQL directly so needs to be untainted.
+    trick_taint($joiner) if $joiner;
     bless { joiner => $joiner || 'AND' }, $class;
 }
 
@@ -41,12 +49,14 @@ sub has_children {
     return scalar(@{ $self->children }) > 0 ? 1 : 0;
 }
 
-sub has_conditions {
+sub has_valid_conditions {
     my ($self) = @_;
     my $children = $self->children;
-    return 1 if grep { $_->isa('Bugzilla::Search::Condition') } @$children;
+    return 1 if grep { $_->isa('Bugzilla::Search::Condition')
+                       && $_->translated } @$children;
     foreach my $child (@$children) {
-        return 1 if $child->has_conditions;
+        next if $child->isa('Bugzilla::Search::Condition');
+        return 1 if $child->has_valid_conditions;
     }
     return 0;
 }
@@ -69,7 +79,7 @@ sub add {
 sub negate {
     my ($self, $value) = @_;
     if (@_ == 2) {
-        $self->{negate} = $value;
+        $self->{negate} = $value ? 1 : 0;
     }
     return $self->{negate};
 }
@@ -90,7 +100,7 @@ sub as_string {
     my ($self) = @_;
     my @strings;
     foreach my $child (@{ $self->children }) {
-        next if $child->isa(__PACKAGE__) && !$child->has_conditions;
+        next if $child->isa(__PACKAGE__) && !$child->has_valid_conditions;
         next if $child->isa('Bugzilla::Search::Condition')
                 && !$child->translated;
 
diff --git a/Bugzilla/Search/Condition.pm b/Bugzilla/Search/Condition.pm
index db20e7f3b..8fe05f065 100644
--- a/Bugzilla/Search/Condition.pm
+++ b/Bugzilla/Search/Condition.pm
@@ -50,7 +50,17 @@ sub translated {
 
 sub as_string {
     my ($self) = @_;
-    return $self->translated->{term};
+    my $term = $self->translated->{term};
+    $term = "NOT( $term )" if $term && $self->negate;
+    return $term;
+}
+
+sub negate {
+    my ($self, $value) = @_;
+    if (@_ == 2) {
+        $self->{negate} = $value ? 1 : 0;
+    }
+    return $self->{negate};
 }
 
 ###########################
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index addde0d8a..a8cdd7ef2 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -413,6 +413,13 @@
     [%+ ELSIF fld == "z" %]the multiple tables/images
     [%+ ELSE %]a report axis[% END %] field.
 
+  [% ELSIF error == "search_cp_without_op" %]
+    Search argument f[% id FILTER html %] is "CP" but there is no
+    matching "OP" before it.
+
+  [% ELSIF error == "search_invalid_joiner" %]
+    '[% joiner FILTER html %]' is not a valid joiner for a search.
+
   [% ELSIF error == "setting_info_invalid" %]
     To create a new setting, you must supply a setting name, a list of 
     value/sortindex pairs, and the default value.