Bug 360028: Bugzilla::Search::Saved should have create() and update(), and buglist.cgi should use it

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=justdave

4 files changed, 127 insertions, 50 deletions

diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index 091362306..7b76f6af0 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -135,6 +135,31 @@ sub canonicalise_query {
     return join("&", @parameters);
 }
 
+sub clean_search_url {
+    my $self = shift;
+    # Delete any empty URL parameter
+    my @cgi_params = $self->param;
+
+    foreach my $param (@cgi_params) {
+        if (defined $self->param($param) && $self->param($param) eq '') {
+            $self->delete($param);
+            $self->delete("${param}_type");
+        }
+
+        # Boolean Chart stuff is empty if it's "noop"
+        if ($param =~ /\d-\d-\d/ && defined $self->param($param)
+            && $self->param($param) eq 'noop')
+        {
+            $self->delete($param);
+        }
+    }
+
+    # Delete certain parameters if the associated parameter is empty.
+    $self->delete('bugidtype')  if !$self->param('bug_id');
+    $self->delete('emailtype1') if !$self->param('email1');
+    $self->delete('emailtype2') if !$self->param('email2');
+}
+
 # Overwrite to ensure nph doesn't get set, and unset HEADERS_ONCE
 sub multipart_init {
     my $self = shift;
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 7fb95e8f2..9c2cf77b4 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -128,6 +128,8 @@ use File::Basename;
     MAX_TOKEN_AGE
 
     SAFE_PROTOCOLS
+
+    MAX_LEN_QUERY_NAME
 );
 
 @Bugzilla::Constants::EXPORT_OK = qw(contenttypes);
@@ -350,6 +352,9 @@ use constant ROOT_USER => $^O =~ /MSWin32/i ? 'Administrator' : 'root';
 # True if we're on Win32.
 use constant ON_WINDOWS => ($^O =~ /MSWin32/i);
 
