aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarc Schumann <wurblzap@gmail.com>2012-10-16 20:26:54 +0200
committerMarc Schumann <wurblzap@gmail.com>2012-10-16 20:26:54 +0200
commitaf38912df5c0f57ad40377119b91dd03d8f9b520 (patch)
treebd04fc880184f7b063e17d12af2d13ef1f5998cc
parentb60afed2cc7c8776217444049e5ebfadf5839a24 (diff)
downloadbugs-af38912df5c0f57ad40377119b91dd03d8f9b520.tar
bugs-af38912df5c0f57ad40377119b91dd03d8f9b520.tar.gz
bugs-af38912df5c0f57ad40377119b91dd03d8f9b520.tar.bz2
bugs-af38912df5c0f57ad40377119b91dd03d8f9b520.tar.xz
bugs-af38912df5c0f57ad40377119b91dd03d8f9b520.zip
Bug 604388 - Filenames used to store data for Old Charts should be based on product IDs, not names (avoid dataloss when renaming products).
r/a=LpSolit
-rw-r--r--Bugzilla/Install/Filesystem.pm61
-rwxr-xr-xcollectstats.pl56
-rwxr-xr-xreports.cgi60
-rw-r--r--template/en/default/global/code-error.html.tmpl4
-rw-r--r--template/en/default/global/user-error.html.tmpl4
-rw-r--r--template/en/default/reports/old-charts.html.tmpl4
6 files changed, 118 insertions, 71 deletions
diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index 6b768cbbb..678e208d4 100644
--- a/Bugzilla/Install/Filesystem.pm
+++ b/Bugzilla/Install/Filesystem.pm
@@ -29,6 +29,7 @@ use File::Find;
use File::Path;
use File::Basename;
use File::Copy qw(move);
+use File::Spec;
use IO::File;
use POSIX ();
@@ -397,6 +398,13 @@ sub update_filesystem {
_update_old_charts($datadir);
}
+ # If there is a file named '-All-' in $datadir/mining, then we're still
+ # having mining files named by product name, and we need to convert them to
+ # files named by product ID.
+ if (-e File::Spec->catfile($datadir, 'mining', '-All-')) {
+ _update_old_mining_filenames(File::Spec->catdir($datadir, 'mining'));
+ }
+
# By sorting the dirs, we assure that shorter-named directories
# (meaning parent directories) are always created before their
# child directories.
@@ -638,6 +646,59 @@ sub _update_old_charts {
}
}
+# The old naming scheme has product names as mining file names; we rename them
+# to product IDs.
+sub _update_old_mining_filenames {
+ my ($miningdir) = @_;
+ my @conversion_errors;
+
+ require Bugzilla::Product;
+
+ # We use a dummy product instance with ID 0, representing all products
+ my $product_all = {id => 0, name => '-All-'};
+ bless($product_all, 'Bugzilla::Product');
+
+ print "Updating old charting data file names...";
+ my @products = Bugzilla::Product->get_all();
+ push(@products, $product_all);
+ foreach my $product (@products) {
+ if (-e File::Spec->catfile($miningdir, $product->id)) {
+ push(@conversion_errors,
+ { product => $product,
+ message => 'A file named "' . $product->id .
+ '" already exists.' });
+ }
+ }
+
+ if (! @conversion_errors) {
+ # Renaming mining files should work now without a hitch.
+ foreach my $product (@products) {
+ if (! rename(File::Spec->catfile($miningdir, $product->name),
+ File::Spec->catfile($miningdir, $product->id))) {
+ push(@conversion_errors,
+ { product => $product,
+ message => $! });
+ }
+ }
+ }
+
+ # Error reporting
+ if (! @conversion_errors) {
+ print " done.\n";
+ }
+ else {
+ print " FAILED:\n";
+ foreach my $error (@conversion_errors) {
+ printf "Cannot rename charting data file for product %d (%s): %s\n",
+ $error->{product}->id, $error->{product}->name,
+ $error->{message};
+ }
+ print "You need to empty the \"$miningdir\" directory, then run\n",
+ " collectstats.pl --regenerate\n",
+ "in order to clean this up.\n";
+ }
+}
+
sub fix_dir_permissions {
my ($dir) = @_;
return if ON_WINDOWS;
diff --git a/collectstats.pl b/collectstats.pl
index 7336714bb..e1a4603f3 100755
--- a/collectstats.pl
+++ b/collectstats.pl
@@ -38,6 +38,10 @@ $| = 1;
my $datadir = bz_locations()->{'datadir'};
my $graphsdir = bz_locations()->{'graphsdir'};
+# We use a dummy product instance with ID 0, representing all products
+my $product_all = {id => 0, name => '-All-'};
+bless($product_all, 'Bugzilla::Product');
+
# Tidy up after graphing module
my $cwd = Cwd::getcwd();
if (chdir($graphsdir)) {
@@ -120,7 +124,7 @@ if ($switch{'regenerate'}) {
my $tstart = time;
my @myproducts = Bugzilla::Product->get_all;
-unshift(@myproducts, "-All-");
+unshift(@myproducts, $product_all);
my $dir = "$datadir/mining";
if (!-d $dir) {
@@ -149,18 +153,8 @@ sub collect_stats {
my $product = shift;
my $when = localtime (time);
my $dbh = Bugzilla->dbh;
- my $product_id;
-
- if (ref $product) {
- $product_id = $product->id;
- $product = $product->name;
- }
- # NB: Need to mangle the product for the filename, but use the real
- # product name in the query
- my $file_product = $product;
- $file_product =~ s/\//-/gs;
- my $file = join '/', $dir, $file_product;
+ my $file = join '/', $dir, $product->id;
my $exists = -f $file;
# if the file exists, get the old status and resolution list for that product.
@@ -186,7 +180,7 @@ sub collect_stats {
my $status_sql = q{SELECT COUNT(*) FROM bugs WHERE bug_status = ?};
my $reso_sql = q{SELECT COUNT(*) FROM bugs WHERE resolution = ?};
- if ($product ne '-All-') {
+ if ($product->id) {
$status_sql .= q{ AND product_id = ?};
$reso_sql .= q{ AND product_id = ?};
}
@@ -197,13 +191,13 @@ sub collect_stats {
my @values ;
foreach my $status (@statuses) {
@values = ($status);
- push (@values, $product_id) if ($product ne '-All-');
+ push (@values, $product->id) if ($product->id);
my $count = $dbh->selectrow_array($sth_status, undef, @values);
push(@row, $count);
}
foreach my $resolution (@resolutions) {
@values = ($resolution);
- push (@values, $product_id) if ($product ne '-All-');
+ push (@values, $product->id) if ($product->id);
my $count = $dbh->selectrow_array($sth_reso, undef, @values);
push(@row, $count);
}
@@ -216,7 +210,7 @@ sub collect_stats {
# Do not edit me! This file is generated.
#
# fields: $fields
-# Product: $product
+# Product: $product->name
# Created: $when
FIN
}
@@ -285,24 +279,14 @@ sub regenerate_stats {
my $when = localtime(time());
my $tstart = time();
- # NB: Need to mangle the product for the filename, but use the real
- # product name in the query
- if (ref $product) {
- $product = $product->name;
- }
- my $file_product = $product;
- $file_product =~ s/\//-/gs;
- my $file = join '/', $dir, $file_product;
+ my $file = join '/', $dir, $product->id;
my $and_product = "";
- my $from_product = "";
my @values = ();
- if ($product ne '-All-') {
- $and_product = q{ AND products.name = ?};
- $from_product = q{ INNER JOIN products
- ON bugs.product_id = products.id};
- push (@values, $product);
+ if ($product->id) {
+ $and_product = q{ AND product_id = ?};
+ push (@values, $product->id);
}
# Determine the start date from the date the first bug in the
@@ -312,7 +296,7 @@ sub regenerate_stats {
$dbh->sql_to_days('creation_ts') . q{ AS start_day, } .
$dbh->sql_to_days('current_date') . q{ AS end_day, } .
$dbh->sql_to_days("'1970-01-01'") .
- qq{ FROM bugs $from_product
+ qq{ FROM bugs
WHERE } . $dbh->sql_to_days('creation_ts') .
qq{ IS NOT NULL $and_product
ORDER BY start_day } . $dbh->sql_limit(1);
@@ -330,7 +314,7 @@ sub regenerate_stats {
# Do not edit me! This file is generated.
#
# fields: $fields
-# Product: $product
+# Product: $product->name
# Created: $when
FIN
# For each day, generate a line of statistics.
@@ -339,12 +323,13 @@ FIN
for (my $day = $start + 1; $day <= $end; $day++) {
# Some output feedback
my $percent_done = ($day - $start - 1) * 100 / $total_days;
- printf "\rRegenerating $product \[\%.1f\%\%]", $percent_done;
+ printf "\rRegenerating %s \[\%.1f\%\%]", $product->name,
+ $percent_done;
# Get a list of bugs that were created the previous day, and
# add those bugs to the list of bugs for this product.
$query = qq{SELECT bug_id
- FROM bugs $from_product
+ FROM bugs
WHERE bugs.creation_ts < } .
$dbh->sql_from_days($day - 1) .
q{ AND bugs.creation_ts >= } .
@@ -387,7 +372,8 @@ FIN
# Finish up output feedback for this product.
my $tend = time;
- say "\rRegenerating $product \[100.0\%] - " . delta_time($tstart, $tend);
+ say "\rRegenerating " . $product->name . ' [100.0%] - ' .
+ delta_time($tstart, $tend);
close DATA;
}
diff --git a/reports.cgi b/reports.cgi
index c8c319f41..c931b1586 100755
--- a/reports.cgi
+++ b/reports.cgi
@@ -26,6 +26,10 @@ my $cgi = Bugzilla->cgi;
my $template = Bugzilla->template;
my $vars = {};
+# We use a dummy product instance with ID 0, representing all products
+my $product_all = {id => 0};
+bless($product_all, 'Bugzilla::Product');
+
if (!Bugzilla->feature('old_charts')) {
ThrowCodeError('feature_disabled', { feature => 'old_charts' });
}
@@ -33,11 +37,11 @@ if (!Bugzilla->feature('old_charts')) {
my $dir = bz_locations()->{'datadir'} . "/mining";
my $graph_dir = bz_locations()->{'graphsdir'};
my $graph_url = basename($graph_dir);
-my $product_name = $cgi->param('product') || '';
+my $product_id = $cgi->param('product_id');
Bugzilla->switch_to_shadow_db();
-if (!$product_name) {
+if (! defined($product_id)) {
# Can we do bug charts?
(-d $dir && -d $graph_dir)
|| ThrowCodeError('chart_dir_nonexistent',
@@ -55,10 +59,10 @@ if (!$product_name) {
push(@datasets, $datasets);
}
- # We only want those products that the user has permissions for.
- my @myproducts = ('-All-');
- # Extract product names from objects and add them to the list.
- push( @myproducts, map { $_->name } @{$user->get_selectable_products} );
+ # Start our product list with an entry for all products, then add those
+ # products that the user has permissions for.
+ my @myproducts = ($product_all);
+ push( @myproducts, @{$user->get_selectable_products} );
$vars->{'datasets'} = \@datasets;
$vars->{'products'} = \@myproducts;
@@ -66,16 +70,21 @@ if (!$product_name) {
print $cgi->header();
}
else {
- # For security and correctness, validate the value of the "product" form variable.
- # Valid values are those products for which the user has permissions which appear
- # in the "product" drop-down menu on the report generation form.
- my ($product) = grep { $_->name eq $product_name } @{$user->get_selectable_products};
- ($product || $product_name eq '-All-')
- || ThrowUserError('invalid_product_name', {product => $product_name});
-
- # Product names can change over time. Their ID cannot; so use the ID
- # to generate the filename.
- my $prod_id = $product ? $product->id : 0;
+ my $product;
+ # For security and correctness, validate the value of the "product_id" form
+ # variable. Valid values are IDs of those products for which the user has
+ # permissions which appear in the "product_id" drop-down menu on the report
+ # generation form. The product_id 0 is a special case, meaning "All
+ # Products".
+ if ($product_id) {
+ $product = Bugzilla::Product->new($product_id);
+ $product && $user->can_see_product($product->name)
+ || ThrowUserError('product_access_denied',
+ {id => $product_id});
+ }
+ else {
+ $product = $product_all;
+ }
# Make sure there is something to plot.
my @datasets = $cgi->param('datasets');
@@ -87,9 +96,9 @@ else {
# Filenames must not be guessable as they can point to products
# you are not allowed to see. Also, different projects can have
- # the same product names.
+ # the same product IDs.
my $project = bz_locations()->{'project'} || '';
- my $image_file = join(':', ($project, $prod_id, @datasets));
+ my $image_file = join(':', ($project, $product->id, @datasets));
my $key = Bugzilla->localconfig->{'site_wide_secret'};
$image_file = hmac_sha256_base64($image_file, $key) . '.png';
$image_file =~ s/\+/-/g;
@@ -116,8 +125,8 @@ sub get_data {
my $dir = shift;
my @datasets;
- open(DATA, '<', "$dir/-All-")
- || ThrowCodeError('chart_file_open_fail', {filename => "$dir/-All-"});
+ open(DATA, '<', "$dir/0")
+ || ThrowCodeError('chart_file_open_fail', {filename => "$dir/0"});
while (<DATA>) {
if (/^# fields?: (.+)\s*$/) {
@@ -131,18 +140,13 @@ sub get_data {
sub generate_chart {
my ($dir, $image_file, $product, $datasets) = @_;
- $product = $product ? $product->name : '-All-';
- my $data_file = $product;
- $data_file =~ s/\//-/gs;
- $data_file = $dir . '/' . $data_file;
+ my $data_file = $dir . '/' . $product->id;
if (! open FILE, $data_file) {
- if ($product eq '-All-') {
- $product = '';
- }
ThrowCodeError('chart_data_not_generated', {'product' => $product});
}
+ my $product_in_title = $product->id ? $product->name : 'All Products';
my @fields;
my @labels = qw(DATE);
my %datasets = map { $_ => 1 } @$datasets;
@@ -205,7 +209,7 @@ sub generate_chart {
my %settings =
(
- "title" => "Status Counts for $product",
+ "title" => "Status Counts for $product_in_title",
"x_label" => "Dates",
"y_label" => "Bug Counts",
"legend_labels" => \@labels,
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 9fd326051..8945ba445 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -55,8 +55,8 @@
[% ELSIF error == "chart_data_not_generated" %]
[% admindocslinks = {'extraconfig.html' => 'Setting up Charting'} %]
- [% IF product %]
- Charts for the <em>[% product FILTER html %]</em> product are not
+ [% IF product.id %]
+ Charts for the <em>[% product.name FILTER html %]</em> product are not
available yet because no charting data has been collected for it since it
was created.
[% ELSE %]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 39d07653c..e7d3061a9 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -962,10 +962,6 @@
[% title = "Invalid Parameter" %]
The new value for [% name FILTER html %] is invalid: [% err FILTER html %].
- [% ELSIF error == "invalid_product_name" %]
- [% title = "Invalid Product Name" %]
- The product name '[% product FILTER html %]' is invalid or does not exist.
-
[% ELSIF error == "invalid_regexp" %]
[% title = "Invalid regular expression" %]
The regular expression you entered is invalid.
diff --git a/template/en/default/reports/old-charts.html.tmpl b/template/en/default/reports/old-charts.html.tmpl
index 12a0cdd83..29098c160 100644
--- a/template/en/default/reports/old-charts.html.tmpl
+++ b/template/en/default/reports/old-charts.html.tmpl
@@ -28,9 +28,9 @@
<tr>
<th>Product:</th>
<td align="center">
- <select id="product" name="product">
+ <select id="product_id" name="product_id">
[% FOREACH product = products %]
- <option value="[% product FILTER html %]">[% product FILTER html %]</option>
+ <option value="[% product.id FILTER html %]">[% product.name || '-All-' FILTER html %]</option>
[% END %]
</select>
</td>