Bug 364780: The keyword cache cannot be fixed with editkeywords privs only - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=justdave

4 files changed, 160 insertions, 129 deletions

diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index 4b25fa038..bf8ba1980 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -22,6 +22,7 @@
 #                 Matthew Tuck <matty@chariot.net.au>
 #                 Max Kanat-Alexander <mkanat@bugzilla.org>
 #                 Marc Schumann <wurblzap@gmail.com>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 use strict;
 
@@ -78,11 +79,13 @@ Bugzilla->login(LOGIN_REQUIRED);
 my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
+my $user = Bugzilla->user;
 
 # Make sure the user is authorized to access sanitycheck.cgi.
 # As this script can now alter the group_control_map table, we no longer
 # let users with editbugs privs run it anymore.
-Bugzilla->user->in_group("editcomponents")
+$user->in_group("editcomponents")
+  || ($user->in_group('editkeywords') && defined $cgi->param('rebuildkeywordcache'))
   || ThrowUserError("auth_failure", {group  => "editcomponents",
                                      action => "run",
                                      object => "sanity_check"});
@@ -94,6 +97,16 @@ my @row;
 $template->put_header("Sanity Check");
 
 ###########################################################################
+# Users with 'editkeywords' privs only can only check keywords.
+###########################################################################
+unless ($user->in_group('editcomponents')) {
+    check_votes_or_keywords('keywords');
+    Status("Sanity check completed.");
+    $template->put_footer();
+    exit;
+}
+
+###########################################################################
 # Fix vote cache
 ###########################################################################
 
@@ -602,164 +615,180 @@ sub AlertBadVoteCache {
     Alert("Bad vote cache for bug " . BugLink($id));
 }
 
-$sth = $dbh->prepare(q{SELECT bug_id, votes, keywords
-                         FROM bugs
-                        WHERE votes != 0 
-                           OR keywords != ''});
-$sth->execute;
-
-my %votes;
-my %bugid;
-my %keyword;
+check_votes_or_keywords();
 
