Bug 176599, Improve performance of duplicates.cgi

original patch iteration by gerv, change to use Bugzilla:Search by me
r=myk, a=justdave

diff --git a/duplicates.cgi b/duplicates.cgi
index 233022eca483140eed795797cdcb76c692cfd63d..5687cefec395aeaddf2ea66b8ee39bf196fc1406 100755 (executable)
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -34,6 +34,9 @@ require "CGI.pl";
 
 use vars qw($buffer);
 
+use Bugzilla::Search;
+use Bugzilla::CGI;
+
 # Go directly to the XUL version of the duplicates report (duplicates.xul)
 # if the user specified ctype=xul.  Adds params if they exist, and directs
 # the user to a signed copy of the script in duplicates.jar if it exists.
@@ -152,27 +155,64 @@ if (!tie(%before, 'AnyDBM_File', "data/duplicates/dupes$whenever",
     $dobefore = 1;
 }
 
+detaint_natural($maxrows)
+  || ThrowUserError("invalid_maxrows", { maxrows => $maxrows});
+
 my @bugs;
 my @bug_ids; 
 
 if (scalar(%count)) {
-    # Don't add CLOSED, and don't add VERIFIED unless they are INVALID or 
-    # WONTFIX. We want to see VERIFIED INVALID and WONTFIX because common 
-    # "bugs" which aren't bugs end up in this state.
-    my $query = "
-      SELECT bugs.bug_id, components.name, bug_severity, op_sys,
-             target_milestone, short_desc, bug_status, resolution
-      FROM bugs, components
-      WHERE (bugs.component_id = components.id)
-      AND   (bug_status != 'CLOSED') 
-      AND   ((bug_status = 'VERIFIED' AND resolution IN ('INVALID', 'WONTFIX'))
-             OR (bug_status != 'VERIFIED'))
-      AND bugs.bug_id IN (" . join(", ", keys %count) . ")";
-
-    # Limit to a single product if requested
-    $query .= (" AND bugs.product_id = " . $product_id) if $product_id;
-
-    SendSQL($query);
+    # use Bugzilla::Search so that we get the security checking
+    my $params = new Bugzilla::CGI({ 'bug_id' => [keys %count] });
+
+    if ($openonly) {
+        $params->param('resolution', '---');
+    } else {
+        # We want to show bugs which:
+        # a) Aren't CLOSED; and
+        # b)  i) Aren't VERIFIED; OR
+        #    ii) Were resolved INVALID/WONTFIX
+
+        # The rationale behind this is that people will eventually stop
+        # reporting fixed bugs when they get newer versions of the software,
+        # but if the bug is determined to be erroneous, people will still
+        # keep reporting it, so we do need to show it here.
+
+        # a)
+        $params->param('field0-0-0', 'bug_status');
+        $params->param('type0-0-0', 'notequals');
+        $params->param('value0-0-0', 'CLOSED');
+
+        # b) i)
+        $params->param('field0-1-0', 'bug_status');
+        $params->param('type0-1-0', 'notequals');
+        $params->param('value0-1-0', 'VERIFIED');
+
+        # b) ii)
+        $params->param('field0-1-1', 'resolution');
+        $params->param('type0-1-1', 'anyexact');
+        $params->param('value0-1-1', 'INVALID,WONTFIX');
+    }
+
+    # Restrict to product if requested
+    if ($::FORM{'product'}) {
+        $params->param('product', $::FORM{'product'});
+    }
+
+    my $query = new Bugzilla::Search('fields' => [qw(bugs.bug_id
+                                                     map_components.name
+                                                     bugs.bug_severity
+                                                     bugs.op_sys
+                                                     bugs.target_milestone
+                                                     bugs.short_desc
+                                                     bugs.bug_status
+                                                     bugs.resolution
+                                                    )
+                                                 ],
+                                     'params' => $params,
+                                    );
+
+    SendSQL($query->getSQL());
 
     while (MoreSQLData()) {
         # Note: maximum row count is dealt with in the template.
@@ -180,10 +220,6 @@ if (scalar(%count)) {
         my ($id, $component, $bug_severity, $op_sys, $target_milestone, 
             $short_desc, $bug_status, $resolution) = FetchSQLData();
 
-        next if (!CanSeeBug($id, $::userid));
-        # Limit to open bugs only if requested
-        next if $openonly && ($resolution ne "");
-
         push (@bugs, { id => $id,
                        count => $count{$id},
                        delta => $delta{$id}, 

diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index d08c96c4bd2859d0bd821e3ae061eacb141673ae..421e396cab71676c6d5da18c3779b27c274deef6 100644 (file)
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -276,6 +276,11 @@
     is either <em>application, audio, image, message, model, multipart, 
     text,</em> or <em>video</em>.
     
+  [% ELSIF error == "invalid_maxrow" %]
+    [% title = "Invalid Max Rows" %]
+    The maximum number of rows, '[% maxrows FILTER html %]', must be a positive
+    integer.
+
   [% ELSIF error == "invalid_product_name" %]
     [% title = "Invalid Product Name" %]
     The product name '[% product FILTER html %]' is invalid or does not exist.

diff --git a/template/en/default/reports/duplicates.html.tmpl b/template/en/default/reports/duplicates.html.tmpl
index 531196eb8216edfd20bb3dd8581d876a637a063f..bb2a3a10cb45f64154778fdfc540ea14307c18d1 100644 (file)
--- a/template/en/default/reports/duplicates.html.tmpl
+++ b/template/en/default/reports/duplicates.html.tmpl
@@ -80,8 +80,8 @@
       <td rowspan="4" valign="top">
         <select name="product" size="5" multiple="multiple">
           [% FOREACH p = products %]
-            <option name="[% p %]"
-            [% " selected" IF product == p %]>[% p %]</option>
+            <option name="[% p FILTER html %]"
+            [% " selected" IF product == p %]>[% p FILTER html %]</option>
           [% END %]
         </select>
       </td>