From 1c5e0c47c92128afebef5614407d84cd72c12b35 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Wed, 28 Nov 2012 16:40:21 +0800 Subject: Bug 814411: Add a caching mechanism to Bugzilla::Object to avoid querying the database repeatedly for the same information --- Bugzilla/Attachment.pm | 2 +- Bugzilla/Bug.pm | 21 +++++++++++++---- Bugzilla/Comment.pm | 3 ++- Bugzilla/Object.pm | 44 ++++++++++++++++++++++++++++++++++- Bugzilla/Product.pm | 4 ++-- Bugzilla/Template.pm | 6 ++--- attachment.cgi | 6 ++--- extensions/BMO/Extension.pm | 3 ++- extensions/InlineHistory/Extension.pm | 2 +- extensions/Splinter/Extension.pm | 2 +- extensions/Splinter/lib/Util.pm | 5 ++-- show_bug.cgi | 6 ++--- 12 files changed, 81 insertions(+), 23 deletions(-) diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index aa7eee2a7..094406c91 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -160,7 +160,7 @@ sub bug { my $self = shift; require Bugzilla::Bug; - $self->{bug} ||= Bugzilla::Bug->new($self->bug_id); + $self->{bug} ||= Bugzilla::Bug->new({ id => $self->bug_id, cache => 1 }); return $self->{bug}; } diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 6973ae2fc..6c23d6992 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -333,10 +333,14 @@ sub new { # If we get something that looks like a word (not a number), # make it the "name" param. - if (!defined $param || (!ref($param) && $param !~ /^\d+$/)) { + if (!defined $param + || (!ref($param) && $param =~ /\D/) + || (ref($param) && $param->{id} =~ /\D/)) + { # But only if aliases are enabled. if (Bugzilla->params->{'usebugaliases'} && $param) { - $param = { name => $param }; + $param = { name => ref($param) ? $param->{id} : $param, + cache => ref($param) ? $param->{cache} : 0 }; } else { # Aliases are off, and we got something that's not a number. @@ -370,6 +374,13 @@ sub new { return $self; } +sub cache_key { + my $class = shift; + my $key = $class->SUPER::cache_key(@_) + || return; + return $key . ',' . Bugzilla->user->id; +} + sub check { my $class = shift; my ($id, $field) = @_; @@ -3241,7 +3252,8 @@ sub component_obj { my ($self) = @_; return $self->{component_obj} if defined $self->{component_obj}; return {} if $self->{error}; - $self->{component_obj} = new Bugzilla::Component($self->{component_id}); + $self->{component_obj} = + new Bugzilla::Component({ id => $self->{component_id}, cache => 1 }); return $self->{component_obj}; } @@ -3410,7 +3422,8 @@ sub product { sub product_obj { my $self = shift; return {} if $self->{error}; - $self->{product_obj} ||= new Bugzilla::Product($self->{product_id}); + $self->{product_obj} ||= + new Bugzilla::Product({ id => $self->{product_id}, cache => 1 }); return $self->{product_obj}; } diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm index ee342fb2d..643767232 100644 --- a/Bugzilla/Comment.pm +++ b/Bugzilla/Comment.pm @@ -143,7 +143,8 @@ sub is_about_attachment { sub attachment { my ($self) = @_; return undef if not $self->is_about_attachment; - $self->{attachment} ||= new Bugzilla::Attachment($self->extra_data); + $self->{attachment} ||= + new Bugzilla::Attachment({ id => $self->extra_data, cache => 1 }); return $self->{attachment}; } diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 8089c6ccc..96651d191 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -72,6 +72,8 @@ sub new { sub _init { my $class = shift; my ($param) = @_; + my $object = $class->_cache_get($param); + return $object if $object; my $dbh = Bugzilla->dbh; my $columns = join(',', $class->_get_db_columns); my $table = $class->DB_TABLE; @@ -82,7 +84,6 @@ sub _init { if (ref $param eq 'HASH') { $id = $param->{id}; } - my $object; if (defined $id) { # We special-case if somebody specifies an ID, so that we can @@ -125,9 +126,48 @@ sub _init { "SELECT $columns FROM $table WHERE $condition", undef, @values); } + $class->_cache_set($param, $object) if $object; return $object; } +# Provides a mechanism for objects to be cached in the request_cahce + +sub _cache_get { + my $class = shift; + my ($param) = @_; + my $cache_key = $class->cache_key($param) + || return; + return Bugzilla->request_cache->{$cache_key}; +} + +sub _cache_set { + my $class = shift; + my ($param, $object) = @_; + my $cache_key = $class->cache_key($param) + || return; + Bugzilla->request_cache->{$cache_key} = $object; +} + +sub _cache_remove { + my $class = shift; + my ($param, $object) = @_; + $param->{cache} = 1; + my $cache_key = $class->cache_key($param) + || return; + delete Bugzilla->request_cache->{$cache_key}; +} + +sub cache_key { + my $class = shift; + my ($param) = @_; + if (ref($param) && $param->{cache} && ($param->{id} || $param->{name})) { + $class = blessed($class) if blessed($class); + return $class . ',' . ($param->{id} || $param->{name}); + } else { + return; + } +} + sub check { my ($invocant, $param) = @_; my $class = ref($invocant) || $invocant; @@ -406,6 +446,7 @@ sub update { $self->audit_log(\%changes) if $self->AUDIT_UPDATES; $dbh->bz_commit_transaction(); + $self->_cache_remove({ id => $self->id }); if (wantarray) { return (\%changes, $old_self); @@ -424,6 +465,7 @@ sub remove_from_db { $self->audit_log(AUDIT_REMOVE) if $self->AUDIT_REMOVES; $dbh->do("DELETE FROM $table WHERE $id_field = ?", undef, $self->id); $dbh->bz_commit_transaction(); + $self->_cache_remove({ id => $self->id }); undef $self; } diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm index 79af9cbf5..452ae90fc 100644 --- a/Bugzilla/Product.pm +++ b/Bugzilla/Product.pm @@ -817,8 +817,8 @@ sub flag_types { sub classification { my $self = shift; - $self->{'classification'} ||= - new Bugzilla::Classification($self->classification_id); + $self->{'classification'} ||= + new Bugzilla::Classification({ id => $self->classification_id, cache => 1 }); return $self->{'classification'}; } diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index 18560c3f5..5d70cb73f 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -281,7 +281,7 @@ sub get_attachment_link { my $dbh = Bugzilla->dbh; $user ||= Bugzilla->user; - my $attachment = new Bugzilla::Attachment($attachid); + my $attachment = new Bugzilla::Attachment({ id => $attachid, cache => 1 }); if ($attachment) { my $title = ""; @@ -333,8 +333,8 @@ sub get_bug_link { $options->{user} ||= Bugzilla->user; my $dbh = Bugzilla->dbh; - if (defined $bug) { - $bug = blessed($bug) ? $bug : new Bugzilla::Bug($bug); + if (defined $bug && $bug ne '') { + $bug = blessed($bug) ? $bug : new Bugzilla::Bug({ id => $bug, cache => 1 }); return $link_text if $bug->{error}; } diff --git a/attachment.cgi b/attachment.cgi index 985430d85..b5c0f8efe 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -180,7 +180,7 @@ sub validateID { { attach_id => scalar $cgi->param($param) }); # Make sure the attachment exists in the database. - my $attachment = new Bugzilla::Attachment($attach_id) + my $attachment = new Bugzilla::Attachment({ id => $attach_id, cache => 1 }) || ThrowUserError("invalid_attach_id", { attach_id => $attach_id }); return $attachment if ($dont_validate_access || check_can_access($attachment)); @@ -192,7 +192,7 @@ sub check_can_access { my $user = Bugzilla->user; # Make sure the user is authorized to access this attachment's bug. - Bugzilla::Bug->check($attachment->bug_id); + Bugzilla::Bug->check({ id => $attachment->bug_id, cache => 1 }); if ($attachment->isprivate && $user->id != $attachment->attacher->id && !$user->is_insider) { @@ -454,7 +454,7 @@ sub diff { # HTML page. sub viewall { # Retrieve and validate parameters - my $bug = Bugzilla::Bug->check(scalar $cgi->param('bugid')); + my $bug = Bugzilla::Bug->check({ id => scalar $cgi->param('bugid'), cache => 1 }); my $bugid = $bug->id; my $attachments = Bugzilla::Attachment->get_attachments_by_bug($bugid); diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm index a276a110e..037cc6689 100644 --- a/extensions/BMO/Extension.pm +++ b/extensions/BMO/Extension.pm @@ -144,7 +144,8 @@ sub template_before_process { if ($file =~ /^attachment\/diff-header\./) { my $attachid = $vars->{attachid} ? $vars->{attachid} : $vars->{newid}; - $vars->{attachment} = Bugzilla::Attachment->new($attachid) if $attachid; + $vars->{attachment} = Bugzilla::Attachment->new({ id => $attachid, cache => 1 }) + if $attachid; } } diff --git a/extensions/InlineHistory/Extension.pm b/extensions/InlineHistory/Extension.pm index 2e388994a..f761a9fbd 100644 --- a/extensions/InlineHistory/Extension.pm +++ b/extensions/InlineHistory/Extension.pm @@ -92,7 +92,7 @@ sub template_before_process { my $field_obj; if ($change->{fieldname} =~ /^cf_/) { - $field_obj = Bugzilla::Field->new({ name => $change->{fieldname} }); + $field_obj = Bugzilla::Field->new({ name => $change->{fieldname}, cache => 1 }); } # identify buglist changes diff --git a/extensions/Splinter/Extension.pm b/extensions/Splinter/Extension.pm index 9c8be4beb..980ab0054 100644 --- a/extensions/Splinter/Extension.pm +++ b/extensions/Splinter/Extension.pm @@ -36,7 +36,7 @@ sub page_before_template { if ($input->{'bug'}) { $vars->{'bug_id'} = $input->{'bug'}; $vars->{'attach_id'} = $input->{'attachment'}; - $vars->{'bug'} = Bugzilla::Bug->check($input->{'bug'}); + $vars->{'bug'} = Bugzilla::Bug->check({ id => $input->{'bug'}, cache => 1 }); } if ($input->{'attachment'}) { diff --git a/extensions/Splinter/lib/Util.pm b/extensions/Splinter/lib/Util.pm index 6305395f9..9f09d3dcc 100644 --- a/extensions/Splinter/lib/Util.pm +++ b/extensions/Splinter/lib/Util.pm @@ -53,7 +53,8 @@ sub attachment_id_is_valid { detaint_natural($attach_id) || return 0; # Make sure the attachment exists in the database. - my $attachment = new Bugzilla::Attachment($attach_id) || return 0; + my $attachment = new Bugzilla::Attachment({ id => $attach_id, cache => 1 }) + || return 0; return $attachment if ($dont_validate_access || attachment_is_visible($attachment)); @@ -135,7 +136,7 @@ sub add_review_links_to_email { if ($email->header('Subject') =~ /^\[Bug\s+(\d+)\]/ && Bugzilla->user->can_see_bug($1)) { - $bug = Bugzilla::Bug->new($1); + $bug = Bugzilla::Bug->new({ id => $1, cache => 1 }); } return unless defined $bug; diff --git a/show_bug.cgi b/show_bug.cgi index 7ea55e732..14e259e48 100755 --- a/show_bug.cgi +++ b/show_bug.cgi @@ -62,7 +62,7 @@ Bugzilla->switch_to_shadow_db unless $user->id; if ($single) { my $id = $cgi->param('id'); - push @bugs, Bugzilla::Bug->check($id); + push @bugs, Bugzilla::Bug->check({ id => $id, cache => 1 }); if (defined $cgi->param('mark')) { foreach my $range (split ',', $cgi->param('mark')) { if ($range =~ /^(\d+)-(\d+)$/) { @@ -78,8 +78,8 @@ if ($single) { foreach my $id ($cgi->param('id')) { # Be kind enough and accept URLs of the form: id=1,2,3. my @ids = split(/,/, $id); - foreach (@ids) { - my $bug = new Bugzilla::Bug($_); + foreach my $bug_id (@ids) { + my $bug = new Bugzilla::Bug({ id => $bug_id, cache => 1 }); # This is basically a backwards-compatibility hack from when # Bugzilla::Bug->new used to set 'NotPermitted' if you couldn't # see the bug. -- cgit v1.2.3-24-g4f1b