Bug 987032: allow memcached to cache bugzilla configuration information

r=dkl, a=glob

.gitrev

@@ -1 +1 @@
-dc9486b05615630a5d6165d8a1bd256284c19268
+3ef245b21a397e5253ba1bda607a1510245d5c09

Bugzilla/Classification.pm

@@ -23,6 +23,8 @@ use parent qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object Exporter);
 ####    Initialization     ####
 ###############################
 
+use constant IS_CONFIG => 1;
+
 use constant DB_TABLE => 'classifications';
 use constant LIST_ORDER => 'sortkey, name';
 
@@ -67,6 +69,7 @@ sub remove_from_db {
         foreach my $id (@$product_ids) {
             Bugzilla->memcached->clear({ table => 'products', id => $id });
         }
+        Bugzilla->memcached->clear_config();
     }
 
     $self->SUPER::remove_from_db();

Bugzilla/Field.pm

@@ -74,6 +74,8 @@ use Scalar::Util qw(blessed);
 ####    Initialization     ####
 ###############################
 
+use constant IS_CONFIG => 1;
+
 use constant DB_TABLE   => 'fielddefs';
 use constant LIST_ORDER => 'sortkey, name';
 
@@ -1078,6 +1080,7 @@ sub create {
           unless $is_obsolete;
 
         Bugzilla->memcached->clear({ table => 'fielddefs', id => $field->id });
+        Bugzilla->memcached->clear_config();
     }
 
     return $field;

Bugzilla/Field/Choice.pm

@@ -24,6 +24,8 @@ use Scalar::Util qw(blessed);
 # Initialization #
 ##################
 