+# The longest that a saved search name can be.
+use constant MAX_LEN_QUERY_NAME => 64;
+
 sub bz_locations {
     # We know that Bugzilla/Constants.pm must be in %INC at this point.
     # So the only question is, what's the name of the directory
diff --git a/Bugzilla/Search/Saved.pm b/Bugzilla/Search/Saved.pm
index 2cb53439d..1ef10c36b 100644
--- a/Bugzilla/Search/Saved.pm
+++ b/Bugzilla/Search/Saved.pm
@@ -29,6 +29,7 @@ use Bugzilla::Constants;
 use Bugzilla::Group;
 use Bugzilla::Search qw(IsValidQueryType);
 use Bugzilla::User;
+use Bugzilla::Util;
 
 #############
 # Constants #
@@ -44,6 +45,76 @@ use constant DB_COLUMNS => qw(
     query_type
 );
 
+use constant REQUIRED_CREATE_FIELDS => qw(name query);
+
+use constant VALIDATORS => {
+    name       => \&_check_name,
+    query      => \&_check_query,
+    query_type => \&_check_query_type,
+    link_in_footer => \&_check_link_in_footer,
+};
+
+use constant UPDATE_COLUMNS => qw(query query_type);
+
+##############
+# Validators #
+##############
+
+sub _check_link_in_footer { return $_[1] ? 1 : 0; }
+
+sub _check_name {
+    my ($invocant, $name) = @_;
+    $name = trim($name);
+    $name || ThrowUserError("query_name_missing");
+    $name !~ /[<>&]/ || ThrowUserError("illegal_query_name");
+    if (length($name) > MAX_LEN_QUERY_NAME) {
+        ThrowUserError("query_name_too_long");
+    }
+    return $name;
+}
+
+sub _check_query {
+    my ($invocant, $query) = @_;
+    $query || ThrowUserError("buglist_parameters_required");
+    my $cgi = new Bugzilla::CGI($query);
+    $cgi->clean_search_url;
+    return $cgi->query_string;
+}
+
+sub _check_query_type {
+    my ($invocant, $type) = @_;
+    # Right now the only query type is LIST_OF_BUGS.
+    return $type ? LIST_OF_BUGS : QUERY_LIST;
+}
+
+#########################
+# Database Manipulation #
+#########################
+
+sub create {
+    my $class = shift;
+    Bugzilla->login(LOGIN_REQUIRED);
+    my $dbh = Bugzilla->dbh;
+    $class->check_required_create_fields(@_);
+    my $params = $class->run_create_validators(@_);
+
+    # Right now you can only create a Saved Search for the current user.
+    $params->{userid} = Bugzilla->user->id;
+
+    $dbh->bz_lock_tables('namedqueries WRITE',
+                         'namedqueries_link_in_footer WRITE');
+    my $lif = delete $params->{link_in_footer};
+    my $obj = $class->insert_create_data($params);
+    if ($lif) {
+        $dbh->do('INSERT INTO namedqueries_link_in_footer 
+                  (user_id, namedquery_id) VALUES (?,?)',
+                 undef, $params->{userid}, $obj->id);
+    }
+    $dbh->bz_unlock_tables();
+
+    return $obj;
+}
+
 #####################
 # Complex Accessors #
 #####################
@@ -112,6 +183,13 @@ sub user {
     return $self->{user};
 }
 
+############
+# Mutators #
+############
+
+sub set_url        { $_[0]->set('query',      $_[1]); }
+sub set_query_type { $_[0]->set('query_type', $_[1]); }
+
 1;
 
 __END__
diff --git a/buglist.cgi b/buglist.cgi
index 44565f1af..fe9ed743d 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -40,6 +40,7 @@ use Bugzilla::Error;
 use Bugzilla::Util;
 use Bugzilla::Search;
 use Bugzilla::Search::Quicksearch;
+use Bugzilla::Search::Saved;
 use Bugzilla::User;
 use Bugzilla::Bug;
 use Bugzilla::Product;
@@ -275,58 +276,26 @@ sub LookupNamedQuery {
 # Returns: A boolean true value if the query existed in the database 
 # before, and we updated it. A boolean false value otherwise.
 sub InsertNamedQuery {
-    my ($userid, $query_name, $query, $link_in_footer, $query_type) = @_;
-    $link_in_footer ||= 0;
-    $query_type ||= QUERY_LIST;
-    $query_name = trim($query_name);
-    Bugzilla->login(LOGIN_REQUIRED);
+    my ($query_name, $query, $link_in_footer, $query_type) = @_;
     my $dbh = Bugzilla->dbh;
-    my $query_existed_before;
-
-    # Validate the query name.
-    $query_name || ThrowUserError("query_name_missing");
-    $query_name !~ /[<>&]/ || ThrowUserError("illegal_query_name");
-    (length($query_name) <= 64) || ThrowUserError("query_name_too_long");
-    trick_taint($query_name);
-
-    detaint_natural($userid);
-    detaint_natural($link_in_footer);
-
-    $query || ThrowUserError("buglist_parameters_required",
-                             {'queryname' => $query});
-    # $query is safe, because we always urlencode or html_quote
-    # it when we display it to the user.
-    trick_taint($query);
-
-    $dbh->bz_lock_tables('namedqueries WRITE',
-                         'namedqueries_link_in_footer WRITE');
-
-    my $result = $dbh->selectrow_array("SELECT userid FROM namedqueries"
-        . " WHERE userid = ? AND name = ?"
-        , undef, ($userid, $query_name));
-    if ($result) {
-        $query_existed_before = 1;
-        $dbh->do("UPDATE namedqueries"
-            . " SET query = ?, query_type = ?"
-            . " WHERE userid = ? AND name = ?"
-            , undef, ($query, $query_type, $userid, $query_name));
+
+    $query_name = trim($query_name);
+    my ($query_obj) = grep {$_->name eq $query_name} @{Bugzilla->user->queries};
+
+    if ($query_obj) {
+        $query_obj->set_url($query);
+        $query_obj->set_query_type($query_type);
+        $query_obj->update();
     } else {
-        $query_existed_before = 0;
-        $dbh->do("INSERT INTO namedqueries"
-            . " (userid, name, query, query_type)"
-            . " VALUES (?, ?, ?, ?)"
-            , undef, ($userid, $query_name, $query, $query_type));
-        if ($link_in_footer) {
-            $dbh->do('INSERT INTO namedqueries_link_in_footer
-                                 (namedquery_id, user_id)
-                          VALUES (?, ?)',
-                     undef,
-                     ($dbh->bz_last_key('namedqueries', 'id'), $userid));
-        }
+        Bugzilla::Search::Saved->create({
+            name           => $query_name,
+            query          => $query,
+            query_type     => $query_type,
+            link_in_footer => $link_in_footer
+        });
     }
 
-    $dbh->bz_unlock_tables();
-    return $query_existed_before;
+    return $query_obj ? 1 : 0;
 }
 
 sub LookupSeries {
@@ -497,7 +466,7 @@ if ($cgi->param('cmdtype') eq "dorem") {
 elsif (($cgi->param('cmdtype') eq "doit") && defined $cgi->param('remtype')) {
     if ($cgi->param('remtype') eq "asdefault") {
         my $user = Bugzilla->login(LOGIN_REQUIRED);
-        InsertNamedQuery($user->id, DEFAULT_QUERY_NAME, $buffer);
+        InsertNamedQuery(DEFAULT_QUERY_NAME, $buffer);
         $vars->{'message'} = "buglist_new_default_query";
     }
     elsif ($cgi->param('remtype') eq "asnamed") {
@@ -551,7 +520,7 @@ elsif (($cgi->param('cmdtype') eq "doit") && defined $cgi->param('remtype')) {
             $query_type = LIST_OF_BUGS;
         }
         my $tofooter = 1;
-        my $existed_before = InsertNamedQuery($user->id, $query_name, $new_query,
+        my $existed_before = InsertNamedQuery($query_name, $new_query,
                                               $tofooter, $query_type);
         if ($existed_before) {
             $vars->{'message'} = "buglist_updated_named_query";