Bug 966180: backport bug 956233 to bmo (enable USE_MEMCACHE on most objects)

16 files changed, 110 insertions, 15 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index bb0f83aba..2b2159346 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -921,6 +921,9 @@ sub update {
         $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
                  undef, ($timestamp, $self->bug_id));
         $self->{modification_time} = $timestamp;
+        # because we updated the attachments table after SUPER::update(), we
+        # need to ensure the cache is flushed.
+        Bugzilla->memcached->clear({ table => 'attachments', id => $self->id });
     }
 
     return $changes;
@@ -945,11 +948,21 @@ sub remove_from_db {
     my $dbh = Bugzilla->dbh;
 
     $dbh->bz_start_transaction();
-    $dbh->do('DELETE FROM flags WHERE attach_id = ?', undef, $self->id);
+    my $flag_ids = $dbh->selectcol_arrayref(
+        'SELECT id FROM flags WHERE attach_id = ?', undef, $self->id);
+    $dbh->do('DELETE FROM flags WHERE ' . $dbh->sql_in('id', $flag_ids))
+        if @$flag_ids;
     $dbh->do('DELETE FROM attach_data WHERE id = ?', undef, $self->id);
     $dbh->do('UPDATE attachments SET mimetype = ?, ispatch = ?, isobsolete = ?
               WHERE attach_id = ?', undef, ('text/plain', 0, 1, $self->id));
     $dbh->bz_commit_transaction();
+
+    # As we don't call SUPER->remove_from_db we need to manually clear
+    # memcached here.
+    Bugzilla->memcached->clear({ table => 'attachments', id => $self->id });
+    foreach my $flag_id (@$flag_ids) {
+        Bugzilla->memcached->clear({ table => 'flags', id => $flag_id });
+    }
 }
 
 ###############################
diff --git a/Bugzilla/Auth/Verify.pm b/Bugzilla/Auth/Verify.pm
index a8cd0af2c..3578631f1 100644
--- a/Bugzilla/Auth/Verify.pm
+++ b/Bugzilla/Auth/Verify.pm
@@ -97,6 +97,7 @@ sub create_or_update_user {
         if ($extern_id && $username_user_id && !$extern_user_id) {
             $dbh->do('UPDATE profiles SET extern_id = ? WHERE userid = ?',
                      undef, $extern_id, $username_user_id);
+            Bugzilla->memcached->clear({ table => 'profiles', id => $username_user_id });
         }
 
         # Finally, at this point, one of these will give us a valid user id.
@@ -109,23 +110,26 @@ sub create_or_update_user {
         'Bugzilla::Auth::Verify::create_or_update_user'})
         unless $user_id;
 
-    my $user = new Bugzilla::User($user_id);
+    my $user = new Bugzilla::User({ id => $user_id, cache => 1 });
 
     # Now that we have a valid User, we need to see if any data has to be
     # updated.
+    my $user_updated = 0;
     if ($username && lc($user->login) ne lc($username)) {
         validate_email_syntax($username)
           || return { failure => AUTH_ERROR, error => 'auth_invalid_email',
                       details => {addr => $username} };
         $user->set_login($username);
+        $user_updated = 1;
     }
     if ($real_name && $user->name ne $real_name) {
         # $real_name is more than likely tainted, but we only use it
         # in a placeholder and we never use it after this.
         trick_taint($real_name);
         $user->set_name($real_name);
+        $user_updated = 1;
     }
-    $user->update();
+    $user->update() if $user_updated;
 
     return { user => $user };
 }
diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm
index 2fcfd4017..783e7490a 100644
--- a/Bugzilla/Auth/Verify/DB.pm
+++ b/Bugzilla/Auth/Verify/DB.pm
@@ -103,6 +103,7 @@ sub change_password {
     my $cryptpassword = bz_crypt($password);
     $dbh->do("UPDATE profiles SET cryptpassword = ? WHERE userid = ?",
              undef, $cryptpassword, $user->id);
+    Bugzilla->memcached->clear({ table => 'profiles', id => $user->id });
 }
 
 1;
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index b6861b509..a99dfaf85 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -76,6 +76,8 @@ use constant LIST_ORDER => ID_FIELD;
 # Bugs have their own auditing table, bugs_activity.
 use constant AUDIT_CREATES => 0;
 use constant AUDIT_UPDATES => 0;
+# This will be enabled later
+use constant USE_MEMCACHED => 0;
 
 # This is a sub because it needs to call other subroutines.
 sub DB_COLUMNS {
diff --git a/Bugzilla/Classification.pm b/Bugzilla/Classification.pm
index 88ec4eb3b..5d75a6dc6 100644
--- a/Bugzilla/Classification.pm
+++ b/Bugzilla/Classification.pm
@@ -56,6 +56,7 @@ use constant VALIDATORS => {
 ###############################
 ####     Constructors     #####
 ###############################
+
 sub remove_from_db {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
@@ -63,9 +64,18 @@ sub remove_from_db {
     ThrowUserError("classification_not_deletable") if ($self->id == 1);
 
     $dbh->bz_start_transaction();
+
     # Reclassify products to the default classification, if needed.
-    $dbh->do("UPDATE products SET classification_id = 1
-              WHERE classification_id = ?", undef, $self->id);
+    my $product_ids = $dbh->selectcol_arrayref(
+        'SELECT id FROM products WHERE classification_id = ?', undef, $self->id);
+
+    if (@$product_ids) {
+        $dbh->do('UPDATE products SET classification_id = 1 WHERE '
+                  . $dbh->sql_in('id', $product_ids));
+        foreach my $id (@$product_ids) {
+            Bugzilla->memcached->clear({ table => 'products', id => $id });
+        }
+    }
 
     $self->SUPER::remove_from_db();
 
diff --git a/Bugzilla/Comment/TagWeights.pm b/Bugzilla/Comment/TagWeights.pm
index 5835efbc4..f1a220a47 100644
--- a/Bugzilla/Comment/TagWeights.pm
+++ b/Bugzilla/Comment/TagWeights.pm
@@ -35,6 +35,9 @@ use constant NAME_FIELD => 'tag';
 use constant LIST_ORDER => 'weight DESC';
 use constant VALIDATORS => { };
 
+# There's no gain to caching these objects
+use constant USE_MEMCACHED => 0;
+
 sub tag    { return $_[0]->{'tag'} }
 sub weight { return $_[0]->{'weight'} }
 
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 5cd246b8e..97d03dc42 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -1078,6 +1078,8 @@ sub create {
             # Restore the original obsolete state of the custom field.
             $dbh->do('UPDATE fielddefs SET obsolete = 0 WHERE id = ?', undef, $field->id)
               unless $is_obsolete;
+
+            Bugzilla->memcached->clear({ table => 'fielddefs', id => $field->id });
         }
     };
 
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 089ac6c9f..6ef7be81e 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -480,6 +480,7 @@ sub update {
         $dbh->do('UPDATE flags SET modification_date = ? WHERE id = ?',
                  undef, ($timestamp, $self->id));
         $self->{'modification_date'} = format_time($timestamp,  '%Y.%m.%d %T');
+        Bugzilla->memcached->clear({ table => 'flags', id => $self->id });
     }
     return $changes;
 }
@@ -626,6 +627,7 @@ sub force_retarget {
         if ($is_retargetted) {
             $dbh->do('UPDATE flags SET type_id = ? WHERE id = ?',
                      undef, ($flag->type_id, $flag->id));
+            Bugzilla->memcached->clear({ table => 'flags', id => $flag->id });
         }
         else {
             # Track deleted attachment flags.
diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index 910e2d425..3325c8c5c 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -200,8 +200,16 @@ sub update {
     # Silently remove requestees from flags which are no longer
     # specifically requestable.
     if (!$self->is_requesteeble) {
-        $dbh->do('UPDATE flags SET requestee_id = NULL WHERE type_id = ?',
-                  undef, $self->id);
+        my $ids = $dbh->selectcol_arrayref(
+            'SELECT id FROM flags WHERE type_id = ? AND requestee_id IS NOT NULL',
+             undef, $self->id);
+
+        if (@$ids) {
+            $dbh->do('UPDATE flags SET requestee_id = NULL WHERE ' . $dbh->sql_in('id', $ids));
+            foreach my $id (@$ids) {
+                Bugzilla->memcached->clear({ table => 'flags', id => $id });
+            }
+        }
     }
 
     Bugzilla::Hook::process('flagtype_end_of_update',
diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index cea5a4a34..384df221e 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -375,6 +375,12 @@ sub OPTIONAL_MODULES {
 
     # memcached
     {
+        package => 'URI-Escape',
+        module  => 'URI::Escape',
+        version => 0,
+        feature => ['memcached'],
+    },
+    {
         package => 'Cache-Memcached',
         module  => 'Cache::Memcached',
         version => '0',
diff --git a/Bugzilla/Memcached.pm b/Bugzilla/Memcached.pm
index b1b10311b..4bfca2b65 100644
--- a/Bugzilla/Memcached.pm
+++ b/Bugzilla/Memcached.pm
@@ -14,6 +14,12 @@ use warnings;
 use Bugzilla::Error;
 use Bugzilla::Util qw(trick_taint);
 use Scalar::Util qw(blessed);
+use URI::Escape;
+use Encode;
+use Sys::Syslog qw(:DEFAULT);
+
+# memcached keys have a maximum length of 250 bytes
+use constant MAX_KEY_LENGTH => 250;
 
 sub _new {
     my $invocant = shift;
@@ -26,10 +32,11 @@ sub _new {
         && Bugzilla->params->{memcached_servers})
     {
         require Cache::Memcached;
+        $self->{namespace} = Bugzilla->params->{memcached_namespace} || '';
         $self->{memcached} =
             Cache::Memcached->new({
                 servers   => [ split(/[, ]+/, Bugzilla->params->{memcached_servers}) ],
-                namespace => Bugzilla->params->{memcached_namespace} || '',
+                namespace => $self->{namespace},
             });
     }
     return bless($self, $class);
@@ -129,6 +136,11 @@ sub clear_all {
     if (!$memcached->incr("prefix", 1)) {
         $memcached->add("prefix", time());
     }
+
+    # BMO - log that we've wiped the cache
+    openlog('apache', 'cons,pid', 'local4');
+    syslog('notice', encode_utf8('[memcached] cache cleared'));
+    closelog();
 }
 
 # in order to clear all our keys, we add a prefix to all our keys.  when we
@@ -155,6 +167,14 @@ sub _prefix {
     return $request_cache->{memcached_prefix};
 }
 
+sub _encode_key {
+    my ($self, $key) = @_;
+    $key = $self->_prefix . ':' . uri_escape_utf8($key);
+    return length($self->{namespace} . $key) > MAX_KEY_LENGTH
+        ? undef
+        : $key;
+}
+
 sub _set {
     my ($self, $key, $value) = @_;
     if (blessed($value)) {
@@ -162,13 +182,17 @@ sub _set {
         ThrowCodeError('param_invalid', { function => "Bugzilla::Memcached::set",
                                           param    => "value" });
     }
-    return $self->{memcached}->set($self->_prefix . ':' . $key, $value);
+    $key = $self->_encode_key($key)
+        or return;
+    return $self->{memcached}->set($key, $value);
 }
 
 sub _get {
     my ($self, $key) = @_;
 
-    my $value = $self->{memcached}->get($self->_prefix . ':' . $key);
+    $key = $self->_encode_key($key)
+        or return;
+    my $value = $self->{memcached}->get($key);
     return unless defined $value;
 
     # detaint returned values
@@ -187,7 +211,9 @@ sub _get {
 
 sub _delete {
     my ($self, $key) = @_;
-    return $self->{memcached}->delete($self->_prefix . ':' . $key);
+    $key = $self->_encode_key($key)
+        or return;
+    return $self->{memcached}->delete($key);
 }
 
 1;
diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm
index 92bc2192a..a5f3ed383 100644
--- a/Bugzilla/Milestone.pm
+++ b/Bugzilla/Milestone.pm
@@ -121,6 +121,7 @@ sub update {
         $dbh->do('UPDATE products SET defaultmilestone = ?
                   WHERE id = ? AND defaultmilestone = ?',
                  undef, ($self->name, $self->product_id, $changes->{value}->[0]));
+        Bugzilla->memcached->clear({ table => 'produles', id => $self->product_id });
     }
     return $changes;
 }
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 43d2c07ac..936465b3f 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -48,9 +48,8 @@ use constant AUDIT_UPDATES => 1;
 use constant AUDIT_REMOVES => 1;
 
 # When USE_MEMCACHED is true, the class is suitable for serialisation to
-# Memcached. This will be flipped to true by default once the majority of
-# Bugzilla Object have been tested with Memcached.
-use constant USE_MEMCACHED => 0;
+# Memcached.  See documentation in Bugzilla::Memcached for more information.
+use constant USE_MEMCACHED => 1;
 
 # This allows the JSON-RPC interface to return Bugzilla::Object instances
 # as though they were hashes. In the future, this may be modified to return
@@ -188,7 +187,20 @@ sub new_from_list {
     # their own implementation of match which is not compatible
     # with this one. However, match() still needs to have the right $invocant
     # in order to do $class->DB_TABLE and so on.
-    return match($invocant, { $id_field => \@detainted_ids });
+    my $list = match($invocant, { $id_field => \@detainted_ids });
+
+    # BMO: Populate the object cache with bug objects, which helps
+    # inline-history when viewing bugs with dependencies.
+    if ($class eq 'Bugzilla::Bug') {
+        foreach my $object (@$list) {
+            $class->_object_cache_set(
+                { id => $object->id, cache => 1 },
+                $object
+            );
+        }
+    }
+
+    return $list;
 }
 
 sub new_from_hash {
diff --git a/Bugzilla/Search/Recent.pm b/Bugzilla/Search/Recent.pm
index 125850e85..b9f6428d7 100644
--- a/Bugzilla/Search/Recent.pm
+++ b/Bugzilla/Search/Recent.pm
@@ -53,6 +53,9 @@ use constant VALIDATORS => {
 
 use constant UPDATE_COLUMNS => qw(bug_list list_order);
 
+# There's no gain to caching these objects
+use constant USE_MEMCACHED => 0;
+
 ###################
 # DB Manipulation #
 ###################
diff --git a/Bugzilla/Search/Saved.pm b/Bugzilla/Search/Saved.pm
index 99194112a..536cf39a4 100644
--- a/Bugzilla/Search/Saved.pm
+++ b/Bugzilla/Search/Saved.pm
@@ -199,6 +199,7 @@ sub rename_field_value {
         }
         $dbh->do("UPDATE $table SET query = ? WHERE $id_field = ?",
                  undef, $query, $id);
+        Bugzilla->memcached->clear({ table => $table, id => $id });
     }
 
     $dbh->bz_commit_transaction();
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 1c6e68078..0429a25e3 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -305,6 +305,7 @@ sub update_last_seen_date {
         # pending changes
         $dbh->do("UPDATE profiles SET last_seen_date = ? WHERE userid = ?",
                  undef, $date, $self->id);
+        Bugzilla->memcached->clear({ table => 'profiles', id => $self->id });
     }
 }