Red Hat Bugzilla – Bug 406261
3.16: Bugzilla/SanityCheck.pm
Last modified: 2013-06-24 00:15:49 EDT
Description: The Bugzilla/SanityCheck.pm module contains the code that was originally in sanitycheck.cgi. The module was created so that different front ends could be wrapped around it such as a command line utility or web UI. Function Requirements: Bugzilla/SanityCheck.pm sanitycheck.pl
Looking at the module Bugzilla/SanityCheck.pm in rh_bugzilla_2_18 and sanitycheck.cgi in Bugzilla_3 , The following is noted: 1- Bugzilla_3 sanitycheck.cgi includes code that checks for user permissions to perform the sanitychecks, code to perform the sanity checking for the following: (referential cross checks, double field referential cross checks, login checks, vote/keyword cache checks, flags being in incorrect products and components, General bug checks, Control Values, Unsent mail) ,, and then code to fix the errors found by the sanity checking. 2- rh_bugzilla_2_18 Bugzilla/SanityCheck.pm includes code that performs sanity checking only ,no code for fixing and no code for checking user permissions, as this module is used by the bugzilla team to sanity check the database. The checks included are : ('foreign_keys', 'profile_loginnames', 'keyword_cache', 'general_bug_check','date_check','control_values', 'unsent_emails','bugs_components') . 3- So basically the checks performed by our rh_bugzilla_2_18 Bugzilla/SanityCheck.pm are similar to the ones in Bugzilla_3 sanitycheck.cgi only one check we are missing which is: flags being in incorrect products and components which is of total 40 lines of code (calculated by running cloc on that code) Also some work will be needed to convert the current SanityCheck.pm code to the coding standards, and few lines of code will be added in sanitycheck.pl to call that new check that will be added So I would expect in total it would be around 60 lines of code + the effort for the conversion of code to comply with coding standards.
1. What about LOC for unit tests? 2. Also, this sounds like we would be trying to get a patch accepted upstream. There is a risk then that the upstream won't accept it. What is your plan B in this case? How will you code it then?
(In reply to comment #2) > 1. What about LOC for unit tests? > Basically for unit test I think sanitycheck.pl will do the job for us ,, as we can only test that extra check by giving option for this check in the command line to sanitycheck.pl > 2. Also, this sounds like we would be trying to get a patch accepted upstream. > There is a risk then that the upstream won't accept it. What is your plan B in > this case? How will you code it then? I am not sure if the upstream would want to replace sanitycheck.cgi with SanityCheck.pm ,, we have done that replacement when someone ran sanitycheck.cgi and produced a lot of annoying unnecessary emails to lots of people in redhat. I think they might accept SanityCheck.pm as an entire new module ,, but this mean that they will have replicated code,, hmm so my plan would be to move all the functionality from sanitycheck.cgi to Bugzilla/SanityCheck.pm that includes all the code for the checks and for the fixing. Then we would code sanitycheck.cgi to call those functions in SanityCheck.pm so it can provide an interface for the users through the web UI and we should keep all the permissions restrictions for running those checks in sanitycheck.cgi ,, and we will have sanitycheck.pl to do similar to what it is doing now in our rh_bugzilla_2_18 which is similar to what it is doing in the upstream version only we would convert it to use SanityCheck.pm instead of using sanitycheck.cgi
(In reply to comment #3) > (In reply to comment #2) > > 1. What about LOC for unit tests? > > > Basically for unit test I think sanitycheck.pl will do the job for us ,, as we > can only test that extra check by giving option for this check in the command > line to sanitycheck.pl That is not a test. A unit test should test the functions in the SanityCheck.pm module. > > 2. Also, this sounds like we would be trying to get a patch accepted upstream. > > There is a risk then that the upstream won't accept it. What is your plan B in > > this case? How will you code it then? > > I am not sure if the upstream would want to replace sanitycheck.cgi with > SanityCheck.pm ,, we have done that replacement when someone ran sanitycheck.cgi > and produced a lot of annoying unnecessary emails to lots of people in redhat. I > think they might accept SanityCheck.pm as an entire new module ,, but this mean > that they will have replicated code,, hmm so my plan would be to move all the > functionality from sanitycheck.cgi to Bugzilla/SanityCheck.pm that includes all > the code for the checks and for the fixing. Then we would code sanitycheck.cgi > to call those functions in SanityCheck.pm so it can provide an interface for the > users through the web UI and we should keep all the permissions restrictions for > running those checks in sanitycheck.cgi ,, and we will have sanitycheck.pl to do > similar to what it is doing now in our rh_bugzilla_2_18 which is similar to what > it is doing in the upstream version only we would convert it to use > SanityCheck.pm instead of using sanitycheck.cgi Talk with the upstream about how best to do this. The only way to know is to ask. ;)
Created attachment 301931 [details] patch to enable specific sanitychecks and diff checks in sanitycheck.cgi and sanitycheck.pl HI,, Attached is a patch to sanitycheck.cgi and sanitycheck.pl ,, it will enable the following: 1- user can run specific sanitycheck reports from the webui by passing cgi params like for example: https://bugdev.devel.redhat.com/bugzilla/sanitycheck.cgi?foreign_keys=1&unsent_emails=1 2- if user didn specify any check cgi params then all checks will run. 4- user also can use commandline script sanitycheck.pl to run specific checks same way as how it worked in RHBZ2.18 for example: perl sanitycheck.pl --profile_loginnames --unsent_emails --login 'bugzilla-owner@redhat.com' 5- user can generate diff reports by running sanitycheck.pl with option --diff_report 6- if user wants to get full sanitycheck reports not only the errors then they can use --verbose option - Please note that there is no need now for Bugzilla/SanityCheck.pm to be borted from 2.18 to 3.2 and sanitycheck.cgi and sanitycheck.pl do its work nicely. - I haven't put REDHAT tags except for the redhat database fields and table in the cross checks , as I was thiking we might contribute this patch to upstream, but I would think maybe better to give me your opinions first before i submit it upstream. -please note the patch looks abit too long but that is becuase there is a lot of identation changes resulted from adding if statements around
Created attachment 301932 [details] patch to enable specific sanitychecks and diff checks in sanitycheck.cgi and sanitycheck.pl HI,, Attached is a patch to sanitycheck.cgi and sanitycheck.pl ,, it will enable the following: 1- user can run specific sanitycheck reports from the webui by passing cgi params like for example: https://bugdev.devel.redhat.com/bugzilla/sanitycheck.cgi?foreign_keys=1&unsent_emails=1 2- if user didn specify any check cgi params then all checks will run. 4- user also can use commandline script sanitycheck.pl to run specific checks same way as how it worked in RHBZ2.18 for example: perl sanitycheck.pl --profile_loginnames --unsent_emails --login 'bugzilla-owner@redhat.com' 5- user can generate diff reports by running sanitycheck.pl with option --diff_report 6- if user wants to get full sanitycheck reports not only the errors then they can use --verbose option - Please note that there is no need now for Bugzilla/SanityCheck.pm to be borted from 2.18 to 3.2 and sanitycheck.cgi and sanitycheck.pl do its work nicely. - I haven't put REDHAT tags except for the redhat database fields and table in the cross checks , as I was thiking we might contribute this patch to upstream, but I would think maybe better to give me your opinions first before i submit it upstream. -please note the patch looks abit too long but that is becuase there is a lot of identation changes resulted from adding if statements around the different check sections in sanitycheck.cgi. please review when you can. Thanks, Noura
worked 12 hours
the patch is applied to bugdev.devel.redhat.com , so you can test it there.
please note that there are 2 checks that we used to have in 2.18 that are not required anymore for 3.2 which are: 1- date_check ,, as that check was used for groups.last_changed and profiles.refreshed_when those fields don't exist anymore. 2- bugs_components,, for some bugs were found by Tony where the component ids in some bugs were inconsistent with the actual component ids of the bug's product, running that check produced no errors at all so I am guess that Tony has fixed them all , so need for that check. Tony can you please confirm. Noura
Comment on attachment 301932 [details] patch to enable specific sanitychecks and diff checks in sanitycheck.cgi and sanitycheck.pl >Index: sanitycheck.cgi >=================================================================== Looks fine to me. >Index: sanitycheck.pl >=================================================================== >RCS file: /cvs/qa/rh_bugzilla_3/sanitycheck.pl,v >retrieving revision 1.2 >diff -p -u -r1.2 sanitycheck.pl >--- sanitycheck.pl 31 Jan 2008 17:48:09 -0000 1.2 >+++ sanitycheck.pl 10 Apr 2008 06:07:50 -0000 >@@ -18,6 +18,7 @@ > # Frédéric Buclin. All Rights Reserved. > # > # Contributor(s): Frédéric Buclin <LpSolit@gmail.com> >+# Noura Elhawary <nelhawar@redhat.com> > > use strict; > >@@ -32,49 +33,157 @@ use Bugzilla::Mailer; > use Getopt::Long; > use Pod::Usage; > >-my $verbose = 0; # Return all comments if true, else errors only. >+use English qw( -no_match_vars ); >+use Text::Diff; You will need to add this to Bugzilla/Install/Requirements.pm so that checksetup.pl will complain if Text::Diff is not installed. Otherwise you should code this to make it an optional feature. my $do_text_diff = 1; eval "use Text::Diff"; $@ && $do_text_diff = 0; >+my $verbose = 0; # Return all comments if true, else errors only. > my $login = ''; # Login name of the user which is used to call sanitycheck.cgi. >-my $help = 0; # Has user asked for help on this script? >+my $help = 0; # Has user asked for help on this script? > Nit: Line variables up with $version = 0; > >-pod2usage({-verbose => 1, -exitval => 1}) if $help; >+GetOptions( \%options, 'verbose', 'login=s', 'help|h|?', @OptionDefs ); >+ >+pod2usage( { -verbose => 1, -exitval => 1 } ) if $help; Nit: Upstream does not like the extra whitespace near the parens ( and ) even though perltidy does. So we should remove them for quicker review. > Bugzilla->usage_mode(USAGE_MODE_CMDLINE); > > # Be sure a login name if given. > $login || ThrowUserError('invalid_username'); > >-my $user = new Bugzilla::User({ name => $login }) >- || ThrowUserError('invalid_username', { name => $login }); >+my $user = new Bugzilla::User( { name => $login } ) >+ || ThrowUserError( 'invalid_username', { name => $login } ); > Nit: same comment about extra whitespace. Also definitely do not change any lines that are not necessary for program proper function or are not additional lines we add. This will keep the patch size smaller and insure quicker review from upstream. >-my $cgi = Bugzilla->cgi; >+my $cgi = Bugzilla->cgi; > my $template = Bugzilla->template; > Same comment as above. > # Authenticate using this user account. > Bugzilla->set_user($user); > >-# Pass this param to sanitycheck.cgi. >-$cgi->param('verbose', $verbose); >+# build check list >+my @checks; >+for my $check (@Checks) { >+ if ( $options{$check} ) { >+ $cgi->param( $check, 1 ); >+ push @checks, $check; >+ } >+} >+ >+# if no checks given on command line then assume all checks >+if ( @checks == 0 ) { >+ push @checks, @Checks; >+ $cgi->param( $_, 1 ) foreach @checks; >+} > >+# run checks and report >+my $msg_body = ''; # accumulate check reports here >+ >+# Pass this param to sanitycheck.cgi. >+$cgi->param( 'verbose', $verbose ); > require 'sanitycheck.cgi'; > >-# Now it's time to send an email to the user if there is something to notify. >-if ($cgi->param('output')) { >+for my $check (@checks) { >+ >+ my $out_rpt = $check . "_report"; >+ >+ # Now it's time to send an email to the user if there is something to notify. > my $message; > my $vars = {}; >- $vars->{'addressee'} = $user->email; >- $vars->{'output'} = $cgi->param('output'); >+ $vars->{'addressee'} = $user->email; >+ $vars->{'output'} = $cgi->param($out_rpt); > $vars->{'error_found'} = $cgi->param('error_found') ? 1 : 0; > >- $template->process('email/sanitycheck.txt.tmpl', $vars, \$message) >- || ThrowTemplateError($template->error()); >+ $template->process( 'email/sanitycheck.txt.tmpl', $vars, \$message ) >+ || ThrowTemplateError( $template->error() ); >+ No white space changes here. >+ # full_report if given then report in case of FAIL or PASS >+ # otherwise only report in case of FAIL >+ if ( $options{diff_report} ) { >+ $msg_body .= create_diff_report( $check, $message ); >+ } Add the $do_text_diff check here: if ($options{diff_report} && $do_text_diff) { $msg_body .= create_diff_report( $check, $message ); } >+ else { >+ $msg_body .= create_report( $check, $message ); >+ } >+} The rest looks fine to me. Dave
Also please make sure you pass back in the enabled checks when clicking on any of the lines in the sanitycheck.cgi report such as "Click here to delete invalid flags", "Click here to rebuild the keyword cache", etc. Otherwise when you click on any of those links, it will try to run all checks on the second pass. You will probably have to do this in the templates as well as sanitycheck.cgi. Here is some patch code to illustrate what I mean: Index: sanitycheck.cgi =================================================================== RCS file: /cvs/qa/rh_bugzilla_3/sanitycheck.cgi,v retrieving revision 1.2 diff -u -r1.2 sanitycheck.cgi --- sanitycheck.cgi 2 Apr 2008 18:48:27 -0000 1.2 +++ sanitycheck.cgi 11 Apr 2008 16:46:18 -0000 @@ -49,6 +50,9 @@ my $cgi = Bugzilla->cgi; return if (!$alert && Bugzilla->usage_mode == USAGE_MODE_CMDLINE && !$cgi->param('verbose')); + # Pass in current query url as well + $vars->{query} = $cgi->canonicalise_query(); + if (Bugzilla->usage_mode == USAGE_MODE_CMDLINE) { my $output = $cgi->param('output') || ''; my $linebreak = $alert ? "\nALERT: " : "\n"; Index: template/en/default/admin/sanitycheck/messages.html.tmpl =================================================================== RCS file: /cvs/qa/rh_bugzilla_3/template/en/default/admin/sanitycheck/messages.html.tmpl,v retrieving revision 1.3 diff -u -r1.3 messages.html.tmpl --- template/en/default/admin/sanitycheck/messages.html.tmpl 27 Feb 2008 20:50:44 -0000 1.3 +++ template/en/default/admin/sanitycheck/messages.html.tmpl 11 Apr 2008 16:46:18 -0000 @@ -34,7 +34,7 @@ [% errortext FILTER html %]: [% INCLUDE bug_list badbugs = badbugs %] [% ELSIF san_tag == "bug_check_repair" %] - <a href="sanitycheck.cgi?[% param FILTER url_quote %]=1">[% text FILTER html %]</a>. + <a href="sanitycheck.cgi?[% param FILTER url_quote %]=1&[% query %]">[% text FILTER html %]</a>. [% ELSIF san_tag == "bug_check_creation_date" %] Checking for [% terms.bugs %] with no creation date (which makes them invisible). Index: template/en/default/filterexceptions.pl =================================================================== RCS file: /cvs/qa/rh_bugzilla_3/template/en/default/filterexceptions.pl,v retrieving revision 1.4 diff -u -r1.4 filterexceptions.pl --- template/en/default/filterexceptions.pl 31 Mar 2008 18:35:32 -0000 1.4 +++ template/en/default/filterexceptions.pl 11 Apr 2008 16:46:18 -0000 @@ -557,6 +557,10 @@ 'new_status.id', ], +'admin/sanitycheck/messages.html.tmpl' => [ + 'query', +], + 'account/login.html.tmpl' => [ 'target', ],
Created attachment 302299 [details] sanitycheck patch Thanks for the review Dave. Here is another patch with all your suggestions applied. Noura
worked one hour
Comment on attachment 302299 [details] sanitycheck patch The recent check in of changes upstream submitted by you causes conflicts with sanitycheck.cgi when applying your patch (upstream bug 427936). I have merged the upstream sanitycheck.cgi changes with our tree recently so update your checkout and generate a new patch for review. >+if ($cgi->param('foreign_keys')) { >+ $cgi->param('output' , ''); >+ >+ CrossCheck('classifications', 'id', >+ ['products', 'classification_id']); >+ >+ CrossCheck("keyworddefs", "id", >+ ["keywords", "keywordid"]); >+ >+ CrossCheck("fielddefs", "id", >+ ["bugs_activity", "fieldid"], >+ ['profiles_activity', 'fieldid']); >+ >+ CrossCheck("flagtypes", "id", >+ ["flags", "type_id"], >+ ["flagexclusions", "type_id"], >+ ["flaginclusions", "type_id"]); >Index: sanitycheck.pl >=================================================================== >-my $result = GetOptions('verbose' => \$verbose, >- 'login=s' => \$login, >- 'help|h|?' => \$help); >+use English qw( -no_match_vars ); >+use Text::Diff; > >-pod2usage({-verbose => 1, -exitval => 1}) if $help; >+my $do_text_diff = 1; >+eval "use Text::Diff"; >+if ($@){ >+ warn $@; >+ $do_text_diff = 0; >+} In sanitycheck.pl you are still doing 'use Text::Diff;' on line 37 even though you are eval'ing 'use Text::Diff;' on line 40. Please remove the first one. Dave
Created attachment 302332 [details] Looks good Noura. Go ahead and check in. Dave Hi Dave,, I fixed the patch so it deosn conflict with sanitycheck.cgi and also made a fix for the use of Text::Diff Thanks, Noura
Comment on attachment 302332 [details] Looks good Noura. Go ahead and check in. Dave >Index: sanitycheck.pl >=================================================================== >RCS file: /cvs/qa/rh_bugzilla_3/sanitycheck.pl,v >retrieving revision 1.2 >diff -p -u -r1.2 sanitycheck.pl >--- sanitycheck.pl 31 Jan 2008 17:48:09 -0000 1.2 >+++ sanitycheck.pl 14 Apr 2008 13:12:58 -0000 >@@ -18,6 +18,7 @@ > # Frédéric Buclin. All Rights Reserved. > # > # Contributor(s): Frédéric Buclin <LpSolit@gmail.com> >+# Noura Elhawary <nelhawar@redhat.com> > > use strict; > >@@ -32,15 +33,41 @@ use Bugzilla::Mailer; > use Getopt::Long; > use Pod::Usage; > >-my $verbose = 0; # Return all comments if true, else errors only. >-my $login = ''; # Login name of the user which is used to call sanitycheck.cgi. >-my $help = 0; # Has user asked for help on this script? >- >-my $result = GetOptions('verbose' => \$verbose, >- 'login=s' => \$login, >- 'help|h|?' => \$help); >+use English qw( -no_match_vars ); > >-pod2usage({-verbose => 1, -exitval => 1}) if $help; >+my $do_text_diff = 1; >+eval {use Text::Diff}; >+if ($@){ >+ warn $@; >+ $do_text_diff = 0; >+} >+ >+my $verbose = 0; # Return all comments if true, else errors only. >+my $login = ''; # Login name of the user which is used to call sanitycheck.cgi. >+my $help = 0; # Has user asked for help on this script? >+ >+my %options = ('verbose' => \$verbose, 'login' => \$login, 'help' => \$help); >+ >+my @OptionDefs = ( >+ 'foreign_keys', 'profile_loginnames', >+ 'keyword_cache', 'general_bug_check', >+ 'control_values', 'unsent_emails', >+ 'flag_check', 'diff_report'); >+ >+# check options >+my @Checks = qw( >+ foreign_keys >+ profile_loginnames >+ keyword_cache >+ general_bug_check >+ control_values >+ unsent_emails >+ flag_check >+); >+ >+GetOptions(\%options, 'verbose', 'login=s', 'help|h|?', @OptionDefs); >+ >+pod2usage({ -verbose => 1, -exitval => 1 }) if $help; > > Bugzilla->usage_mode(USAGE_MODE_CMDLINE); > >@@ -56,25 +83,113 @@ my $template = Bugzilla->template; > # Authenticate using this user account. > Bugzilla->set_user($user); > >+# build check list >+my @checks; >+for my $check (@Checks) { >+ if ( $options{$check} ) { >+ $cgi->param( $check, 1 ); >+ push @checks, $check; >+ } >+} >+ >+# if no checks given on command line then assume all checks >+if (@checks == 0) { >+ push @checks, @Checks; >+ $cgi->param($_, 1) foreach @checks; >+} >+ >+# run checks and report >+my $msg_body = ''; # accumulate check reports here >+ > # Pass this param to sanitycheck.cgi. > $cgi->param('verbose', $verbose); > > require 'sanitycheck.cgi'; > >-# Now it's time to send an email to the user if there is something to notify. >-if ($cgi->param('output')) { >+for my $check (@checks) { >+ >+ my $out_rpt = $check . "_report"; >+ >+ # Now it's time to send an email to the user if there is something to notify. > my $message; > my $vars = {}; > $vars->{'addressee'} = $user->email; >- $vars->{'output'} = $cgi->param('output'); >+ $vars->{'output'} = $cgi->param($out_rpt); > $vars->{'error_found'} = $cgi->param('error_found') ? 1 : 0; > > $template->process('email/sanitycheck.txt.tmpl', $vars, \$message) > || ThrowTemplateError($template->error()); > >- MessageToMTA($message); >+ # full_report if given then report in case of FAIL or PASS >+ # otherwise only report in case of FAIL >+ if ($options{diff_report} && $do_text_diff) { >+ $msg_body .= create_diff_report($check, $message); >+ } >+ else { >+ $msg_body .= create_report($check, $message); >+ } >+} >+ >+if (Bugzilla->params->{mail_delivery_method} eq 'None') { >+ print "$msg_body\n"; >+} >+else { >+ MessageToMTA($msg_body); >+} >+ >+# returns string report >+sub create_report { >+ my ($check, $rpt) = @_; >+ >+ my $check_rpt = >+ "Check: $check\n====================================" >+ . "===============================\n$rpt\n"; >+ >+ return $check_rpt; >+} >+ >+# returns empty string '' if no difference, otherwise a string representing the diff report >+sub create_diff_report { >+ my ($check, $rpt) = @_; >+ >+ my $report = create_report($check, $rpt); >+ >+ my $baseline_fn = $check . ".orig"; >+ >+ $baseline_fn = "data/sanitycheck/$baseline_fn"; >+ >+ if (not -e $baseline_fn) { >+ create_report_file($baseline_fn, $report); >+ return ''; >+ } >+ >+ # write the current report to a file to diff it against >+ # the baseline report >+ my $rpt_file = 'rpt_tmp_file'; >+ create_report_file($rpt_file, $report); >+ >+ my $diff = diff $baseline_fn, $rpt_file; >+ >+ if ($diff) { >+ return "Difference Check: $check\n" >+ . "===================================================================\n" >+ . "$diff\n\n"; >+ } >+ >+ unlink $rpt_file; >+ return ''; > } > >+sub create_report_file { >+ >+ my ($file_name, $report) = @_; >+ open my $FILE, ">", $file_name >+ or die "Couldn't open '$file_name' : $OS_ERROR"; >+ print {$FILE} $report >+ or die "Couldn't write to '$file_name': $OS_ERROR"; >+ close $FILE or die "Couldn't close '$file_name': $OS_ERROR"; >+ >+} > > __END__ > >@@ -86,6 +201,8 @@ sanitycheck.pl - Perl script to perform > > ./sanitycheck.pl [--help] > ./sanitycheck.pl [--verbose] --login <user@domain.com> >+ ./sanitycheck.pl [check_name] --login <user@domain.com> >+ ./sanitycheck.pl [check_name] [--diff_report] --login <user@domain.com> > > =head1 OPTIONS > >@@ -107,10 +224,70 @@ This should be passed the email address > running the Sanity Check process, a user with the editcomponents priv. This > user will receive an email with the results of the script run. > >+=item B<--diff_report> >+ >+Generates only diff report for checks by comparing freshly generated check report >+with stored report for that check. >+ >+=item B<check_name> >+ >+This can be any one of the following checknames or a combination of them: >+ >+=over >+ >+=item B<--foreign_keys> >+ >+Performs referential integrity checks (cross and double cross). >+ >+=item B<--profile_loginname> >+ >+Performs login email checks for format and lowercase. >+ >+=item B<--keyword_cache> >+ >+Performs vote/keyword cache checks. >+ >+=item B<--general_bug_check> >+ >+Performs general bug checks ex: resolutions, statuses etc. >+ >+=item B<--control_values> >+ >+Performs group control values are validation check. >+ >+=item B<--unsent_emails> >+ >+Performs checks for bugs that have changes but no mail sent for at least half an hour. >+ >+=item B<--flag_check> >+ >+Check for flags being in incorrect products and components. >+ >+=back >+ > =back > > =head1 DESCRIPTION > > This script provides a way of running a 'Sanity Check' on the database > via either a CLI or cron. It is equivalent to calling sanitycheck.cgi >-via a web broswer. >+via a web broswer with an extra feature for generating diff sanitycheck >+reports. If the user specified the checks they want to perform then only >+those checks will be performed, otherwise all checks will be performed. >+ >+The generated check reports can be of three types: >+ >+"full reports" >+ that include errors as well as successful checks, to get >+ those the --verbose option must be set. >+ >+"fail reports" >+ that include only errors in the reports, to get those >+ the --verbose must be ignored. >+ >+"diff reports" >+ to produce diff check reports. freshly generated reports >+ are diffed against original ones kept in the data/sanitycheck >+ dir and the result is the desired diff reports. to get those >+ the --diff_report option must be set. >+ >Index: sanitycheck.cgi >=================================================================== >RCS file: /cvs/qa/rh_bugzilla_3/sanitycheck.cgi,v >retrieving revision 1.3 >diff -p -u -r1.3 sanitycheck.cgi >--- sanitycheck.cgi 12 Apr 2008 05:01:32 -0000 1.3 >+++ sanitycheck.cgi 14 Apr 2008 13:13:11 -0000 >@@ -23,6 +23,7 @@ > # Max Kanat-Alexander <mkanat@bugzilla.org> > # Marc Schumann <wurblzap@gmail.com> > # Frédéric Buclin <LpSolit@gmail.com> >+# Noura Elhawary <nelhawar@redhat.com> > > use strict; > >@@ -49,6 +50,9 @@ sub Status { > my $cgi = Bugzilla->cgi; > return if (!$alert && Bugzilla->usage_mode == USAGE_MODE_CMDLINE && !$cgi->param('verbose')); > >+ # Pass in current query url as well >+ $vars->{query} = $cgi->canonicalise_query(); >+ > if (Bugzilla->usage_mode == USAGE_MODE_CMDLINE) { > my $output = $cgi->param('output') || ''; > my $linebreak = $alert ? "\nALERT: " : "\n"; >@@ -96,6 +100,28 @@ unless (Bugzilla->usage_mode == USAGE_MO > || ThrowTemplateError($template->error()); > } > >+# check if any cgi params were set for specific checks >+# if not then all checks should run >+my $check_all = 1; >+ >+my @check_params = qw( >+ foreign_keys >+ profile_loginnames >+ keyword_cache >+ general_bug_check >+ control_values >+ unsent_emails >+ flag_check >+); >+for my $chk (@check_params) { >+ if ($cgi->param($chk)){ >+ $check_all = 0; >+ } >+} >+if ($check_all) { >+ $cgi->param($_, 1) foreach @check_params; >+} >+ > ########################################################################### > # Users with 'editkeywords' privs only can only check keywords. > ########################################################################### >@@ -390,133 +416,168 @@ sub CrossCheck { > } > } > >-CrossCheck('classifications', 'id', >- ['products', 'classification_id']); >+if ($cgi->param('foreign_keys')) { >+ $cgi->param('output' , ''); >+ >+ CrossCheck('classifications', 'id', >+ ['products', 'classification_id']); >+ >+ CrossCheck("keyworddefs", "id", >+ ["keywords", "keywordid"]); >+ >+ CrossCheck("fielddefs", "id", >+ ["bugs_activity", "fieldid"], >+ ['profiles_activity', 'fieldid']); >+ >+ CrossCheck("flagtypes", "id", >+ ["flags", "type_id"], >+ ["flagexclusions", "type_id"], >+ ["flaginclusions", "type_id"]); >+ >+ CrossCheck("bugs", "bug_id", >+ ["bugs_activity", "bug_id"], >+ ["bug_group_map", "bug_id"], >+ ["attachments", "bug_id"], >+ ["cc", "bug_id"], >+ ["longdescs", "bug_id"], >+ ["dependencies", "blocked"], >+ ["dependencies", "dependson"], >+ ['flags', 'bug_id'], >+ ["votes", "bug_id"], >+ ["keywords", "bug_id"], >+ ["duplicates", "dupe_of", "dupe"], >+ ["duplicates", "dupe", "dupe_of"], >+ # REDHAT EXTENSION START 406261 >+ ["bz_it_map", "bz_id", "it_id"], >+ ["ext_bz_bug_map", "bug_id", "ext_bz_id"]); >+ # REDHAT EXTENSION END 406261 >+ >+ CrossCheck("groups", "id", >+ ["bug_group_map", "group_id"], >+ ['category_group_map', 'group_id'], >+ ["group_group_map", "grantor_id"], >+ ["group_group_map", "member_id"], >+ ["group_control_map", "group_id"], >+ ["namedquery_group_map", "group_id"], >+ ["user_group_map", "group_id"], >+ ["flagtypes", "grant_group_id"], >+ ["flagtypes", "request_group_id"], >+ # REDHAT EXTENSION START 406261 >+ ["flagtypes", "view_group_id"]); >+ # REDHAT EXTENSION END 406261 >+ >+ CrossCheck("namedqueries", "id", >+ ["namedqueries_link_in_footer", "namedquery_id"], >+ ["namedquery_group_map", "namedquery_id"], >+ ); >+ >+ CrossCheck("profiles", "userid", >+ ['profiles_activity', 'userid'], >+ ['profiles_activity', 'who'], >+ ['email_setting', 'user_id'], >+ ['profile_setting', 'user_id'], >+ ["bugs", "reporter", "bug_id"], >+ ["bugs", "assigned_to", "bug_id"], >+ ["bugs", "qa_contact", "bug_id"], >+ ["attachments", "submitter_id", "bug_id"], >+ ['flags', 'setter_id', 'bug_id'], >+ ['flags', 'requestee_id', 'bug_id'], >+ ["bugs_activity", "who", "bug_id"], >+ ["cc", "who", "bug_id"], >+ ['quips', 'userid'], >+ ["votes", "who", "bug_id"], >+ ["longdescs", "who", "bug_id"], >+ ["logincookies", "userid"], >+ ["namedqueries", "userid"], >+ ["namedqueries_link_in_footer", "user_id"], >+ ['series', 'creator', 'series_id'], >+ ["watch", "watcher"], >+ ["watch", "watched"], >+ ['whine_events', 'owner_userid'], >+ ["tokens", "userid"], >+ ["user_group_map", "user_id"], >+ ["components", "initialowner", "name"], >+ ["components", "initialqacontact", "name"], >+ ["component_cc", "user_id"]); >+ >+ CrossCheck("products", "id", >+ ["bugs", "product_id", "bug_id"], >+ ["components", "product_id", "name"], >+ ["milestones", "product_id", "value"], >+ ["versions", "product_id", "value"], >+ ["group_control_map", "product_id"], >+ ["flaginclusions", "product_id", "type_id"], >+ ["flagexclusions", "product_id", "type_id"]); >+ >+ CrossCheck("components", "id", >+ ["component_cc", "component_id"], >+ ["bugs", "component_id"], >+ ["flagexclusions", "component_id", "type_id"], >+ ["flaginclusions", "component_id", "type_id"]); >+ >+ # Check the former enum types -mkanat@bugzilla.org >+ CrossCheck("bug_status", "value", >+ ["bugs", "bug_status", "bug_id"]); >+ >+ CrossCheck("resolution", "value", >+ ["bugs", "resolution", "bug_id"]); >+ >+ CrossCheck("bug_severity", "value", >+ ["bugs", "bug_severity", "bug_id"]); >+ >+ CrossCheck("op_sys", "value", >+ ["bugs", "op_sys", "bug_id"]); >+ >+ CrossCheck("priority", "value", >+ ["bugs", "priority", "bug_id"]); >+ >+ CrossCheck("rep_platform", "value", >+ ["bugs", "rep_platform", "bug_id"]); >+ >+ CrossCheck('series', 'series_id', >+ ['series_data', 'series_id']); >+ >+ CrossCheck('series_categories', 'id', >+ ['series', 'category'], >+ ["category_group_map", "category_id"], >+ ["series", "subcategory", "name"]); >+ >+ # REDHAT EXTENSION START 406261 >+ CrossCheck("external_bugzilla", "id", >+ ["ext_bz_bug_map", "ext_bz_id", "bug_id"]); >+ # REDHAT EXTENSION END 406261 >+ >+ CrossCheck('whine_events', 'id', >+ ['whine_queries', 'eventid'], >+ ['whine_schedules', 'eventid']); >+ >+ CrossCheck('attachments', 'attach_id', >+ ['attach_data', 'id'], >+ ['bugs_activity', 'attach_id']); >+ >+ CrossCheck('bug_status', 'id', >+ ['status_workflow', 'old_status'], >+ ['status_workflow', 'new_status']); >+ >+ DoubleCrossCheck('attachments', 'bug_id', 'attach_id', >+ ['flags', 'bug_id', 'attach_id'], >+ ['bugs_activity', 'bug_id', 'attach_id']); >+ >+ DoubleCrossCheck("components", "product_id", "id", >+ ["bugs", "product_id", "component_id", "bug_id"], >+ ['flagexclusions', 'product_id', 'component_id'], >+ ['flaginclusions', 'product_id', 'component_id']); >+ >+ DoubleCrossCheck("versions", "product_id", "value", >+ ["bugs", "product_id", "version", "bug_id"]); >+ >+ DoubleCrossCheck("milestones", "product_id", "value", >+ ["bugs", "product_id", "target_milestone", "bug_id"], >+ ["products", "id", "defaultmilestone", "name"]); > >-CrossCheck("keyworddefs", "id", >- ["keywords", "keywordid"]); >- >-CrossCheck("fielddefs", "id", >- ["bugs_activity", "fieldid"], >- ['profiles_activity', 'fieldid']); >- >-CrossCheck("flagtypes", "id", >- ["flags", "type_id"], >- ["flagexclusions", "type_id"], >- ["flaginclusions", "type_id"]); >- >-CrossCheck("bugs", "bug_id", >- ["bugs_activity", "bug_id"], >- ["bug_group_map", "bug_id"], >- ["attachments", "bug_id"], >- ["cc", "bug_id"], >- ["longdescs", "bug_id"], >- ["dependencies", "blocked"], >- ["dependencies", "dependson"], >- ['flags', 'bug_id'], >- ["votes", "bug_id"], >- ["keywords", "bug_id"], >- ["duplicates", "dupe_of", "dupe"], >- ["duplicates", "dupe", "dupe_of"]); >- >-CrossCheck("groups", "id", >- ["bug_group_map", "group_id"], >- ['category_group_map', 'group_id'], >- ["group_group_map", "grantor_id"], >- ["group_group_map", "member_id"], >- ["group_control_map", "group_id"], >- ["namedquery_group_map", "group_id"], >- ["user_group_map", "group_id"], >- ["flagtypes", "grant_group_id"], >- ["flagtypes", "request_group_id"]); >- >-CrossCheck("namedqueries", "id", >- ["namedqueries_link_in_footer", "namedquery_id"], >- ["namedquery_group_map", "namedquery_id"], >- ); >- >-CrossCheck("profiles", "userid", >- ['profiles_activity', 'userid'], >- ['profiles_activity', 'who'], >- ['email_setting', 'user_id'], >- ['profile_setting', 'user_id'], >- ["bugs", "reporter", "bug_id"], >- ["bugs", "assigned_to", "bug_id"], >- ["bugs", "qa_contact", "bug_id"], >- ["attachments", "submitter_id", "bug_id"], >- ['flags', 'setter_id', 'bug_id'], >- ['flags', 'requestee_id', 'bug_id'], >- ["bugs_activity", "who", "bug_id"], >- ["cc", "who", "bug_id"], >- ['quips', 'userid'], >- ["votes", "who", "bug_id"], >- ["longdescs", "who", "bug_id"], >- ["logincookies", "userid"], >- ["namedqueries", "userid"], >- ["namedqueries_link_in_footer", "user_id"], >- ['series', 'creator', 'series_id'], >- ["watch", "watcher"], >- ["watch", "watched"], >- ['whine_events', 'owner_userid'], >- ["tokens", "userid"], >- ["user_group_map", "user_id"], >- ["components", "initialowner", "name"], >- ["components", "initialqacontact", "name"], >- ["component_cc", "user_id"]); >- >-CrossCheck("products", "id", >- ["bugs", "product_id", "bug_id"], >- ["components", "product_id", "name"], >- ["milestones", "product_id", "value"], >- ["versions", "product_id", "value"], >- ["group_control_map", "product_id"], >- ["flaginclusions", "product_id", "type_id"], >- ["flagexclusions", "product_id", "type_id"]); >- >-CrossCheck("components", "id", >- ["component_cc", "component_id"], >- ["flagexclusions", "component_id", "type_id"], >- ["flaginclusions", "component_id", "type_id"]); >- >-# Check the former enum types -mkanat@bugzilla.org >-CrossCheck("bug_status", "value", >- ["bugs", "bug_status", "bug_id"]); >- >-CrossCheck("resolution", "value", >- ["bugs", "resolution", "bug_id"]); >- >-CrossCheck("bug_severity", "value", >- ["bugs", "bug_severity", "bug_id"]); >- >-CrossCheck("op_sys", "value", >- ["bugs", "op_sys", "bug_id"]); >- >-CrossCheck("priority", "value", >- ["bugs", "priority", "bug_id"]); >- >-CrossCheck("rep_platform", "value", >- ["bugs", "rep_platform", "bug_id"]); >- >-CrossCheck('series', 'series_id', >- ['series_data', 'series_id']); >- >-CrossCheck('series_categories', 'id', >- ['series', 'category'], >- ["category_group_map", "category_id"], >- ["series", "subcategory"]); >- >-CrossCheck('whine_events', 'id', >- ['whine_queries', 'eventid'], >- ['whine_schedules', 'eventid']); >- >-CrossCheck('attachments', 'attach_id', >- ['attach_data', 'id'], >- ['bugs_activity', 'attach_id']); >- >-CrossCheck('bug_status', 'id', >- ['status_workflow', 'old_status'], >- ['status_workflow', 'new_status']); >+ $cgi->param('foreign_keys_report', $cgi->param('output')); > >+} > ########################################################################### > # Perform double field referential (cross) checks > ########################################################################### >@@ -575,41 +636,37 @@ sub DoubleCrossCheck { > } > } > >-DoubleCrossCheck('attachments', 'bug_id', 'attach_id', >- ['flags', 'bug_id', 'attach_id'], >- ['bugs_activity', 'bug_id', 'attach_id']); >- >-DoubleCrossCheck("components", "product_id", "id", >- ["bugs", "product_id", "component_id", "bug_id"], >- ['flagexclusions', 'product_id', 'component_id'], >- ['flaginclusions', 'product_id', 'component_id']); >- >-DoubleCrossCheck("versions", "product_id", "value", >- ["bugs", "product_id", "version", "bug_id"]); >- >-DoubleCrossCheck("milestones", "product_id", "value", >- ["bugs", "product_id", "target_milestone", "bug_id"], >- ["products", "id", "defaultmilestone", "name"]); >- > ########################################################################### > # Perform login checks > ########################################################################### > >-Status('profile_login_start'); >+if ($cgi->param('profile_loginnames')) { > >-my $sth = $dbh->prepare(q{SELECT userid, login_name FROM profiles}); >-$sth->execute; >+ $cgi->param('output', ''); > >-while (my ($id, $email) = $sth->fetchrow_array) { >- validate_email_syntax($email) >- || Status('profile_login_alert', {id => $id, email => $email}, 'alert'); >-} >+ Status('profile_login_start'); >+ >+ my $sth = $dbh->prepare(q{SELECT userid, login_name FROM profiles}); >+ $sth->execute; >+ >+ while (my ($id, $email) = $sth->fetchrow_array) { >+ validate_email_syntax($email) >+ || Status('profile_login_alert', {id => $id, email => $email}, 'alert'); >+ } > >+ $cgi->param('profile_loginnames_report', $cgi->param('output')); >+} > ########################################################################### > # Perform vote/keyword cache checks > ########################################################################### > >-check_votes_or_keywords(); >+if ($cgi->param('keyword_cache')) { >+ $cgi->param('output', ''); >+ >+ check_votes_or_keywords(); >+ >+ $cgi->param('keyword_cache_report', $cgi->param('output')); >+} > > sub check_votes_or_keywords { > my $check = shift || 'all'; >@@ -786,52 +843,57 @@ sub _check_keywords { > # Check for flags being in incorrect products and components > ########################################################################### > >-Status('flag_check_start'); >+if ($cgi->param('flag_check')) { > >-my $invalid_flags = $dbh->selectall_arrayref( >- 'SELECT DISTINCT flags.id, flags.bug_id, flags.attach_id >- FROM flags >- INNER JOIN bugs >- ON flags.bug_id = bugs.bug_id >- LEFT JOIN flaginclusions AS i >- ON flags.type_id = i.type_id >- AND (bugs.product_id = i.product_id OR i.product_id IS NULL) >- AND (bugs.component_id = i.component_id OR i.component_id IS NULL) >- WHERE i.type_id IS NULL'); >- >-my @invalid_flags = @$invalid_flags; >- >-$invalid_flags = $dbh->selectall_arrayref( >- 'SELECT DISTINCT flags.id, flags.bug_id, flags.attach_id >- FROM flags >- INNER JOIN bugs >- ON flags.bug_id = bugs.bug_id >- INNER JOIN flagexclusions AS e >- ON flags.type_id = e.type_id >- WHERE (bugs.product_id = e.product_id OR e.product_id IS NULL) >- AND (bugs.component_id = e.component_id OR e.component_id IS NULL)'); >- >-push(@invalid_flags, @$invalid_flags); >- >-if (scalar(@invalid_flags)) { >- if ($cgi->param('remove_invalid_flags')) { >- Status('flag_deletion_start'); >- my @flag_ids = map {$_->[0]} @invalid_flags; >- # Silently delete these flags, with no notification to requesters/setters. >- $dbh->do('DELETE FROM flags WHERE id IN (' . join(',', @flag_ids) .')'); >- Status('flag_deletion_end'); >- } >- else { >- foreach my $flag (@$invalid_flags) { >- my ($flag_id, $bug_id, $attach_id) = @$flag; >- Status('flag_alert', >- {flag_id => $flag_id, attach_id => $attach_id, bug_id => $bug_id}, >- 'alert'); >+ $cgi->param('output', ''); >+ >+ Status('flag_check_start'); >+ >+ my $invalid_flags = $dbh->selectall_arrayref( >+ 'SELECT DISTINCT flags.id, flags.bug_id, flags.attach_id >+ FROM flags >+ INNER JOIN bugs >+ ON flags.bug_id = bugs.bug_id >+ LEFT JOIN flaginclusions AS i >+ ON flags.type_id = i.type_id >+ AND (bugs.product_id = i.product_id OR i.product_id IS NULL) >+ AND (bugs.component_id = i.component_id OR i.component_id IS NULL) >+ WHERE i.type_id IS NULL'); >+ >+ my @invalid_flags = @$invalid_flags; >+ >+ $invalid_flags = $dbh->selectall_arrayref( >+ 'SELECT DISTINCT flags.id, flags.bug_id, flags.attach_id >+ FROM flags >+ INNER JOIN bugs >+ ON flags.bug_id = bugs.bug_id >+ INNER JOIN flagexclusions AS e >+ ON flags.type_id = e.type_id >+ WHERE (bugs.product_id = e.product_id OR e.product_id IS NULL) >+ AND (bugs.component_id = e.component_id OR e.component_id IS NULL)'); >+ >+ push(@invalid_flags, @$invalid_flags); >+ >+ if (scalar(@invalid_flags)) { >+ if ($cgi->param('remove_invalid_flags')) { >+ Status('flag_deletion_start'); >+ my @flag_ids = map {$_->[0]} @invalid_flags; >+ # Silently delete these flags, with no notification to requesters/setters. >+ $dbh->do('DELETE FROM flags WHERE id IN (' . join(',', @flag_ids) .')'); >+ Status('flag_deletion_end'); >+ } >+ else { >+ foreach my $flag (@$invalid_flags) { >+ my ($flag_id, $bug_id, $attach_id) = @$flag; >+ Status('flag_alert', >+ {flag_id => $flag_id, attach_id => $attach_id, bug_id => $bug_id}, >+ 'alert'); >+ } >+ Status('flag_fix'); > } >- Status('flag_fix'); > } >+ $cgi->param('flag_check_report', 'output'); > } >- > ########################################################################### > # General bug checks > ########################################################################### >@@ -857,113 +919,134 @@ sub BugCheck { > } > } > >-Status('bug_check_creation_date'); >- >-BugCheck("bugs WHERE creation_ts IS NULL", 'bug_check_creation_date_error_text', >- 'repair_creation_date', 'bug_check_creation_date_repair_text'); >+if ($cgi->param('general_bug_check')) { >+ >+ $cgi->param('output', ''); >+ >+ Status('bug_check_creation_date'); > >-Status('bug_check_res_dupl'); >+ BugCheck("bugs WHERE creation_ts IS NULL", 'bug_check_creation_date_error_text', >+ 'repair_creation_date', 'bug_check_creation_date_repair_text'); > >-BugCheck("bugs INNER JOIN duplicates ON bugs.bug_id = duplicates.dupe " . >- "WHERE bugs.resolution != 'DUPLICATE'", 'bug_check_res_dupl_error_text'); >+ Status('bug_check_res_dupl'); > >-BugCheck("bugs LEFT JOIN duplicates ON bugs.bug_id = duplicates.dupe WHERE " . >- "bugs.resolution = 'DUPLICATE' AND " . >- "duplicates.dupe IS NULL", 'bug_check_res_dupl_error_text2'); >+ BugCheck("bugs INNER JOIN duplicates ON bugs.bug_id = duplicates.dupe " . >+ "WHERE bugs.resolution != 'DUPLICATE'", 'bug_check_res_dupl_error_text'); > >-Status('bug_check_status_res'); >+ BugCheck("bugs LEFT JOIN duplicates ON bugs.bug_id = duplicates.dupe WHERE " . >+ "bugs.resolution = 'DUPLICATE' AND " . >+ "duplicates.dupe IS NULL", 'bug_check_res_dupl_error_text2'); > >-my @open_states = map($dbh->quote($_), BUG_STATE_OPEN); >-my $open_states = join(', ', @open_states); >+ Status('bug_check_status_res'); > >-BugCheck("bugs WHERE bug_status IN ($open_states) AND resolution != ''", >- 'bug_check_status_res_error_text'); >-BugCheck("bugs WHERE bug_status NOT IN ($open_states) AND resolution = ''", >- 'bug_check_status_res_error_text2'); >+ my @open_states = map($dbh->quote($_), BUG_STATE_OPEN); >+ my $open_states = join(', ', @open_states); > >-Status('bug_check_status_everconfirmed'); >+ BugCheck("bugs WHERE bug_status IN ($open_states) AND resolution != ''", >+ 'bug_check_status_res_error_text'); >+ BugCheck("bugs WHERE bug_status NOT IN ($open_states) AND resolution = ''", >+ 'bug_check_status_res_error_text2'); > >-BugCheck("bugs WHERE bug_status = 'UNCONFIRMED' AND everconfirmed = 1", >- 'bug_check_status_everconfirmed_error_text'); >+ Status('bug_check_status_everconfirmed'); > >-my @confirmed_open_states = grep {$_ ne 'UNCONFIRMED'} BUG_STATE_OPEN; >-my $confirmed_open_states = join(', ', map {$dbh->quote($_)} @confirmed_open_states); >+ BugCheck("bugs WHERE bug_status = 'UNCONFIRMED' AND everconfirmed = 1", >+ 'bug_check_status_everconfirmed_error_text'); > >-BugCheck("bugs WHERE bug_status IN ($confirmed_open_states) AND everconfirmed = 0", >- 'bug_check_status_everconfirmed_error_text2'); >+ my @confirmed_open_states = grep {$_ ne 'UNCONFIRMED'} BUG_STATE_OPEN; >+ my $confirmed_open_states = join(', ', map {$dbh->quote($_)} @confirmed_open_states); > >-Status('bug_check_votes_everconfirmed'); >+ BugCheck("bugs WHERE bug_status IN ($confirmed_open_states) AND everconfirmed = 0", >+ 'bug_check_status_everconfirmed_error_text2'); > >-BugCheck("bugs INNER JOIN products ON bugs.product_id = products.id " . >- "WHERE everconfirmed = 0 AND votestoconfirm <= votes", >- 'bug_check_votes_everconfirmed_error_text'); >+ Status('bug_check_votes_everconfirmed'); > >+ BugCheck("bugs INNER JOIN products ON bugs.product_id = products.id " . >+ "WHERE everconfirmed = 0 AND votestoconfirm <= votes", >+ 'bug_check_votes_everconfirmed_error_text'); >+ >+ $cgi->param('general_bug_check_report', $cgi->param('output')); >+} > ########################################################################### > # Control Values > ########################################################################### > >-# Checks for values that are invalid OR >-# not among the 9 valid combinations >-Status('bug_check_control_values'); >-my $groups = join(", ", (CONTROLMAPNA, CONTROLMAPSHOWN, CONTROLMAPDEFAULT, >-CONTROLMAPMANDATORY)); >-my $query = qq{ >- SELECT COUNT(product_id) >- FROM group_control_map >- WHERE membercontrol NOT IN( $groups ) >- OR othercontrol NOT IN( $groups ) >- OR ((membercontrol != othercontrol) >- AND (membercontrol != } . CONTROLMAPSHOWN . q{) >- AND ((membercontrol != } . CONTROLMAPDEFAULT . q{) >- OR (othercontrol = } . CONTROLMAPSHOWN . q{)))}; >- >-my $entries = $dbh->selectrow_array($query); >-Status('bug_check_control_values_alert', {entries => $entries}, 'alert') if $entries; >- >-Status('bug_check_control_values_violation'); >-BugCheck("bugs >- INNER JOIN bug_group_map >- ON bugs.bug_id = bug_group_map.bug_id >- LEFT JOIN group_control_map >- ON bugs.product_id = group_control_map.product_id >- AND bug_group_map.group_id = group_control_map.group_id >- WHERE ((group_control_map.membercontrol = " . CONTROLMAPNA . ") >- OR (group_control_map.membercontrol IS NULL))", >- 'bug_check_control_values_error_text', >- 'createmissinggroupcontrolmapentries', >- 'bug_check_control_values_repair_text'); >- >-BugCheck("bugs >- INNER JOIN group_control_map >- ON bugs.product_id = group_control_map.product_id >- INNER JOIN groups >- ON group_control_map.group_id = groups.id >- LEFT JOIN bug_group_map >- ON bugs.bug_id = bug_group_map.bug_id >- AND group_control_map.group_id = bug_group_map.group_id >- WHERE group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " >- AND bug_group_map.group_id IS NULL >- AND groups.isactive != 0", >- 'bug_check_control_values_error_text2'); >+if ($cgi->param('control_values')) { >+ >+ $cgi->param('output', ''); >+ >+ # Checks for values that are invalid OR >+ # not among the 9 valid combinations >+ Status('bug_check_control_values'); >+ my $groups = join(", ", (CONTROLMAPNA, CONTROLMAPSHOWN, CONTROLMAPDEFAULT, >+ CONTROLMAPMANDATORY)); >+ my $query = qq{ >+ SELECT COUNT(product_id) >+ FROM group_control_map >+ WHERE membercontrol NOT IN( $groups ) >+ OR othercontrol NOT IN( $groups ) >+ OR ((membercontrol != othercontrol) >+ AND (membercontrol != } . CONTROLMAPSHOWN . q{) >+ AND ((membercontrol != } . CONTROLMAPDEFAULT . q{) >+ OR (othercontrol = } . CONTROLMAPSHOWN . q{)))}; >+ >+ my $entries = $dbh->selectrow_array($query); >+ Status('bug_check_control_values_alert', {entries => $entries}, 'alert') if $entries; >+ >+ Status('bug_check_control_values_violation'); >+ BugCheck("bugs >+ INNER JOIN bug_group_map >+ ON bugs.bug_id = bug_group_map.bug_id >+ LEFT JOIN group_control_map >+ ON bugs.product_id = group_control_map.product_id >+ AND bug_group_map.group_id = group_control_map.group_id >+ WHERE ((group_control_map.membercontrol = " . CONTROLMAPNA . ") >+ OR (group_control_map.membercontrol IS NULL))", >+ 'bug_check_control_values_error_text', >+ 'createmissinggroupcontrolmapentries', >+ 'bug_check_control_values_repair_text'); >+ >+ BugCheck("bugs >+ INNER JOIN group_control_map >+ ON bugs.product_id = group_control_map.product_id >+ INNER JOIN groups >+ ON group_control_map.group_id = groups.id >+ LEFT JOIN bug_group_map >+ ON bugs.bug_id = bug_group_map.bug_id >+ AND group_control_map.group_id = bug_group_map.group_id >+ WHERE group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " >+ AND bug_group_map.group_id IS NULL >+ AND groups.isactive != 0", >+ 'bug_check_control_values_error_text2'); >+ >+ $cgi->param('control_values_report', $cgi->param('output')); >+} > > ########################################################################### > # Unsent mail > ########################################################################### > >-Status('unsent_bugmail_check'); >+if ($cgi->param('unsent_emails')) { >+ >+ $cgi->param('output' , ''); >+ >+ Status('unsent_bugmail_check'); > >-my $time = $dbh->sql_interval(30, 'MINUTE'); >-my $badbugs = $dbh->selectcol_arrayref(qq{ >- SELECT bug_id >- FROM bugs >- WHERE (lastdiffed IS NULL OR lastdiffed < delta_ts) >- AND delta_ts < now() - $time >- ORDER BY bug_id}); >+ my $time = $dbh->sql_interval(30, 'MINUTE'); >+ my $badbugs = $dbh->selectcol_arrayref(qq{ >+ SELECT bug_id >+ FROM bugs >+ WHERE (lastdiffed IS NULL OR lastdiffed < delta_ts) >+ AND delta_ts < now() - $time >+ ORDER BY bug_id}); >+ >+ >+ if (scalar(@$badbugs > 0)) { >+ Status('unsent_bugmail_alert', {badbugs => $badbugs}, 'alert'); >+ Status('unsent_bugmail_fix'); >+ } > >+ $cgi->param('unsent_emails_report', $cgi->param('output')); > >-if (scalar(@$badbugs > 0)) { >- Status('unsent_bugmail_alert', {badbugs => $badbugs}, 'alert'); >- Status('unsent_bugmail_fix'); > } > > ########################################################################### >Index: template/en/default/filterexceptions.pl >=================================================================== >RCS file: /cvs/qa/rh_bugzilla_3/template/en/default/filterexceptions.pl,v >retrieving revision 1.4 >diff -p -u -r1.4 filterexceptions.pl >--- template/en/default/filterexceptions.pl 31 Mar 2008 18:35:32 -0000 1.4 >+++ template/en/default/filterexceptions.pl 14 Apr 2008 13:13:22 -0000 >@@ -557,6 +557,10 @@ > 'new_status.id', > ], > >+'admin/sanitycheck/messages.html.tmpl' => [ >+ 'query', >+], >+ > 'account/login.html.tmpl' => [ > 'target', > ], >Index: template/en/default/admin/sanitycheck/messages.html.tmpl >=================================================================== >RCS file: /cvs/qa/rh_bugzilla_3/template/en/default/admin/sanitycheck/messages.html.tmpl,v >retrieving revision 1.3 >diff -p -u -r1.3 messages.html.tmpl >--- template/en/default/admin/sanitycheck/messages.html.tmpl 27 Feb 2008 20:50:44 -0000 1.3 >+++ template/en/default/admin/sanitycheck/messages.html.tmpl 14 Apr 2008 13:13:38 -0000 >@@ -34,7 +34,8 @@ > [% errortext FILTER html %]: [% INCLUDE bug_list badbugs = badbugs %] > > [% ELSIF san_tag == "bug_check_repair" %] >- <a href="sanitycheck.cgi?[% param FILTER url_quote %]=1">[% text FILTER html %]</a>. >+ <a href="sanitycheck.cgi?[% param FILTER url_quote %]=1&[% query %]">[% >+text FILTER html %]</a>. > > [% ELSIF san_tag == "bug_check_creation_date" %] > Checking for [% terms.bugs %] with no creation date (which makes them invisible). >@@ -127,11 +128,11 @@ > [% END %] > > [% ELSIF san_tag == "cross_check_attachment_has_references" %] >- <a href="sanitycheck.cgi?remove_invalid_attach_references=1">Remove >+ <a href="sanitycheck.cgi?remove_invalid_attach_references=1&[% query %]">Remove > invalid references to non existent attachments.</a> > > [% ELSIF san_tag == "cross_check_bug_has_references" %] >- <a href="sanitycheck.cgi?remove_invalid_bug_references=1">Remove >+ <a href="sanitycheck.cgi?remove_invalid_bug_references=1&[% query %]">Remove > invalid references to non existent [% terms.bugs %].</a> > > [% ELSIF san_tag == "double_cross_check_to" %] >@@ -171,7 +172,7 @@ > [%+ PROCESS bug_link bug_id = bug_id %]. > > [% ELSIF san_tag == "flag_fix" %] >- <a href="sanitycheck.cgi?remove_invalid_flags=1">Click >+ <a href="sanitycheck.cgi?remove_invalid_flags=1&[% query %]">Click > here to delete invalid flags</a> > > [% ELSIF san_tag == "group_control_map_entries_creation" %] >@@ -221,7 +222,7 @@ > Keyword cache fixed. > > [% ELSIF san_tag == "keyword_cache_rebuild" %] >- <a href="sanitycheck.cgi?rebuildkeywordcache=1">Click here to >+ <a href="sanitycheck.cgi?rebuildkeywordcache=1&[% query %]">Click here to > rebuild the keyword cache</a>. > > [% ELSIF san_tag == "profile_login_start" %] >@@ -252,7 +253,7 @@ > half an hour: [% INCLUDE bug_list badbugs = badbugs %] > > [% ELSIF san_tag == "unsent_bugmail_fix" %] >- <a href="sanitycheck.cgi?rescanallBugMail=1">Send these mails</a>. >+ <a href="sanitycheck.cgi?rescanallBugMail=1&[% query %]">Send these mails</a>. > > [% ELSIF san_tag == "vote_cache_rebuild_start" %] > OK, now rebuilding vote cache. >@@ -261,7 +262,7 @@ > Vote cache has been rebuilt. > > [% ELSIF san_tag == "vote_cache_rebuild_fix" %] >- <a href="sanitycheck.cgi?rebuildvotecache=1">Click here to >+ <a href="sanitycheck.cgi?rebuildvotecache=1&[% query %]">Click here to > rebuild the vote cache</a> > > [% ELSIF san_tag == "vote_cache_alert" %]
Created attachment 302410 [details] sanitycheck patch as agreed in the meeting ,, I added the extra check for bugs_components, attaching new patch that has it. Noura
Comment on attachment 302410 [details] sanitycheck patch Looks good Noura. Testing worked well. Please check in. Dave
(In reply to comment #18) > (From update of attachment 302410 [details] [edit]) > Looks good Noura. Testing worked well. Please check in. > > Dave > Thanks Dave.. Committed to cvs. Noura
Looks ok. The only piece of trivia I can add is to use HERE docs instead of appending strings together For example, instead of this +sub create_report { + my ($check, $rpt) = @_; + + my $check_rpt = + "Check: $check\n====================================" + . "===============================\n$rpt\n"; + Do this my $check_rpt = <<CHECK_RPT Check: $check =================================================================== $rpt CHECK_RPT IMHO it's easier to look at that in the code and visualise how it will look on the final output. YMMV Not asking you to fix it now, just for next time.
I cleaned up the perldoc in sanitycheck.pl. The only question I have is what is the meaning of "more output" with the --verbose flag? I didn't see a good explanation of what it might be. Not a big deal
(In reply to comment #21) > I cleaned up the perldoc in sanitycheck.pl. Thanks Kevin. >The only question I have is what > is the meaning of "more output" with the --verbose flag? I didn't see a good > explanation of what it might be. Not a big deal > I have added clearer explanation to the documentation. so basically if we don't have the --verbose option then only checks that produced errors will be in the output but with the option then the output will include also the comments for the checks that passed along with the ones that produced errors. Noura