Bug 814411: Add a caching mechanism to Bugzilla::Object to avoid querying the database repeatedly for the same information

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.