+use constant IS_CONFIG => 1;
+
 use constant DB_COLUMNS => qw(
     id
     value

Bugzilla/Group.pm

@@ -21,6 +21,8 @@ use Bugzilla::Config qw(:admin);
 ##### Module Initialization ###
 ###############################
 
+use constant IS_CONFIG => 1;
+
 use constant DB_COLUMNS => qw(
     groups.id
     groups.name

Bugzilla/Keyword.pm

@@ -19,6 +19,8 @@ use Bugzilla::Util;
 ####    Initialization     ####
 ###############################
 
+use constant IS_CONFIG => 1;
+
 use constant DB_COLUMNS => qw(
    keyworddefs.id
    keyworddefs.name

Bugzilla/Memcached.pm

@@ -40,6 +40,10 @@ sub _new {
     return bless($self, $class);
 }
 
+sub enabled {
+    return $_[0]->{memcached} ? 1 : 0;
+}
+
 sub set {
     my ($self, $args) = @_;
     return unless $self->{memcached};
@@ -95,6 +99,32 @@ sub get {
     }
 }
 
+sub set_config {
+    my ($self, $args) = @_;
+    return unless $self->{memcached};
+
+    if (exists $args->{key}) {
+        return $self->_set($self->_config_prefix . ':' . $args->{key}, $args->{data});
+    }
+    else {
+        ThrowCodeError('params_required', { function => "Bugzilla::Memcached::set_config",
+                                            params   => [ 'key' ] });
+    }
+}
+
+sub get_config {
+    my ($self, $args) = @_;
+    return unless $self->{memcached};
+
+    if (exists $args->{key}) {
+        return $self->_get($self->_config_prefix . ':' . $args->{key});
+    }
+    else {
+        ThrowCodeError('params_required', { function => "Bugzilla::Memcached::get_config",
+                                            params   => [ 'key' ] });
+    }
+}
+
 sub clear {
     my ($self, $args) = @_;
     return unless $self->{memcached};
@@ -130,39 +160,61 @@ sub clear {
 
 sub clear_all {
     my ($self) = @_;
-    return unless my $memcached = $self->{memcached};
-    if (!$memcached->incr("prefix", 1)) {
-        $memcached->add("prefix", time());
-    }
+    return unless $self->{memcached};
+    $self->_inc_prefix("global");
+}
+
+sub clear_config {
+    my ($self) = @_;
+    return unless $self->{memcached};
+    $self->_inc_prefix("config");
 }
 
 # in order to clear all our keys, we add a prefix to all our keys.  when we
 # need to "clear" all current keys, we increment the prefix.
 sub _prefix {
-    my ($self) = @_;
+    my ($self, $name) = @_;
     # we don't want to change prefixes in the middle of a request
     my $request_cache = Bugzilla->request_cache;
-    if (!$request_cache->{memcached_prefix}) {
+    my $request_cache_key = "memcached_prefix_$name";
+    if (!$request_cache->{$request_cache_key}) {
         my $memcached = $self->{memcached};
-        my $prefix = $memcached->get("prefix");
+        my $prefix = $memcached->get($name);
         if (!$prefix) {
             $prefix = time();
-            if (!$memcached->add("prefix", $prefix)) {
+            if (!$memcached->add($name, $prefix)) {
                 # if this failed, either another process set the prefix, or
                 # memcached is down.  assume we lost the race, and get the new
                 # value.  if that fails, memcached is down so use a dummy
                 # prefix for this request.
-                $prefix = $memcached->get("prefix") || 0;
+                $prefix = $memcached->get($name) || 0;
             }
         }
-        $request_cache->{memcached_prefix} = $prefix;
+        $request_cache->{$request_cache_key} = $prefix;
     }
-    return $request_cache->{memcached_prefix};
+    return $request_cache->{$request_cache_key};
+}
+
+sub _inc_prefix {
+    my ($self, $name) = @_;
+    my $memcached = $self->{memcached};
+    if (!$memcached->incr($name, 1)) {
+        $memcached->add($name, time());
+    }
+    delete Bugzilla->request_cache->{"memcached_prefix_$name"};
+}
+
+sub _global_prefix {
+    return $_[0]->_prefix("global");
+}
+
+sub _config_prefix {
+    return $_[0]->_prefix("config");
 }
 
 sub _encode_key {
     my ($self, $key) = @_;
-    $key = $self->_prefix . ':' . uri_escape_utf8($key);
+    $key = $self->_global_prefix . ':' . uri_escape_utf8($key);
     return length($self->{namespace} . $key) > MAX_KEY_LENGTH
         ? undef
         : $key;
@@ -175,6 +227,7 @@ sub _set {
         ThrowCodeError('param_invalid', { function => "Bugzilla::Memcached::set",
                                           param    => "value" });
     }
+
     $key = $self->_encode_key($key)
         or return;
     return $self->{memcached}->set($key, $value);
@@ -191,10 +244,19 @@ sub _get {
     # detaint returned values
     # hashes and arrays are detainted just one level deep
     if (ref($value) eq 'HASH') {
-        map { defined($_) && trick_taint($_) } values %$value;
+        _detaint_hashref($value);
     }
     elsif (ref($value) eq 'ARRAY') {
-        trick_taint($_) foreach @$value;
+        foreach my $value (@$value) {
+            next unless defined $value;
+            # arrays of hashes are common
+            if (ref($value) eq 'HASH') {
+                _detaint_hashref($value);
+            }
+            elsif (!ref($value)) {
+                trick_taint($value);
+            }
+        }
     }
     elsif (!ref($value)) {
         trick_taint($value);
@@ -202,6 +264,15 @@ sub _get {
     return $value;
 }
 
+sub _detaint_hashref {
+    my ($hashref) = @_;
+    foreach my $value (values %$hashref) {
+        if (defined($value) && !ref($value)) {
+            trick_taint($value);
+        }
+    }
+}
+
 sub _delete {
     my ($self, $key) = @_;
     $key = $self->_encode_key($key)
@@ -266,6 +337,14 @@ L<Bugzilla-E<gt>memcached()|Bugzilla/memcached>.
 
 =head1 METHODS
 
+=over
+
+=item C<enabled>
+
+Returns true if Memcached support is available and enabled.
+
+=back
+
 =head2 Setting
 
 Adds a value to Memcached.
@@ -285,6 +364,13 @@ to C<undef>.
 This is a convenience method which allows cached data to be later retrieved by
 specifying the C<table> and either the C<id> or C<name>.
 
+=item C<set_config({ key =E<gt> $key, data =E<gt> $data })>
+
+Adds the C<data> using the C<key> while identifying the data as part of
+Bugzilla's configuration (such as fields, products, components, groups, etc).
+Values set with C<set_config> are automatically cleared when changes are made
+to Bugzilla's configuration.
+
 =back
 
 =head2 Getting
@@ -306,6 +392,11 @@ Return C<value> with the specified C<table> and C<id>.
 
 Return C<value> with the specified C<table> and C<name>.
 
+=item C<get_config({ key =E<gt> $key })>
+
+Return C<value> with the specified C<key> from the configuration cache.  See
+C<set_config> for more information.
+
 =back
 
 =head2 Clearing
@@ -328,6 +419,11 @@ corresponding C<table> and C<name> entry.
 Removes C<value> with the specified C<table> and C<name>, as well as the
 corresponding C<table> and C<id> entry.
 
+=item C<clear_config>
+
+Removes all configuration related values from the cache.  See C<set_config> for
+more information.
+
 =item C<clear_all>
 
 Removes all values from the cache.

Bugzilla/Milestone.pm

@@ -114,6 +114,7 @@ sub update {
                   WHERE id = ? AND defaultmilestone = ?',
                  undef, ($self->name, $self->product_id, $changes->{value}->[0]));
         Bugzilla->memcached->clear({ table => 'produles', id => $self->product_id });
+        Bugzilla->memcached->clear_config();
     }
     $dbh->bz_commit_transaction();
 

Bugzilla/Object.pm

@@ -38,6 +38,11 @@ use constant AUDIT_REMOVES => 1;
 # Memcached.  See documentation in Bugzilla::Memcached for more information.
 use constant USE_MEMCACHED => 1;
 
+# When IS_CONFIG is true, the class is used to track seldom changed
+# configuration objects.  This includes, but is not limited to, fields, field
+# values, keywords, products, classifications, priorities, severities, etc.
+use constant IS_CONFIG => 0;
+
 # 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
 # less information.
@@ -56,7 +61,7 @@ sub new {
     return $object if $object;
 
     my ($data, $set_memcached);
-    if (Bugzilla->feature('memcached')
+    if (Bugzilla->memcached->enabled
         && $class->USE_MEMCACHED
         && ref($param) eq 'HASH' && $param->{cache})
     {
@@ -352,22 +357,42 @@ sub _do_list_select {
     my $cols  = join(',', $class->_get_db_columns);
     my $order = $class->LIST_ORDER;
 
-    my $sql = "SELECT $cols FROM $table";
-    if (defined $where) {
-        $sql .= " WHERE $where ";
+    # Unconditional requests for configuration data are cacheable.
+    my ($objects, $set_memcached, $memcached_key);
+    if (!defined $where
+        && Bugzilla->memcached->enabled
+        && $class->IS_CONFIG)
+    {
+        $memcached_key = "$class:get_all";
+        $objects = Bugzilla->memcached->get_config({ key => $memcached_key });
+        $set_memcached = $objects ? 0 : 1;
     }
-    $sql .= " ORDER BY $order";
-    
-    $sql .= " $postamble" if $postamble;
-        
-    my $dbh = Bugzilla->dbh;
-    # Sometimes the values are tainted, but we don't want to untaint them
-    # for the caller. So we copy the array. It's safe to untaint because
-    # they're only used in placeholders here.
-    my @untainted = @{ $values || [] };
-    trick_taint($_) foreach @untainted;
-    my $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @untainted);
-    $class->_serialisation_keys($objects->[0]) if @$objects;
+
+    if (!$objects) {
+        my $sql = "SELECT $cols FROM $table";
+        if (defined $where) {
+            $sql .= " WHERE $where ";
+        }
+        $sql .= " ORDER BY $order";
+        $sql .= " $postamble" if $postamble;
+
+        my $dbh = Bugzilla->dbh;
+        # Sometimes the values are tainted, but we don't want to untaint them
+        # for the caller. So we copy the array. It's safe to untaint because
+        # they're only used in placeholders here.
+        my @untainted = @{ $values || [] };
+        trick_taint($_) foreach @untainted;
+        $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @untainted);
+        $class->_serialisation_keys($objects->[0]) if @$objects;
+    }
+
+    if ($objects && $set_memcached) {
+        Bugzilla->memcached->set_config({
+            key  => $memcached_key,
+            data => $objects
+        });
+    }
+
     foreach my $object (@$objects) {
         $object = $class->new_from_hash($object);
     }
@@ -495,8 +520,11 @@ sub update {
     $self->audit_log(\%changes) if $self->AUDIT_UPDATES;
 
     $dbh->bz_commit_transaction();
-    Bugzilla->memcached->clear({ table => $table, id => $self->id })
-        if $self->USE_MEMCACHED && @values;
+    if ($self->USE_MEMCACHED && @values) {
+        Bugzilla->memcached->clear({ table => $table, id => $self->id });
+        Bugzilla->memcached->clear_config()
+            if $self->IS_CONFIG;
+    }
     $self->_object_cache_remove({ id => $self->id });
     $self->_object_cache_remove({ name => $self->name }) if $self->name;
 
@@ -517,8 +545,11 @@ 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();
-    Bugzilla->memcached->clear({ table => $table, id => $self->id })
-        if $self->USE_MEMCACHED;
+    if ($self->USE_MEMCACHED) {
+        Bugzilla->memcached->clear({ table => $table, id => $self->id });
+        Bugzilla->memcached->clear_config()
+            if $self->IS_CONFIG;
+    }
     $self->_object_cache_remove({ id => $self->id });
     $self->_object_cache_remove({ name => $self->name }) if $self->name;
     undef $self;
@@ -585,6 +616,13 @@ sub create {
     my $object = $class->insert_create_data($field_values);
     $dbh->bz_commit_transaction();
 
+    if (Bugzilla->memcached->enabled
+        && $class->USE_MEMCACHED
+        && $class->IS_CONFIG)
+    {
+        Bugzilla->memcached->clear_config();
+    }
+
     return $object;
 }
 
@@ -680,7 +718,7 @@ sub insert_create_data {
 
 sub get_all {
     my $class = shift;
-    return @{$class->_do_list_select()};
+    return @{ $class->_do_list_select() };
 }
 
 ###############################

Bugzilla/Product.pm

@@ -34,6 +34,8 @@ use constant DEFAULT_CLASSIFICATION_ID => 1;
 ####    Initialization     ####
 ###############################
 
+use constant IS_CONFIG => 1;
+
 use constant DB_TABLE => 'products';
 
 use constant DB_COLUMNS => qw(

Bugzilla/Status.pm

@@ -108,11 +108,21 @@ sub _check_value {
 
 sub BUG_STATE_OPEN {
     my $dbh = Bugzilla->dbh;
-    my $cache = Bugzilla->request_cache;
-    $cache->{status_bug_state_open} ||=
-        $dbh->selectcol_arrayref('SELECT value FROM bug_status 
-                                   WHERE is_open = 1');
-    return @{ $cache->{status_bug_state_open} };
+    my $request_cache = Bugzilla->request_cache;
+    my $cache_key = 'status_bug_state_open';
+    return @{ $request_cache->{$cache_key} }
+        if exists $request_cache->{$cache_key};
+
+    my $rows = Bugzilla->memcached->get_config({ key => $cache_key });
+    if (!$rows) {
+        $rows = $dbh->selectcol_arrayref(
+            'SELECT value FROM bug_status WHERE is_open = 1'
+        );
+        Bugzilla->memcached->set_config({ key => $cache_key, data => $rows });
+    }
+
+    $request_cache->{$cache_key} = $rows;
+    return @$rows;
 }
 
 # Tells you whether or not the argument is a valid "open" state.

Bugzilla/User.pm

@@ -675,13 +675,23 @@ sub groups {
             FROM user_group_map
            WHERE user_id = ? AND isbless = 0}, undef, $self->id);
 
-    my $rows = $dbh->selectall_arrayref(
-        "SELECT DISTINCT grantor_id, member_id
-           FROM group_group_map
-          WHERE grant_type = " . GROUP_MEMBERSHIP);
+    my $cache_key = 'group_grant_type_' . GROUP_MEMBERSHIP;
+    my $membership_rows = Bugzilla->memcached->get_config({
+        key => $cache_key,
+    });
+    if (!$membership_rows) {
+        $membership_rows = $dbh->selectall_arrayref(
+            "SELECT DISTINCT grantor_id, member_id
+               FROM group_group_map
+              WHERE grant_type = " . GROUP_MEMBERSHIP);
+        Bugzilla->memcached->set_config({
+            key  => $cache_key,
+            data => $membership_rows,
+        });
+    }
 
     my %group_membership;
-    foreach my $row (@$rows) {
+    foreach my $row (@$membership_rows) {
         my ($grantor_id, $member_id) = @$row; 
         push (@{ $group_membership{$member_id} }, $grantor_id);
     }

contrib/convert-workflow.pl

@@ -151,6 +151,7 @@ if ($enable_unconfirmed) {
     $dbh->do('UPDATE products SET allows_unconfirmed = 1');
 }
 $dbh->bz_commit_transaction();
+Bugzilla->memcached->clear_all();
 
 print <<END;
 Done. There are some things you may want to fix, now:

editclassifications.cgi

@@ -222,6 +222,7 @@ if ($action eq 'reclassify') {
     foreach my $name (@names) {
         Bugzilla->memcached->clear({ table => 'products', name => $name });
     }
+    Bugzilla->memcached->clear_config();
 
     LoadTemplate($action);
 }