Bug 956233: enable USE_MEMCACHE on most objects

r=dkl, a=glob

23 files changed, 130 insertions, 24 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index c0521c9b4..d0bd8e4c2 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -908,6 +908,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;
@@ -932,7 +935,10 @@ 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->selectrow_array(
+        '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));
@@ -942,6 +948,13 @@ sub remove_from_db {
     if (-e $filename) {
         unlink $filename or warn "Couldn't unlink $filename: $!";
     }
+
+    # 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 0c552f40b..ecb64e22a 100644
--- a/Bugzilla/Auth/Verify.pm
+++ b/Bugzilla/Auth/Verify.pm
@@ -89,6 +89,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.
diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm
index 82fa662dc..a5b78797b 100644
--- a/Bugzilla/Auth/Verify/DB.pm
+++ b/Bugzilla/Auth/Verify/DB.pm
@@ -95,6 +95,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 95bf293a5..3e48312a9 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -54,6 +54,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 771a894b4..5d488e3b9 100644
--- a/Bugzilla/Classification.pm
+++ b/Bugzilla/Classification.pm
@@ -55,9 +55,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->selectrow_array(
+        "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 82c82161a..dce16f83b 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -1075,6 +1075,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 });
     }
 
     return $field;
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 8931649e4..f365afd98 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -461,6 +461,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;
 }
@@ -607,6 +608,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 5fee86b30..c70c1598c 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -185,8 +185,13 @@ 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->selectrow_array(
+            "SELECT id FROM flags WHERE type_id = ?", undef, $self->id);
+        $dbh->do("UPDATE flags SET requestee_id = NULL WHERE "
+            . $dbh->sql_in('type_id', \@ids));
+        foreach my $id (@ids) {
+            Bugzilla->memcached->clear({ table => 'flags', id => $id });
+        }
     }
 
     $dbh->bz_commit_transaction();
diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index cd8978eb1..e72129153 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -397,6 +397,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..0752bcce9 100644
--- a/Bugzilla/Memcached.pm
+++ b/Bugzilla/Memcached.pm
@@ -14,6 +14,10 @@ use warnings;
 use Bugzilla::Error;
 use Bugzilla::Util qw(trick_taint);
 use Scalar::Util qw(blessed);
+use URI::Escape;
+
+# memcached keys have a maximum length of 250 bytes
+use constant MAX_KEY_LENGTH => 250;
 
 sub _new {
     my $invocant = shift;
@@ -26,10 +30,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);
@@ -155,6 +160,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 +175,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 +204,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 6d6465e4d..b1783de88 100644
--- a/Bugzilla/Milestone.pm
+++ b/Bugzilla/Milestone.pm
@@ -113,6 +113,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 });
     }
     $dbh->bz_commit_transaction();
 
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index c4dfe8cf7..ff1454539 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -35,9 +35,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
diff --git a/Bugzilla/Search/Recent.pm b/Bugzilla/Search/Recent.pm
index 6d349d129..cc1c3c582 100644
--- a/Bugzilla/Search/Recent.pm
+++ b/Bugzilla/Search/Recent.pm
@@ -42,6 +42,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 e81035ada..5dfce611b 100644
--- a/Bugzilla/Search/Saved.pm
+++ b/Bugzilla/Search/Saved.pm
@@ -187,6 +187,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 5ded9f06e..872c742db 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -278,6 +278,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 });
     }
 }
 
diff --git a/buglist.cgi b/buglist.cgi
index 281504c66..600e14ef7 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -382,6 +382,7 @@ if ($cmdtype eq "dorem") {
             $dbh->do('DELETE FROM namedquery_group_map
                             WHERE namedquery_id = ?',
                      undef, $query_id);
+            Bugzilla->memcached->clear({ table => 'namedqueries', id => $query_id });
         }
 
         # Now reset the cached queries
diff --git a/contrib/merge-users.pl b/contrib/merge-users.pl
index 19cb8d4c9..eb205cac2 100755
--- a/contrib/merge-users.pl
+++ b/contrib/merge-users.pl
@@ -222,4 +222,9 @@ $user->derive_regexp_groups();
 # Commit the transaction
 $dbh->bz_commit_transaction();
 
