summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kanat-Alexander <mkanat@bugzilla.org>2011-01-04 03:09:42 +0100
committerMax Kanat-Alexander <mkanat@bugzilla.org>2011-01-04 03:09:42 +0100
commitb6d1cc32f75d47437e382caccfccc5b2d98af765 (patch)
tree2ceff059de13f820287c0a84f248d55ef5630aff
parenta1fa8d5f9f196d8611c64c1625e8269c9df52214 (diff)
downloadbugzilla-b6d1cc32f75d47437e382caccfccc5b2d98af765.tar.gz
bugzilla-b6d1cc32f75d47437e382caccfccc5b2d98af765.tar.xz
Bug 595410: Make it faster to display a bug that has a lot of dependencies.
r=LpSolit, a=LpSolit
-rw-r--r--Bugzilla/Bug.pm43
-rw-r--r--Bugzilla/Template.pm53
-rw-r--r--Bugzilla/Util.pm9
-rwxr-xr-xshow_bug.cgi4
-rw-r--r--template/en/default/bug/edit.html.tmpl50
-rw-r--r--template/en/default/bug/link.html.tmpl61
-rw-r--r--template/en/default/filterexceptions.pl4
-rw-r--r--template/en/default/global/field-descs.none.tmpl42
8 files changed, 168 insertions, 98 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index f3a28658a..398843009 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -472,6 +472,26 @@ sub match {
return $class->SUPER::match(@_);
}
+# Helps load up information for bugs for show_bug.cgi and other situations
+# that will need to access info on lots of bugs.
+sub preload {
+ my ($class, $bugs) = @_;
+ my $user = Bugzilla->user;
+
+ # It would be faster but MUCH more complicated to select all the
+ # deps for the entire list in one SQL statement. If we ever have
+ # a profile that proves that that's necessary, we can switch over
+ # to the more complex method.
+ my @all_dep_ids;
+ foreach my $bug (@$bugs) {
+ push(@all_dep_ids, @{ $bug->blocked }, @{ $bug->dependson });
+ }
+ @all_dep_ids = uniq @all_dep_ids;
+ # If we don't do this, can_see_bug will do one call per bug in
+ # the dependency lists, during get_bug_link in Bugzilla::Template.
+ $user->visible_bugs(\@all_dep_ids);
+}
+
sub possible_duplicates {
my ($class, $params) = @_;
my $short_desc = $params->{summary};
@@ -2302,6 +2322,8 @@ sub set_dependencies {
detaint_natural($_) foreach (@$dependson, @$blocked);
$self->{'dependson'} = $dependson;
$self->{'blocked'} = $blocked;
+ delete $self->{depends_on_obj};
+ delete $self->{blocks_obj};
}
sub _clear_dup_id { $_[0]->{dup_id} = undef; }
sub set_dup_id {
@@ -3003,6 +3025,12 @@ sub blocked {
return $self->{'blocked'};
}
+sub blocks_obj {
+ my ($self) = @_;
+ $self->{blocks_obj} ||= $self->_bugs_in_order($self->blocked);
+ return $self->{blocks_obj};
+}
+
sub bug_group {
my ($self) = @_;
return join(', ', (map { $_->name } @{$self->groups_in}));
@@ -3096,6 +3124,12 @@ sub dependson {
return $self->{'dependson'};
}
+sub depends_on_obj {
+ my ($self) = @_;
+ $self->{depends_on_obj} ||= $self->_bugs_in_order($self->dependson);
+ return $self->{depends_on_obj};
+}
+
sub flag_types {
my ($self) = @_;
return $self->{'flag_types'} if exists $self->{'flag_types'};
@@ -3496,6 +3530,15 @@ sub EmitDependList {
return $list_ref;
}
+# Creates a lot of bug objects in the same order as the input array.
+sub _bugs_in_order {
+ my ($self, $bug_ids) = @_;
+ my $bugs = $self->new_from_list($bug_ids);
+ my %bug_map = map { $_->id => $_ } @$bugs;
+ my @result = map { $bug_map{$_} } @$bug_ids;
+ return \@result;
+}
+
# Get the activity of a bug, starting from $starttime (if given).
# This routine assumes Bugzilla::Bug->check has been previously called.
sub GetBugActivity {
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index 45b61b0de..453ca5596 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -317,51 +317,19 @@ sub get_attachment_link {
sub get_bug_link {
my ($bug, $link_text, $options) = @_;
+ $options ||= {};
my $dbh = Bugzilla->dbh;
- if (!$bug) {
- return html_quote('<missing bug number>');
+ if (defined $bug) {
+ $bug = blessed($bug) ? $bug : new Bugzilla::Bug($bug);
+ return $link_text if $bug->{error};
}
- $bug = blessed($bug) ? $bug : new Bugzilla::Bug($bug);
- return $link_text if $bug->{error};
-
- # Initialize these variables to be "" so that we don't get warnings
- # if we don't change them below (which is highly likely).
- my ($pre, $title, $post) = ("", "", "");
- my @css_classes = ("bz_bug_link");
-
- $title = get_text('get_status', { status => $bug->bug_status });
-
- push @css_classes, "bz_status_" . css_class_quote($bug->bug_status);
-
- if ($bug->resolution) {
- push @css_classes, "bz_closed";
- $title .= ' ' . get_text('get_resolution',
- { resolution => $bug->resolution });
- }
- if (Bugzilla->user->can_see_bug($bug)) {
- $title .= " - " . $bug->short_desc;
- if ($options->{use_alias} && $link_text =~ /^\d+$/ && $bug->alias) {
- $link_text = $bug->alias;
- }
- }
- # Prevent code injection in the title.
- $title = html_quote(clean_text($title));
- my $linkval = "show_bug.cgi?id=" . $bug->id;
-
- if ($options->{full_url}) {
- $linkval = correct_urlbase() . $linkval;
- }
-
- if (defined $options->{comment_num}) {
- $linkval .= "#c" . $options->{comment_num};
- }
-
- $pre = '<span class="' . join(" ", @css_classes) . '">';
- $post = '</span>';
-
- return qq{$pre<a href="$linkval" title="$title">$link_text</a>$post};
+ my $template = Bugzilla->template_inner;
+ my $linkified;
+ $template->process('bug/link.html.tmpl',
+ { bug => $bug, link_text => $link_text, %$options }, \$linkified);
+ return $linkified;
}
# We use this instead of format because format doesn't deal well with
@@ -948,6 +916,9 @@ sub create {
# it only once per-language no matter how many times
# $template->process() is called.
'field_descs' => sub { return template_var('field_descs') },
+ # This way we don't have to load field-descs.none.tmpl in
+ # many templates.
+ 'display_value' => \&Bugzilla::Util::display_value,
'install_string' => \&Bugzilla::Install::Util::install_string,
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 6f29a1201..457eb7d02 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -639,6 +639,15 @@ sub template_var {
return $vars{$name};
}
+sub display_value {
+ my ($field, $value) = @_;
+ my $value_descs = template_var('value_descs');
+ if (defined $value_descs->{$field}->{$value}) {
+ return $value_descs->{$field}->{$value};
+ }
+ return $value;
+}
+
sub disable_utf8 {
if (Bugzilla->params->{'utf8'}) {
binmode STDOUT, ':bytes'; # Turn off UTF8 encoding.
diff --git a/show_bug.cgi b/show_bug.cgi
index 64d2e875f..7ea55e732 100755
--- a/show_bug.cgi
+++ b/show_bug.cgi
@@ -52,7 +52,7 @@ if (!$cgi->param('id') && $single) {
my $format = $template->get_format("bug/show", scalar $cgi->param('format'),
scalar $cgi->param('ctype'));
-my @bugs = ();
+my @bugs;
my %marks;
# If the user isn't logged in, we use data from the shadow DB. If he plans
@@ -91,6 +91,8 @@ if ($single) {
}
}
+Bugzilla::Bug->preload(\@bugs);
+
$vars->{'bugs'} = \@bugs;
$vars->{'marks'} = \%marks;
diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index 2a9a0776e..744afeb2d 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -620,13 +620,13 @@
[%############################################################################%]
[% BLOCK section_dependson_blocks %]
<tr>
- [% PROCESS dependencies
- dep = { title => "Depends&nbsp;on", fieldname => "dependson" } %]
+ [% INCLUDE dependencies
+ field = bug_fields.dependson deps = bug.depends_on_obj %]
</tr>
<tr>
- [% PROCESS dependencies accesskey = "b"
- dep = { title => "<u>B</u>locks", fieldname => "blocked" } %]
+ [% INCLUDE dependencies
+ field = bug_fields.blocked deps = bug.blocks_obj %]
<tr>
<th>&nbsp;</th>
@@ -936,38 +936,36 @@
[% BLOCK dependencies %]
- <th class="field_label">
- <label for="[% dep.fieldname %]"[% " accesskey=\"$accesskey\"" IF accesskey %]>
- [% dep.title %]</label>:
- </th>
- <td>
- <span id="[% dep.fieldname %]_input_area">
- [% IF bug.check_can_change_field(dep.fieldname, 0, 1) %]
- <input name="[% dep.fieldname %]" id="[% dep.fieldname %]"
- class="text_input"
- value="[% bug.${dep.fieldname}.join(', ') %]">
+ [% INCLUDE "bug/field-label.html.tmpl" %]
+
+ <td>
+ <span id="[% field.name FILTER html %]_input_area">
+ [% IF bug.check_can_change_field(field.name, 0, 1) %]
+ <input name="[% field.name FILTER html %]"
+ id="[% field.name FILTER html %]" class="text_input"
+ value="[% bug.${field.name}.join(', ') FILTER html %]">
[% END %]
</span>
- [% FOREACH depbug = bug.${dep.fieldname} %]
- [% depbug FILTER bug_link(depbug, use_alias => 1) FILTER none %][% " " %]
+ [% FOREACH dep_bug = deps %]
+ [% dep_bug.id FILTER bug_link(dep_bug, use_alias => 1)
+ FILTER none %][% " " %]
[% END %]
- [% IF bug.check_can_change_field(dep.fieldname, 0, 1) %]
- <span id="[% dep.fieldname %]_edit_container" class="edit_me bz_default_hidden" >
- (<a href="#" id="[% dep.fieldname %]_edit_action">edit</a>)
+ [% IF bug.check_can_change_field(field.name, 0, 1) %]
+ <span id="[% field.name FILTER html %]_edit_container"
+ class="edit_me bz_default_hidden">
+ (<a href="#" id="[% field.name FILTER html %]_edit_action">edit</a>)
</span>
<script type="text/javascript">
- hideEditableField('[% dep.fieldname %]_edit_container',
- '[% dep.fieldname %]_input_area',
- '[% dep.fieldname %]_edit_action',
- '[% dep.fieldname %]',
- "[% bug.${dep.fieldname}.join(', ') %]");
+ hideEditableField('[% field.name FILTER js %]_edit_container',
+ '[% field.name FILTER js %]_input_area',
+ '[% field.name FILTER js %]_edit_action',
+ '[% field.name FILTER js %]',
+ '[% bug.${field.name}.join(', ') FILTER js %]');
</script>
[% END %]
</td>
- [% accesskey = undef %]
-
[% END %]
[%############################################################################%]
diff --git a/template/en/default/bug/link.html.tmpl b/template/en/default/bug/link.html.tmpl
new file mode 100644
index 000000000..b13866850
--- /dev/null
+++ b/template/en/default/bug/link.html.tmpl
@@ -0,0 +1,61 @@
+[%# The contents of this file are subject to the Mozilla Public
+ # License Version 1.1 (the "License"); you may not use this file
+ # except in compliance with the License. You may obtain a copy of
+ # the License at http://www.mozilla.org/MPL/
+ #
+ # Software distributed under the License is distributed on an "AS
+ # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ # implied. See the License for the specific language governing
+ # rights and limitations under the License.
+ #
+ # The Original Code is the Bugzilla Bug Tracking System.
+ #
+ # The Initial Developer of the Original Code is Everything Solved, Inc.
+ # Portions created by the Initial Developer are Copyright (C) 2010 the
+ # Initial Developer. All Rights Reserved.
+ #
+ # Contributor(s):
+ # Max Kanat-Alexander <mkanat@bugzilla.org>
+ #%]
+
+[%# INTERFACE:
+ # bug: a Bugzilla::Bug object
+ # link_text: the text that we're highlighting.
+ # use_alias: boolean; If true, we display the bug's alias as the link
+ # text instead of link_text.
+ # comment_num: If defined, make this a link to that comment on the bug.
+ # full_url: boolean; If true, generate links that include the full
+ # urlbase. (This is for emails, mostly.)
+ #%]
+
+[% IF !bug %]
+ &lt;missing&gt;
+ [% RETURN %]
+[% END %]
+
+[%# We use "FILTER none" here because link_title is filtered down below. %]
+[% link_title = BLOCK %]
+ [% display_value('bug_status', bug.bug_status) FILTER none %]
+ [%+ display_value('resolution', bug.resolution) FILTER none %]
+[% END %]
+
+[% IF user.can_see_bug(bug) %]
+ [% link_title = link_title _ ' - ' _ bug.short_desc %]
+
+ [% IF use_alias && bug.alias %]
+ [% link_text = bug.alias %]
+ [% END %]
+[% END %]
+
+[% SET anchor = '' %]
+[% IF comment_num.defined %]
+ [% anchor = "#c$comment_num" %]
+[% END %]
+
+<a class="bz_bug_link
+ bz_status_[% bug.bug_status FILTER css_class_quote %]
+ [% ' bz_closed' IF !bug.isopened %]"
+ title="[% link_title FILTER collapse FILTER html %]"
+ href="[% urlbase FILTER html IF full_url %]show_bug.cgi?id=
+ [%~ bug.id FILTER uri %][% anchor FILTER html %]">
+ [%~ link_text FILTER html %]</a>
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index b85bb7acd..abc57008c 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -258,11 +258,7 @@
'bug.delta_ts',
'bug.bug_id',
'group.bit',
- 'dep.title',
- 'dep.fieldname',
- 'bug.${dep.fieldname}.join(\', \')',
'selname',
- '" accesskey=\"$accesskey\"" IF accesskey',
'inputname',
'" colspan=\"$colspan\"" IF colspan',
'" size=\"$size\"" IF size',
diff --git a/template/en/default/global/field-descs.none.tmpl b/template/en/default/global/field-descs.none.tmpl
index efcce6c64..ff6033783 100644
--- a/template/en/default/global/field-descs.none.tmpl
+++ b/template/en/default/global/field-descs.none.tmpl
@@ -61,35 +61,25 @@
${constants.FIELD_TYPE_BUG_ID} => "$terms.Bug ID",
} %]
-[%# You can use this hash to localize (translate) the values displayed
- # for drop-down and multiple-select fields. Lines starting with "#"
- # are comments.
- #%]
-[% value_descs = {
- "bug_status" => {
- # "UNCONFIRMED" => "UNCO",
- # "CONFIRMED" => "ITSABUG",
- },
+[% IF in_template_var %]
- "resolution" => {
- "" => "---",
- # "FIXED" => "NO LONGER AN ISSUE",
- # "WORKSFORME" => "NOTMYPROBLEM!",
- },
-} %]
+ [%# You can use this hash to localize (translate) the values displayed
+ # for drop-down and multiple-select fields. Lines starting with "#"
+ # are comments.
+ #%]
+ [% value_descs = {
+ "bug_status" => {
+ # "UNCONFIRMED" => "UNCO",
+ # "CONFIRMED" => "ITSABUG",
+ },
-[%# We use "FILTER none" here because only the caller can know how to
- # filter the result appropriately.
- #%]
-[% MACRO display_value(field_name, value_name) BLOCK %][% FILTER trim %]
- [% IF value_descs.${field_name}.${value_name}.defined %]
- [% value_descs.${field_name}.${value_name} FILTER none %]
- [% ELSE %]
- [% value_name FILTER none %]
- [% END %]
-[% END %][% END %]
+ "resolution" => {
+ "" => "---",
+ # "FIXED" => "NO LONGER AN ISSUE",
+ # "WORKSFORME" => "NOTMYPROBLEM!",
+ },
+ } %]
-[% IF in_template_var %]
[% vars.terms = terms %]
[%# field_descs is loaded as a global template variable and cached