From 6d730fae2211b9319037b7cb0515351fa529e78c Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Wed, 28 May 2014 13:53:48 +0800 Subject: Bug 993939: Bugzilla::User::Setting::groups() should use memcached r=dkl, a=justdave --- Bugzilla/Memcached.pm | 22 +++++++--- Bugzilla/User.pm | 118 ++++++++++++++++++++++++++++---------------------- editusers.cgi | 1 + 3 files changed, 83 insertions(+), 58 deletions(-) diff --git a/Bugzilla/Memcached.pm b/Bugzilla/Memcached.pm index 2bf7f1393..819b6d8b6 100644 --- a/Bugzilla/Memcached.pm +++ b/Bugzilla/Memcached.pm @@ -104,7 +104,7 @@ sub set_config { return unless $self->{memcached}; if (exists $args->{key}) { - return $self->_set($self->_config_prefix . ':' . $args->{key}, $args->{data}); + return $self->_set($self->_config_prefix . '.' . $args->{key}, $args->{data}); } else { ThrowCodeError('params_required', { function => "Bugzilla::Memcached::set_config", @@ -117,7 +117,7 @@ sub get_config { return unless $self->{memcached}; if (exists $args->{key}) { - return $self->_get($self->_config_prefix . ':' . $args->{key}); + return $self->_get($self->_config_prefix . '.' . $args->{key}); } else { ThrowCodeError('params_required', { function => "Bugzilla::Memcached::get_config", @@ -165,9 +165,14 @@ sub clear_all { } sub clear_config { - my ($self) = @_; - return unless $self->{memcached}; - $self->_inc_prefix("config"); + my ($self, $args) = @_; + if ($args && exists $args->{key}) { + $self->_delete($self->_config_prefix . '.' . $args->{key}); + } + else { + 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 @@ -214,7 +219,7 @@ sub _config_prefix { sub _encode_key { my ($self, $key) = @_; - $key = $self->_global_prefix . ':' . uri_escape_utf8($key); + $key = $self->_global_prefix . '.' . uri_escape_utf8($key); return length($self->{namespace} . $key) > MAX_KEY_LENGTH ? undef : $key; @@ -419,6 +424,11 @@ corresponding C and C entry. Removes C with the specified C
and C, as well as the corresponding C
and C entry. +=item C $key })> + +Remove C with the specified C from the configuration cache. See +C for more information. + =item C Removes all configuration related values from the cache. See C for diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 0b644a3b4..6b5d13cbe 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -668,66 +668,78 @@ sub flush_queries_cache { sub groups { my $self = shift; - return $self->{groups} if defined $self->{groups}; return [] unless $self->id; + return $self->{groups} if defined $self->{groups}; - my $dbh = Bugzilla->dbh; - my $groups_to_check = $dbh->selectcol_arrayref( - q{SELECT DISTINCT group_id - FROM user_group_map - WHERE user_id = ? AND isbless = 0}, undef, $self->id); - - my $cache_key = 'group_grant_type_' . GROUP_MEMBERSHIP; - my $membership_rows = Bugzilla->memcached->get_config({ - key => $cache_key, + my $user_groups_key = "user_groups." . $self->id; + my $groups = Bugzilla->memcached->get_config({ + key => $user_groups_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, + + if (!$groups) { + my $dbh = Bugzilla->dbh; + my $groups_to_check = $dbh->selectcol_arrayref( + "SELECT DISTINCT group_id + FROM user_group_map + WHERE user_id = ? AND isbless = 0", undef, $self->id); + + my $grant_type_key = 'group_grant_type_' . GROUP_MEMBERSHIP; + my $membership_rows = Bugzilla->memcached->get_config({ + key => $grant_type_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 => $grant_type_key, + data => $membership_rows, + }); + } - my %group_membership; - foreach my $row (@$membership_rows) { - my ($grantor_id, $member_id) = @$row; - push (@{ $group_membership{$member_id} }, $grantor_id); - } - - # Let's walk the groups hierarchy tree (using FIFO) - # On the first iteration it's pre-filled with direct groups - # membership. Later on, each group can add its own members into the - # FIFO. Circular dependencies are eliminated by checking - # $checked_groups{$member_id} hash values. - # As a result, %groups will have all the groups we are the member of. - my %checked_groups; - my %groups; - while (scalar(@$groups_to_check) > 0) { - # Pop the head group from FIFO - my $member_id = shift @$groups_to_check; - - # Skip the group if we have already checked it - if (!$checked_groups{$member_id}) { - # Mark group as checked - $checked_groups{$member_id} = 1; - - # Add all its members to the FIFO check list - # %group_membership contains arrays of group members - # for all groups. Accessible by group number. - my $members = $group_membership{$member_id}; - my @new_to_check = grep(!$checked_groups{$_}, @$members); - push(@$groups_to_check, @new_to_check); - - $groups{$member_id} = 1; + my %group_membership; + foreach my $row (@$membership_rows) { + my ($grantor_id, $member_id) = @$row; + push (@{ $group_membership{$member_id} }, $grantor_id); } - } - $self->{groups} = Bugzilla::Group->new_from_list([keys %groups]); + # Let's walk the groups hierarchy tree (using FIFO) + # On the first iteration it's pre-filled with direct groups + # membership. Later on, each group can add its own members into the + # FIFO. Circular dependencies are eliminated by checking + # $checked_groups{$member_id} hash values. + # As a result, %groups will have all the groups we are the member of. + my %checked_groups; + my %groups; + while (scalar(@$groups_to_check) > 0) { + # Pop the head group from FIFO + my $member_id = shift @$groups_to_check; + + # Skip the group if we have already checked it + if (!$checked_groups{$member_id}) { + # Mark group as checked + $checked_groups{$member_id} = 1; + + # Add all its members to the FIFO check list + # %group_membership contains arrays of group members + # for all groups. Accessible by group number. + my $members = $group_membership{$member_id}; + my @new_to_check = grep(!$checked_groups{$_}, @$members); + push(@$groups_to_check, @new_to_check); + + $groups{$member_id} = 1; + } + } + $groups = [ keys %groups ]; + Bugzilla->memcached->set_config({ + key => $user_groups_key, + data => $groups, + }); + } + + $self->{groups} = Bugzilla::Group->new_from_list($groups); return $self->{groups}; } @@ -1448,6 +1460,8 @@ sub derive_regexp_groups { $group_delete->execute($id, $group, GRANT_REGEXP) if $present; } } + + Bugzilla->memcached->clear_config({ key => "user_groups.$id" }); } sub product_responsibilities { diff --git a/editusers.cgi b/editusers.cgi index 83f364528..650784d5d 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -346,6 +346,7 @@ if ($action eq 'search') { ($otherUserID, $userid, get_field_id('bug_group'), join(', ', @groupsRemovedFrom), join(', ', @groupsAddedTo))); + Bugzilla->memcached->clear_config({ key => "user_groups.$otherUserID" }) } # XXX: should create profiles_activity entries for blesser changes. -- cgit v1.2.3-24-g4f1b