+# It's complex to determine which items now need to be flushed from memcached.
+# As user merge is expected to be a rare event, we just flush the entire cache
+# when users are merged.
+Bugzilla->memcached->clear_all();
+
 print "Done.\n";
diff --git a/editclassifications.cgi b/editclassifications.cgi
index 3d93057b9..d47853133 100755
--- a/editclassifications.cgi
+++ b/editclassifications.cgi
@@ -188,9 +188,10 @@ if ($action eq 'update') {
 
 if ($action eq 'reclassify') {
     my $classification = Bugzilla::Classification->check($class_name);
-   
+
     my $sth = $dbh->prepare("UPDATE products SET classification_id = ?
                              WHERE name = ?");
+    my @names;
 
     if (defined $cgi->param('add_products')) {
         check_token_data($token, 'reclassify_classifications');
@@ -198,6 +199,7 @@ if ($action eq 'reclassify') {
             foreach my $prod ($cgi->param("prodlist")) {
                 trick_taint($prod);
                 $sth->execute($classification->id, $prod);
+                push @names, $prod;
             }
         }
         delete_token($token);
@@ -206,7 +208,8 @@ if ($action eq 'reclassify') {
         if (defined $cgi->param('myprodlist')) {
             foreach my $prod ($cgi->param("myprodlist")) {
                 trick_taint($prod);
-                $sth->execute(1,$prod);
+                $sth->execute(1, $prod);
+                push @names, $prod;
             }
         }
         delete_token($token);
@@ -216,6 +219,10 @@ if ($action eq 'reclassify') {
     $vars->{'classification'} = $classification;
     $vars->{'token'} = issue_session_token('reclassify_classifications');
 
+    foreach my $name (@names) {
+        Bugzilla->memcached->clear({ table => 'products', name => $name });
+    }
+
     LoadTemplate($action);
 }
 
diff --git a/editusers.cgi b/editusers.cgi
index f4e3c0841..83f364528 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -650,6 +650,11 @@ if ($action eq 'search') {
     $dbh->bz_commit_transaction();
     delete_token($token);
 
+    # It's complex to determine which items now need to be flushed from
+    # memcached.  As user deletion is expected to be a rare event, we just
+    # flush the entire cache when a user is deleted.
+    Bugzilla->memcached->clear_all();
+
     $vars->{'message'} = 'account_deleted';
     $vars->{'otheruser'}{'login'} = $otherUser->login;
     $vars->{'restrictablegroups'} = $user->bless_groups();
diff --git a/query.cgi b/query.cgi
index 65a7d3c3d..620cf2737 100755
--- a/query.cgi
+++ b/query.cgi
@@ -14,6 +14,7 @@ use Bugzilla;
 use Bugzilla::Bug;
 use Bugzilla::Constants;
 use Bugzilla::Search;
+use Bugzilla::Search::Saved;
 use Bugzilla::User;
 use Bugzilla::Util;
 use Bugzilla::Error;
@@ -76,9 +77,11 @@ if ($cgi->param('nukedefaultquery')) {
     if ($userid) {
         my $token = $cgi->param('token');
         check_hash_token($token, ['nukedefaultquery']);
-        $dbh->do("DELETE FROM namedqueries" .
-                 " WHERE userid = ? AND name = ?", 
-                 undef, ($userid, DEFAULT_QUERY_NAME));
+        my $named_queries = Bugzilla::Search::Saved->match(
+            { userid => $userid, name => DEFAULT_QUERY_NAME });
+        if (@$named_queries) {
+            $named_queries->[0]->remove_from_db();
+        }
     }
     $buffer = "";
 }
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index 256438bd8..2f87fb828 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -73,6 +73,7 @@ else {
     }
 }
 my $vars = {};
+my $clear_memcached = 0;
 
 print $cgi->header() unless Bugzilla->usage_mode == USAGE_MODE_CMDLINE;
 
@@ -149,6 +150,7 @@ if ($cgi->param('createmissinggroupcontrolmapentries')) {
     }
 
     Status('group_control_map_entries_repaired', {counter => $counter});
+    $clear_memcached = 1 if $counter;
 }
 
 ###########################################################################
@@ -175,6 +177,7 @@ if ($cgi->param('repair_creation_date')) {
         $sth_UpdateDate->execute($date, $bugid);
     }
     Status('bug_creation_date_fixed', {bug_count => scalar(@$bug_ids)});
+    $clear_memcached = 1 if @$bug_ids;
 }
 
 ###########################################################################
@@ -191,6 +194,7 @@ if ($cgi->param('repair_everconfirmed')) {
     $dbh->do("UPDATE bugs SET everconfirmed = 1 WHERE bug_status IN ($confirmed_open_states)");
 
     Status('everconfirmed_end');
+    $clear_memcached = 1;
 }
 
 ###########################################################################
@@ -206,11 +210,12 @@ if ($cgi->param('repair_bugs_fulltext')) {
                                             ON bugs_fulltext.bug_id = bugs.bug_id
                                             WHERE bugs_fulltext.bug_id IS NULL');
 
-   foreach my $bugid (@$bug_ids) {
-       Bugzilla::Bug->new($bugid)->_sync_fulltext( new_bug => 1 );
-   }
+    foreach my $bugid (@$bug_ids) {
+        Bugzilla::Bug->new($bugid)->_sync_fulltext( new_bug => 1 );
+    }
 
-   Status('bugs_fulltext_fixed', {bug_count => scalar(@$bug_ids)});
+    Status('bugs_fulltext_fixed', {bug_count => scalar(@$bug_ids)});
+    $clear_memcached = 1 if @$bug_ids;
 }
 
 ###########################################################################
@@ -246,7 +251,10 @@ if ($cgi->param('rescanallBugMail')) {
         Bugzilla::BugMail::Send($bugid, $vars);
     }
 
-    Status('send_bugmail_end') if scalar(@$list);
+    if (@$list) {
+        Status('send_bugmail_end');
+        Bugzilla->memcached->clear_all();
+    }
 
     unless (Bugzilla->usage_mode == USAGE_MODE_CMDLINE) {
         $template->process('global/footer.html.tmpl', $vars)
@@ -280,6 +288,7 @@ if ($cgi->param('remove_invalid_bug_references')) {
 
         if (scalar(@$bug_ids)) {
             $dbh->do("DELETE FROM $table WHERE $field IN (" . join(',', @$bug_ids) . ")");
+            $clear_memcached = 1;
         }
     }
 
@@ -310,6 +319,7 @@ if ($cgi->param('remove_invalid_attach_references')) {
 
     $dbh->bz_commit_transaction();
     Status('attachment_reference_deletion_end');
+    $clear_memcached = 1 if @$attach_ids;
 }
 
 ###########################################################################
@@ -336,12 +346,17 @@ if ($cgi->param('remove_old_whine_targets')) {
             $dbh->do("DELETE FROM whine_schedules
                        WHERE mailto_type = $type AND mailto IN (" .
                        join(',', @$old_ids) . ")");
+            $clear_memcached = 1;
         }
     }
     $dbh->bz_commit_transaction();
     Status('whines_obsolete_target_deletion_end');
 }
 
+# If any repairs were attempted or made, we need to clear memcached to ensure
+# state is consistent.
+Bugzilla->memcached->clear_all() if $clear_memcached;
+
 ###########################################################################
 # Repair hook
 ###########################################################################
@@ -717,6 +732,7 @@ if (scalar(@invalid_flags)) {
         # Silently delete these flags, with no notification to requesters/setters.
         $dbh->do('DELETE FROM flags WHERE id IN (' . join(',', @flag_ids) .')');
         Status('flag_deletion_end');
+        Bugzilla->memcached->clear_all();
     }
     else {
         foreach my $flag (@$invalid_flags) {
diff --git a/userprefs.cgi b/userprefs.cgi
index 638004c1e..8a93bf5d3 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -491,12 +491,13 @@ sub SaveSavedSearches {
     }
 
     $user->flush_queries_cache;
-    
+
     # Update profiles.mybugslink.
     my $showmybugslink = defined($cgi->param("showmybugslink")) ? 1 : 0;
     $dbh->do("UPDATE profiles SET mybugslink = ? WHERE userid = ?",
-             undef, ($showmybugslink, $user->id));    
+             undef, ($showmybugslink, $user->id));
     $user->{'showmybugslink'} = $showmybugslink;
+    Bugzilla->memcached->clear({ table => 'profiles', id => $user->id });
 }