From 81259736a5b843dcb87ad531aa6990c9bd30d676 Mon Sep 17 00:00:00 2001 From: vfilippov Date: Fri, 26 Aug 2011 14:40:49 +0000 Subject: [PATCH] Bug 82520 - Full diagnostics, full commit (no "non-fatal" checks support) git-svn-id: svn://svn.office.custis.ru/3rdparty/bugzilla.org/trunk@1354 6955db30-a419-402b-8a0d-67ecbb4d7f56 --- Bugzilla/Search.pm | 8 +-- buglist.cgi | 24 +++++---- extensions/custis/lib/BugWorkTime.pm | 62 +++++++++++++++++++---- extensions/custis/lib/Bugzilla/Checker.pm | 2 +- extensions/custis/lib/Checkers.pm | 48 ++++++++++++------ fill-day-worktime.cgi | 26 ++++++++-- post_bug.cgi | 2 - process_bug.cgi | 21 +++----- report.cgi | 3 +- rss-comments.cgi | 1 - userprefs.cgi | 4 +- 11 files changed, 138 insertions(+), 63 deletions(-) diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 30bc0d20d..79f378213 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -8,10 +8,10 @@ This is totally rewritten Bugzilla4Intranet search engine. Original Bugzilla::Search was ugly and stupid. It contained a lot of legacy code and often generated queries very complex -for the DBMS, which leads to awful performance on big databases. +for the DBMS, and search performance was awful on big databases. -Yet in Bugzilla 4.0, although we could expect the authors to refactor -it, they've just decomposed overlong subroutines into small ones. +Although we could expect the authors to refactor it in Bugzilla 4.0, +they've just decomposed overlong subroutines into small ones. So in Bugzilla4Intranet, I (Vitaliy Filippov) have rewritten it totally. @@ -19,7 +19,7 @@ License is dual-license GPL 3.0+ or MPL 1.1+ ("+" means "or any later version"), to be compatible with both GPL and Bugzilla code. Most of the functionality remains unchanged, but the internals are totally -different, as well as the query performance (tested on MySQL) :) +different, as well as query performance is (tested on MySQL) :) =head1 BOOLEAN CHARTS diff --git a/buglist.cgi b/buglist.cgi index 8bcdeabfd..3dc61edad 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -897,24 +897,30 @@ elsif ($fulltext) { if ($superworktime) { # Must come after Bugzilla::Search::getSQL - my $d = $Bugzilla::Search::interval_to; if (Bugzilla->user->in_group('worktimeadmin')) { - # Use DateTime instead of SQL functions to be more DBMS-independent - $d =~ s/(\d)( .*)?$/$1 00:00:00/; - $d ||= POSIX::strftime("%Y-%m-%d 00:00:00", localtime); - $d = datetime_from($d); - $d->subtract(days => 1); - $d = $d->ymd; + my $d = $Bugzilla::Search::interval_to; + if ($d) + { + # Use DateTime instead of SQL functions to be more DBMS-independent + $d =~ s/(\d)( .*)?$/$1 00:00:00/; + $d = datetime_from($d); + $d->subtract(days => 1); + $d = $d->ymd; + } + else + { + $d = POSIX::strftime("%Y-%m-%d", localtime); + } $vars->{worktime_user} = $cgi->param('worktime_user') || ($Bugzilla::Search::interval_who ? $Bugzilla::Search::interval_who->login : undef); + $vars->{worktime_date} = $cgi->param('worktime_date') || $d; } else { - $d = POSIX::strftime("%Y-%m-%d %H:%M:%S", localtime); + $vars->{worktime_date} = POSIX::strftime("%Y-%m-%d", localtime); $vars->{worktime_user} = Bugzilla->user->login; } $vars->{token} = issue_session_token('superworktime'); - ($vars->{worktime_date}) = $cgi->param('worktime_date') || $d; } ################################################################################ diff --git a/extensions/custis/lib/BugWorkTime.pm b/extensions/custis/lib/BugWorkTime.pm index 19f030c59..f1e5d350d 100644 --- a/extensions/custis/lib/BugWorkTime.pm +++ b/extensions/custis/lib/BugWorkTime.pm @@ -10,12 +10,14 @@ use Bugzilla::Bug; use Bugzilla::Util; use Bugzilla::Token; -# used by mass worktime editing forms +# Used by mass worktime editing forms +# Returns 0 if update is unsuccessful, 1 if OK sub FixWorktime { my ($bug, $wtime, $comment, $timestamp, $userid) = @_; $bug = Bugzilla::Bug->new({ id => $bug, for_update => 1 }) unless ref $bug; - return undef unless $bug && ($comment || $wtime); + return 0 unless $bug; + return 1 unless $comment || $wtime; my $remaining_time = $bug->remaining_time; my $newrtime = $remaining_time - $wtime; @@ -29,8 +31,15 @@ sub FixWorktime $bug->remaining_time($newrtime) if $newrtime != $remaining_time; $bug->update(); - # stop bugmail - Bugzilla->dbh->do('UPDATE bugs SET lastdiffed=NOW() WHERE bug_id=?', undef, $bug->id); + if (@{$bug->{failed_checkers} || []} && !$bug->{passed_checkers}) + { + return 0; + } + else + { + # stop bugmail + Bugzilla->dbh->do('UPDATE bugs SET lastdiffed=NOW() WHERE bug_id=?', undef, $bug->id); + } return 1; } @@ -95,8 +104,17 @@ sub DistributeWorktime $bug->update(); - # stop bugmail - Bugzilla->dbh->do('UPDATE bugs SET lastdiffed=NOW() WHERE bug_id=?', undef, $bug->id); + if (@{$bug->{failed_checkers} || []} && !$bug->{passed_checkers}) + { + return 0; + } + else + { + # stop bugmail + Bugzilla->dbh->do('UPDATE bugs SET lastdiffed=NOW() WHERE bug_id=?', undef, $bug->id); + } + + return 1; } # CustIS Bug 68921 - "Супер-TodayWorktime", или массовая фиксация трудозатрат @@ -167,6 +185,9 @@ sub HandleSuperWorktime # перемещаем $other_bug_id в конец, чтобы не сбить пропорцию в процессе списывания времени @bugids = (@bi, $other_bug_id); } + my ($ok, $rollback) = 0; + Bugzilla->request_cache->{checkers_hide_error} = 1; + Bugzilla->dbh->bz_start_transaction(); foreach (@bugids) { $t = $args->{"wtime_$_"}; @@ -175,24 +196,43 @@ sub HandleSuperWorktime Bugzilla->dbh->bz_start_transaction(); if ($bug = Bugzilla::Bug->new({ id => $_, for_update => 1 })) { + # Юзеры хотят цельный коммит и построчную диагностику... + # Так что если хотя бы одно изменение не пройдёт, потом сделаем полный откат if ($wt_user) { - # списываем время на одного юзера - BugWorkTime::FixWorktime($bug, $t, $comment, $wt_date, $wt_user->id); + # Списываем время на одного юзера + $ok = BugWorkTime::FixWorktime($bug, $t, $comment, $wt_date, $wt_user->id); } else { - # распределяем время по участникам - BugWorkTime::DistributeWorktime( + # Распределяем время по участникам + $ok = BugWorkTime::DistributeWorktime( $bug, $t, $comment, $wt_date, $tsfrom, $tsto, $args->{divide_min_inc}, $other_bug_id ); } } - Bugzilla->dbh->bz_commit_transaction(); + if ($ok) + { + Bugzilla->dbh->bz_commit_transaction(); + } + else + { + $rollback = 1; + } } $cgi->delete("wtime_$_"); } + if ($rollback) + { + # Цельный откат, если хотя бы одно изменение блокируется + Bugzilla->dbh->bz_rollback_transaction(); + } + else + { + Bugzilla->dbh->bz_commit_transaction(); + } + Checkers::show_checker_errors(); $cgi->delete('save_worktime'); print $cgi->redirect(-location => $cgi->self_url); exit; diff --git a/extensions/custis/lib/Bugzilla/Checker.pm b/extensions/custis/lib/Bugzilla/Checker.pm index a0e698cc9..01871ff73 100644 --- a/extensions/custis/lib/Bugzilla/Checker.pm +++ b/extensions/custis/lib/Bugzilla/Checker.pm @@ -76,7 +76,7 @@ sub refresh_sql my $search = new Bugzilla::Search( params => $params, fields => [ 'bug_id' ], - user => Bugzilla::User->super_user, + user => $query->user, ); my $terms = Bugzilla::Search::simplify_expression([ 'AND_MANY', { term => 'bugs.bug_id=?' }, diff --git a/extensions/custis/lib/Checkers.pm b/extensions/custis/lib/Checkers.pm index cc89460d8..1886a250f 100644 --- a/extensions/custis/lib/Checkers.pm +++ b/extensions/custis/lib/Checkers.pm @@ -15,8 +15,6 @@ use Bugzilla::Checker; use Bugzilla::Search::Saved; use Bugzilla::Error; -our $THROW_ERROR = 1; # 0 во время массовых изменений - sub refresh_checker { my ($query) = @_; @@ -69,11 +67,14 @@ sub check return $checked; } +# Откатывает изменения до последней SAVEPOINT, если проверки не прошли. +# Ставит $bug->{passed_checkers} = прошли ли проверки (с учётом "do what i say"), и возвращает его же. +# Если завалены только мягкие проверки и !Bugzilla->request_cache->{checkers_hide_error} - показывает предупреждение. sub alert { my ($bug, $is_new) = @_; my (@fatal, @warn); - map { $_->is_fatal ? push(@fatal, $_) : push(@warn, $_) } @{$bug->{failed_checkers}}; + map { $_->is_fatal ? push(@fatal, $_) : push(@warn, $_) } @{$bug->{failed_checkers} || []}; my $force = 1 && Bugzilla->cgi->param('force_checkers'); if (!@fatal && (!@warn || $force)) { @@ -95,23 +96,34 @@ sub alert } # нужно откатить изменения ТОЛЬКО ОДНОГО бага (см. process_bug.cgi) Bugzilla->dbh->bz_rollback_to_savepoint; - if ($THROW_ERROR) + if (!Bugzilla->request_cache->{checkers_hide_error}) { - Bugzilla->template->process("verify-checkers.html.tmpl", { - failed => [ $bug ], - allow_commit => !@fatal, - exclude_params_re => '^force_checkers$', - }) || ThrowTemplateError(Bugzilla->template->error); - exit; + show_checker_errors([ $bug ]); } } + return $bug->{passed_checkers}; +} + +# Показать сообщение об ошибке проверки/проверок +sub show_checker_errors +{ + my ($bugs) = @_; + $bugs ||= Bugzilla->request_cache->{failed_checkers}; + return if !$bugs || !grep { @{$_->{failed_checkers} || []} } @$bugs; + my $fatal = 1 && (grep { grep { $_->is_fatal } @{$_->{failed_checkers} || []} } @$bugs); + Bugzilla->template->process("verify-checkers.html.tmpl", { + failed => $bugs, + allow_commit => !$fatal, + exclude_params_re => '^force_checkers$', + }) || ThrowTemplateError(Bugzilla->template->error); + exit; } sub freeze_failed_checkers { my $failedbugs = shift; $failedbugs && @$failedbugs || return undef; - return [ map { [ $_->bug_id, [ map { $_->id } @{$_->{failed_checkers}} ] ] } @$failedbugs ]; + return [ map { [ $_->bug_id, [ map { $_->id } @{$_->{failed_checkers}} ] ] } grep { $_->{failed_checkers} } @$failedbugs ]; } sub unfreeze_failed_checkers @@ -215,18 +227,24 @@ sub bug_end_of_update # запускаем проверки, работающие ПОСЛЕ внесения изменений push @{$bug->{failed_checkers}}, @{ check($bug->bug_id, CF_UPDATE, CF_FREEZE | CF_UPDATE) }; + # фильтруем по изменениям if (@{$bug->{failed_checkers}}) { filter_failed_checkers($bug->{failed_checkers}, $changes, $bug); } - # ругаемся/откатываем изменения, если что-то есть + # ругаемся и откатываем изменения, если есть заваленные проверки if (@{$bug->{failed_checkers}}) { - alert($bug); - %{ $args->{changes} } = (); - $bug->{added_comments} = undef; + if (!alert($bug)) + { + %{ $args->{changes} } = (); + $bug->{added_comments} = undef; + } + # запоминаем объект бага с зафейленными проверками в request_cache + push @{Bugzilla->request_cache->{failed_checkers} ||= []}, $bug; } + return 1; } diff --git a/fill-day-worktime.cgi b/fill-day-worktime.cgi index 389b95309..74bde4287 100755 --- a/fill-day-worktime.cgi +++ b/fill-day-worktime.cgi @@ -84,12 +84,33 @@ if (@idlist || @lines) add_wt($wtime, $id, $time, $comment); } } + Bugzilla->request_cache->{checkers_hide_error} = 1; + my $rollback = 0; + $dbh->bz_start_transaction(); foreach my $id (@{$wtime->{IDS}}) { $dbh->bz_start_transaction(); - BugWorkTime::FixWorktime($id, $wtime->{$id}->{time}, join("\n", @{$wtime->{$id}->{comments} || []})); - $dbh->bz_commit_transaction(); + if (!BugWorkTime::FixWorktime($id, $wtime->{$id}->{time}, join("\n", @{$wtime->{$id}->{comments} || []}))) + { + $rollback = 1; + } + else + { + $dbh->bz_commit_transaction(); + } } + if ($rollback) + { + # Rollback all changes if some are blocked by Checkers + # Note that there is no support for "non-fatal" checks, + # and the warning with "do what i say..." text is not shown + Bugzilla->dbh->bz_rollback_transaction(); + } + else + { + Bugzilla->dbh->bz_commit_transaction(); + } + Checkers::show_checker_errors(); print $cgi->redirect(-location => "fill-day-worktime.cgi?lastdays=" . $lastdays); exit; } @@ -105,7 +126,6 @@ if ($query_id) my $search = new Bugzilla::Search( params => $queryparams, fields => [ "bugs.bug_id" ], - user => $user, ); $sqlquery = $search->bugid_query; } diff --git a/post_bug.cgi b/post_bug.cgi index 3b499e34f..a19ddf20a 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -177,8 +177,6 @@ foreach my $field (@multi_selects) { $bug_params{$field->name} = [$cgi->param($field->name)]; } -$Checkers::THROW_ERROR = 1; - # CustIS Bug 63152 - Duplicated bugs on attachment create errors Bugzilla->dbh->bz_start_transaction; diff --git a/process_bug.cgi b/process_bug.cgi index 9b71e9ded..eb770062c 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -556,12 +556,10 @@ foreach my $b (@bug_objects) { Bugzilla::Hook::process('process_bug-pre_update', { bugs => \@bug_objects }); -my $failed_checkers = []; - # @msgs will store emails which have to be sent to voters, if any. my @msgs; -$Checkers::THROW_ERROR = @bug_objects == 1; +Bugzilla->request_cache->{checkers_hide_error} = 1 if @bug_objects > 1; ############################## # Do Actual Database Updates # @@ -572,15 +570,12 @@ foreach my $bug (@bug_objects) { my $timestamp = $dbh->selectrow_array(q{SELECT LOCALTIMESTAMP(0)}); my $changes = $bug->update($timestamp); - if ($bug->{failed_checkers} && @{$bug->{failed_checkers}}) + if ($bug->{failed_checkers} && @{$bug->{failed_checkers}} && + !$bug->{passed_checkers}) { - push @$failed_checkers, $bug; - unless ($bug->{passed_checkers}) - { - # This means update is blocked - # and rollback_to_savepoint is already done in Checkers.pm - next; - } + # This means update is blocked + # and rollback_to_savepoint is already done in Checkers.pm + next; } my %notify_deps; @@ -698,7 +693,7 @@ foreach my $msg (@msgs) { # Send bugmail send_results($_) for @$send_results; $vars->{sentmail} = $send_results; -$vars->{failed_checkers} = $failed_checkers; +$vars->{failed_checkers} = Bugzilla->request_cache->{failed_checkers}; my $bug; if (Bugzilla->usage_mode == USAGE_MODE_EMAIL) { @@ -731,7 +726,7 @@ elsif (($action eq 'next_bug' or $action eq 'same_bug') && ($bug = $vars->{bug}) title => $title, sent_attrs => $send_attrs, # CustIS Bug 68921 - Correctness checkers - failed_checkers => Checkers::freeze_failed_checkers($failed_checkers), + failed_checkers => Checkers::freeze_failed_checkers(Bugzilla->request_cache->{failed_checkers}), }; # CustIS Bug 38616 - CC list restriction if (scalar(@bug_objects) == 1 && $bug_objects[0]->{restricted_cc}) diff --git a/report.cgi b/report.cgi index 7fcc055cd..bfe570bbb 100755 --- a/report.cgi +++ b/report.cgi @@ -29,6 +29,7 @@ use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::Field; +use Bugzilla::Search; my $cgi = Bugzilla->cgi; my $template = Bugzilla->template; @@ -45,8 +46,6 @@ if (grep(/^cmd-/, $cgi->param())) { exit; } -use Bugzilla::Search; - Bugzilla->login(); my $dbh = Bugzilla->switch_to_shadow_db(); diff --git a/rss-comments.cgi b/rss-comments.cgi index e1bd31808..d33cf33ea 100755 --- a/rss-comments.cgi +++ b/rss-comments.cgi @@ -55,7 +55,6 @@ $vars->{urlquerypart} = $queryparams->canonicalise_query('order', 'cmdtype', 'qu my $search = new Bugzilla::Search( params => $queryparams, fields => [ "bug_id" ], - user => $user ); my $sqlquery = $search->bugid_query; diff --git a/userprefs.cgi b/userprefs.cgi index be0c4f3e1..a06b8c59c 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -212,7 +212,7 @@ sub DoEmail if ($_->[1] eq $userid) { push @{$vars->{watchedusers}}, Bugzilla::User->new($_->[0]); - } + } else { push @{$vars->{watchers}}, Bugzilla::User->new($_->[1]); @@ -234,7 +234,7 @@ sub DoEmail $mail{$relationship}{$event} = 1; } - $vars->{'mail'} = \%mail; + $vars->{'mail'} = \%mail; } sub SaveEmail {