Bug 851267: Bugzilla times out when a user has several thousands of votes

r=dkl a=justdave
4.4
Frédéric Buclin 2013-09-28 00:55:48 +02:00
parent b59b75ba90
commit 6c07ea5a3c
3 changed files with 90 additions and 93 deletions

View File

@ -466,9 +466,9 @@ sub redirect_search_url {
# GET requests that lacked a list_id are always redirected. POST requests
# are only redirected if they're under the CGI_URI_LIMIT though.
my $uri_length = length($self->self_url());
if ($self->request_method() ne 'POST' or $uri_length < CGI_URI_LIMIT) {
print $self->redirect(-url => $self->self_url());
my $self_url = $self->self_url();
if ($self->request_method() ne 'POST' or length($self_url) < CGI_URI_LIMIT) {
print $self->redirect(-url => $self_url);
exit;
}
}
@ -522,7 +522,7 @@ sub url_is_attachment_base {
$regex =~ s/\\\%bugid\\\%/\\d+/;
}
$regex = "^$regex";
return ($self->self_url =~ $regex) ? 1 : 0;
return ($self->url =~ $regex) ? 1 : 0;
}
##########################

View File

@ -19,7 +19,7 @@ use Bugzilla::User;
use Bugzilla::Util qw(detaint_natural);
use Bugzilla::Token;
use List::Util qw(min);
use List::Util qw(min sum);
use constant VERSION => BUGZILLA_VERSION;
use constant DEFAULT_VOTES_PER_BUG => 1;
@ -185,7 +185,7 @@ sub bug_end_of_update {
# If some votes have been removed, RemoveVotes() returns
# a list of messages to send to voters.
@msgs = _remove_votes($bug->id, 0, 'votes_bug_moved');
_confirm_if_vote_confirmed($bug->id);
_confirm_if_vote_confirmed($bug);
foreach my $msg (@msgs) {
MessageToMTA($msg);
@ -418,53 +418,38 @@ sub _page_user {
foreach my $product (@{ $user->get_selectable_products }) {
next unless ($product->{votesperuser} > 0);
my @bugs;
my @bug_ids;
my $total = 0;
my $onevoteonly = 0;
my $vote_list =
$dbh->selectall_arrayref('SELECT votes.bug_id, votes.vote_count,
bugs.short_desc
FROM votes
INNER JOIN bugs
ON votes.bug_id = bugs.bug_id
WHERE votes.who = ?
AND bugs.product_id = ?
ORDER BY votes.bug_id',
undef, ($who->id, $product->id));
$dbh->selectall_arrayref('SELECT votes.bug_id, votes.vote_count
FROM votes
INNER JOIN bugs
ON votes.bug_id = bugs.bug_id
WHERE votes.who = ?
AND bugs.product_id = ?',
undef, ($who->id, $product->id));
$user->visible_bugs([map { $_->[0] } @$vote_list]);
foreach (@$vote_list) {
my ($id, $count, $summary) = @$_;
$total += $count;
my %votes = map { $_->[0] => $_->[1] } @$vote_list;
my @bug_ids = sort keys %votes;
# Exclude bugs that the user can no longer see.
@bug_ids = @{ $user->visible_bugs(\@bug_ids) };
next unless scalar @bug_ids;
# Next if user can't see this bug. So, the totals will be correct
# and they can see there are votes 'missing', but not on what bug
# they are. This seems a reasonable compromise; the alternative is
# to lie in the totals.
next if !$user->can_see_bug($id);
push (@bugs, { id => $id,
summary => $summary,
count => $count });
push (@bug_ids, $id);
push (@all_bug_ids, $id);
}
push(@all_bug_ids, @bug_ids);
my @bugs = @{ Bugzilla::Bug->new_from_list(\@bug_ids) };
$_->{count} = $votes{$_->id} foreach @bugs;
# We include votes from bugs that the user can no longer see.
my $total = sum(values %votes) || 0;
my $onevoteonly = 0;
$onevoteonly = 1 if (min($product->{votesperuser},
$product->{maxvotesperbug}) == 1);
# Only add the product for display if there are any bugs in it.
if ($#bugs > -1) {
push (@products, { name => $product->name,
bugs => \@bugs,
bug_ids => \@bug_ids,
onevoteonly => $onevoteonly,
total => $total,
maxvotes => $product->{votesperuser},
maxperbug => $product->{maxvotesperbug} });
}
push(@products, { name => $product->name,
bugs => \@bugs,
bug_ids => \@bug_ids,
onevoteonly => $onevoteonly,
total => $total,
maxvotes => $product->{votesperuser},
maxperbug => $product->{maxvotesperbug} });
}
if ($canedit && $bug) {
@ -498,6 +483,7 @@ sub _update_votes {
# IDs and the field values are the number of votes.
my @buglist = grep {/^\d+$/} keys %$input;
my (%bugs, %votes);
# If no bugs are in the buglist, let's make sure the user gets notified
# that their votes will get nuked if they continue.
@ -513,20 +499,23 @@ sub _update_votes {
exit;
}
}
else {
$user->visible_bugs(\@buglist);
my $bugs_obj = Bugzilla::Bug->new_from_list(\@buglist);
$bugs{$_->id} = $_ foreach @$bugs_obj;
}
# Call check() on each bug ID to make sure it is a positive
# integer representing an existing bug that the user is authorized
# to access, and make sure the number of votes submitted is also
# a non-negative integer (a series of digits not preceded by a
# minus sign).
my (%votes, @bugs);
# Call check_is_visible() on each bug to make sure it is an existing bug
# that the user is authorized to access, and make sure the number of votes
# submitted is also an integer.
foreach my $id (@buglist) {
my $bug = Bugzilla::Bug->check($id);
push(@bugs, $bug);
$id = $bug->id;
$votes{$id} = $input->{$id};
detaint_natural($votes{$id})
|| ThrowUserError("voting_must_be_nonnegative");
my $bug = $bugs{$id}
or ThrowUserError('bug_id_does_not_exist', { bug_id => $id });
$bug->check_is_visible;
$id = $bug->id;
$votes{$id} = $input->{$id};
detaint_natural($votes{$id})
|| ThrowUserError("voting_must_be_nonnegative");
}
my $token = $cgi->param('token');
@ -539,10 +528,10 @@ sub _update_votes {
# If the user is voting for bugs, make sure they aren't overstuffing
# the ballot box.
if (scalar @bugs) {
if (scalar @buglist) {
my (%prodcount, %products);
foreach my $bug (@bugs) {
my $bug_id = $bug->id;
foreach my $bug_id (keys %bugs) {
my $bug = $bugs{$bug_id};
my $prod = $bug->product;
$products{$prod} ||= $bug->product_obj;
$prodcount{$prod} ||= 0;
@ -566,56 +555,65 @@ sub _update_votes {
}
}
# Update the user's votes in the database. If the user did not submit
# any votes, they may be using a form with checkboxes to remove all their
# votes (checkboxes are not submitted along with other form data when
# they are not checked, and Bugzilla uses them to represent single votes
# for products that only allow one vote per bug). In that case, we still
# need to clear the user's votes from the database.
my %affected;
# Update the user's votes in the database.
$dbh->bz_start_transaction();
# Take note of, and delete the user's old votes from the database.
my $bug_list = $dbh->selectcol_arrayref('SELECT bug_id FROM votes
my $old_list = $dbh->selectall_arrayref('SELECT bug_id, vote_count FROM votes
WHERE who = ?', undef, $who);
foreach my $id (@$bug_list) {
$affected{$id} = 1;
}
$dbh->do('DELETE FROM votes WHERE who = ?', undef, $who);
my %old_votes = map { $_->[0] => $_->[1] } @$old_list;
my $sth_insertVotes = $dbh->prepare('INSERT INTO votes (who, bug_id, vote_count)
VALUES (?, ?, ?)');
my $sth_updateVotes = $dbh->prepare('UPDATE votes SET vote_count = ?
WHERE bug_id = ? AND who = ?');
# Insert the new values in their place
foreach my $id (@buglist) {
if ($votes{$id} > 0) {
my %affected = map { $_ => 1 } (@buglist, keys %old_votes);
my @deleted_votes;
foreach my $id (keys %affected) {
if (!$votes{$id}) {
push(@deleted_votes, $id);
next;
}
if ($votes{$id} == ($old_votes{$id} || 0)) {
delete $affected{$id};
next;
}
# We use 'defined' in case 0 was accidentally stored in the DB.
if (defined $old_votes{$id}) {
$sth_updateVotes->execute($votes{$id}, $id, $who);
}
else {
$sth_insertVotes->execute($who, $id, $votes{$id});
}
$affected{$id} = 1;
}
if (@deleted_votes) {
$dbh->do('DELETE FROM votes WHERE who = ? AND ' .
$dbh->sql_in('bug_id', \@deleted_votes), undef, $who);
}
# Update the cached values in the bugs table
print $cgi->header();
my @updated_bugs = ();
my $sth_getVotes = $dbh->prepare("SELECT SUM(vote_count) FROM votes
WHERE bug_id = ?");
my $sth_updateVotes = $dbh->prepare("UPDATE bugs SET votes = ?
WHERE bug_id = ?");
$sth_updateVotes = $dbh->prepare('UPDATE bugs SET votes = ? WHERE bug_id = ?');
foreach my $id (keys %affected) {
$sth_getVotes->execute($id);
my $v = $sth_getVotes->fetchrow_array || 0;
$sth_updateVotes->execute($v, $id);
my $confirmed = _confirm_if_vote_confirmed($id);
my $confirmed = _confirm_if_vote_confirmed($bugs{$id} || $id);
push (@updated_bugs, $id) if $confirmed;
}
$dbh->bz_commit_transaction();
print $cgi->header() if scalar @updated_bugs;
$vars->{'type'} = "votes";
$vars->{'title_tag'} = 'change_votes';
foreach my $bug_id (@updated_bugs) {
@ -826,7 +824,7 @@ sub _remove_votes {
# confirm a bug has been reduced, check if the bug is now confirmed.
sub _confirm_if_vote_confirmed {
my $id = shift;
my $bug = new Bugzilla::Bug($id);
my $bug = ref $id ? $id : new Bugzilla::Bug($id);
my $ret = 0;
if (!$bug->everconfirmed

View File

@ -95,8 +95,7 @@
</tr>
[% FOREACH bug = product.bugs %]
<tr [% IF bug.id == this_bug.id && canedit %]
class="bz_bug_being_voted_on" [% END %]>
<tr [% IF bug.id == this_bug.id && canedit %] class="bz_bug_being_voted_on"[% END %]>
<td>
[% IF bug.id == this_bug.id && canedit %]
[% IF product.onevoteonly %]
@ -106,25 +105,25 @@
[% END %]
[%- END %]
</td>
<td align="right"><a name="vote_[% bug.id FILTER html %]">
<td align="right"><a name="vote_[% bug.id FILTER none %]">
[% IF canedit %]
[% IF product.onevoteonly %]
<input type="checkbox" name="[% bug.id FILTER html %]" value="1"
[% " checked" IF bug.count %] id="bug_[% bug.id FILTER html %]">
<input type="checkbox" name="[% bug.id FILTER none %]" value="1"
[% " checked" IF bug.count %] id="bug_[% bug.id FILTER none %]">
[% ELSE %]
<input name="[% bug.id FILTER html %]" value="[% bug.count FILTER html %]"
size="2" id="bug_[% bug.id FILTER html %]">
<input name="[% bug.id FILTER none %]" value="[% bug.count FILTER html %]"
size="2" id="bug_[% bug.id FILTER none %]">
[% END %]
[% ELSE %]
[% bug.count FILTER html %]
[% END %]
</a></td>
<td align="center">
[% bug.id FILTER bug_link(bug) FILTER none %]
[% PROCESS bug/link.html.tmpl bug = bug, link_text = bug.id %]
</td>
<td>
[% bug.summary FILTER html %]
(<a href="page.cgi?id=voting/bug.html&amp;bug_id=[% bug.id FILTER uri %]">Show Votes</a>)
[% bug.short_desc FILTER html %]
(<a href="page.cgi?id=voting/bug.html&amp;bug_id=[% bug.id FILTER none %]">Show Votes</a>)
</td>
</tr>
[% END %]