-while (my ($id, $v, $k) = $sth->fetchrow_array) {
-    if ($v != 0) {
-        $votes{$id} = $v;
-    }
-    if ($k) {
-        $keyword{$id} = $k;
-    }
-}
+sub check_votes_or_keywords {
+    my $check = shift || 'all';
 
-Status("Checking cached vote counts");
-$sth = $dbh->prepare(q{SELECT bug_id, SUM(vote_count)
-                         FROM votes }.
-                       $dbh->sql_group_by('bug_id'));
-$sth->execute;
+    my $dbh = Bugzilla->dbh;
+    my $sth = $dbh->prepare(q{SELECT bug_id, votes, keywords
+                                FROM bugs
+                               WHERE votes != 0 OR keywords != ''});
+    $sth->execute;
 
-my $offer_votecache_rebuild = 0;
+    my %votes;
+    my %keyword;
 
-while (my ($id, $v) = $sth->fetchrow_array) {
-    if ($v <= 0) {
-        Alert("Bad vote sum for bug $id");
-    } else {
-        if (!defined $votes{$id} || $votes{$id} != $v) {
-            AlertBadVoteCache($id);
-            $offer_votecache_rebuild = 1;
+    while (my ($id, $v, $k) = $sth->fetchrow_array) {
+        if ($v != 0) {
+            $votes{$id} = $v;
+        }
+        if ($k) {
+            $keyword{$id} = $k;
         }
-        delete $votes{$id};
     }
-}
-foreach my $id (keys %votes) {
-    AlertBadVoteCache($id);
-    $offer_votecache_rebuild = 1;
-}
 
-if ($offer_votecache_rebuild) {
-    print qq{<a href="sanitycheck.cgi?rebuildvotecache=1">Click here to rebuild the vote cache</a><p>\n};
+    # If we only want to check keywords, skip checks about votes.
+    _check_votes(\%votes) unless ($check eq 'keywords');
+    # If we only want to check votes, skip checks about keywords.
+    _check_keywords(\%keyword) unless ($check eq 'votes');
 }
 
+sub _check_votes {
+    my $votes = shift;
 
-Status("Checking keywords table");
-
-my %keywordids;
+    Status("Checking cached vote counts");
+    my $dbh = Bugzilla->dbh;
+    my $sth = $dbh->prepare(q{SELECT bug_id, SUM(vote_count)
+                                FROM votes }.
+                                $dbh->sql_group_by('bug_id'));
+    $sth->execute;
 
-my $keywords = $dbh->selectall_arrayref(q{SELECT id, name 
-                                            FROM keyworddefs});
+    my $offer_votecache_rebuild = 0;
 
-foreach my $keyword (@$keywords) {
-    my ($id, $name) = @$keyword;
-    if ($keywordids{$id}) {
-        Alert("Duplicate entry in keyworddefs for id $id");
+    while (my ($id, $v) = $sth->fetchrow_array) {
+        if ($v <= 0) {
+            Alert("Bad vote sum for bug $id");
+        } else {
+            if (!defined $votes->{$id} || $votes->{$id} != $v) {
+                AlertBadVoteCache($id);
+                $offer_votecache_rebuild = 1;
+            }
+            delete $votes->{$id};
+        }
     }
-    $keywordids{$id} = 1;
-    if ($name =~ /[\s,]/) {
-        Alert("Bogus name in keyworddefs for id $id");
+    foreach my $id (keys %$votes) {
+        AlertBadVoteCache($id);
+        $offer_votecache_rebuild = 1;
     }
-}
 
-$sth = $dbh->prepare(q{SELECT bug_id, keywordid
-                         FROM keywords
-                     ORDER BY bug_id, keywordid});
-$sth->execute;
-my $lastid;
-my $lastk;
-while (my ($id, $k) = $sth->fetchrow_array) {
-    if (!$keywordids{$k}) {
-        Alert("Bogus keywordids $k found in keywords table");
+    if ($offer_votecache_rebuild) {
+        print qq{<a href="sanitycheck.cgi?rebuildvotecache=1">Click here to rebuild the vote cache</a><p>\n};
     }
-    if (defined $lastid && $id eq $lastid && $k eq $lastk) {
-        Alert("Duplicate keyword ids found in bug " . BugLink($id));
-    }
-    $lastid = $id;
-    $lastk = $k;
 }
 
-Status("Checking cached keywords");
-
-my %realk;
+sub _check_keywords {
+    my $keyword = shift;
 
-if (defined $cgi->param('rebuildkeywordcache')) {
-    $dbh->bz_lock_tables('bugs write', 'keywords read',
-                                  'keyworddefs read');
-}
+    Status("Checking keywords table");
+    my $dbh = Bugzilla->dbh;
+    my $cgi = Bugzilla->cgi;
 
-my $query = q{SELECT keywords.bug_id, keyworddefs.name
-                FROM keywords
-          INNER JOIN keyworddefs
-                  ON keyworddefs.id = keywords.keywordid
-          INNER JOIN bugs
-                  ON keywords.bug_id = bugs.bug_id
-            ORDER BY keywords.bug_id, keyworddefs.name};
+    my %keywordids;
+    my $keywords = $dbh->selectall_arrayref(q{SELECT id, name
+                                                FROM keyworddefs});
 
-$sth = $dbh->prepare($query);
-$sth->execute;
+    foreach (@$keywords) {
+        my ($id, $name) = @$_;
+        if ($keywordids{$id}) {
+            Alert("Duplicate entry in keyworddefs for id $id");
+        }
+        $keywordids{$id} = 1;
+        if ($name =~ /[\s,]/) {
+            Alert("Bogus name in keyworddefs for id $id");
+        }
+    }
 
-my $lastb = 0;
-my @list;
-while (1) {
-    my ($b, $k) = $sth->fetchrow_array; 
-    if (!defined $b || $b != $lastb) {
-        if (@list) {
-            $realk{$lastb} = join(', ', @list);
+    my $sth = $dbh->prepare(q{SELECT bug_id, keywordid
+                                FROM keywords
+                            ORDER BY bug_id, keywordid});
+    $sth->execute;
+    my $lastid;
+    my $lastk;
+    while (my ($id, $k) = $sth->fetchrow_array) {
+        if (!$keywordids{$k}) {
+            Alert("Bogus keywordids $k found in keywords table");
         }
-        if (!$b) {
-            last;
+        if (defined $lastid && $id eq $lastid && $k eq $lastk) {
+            Alert("Duplicate keyword ids found in bug " . BugLink($id));
         }
-        $lastb = $b;
-        @list = ();
+        $lastid = $id;
+        $lastk = $k;
     }
-    push(@list, $k);
-}
 
-my @badbugs = ();
+    Status("Checking cached keywords");
 
-foreach my $b (keys(%keyword)) {
-    if (!exists $realk{$b} || $realk{$b} ne $keyword{$b}) {
-        push(@badbugs, $b);
+    if (defined $cgi->param('rebuildkeywordcache')) {
+        $dbh->bz_lock_tables('bugs write', 'keywords read', 'keyworddefs read');
     }
-}
-foreach my $b (keys(%realk)) {
-    if (!exists $keyword{$b}) {
-        push(@badbugs, $b);
+
+    my $query = q{SELECT keywords.bug_id, keyworddefs.name
+                    FROM keywords
+              INNER JOIN keyworddefs
+                      ON keyworddefs.id = keywords.keywordid
+              INNER JOIN bugs
+                      ON keywords.bug_id = bugs.bug_id
+                ORDER BY keywords.bug_id, keyworddefs.name};
+
+    $sth = $dbh->prepare($query);
+    $sth->execute;
+
+    my $lastb = 0;
+    my @list;
+    my %realk;
+    while (1) {
+        my ($b, $k) = $sth->fetchrow_array;
+        if (!defined $b || $b != $lastb) {
+            if (@list) {
+                $realk{$lastb} = join(', ', @list);
+            }
+            last unless $b;
+
+            $lastb = $b;
+            @list = ();
+        }
+        push(@list, $k);
     }
-}
-if (@badbugs) {
-    @badbugs = sort {$a <=> $b} @badbugs;
-    Alert(scalar(@badbugs) . " bug(s) found with incorrect keyword cache: " .
-          BugListLinks(@badbugs));
-          
-    my $sth_update = $dbh->prepare(q{UPDATE bugs
-                                        SET keywords = ?
-                                      WHERE bug_id = ?});
-                                      
-    if (defined $cgi->param('rebuildkeywordcache')) {
-        Status("OK, now fixing keyword cache.");
-        foreach my $b (@badbugs) {
-            my $k = '';
-            if (exists($realk{$b})) {
-                $k = $realk{$b};
+
+    my @badbugs = ();
+
+    foreach my $b (keys(%$keyword)) {
+        if (!exists $realk{$b} || $realk{$b} ne $keyword->{$b}) {
+            push(@badbugs, $b);
+        }
+    }
+    foreach my $b (keys(%realk)) {
+        if (!exists $keyword->{$b}) {
+            push(@badbugs, $b);
+        }
+    }
+    if (@badbugs) {
+        @badbugs = sort {$a <=> $b} @badbugs;
+        Alert(scalar(@badbugs) . " bug(s) found with incorrect keyword cache: " .
+              BugListLinks(@badbugs));
+
+        my $sth_update = $dbh->prepare(q{UPDATE bugs
+                                            SET keywords = ?
+                                          WHERE bug_id = ?});
+
+        if (defined $cgi->param('rebuildkeywordcache')) {
+            Status("OK, now fixing keyword cache.");
+            foreach my $b (@badbugs) {
+                my $k = '';
+                if (exists($realk{$b})) {
+                    $k = $realk{$b};
+                }
+                $sth_update->execute($k, $b);
             }
-            $sth_update->execute($k, $b);
+            Status("Keyword cache fixed.");
+        } else {
+            print qq{<a href="sanitycheck.cgi?rebuildkeywordcache=1">Click here to rebuild the keyword cache</a><p>\n};
         }
-        Status("Keyword cache fixed.");
-    } else {
-        print qq{<a href="sanitycheck.cgi?rebuildkeywordcache=1">Click here to rebuild the keyword cache</a><p>\n};
     }
-}
 
-if (defined $cgi->param('rebuildkeywordcache')) {
-    $dbh->bz_unlock_tables();
+    if (defined $cgi->param('rebuildkeywordcache')) {
+        $dbh->bz_unlock_tables();
+    }
 }
 
 ###########################################################################
@@ -888,7 +917,7 @@ BugCheck("bugs INNER JOIN products ON bugs.product_id = products.id " .
 Status("Checking for bad values in group_control_map");
 my $groups = join(", ", (CONTROLMAPNA, CONTROLMAPSHOWN, CONTROLMAPDEFAULT,
 CONTROLMAPMANDATORY));
-$query = qq{
+my $query = qq{
      SELECT COUNT(product_id) 
        FROM group_control_map 
       WHERE membercontrol NOT IN( $groups )
diff --git a/template/en/default/global/site-navigation.html.tmpl b/template/en/default/global/site-navigation.html.tmpl
index 7c2eabecf..507b779c8 100644
--- a/template/en/default/global/site-navigation.html.tmpl
+++ b/template/en/default/global/site-navigation.html.tmpl
@@ -122,6 +122,6 @@
     [% '<link rel="Administration" title="Whining"       
               href="editwhines.cgi">' IF user.groups.bz_canusewhines %]
     [% '<link rel="Administration" title="Sanity Check"  
-              href="sanitycheck.cgi">' IF user.groups.tweakparams %]
+              href="sanitycheck.cgi">' IF user.groups.editcomponents %]
   [% END %]  
 [% END %]
diff --git a/template/en/default/global/useful-links.html.tmpl b/template/en/default/global/useful-links.html.tmpl
index 8078a5d80..cd485ad8f 100644
--- a/template/en/default/global/useful-links.html.tmpl
+++ b/template/en/default/global/useful-links.html.tmpl
@@ -48,6 +48,8 @@
           <li><span class="separator">[% sep %]</span><a href="editparams.cgi">Parameters</a></li>
           [% sep = "| " %]
           <li><span class="separator">[% sep %]</span><a href="editsettings.cgi">User Preferences</a></li>
+        [% END %]
+        [% IF user.groups.editcomponents %]
           <li><span class="separator">[% sep %]</span><a href="sanitycheck.cgi">Sanity Check</a></li>
         [% END %]
         [% IF user.groups.editusers || user.can_bless %]
diff --git a/template/en/default/sidebar.xul.tmpl b/template/en/default/sidebar.xul.tmpl
index 9ad50e07b..6742889da 100644
--- a/template/en/default/sidebar.xul.tmpl
+++ b/template/en/default/sidebar.xul.tmpl
@@ -94,7 +94,7 @@ function normal_keypress_handler( aEvent ) {
   [%- IF user.groups.bz_canusewhines %]
       <text class="text-link" onclick="load_relative_url('editwhines.cgi')" value="edit whining"/>
   [%- END %]
-  [%- IF user.groups.tweakparams %]
+  [%- IF user.groups.editcomponents %]
       <text class="text-link" onclick="load_relative_url('sanitycheck.cgi')" value="sanity check"/>
   [%- END %]
   [%- IF user.authorizer.can_logout %]