opendev/gerrit
Code Issues Proposed changes

Improve method and variable names related to group inclusion caches

Browse Source 
In [1] group's accounts will be cached, thus in the semantic
environment of group inclusion caches the words 'member' and
'include' will not only be limited to subgroups and parent groups,
but will also include accounts.

This commit does not update cache names and does not affect users.
It only updates related method names to be more meaningful.

[1] https://gerrit-review.googlesource.com/58302

Change-Id: I9a5be1cb08ff53e2cede251a6f63e16ec25cdcfa
This commit is contained in:
Bruce Zu
2014-09-28 13:04:54 +08:00
committed by Saša Živkov
parent 0524c80b88
commit dd83336fd8
9 changed files with 48 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java
View File

@@ -21,14 +21,14 @@ import java.util.Set;
/** Tracks group inclusions in memory for efficient access. */ /** Tracks group inclusions in memory for efficient access. */
public interface GroupIncludeCache { public interface GroupIncludeCache {
/** @return groups directly a member of the passed group. */ /** @return groups directly a member of the passed group. */
public Set<AccountGroup.UUID> membersOf(AccountGroup.UUID group); public Set<AccountGroup.UUID> subgroupsOf(AccountGroup.UUID group);
/** @return any groups the passed group belongs to. */ /** @return any groups the passed group belongs to. */
public Set<AccountGroup.UUID> memberIn(AccountGroup.UUID groupId); public Set<AccountGroup.UUID> parentGroupsOf(AccountGroup.UUID groupId);
/** @return set of any UUIDs that are not internal groups. */ /** @return set of any UUIDs that are not internal groups. */
public Set<AccountGroup.UUID> allExternalMembers(); public Set<AccountGroup.UUID> allExternalMembers();
public void evictMembersOf(AccountGroup.UUID groupId); public void evictSubgroupsOf(AccountGroup.UUID groupId);
public void evictMemberIn(AccountGroup.UUID groupId); public void evictParentGroupsOf(AccountGroup.UUID groupId);
} }

48
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java
View File

@@ -42,23 +42,23 @@ import java.util.concurrent.ExecutionException;
public class GroupIncludeCacheImpl implements GroupIncludeCache { public class GroupIncludeCacheImpl implements GroupIncludeCache {
private static final Logger log = LoggerFactory private static final Logger log = LoggerFactory
.getLogger(GroupIncludeCacheImpl.class); .getLogger(GroupIncludeCacheImpl.class);
private static final String BYINCLUDE_NAME = "groups_byinclude"; private static final String PARENT_GROUPS_NAME = "groups_byinclude";
private static final String MEMBERS_NAME = "groups_members"; private static final String SUBGROUPS_NAME = "groups_members";
private static final String EXTERNAL_NAME = "groups_external"; private static final String EXTERNAL_NAME = "groups_external";
public static Module module() { public static Module module() {
return new CacheModule() { return new CacheModule() {
@Override @Override
protected void configure() { protected void configure() {
cache(BYINCLUDE_NAME, cache(PARENT_GROUPS_NAME,
AccountGroup.UUID.class, AccountGroup.UUID.class,
new TypeLiteral<Set<AccountGroup.UUID>>() {}) new TypeLiteral<Set<AccountGroup.UUID>>() {})
.loader(MemberInLoader.class); .loader(parentGroupsLoader.class);
cache(MEMBERS_NAME, cache(SUBGROUPS_NAME,
AccountGroup.UUID.class, AccountGroup.UUID.class,
new TypeLiteral<Set<AccountGroup.UUID>>() {}) new TypeLiteral<Set<AccountGroup.UUID>>() {})
.loader(MembersOfLoader.class); .loader(subgroupsLoader.class);
cache(EXTERNAL_NAME, cache(EXTERNAL_NAME,
String.class, String.class,
@@ -71,24 +71,24 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
}; };
} }
private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> membersOf; private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> subgroups;
private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> memberIn; private final LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> parentGroups;
private final LoadingCache<String, Set<AccountGroup.UUID>> external; private final LoadingCache<String, Set<AccountGroup.UUID>> external;
@Inject @Inject
GroupIncludeCacheImpl( GroupIncludeCacheImpl(
@Named(MEMBERS_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> membersOf, @Named(SUBGROUPS_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> subgroups,
@Named(BYINCLUDE_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> memberIn, @Named(PARENT_GROUPS_NAME) LoadingCache<AccountGroup.UUID, Set<AccountGroup.UUID>> parentGroups,
@Named(EXTERNAL_NAME) LoadingCache<String, Set<AccountGroup.UUID>> external) { @Named(EXTERNAL_NAME) LoadingCache<String, Set<AccountGroup.UUID>> external) {
this.membersOf = membersOf; this.subgroups = subgroups;
this.memberIn = memberIn; this.parentGroups = parentGroups;
this.external = external; this.external = external;
} }
@Override @Override
public Set<AccountGroup.UUID> membersOf(AccountGroup.UUID groupId) { public Set<AccountGroup.UUID> subgroupsOf(AccountGroup.UUID groupId) {
try { try {
return membersOf.get(groupId); return subgroups.get(groupId);
} catch (ExecutionException e) { } catch (ExecutionException e) {
log.warn("Cannot load members of group", e); log.warn("Cannot load members of group", e);
return Collections.emptySet(); return Collections.emptySet();
@@ -96,9 +96,9 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
} }
@Override @Override
public Set<AccountGroup.UUID> memberIn(AccountGroup.UUID groupId) { public Set<AccountGroup.UUID> parentGroupsOf(AccountGroup.UUID groupId) {
try { try {
return memberIn.get(groupId); return parentGroups.get(groupId);
} catch (ExecutionException e) { } catch (ExecutionException e) {
log.warn("Cannot load included groups", e); log.warn("Cannot load included groups", e);
return Collections.emptySet(); return Collections.emptySet();
@@ -106,16 +106,16 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
} }
@Override @Override
public void evictMembersOf(AccountGroup.UUID groupId) { public void evictSubgroupsOf(AccountGroup.UUID groupId) {
if (groupId != null) { if (groupId != null) {
membersOf.invalidate(groupId); subgroups.invalidate(groupId);
} }
} }
@Override @Override
public void evictMemberIn(AccountGroup.UUID groupId) { public void evictParentGroupsOf(AccountGroup.UUID groupId) {
if (groupId != null) { if (groupId != null) {
memberIn.invalidate(groupId); parentGroups.invalidate(groupId);
if (!AccountGroup.isInternalGroup(groupId)) { if (!AccountGroup.isInternalGroup(groupId)) {
external.invalidate(EXTERNAL_NAME); external.invalidate(EXTERNAL_NAME);
@@ -133,12 +133,12 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
} }
} }
static class MembersOfLoader extends static class subgroupsLoader extends
CacheLoader<AccountGroup.UUID, Set<AccountGroup.UUID>> { CacheLoader<AccountGroup.UUID, Set<AccountGroup.UUID>> {
private final SchemaFactory<ReviewDb> schema; private final SchemaFactory<ReviewDb> schema;
@Inject @Inject
MembersOfLoader(final SchemaFactory<ReviewDb> sf) { subgroupsLoader(final SchemaFactory<ReviewDb> sf) {
schema = sf; schema = sf;
} }
@@ -163,12 +163,12 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
} }
} }
static class MemberInLoader extends static class parentGroupsLoader extends
CacheLoader<AccountGroup.UUID, Set<AccountGroup.UUID>> { CacheLoader<AccountGroup.UUID, Set<AccountGroup.UUID>> {
private final SchemaFactory<ReviewDb> schema; private final SchemaFactory<ReviewDb> schema;
@Inject @Inject
MemberInLoader(final SchemaFactory<ReviewDb> sf) { parentGroupsLoader(final SchemaFactory<ReviewDb> sf) {
schema = sf; schema = sf;
} }

4
gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java
View File

@@ -90,7 +90,7 @@ public class IncludingGroupMembership implements GroupMembership {
} }
memberOf.put(id, false); memberOf.put(id, false);
if (search(includeCache.membersOf(id))) { if (search(includeCache.subgroupsOf(id))) {
memberOf.put(id, true); memberOf.put(id, true);
return true; return true;
} }
@@ -131,7 +131,7 @@ public class IncludingGroupMembership implements GroupMembership {
while (!q.isEmpty()) { while (!q.isEmpty()) {
AccountGroup.UUID id = q.remove(q.size() - 1); AccountGroup.UUID id = q.remove(q.size() - 1);
for (AccountGroup.UUID g : includeCache.memberIn(id)) { for (AccountGroup.UUID g : includeCache.parentGroupsOf(id)) {
if (g != null && r.add(g)) { if (g != null && r.add(g)) {
q.add(g); q.add(g);
memberOf.put(g, true); memberOf.put(g, true);

4
gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java
View File

@@ -116,7 +116,7 @@ public class PerformCreateGroup {
if (createGroupArgs.initialGroups != null) { if (createGroupArgs.initialGroups != null) {
addGroups(groupId, createGroupArgs.initialGroups); addGroups(groupId, createGroupArgs.initialGroups);
groupIncludeCache.evictMembersOf(uuid); groupIncludeCache.evictSubgroupsOf(uuid);
} }
groupCache.onCreateGroup(createGroupArgs.getGroup()); groupCache.onCreateGroup(createGroupArgs.getGroup());
@@ -162,7 +162,7 @@ public class PerformCreateGroup {
db.accountGroupByIdAud().insert(includesAudit); db.accountGroupByIdAud().insert(includesAudit);
for (AccountGroup.UUID uuid : groups) { for (AccountGroup.UUID uuid : groups) {
groupIncludeCache.evictMemberIn(uuid); groupIncludeCache.evictParentGroupsOf(uuid);
} }
} }
} }

18
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
View File

@@ -91,7 +91,7 @@ import javax.security.auth.login.LoginException;
return null; return null;
} }
private final Cache<String, ImmutableSet<String>> groupsByInclude; private final Cache<String, ImmutableSet<String>> parentGroups;
private final Config config; private final Config config;
private final String server; private final String server;
private final String username; private final String username;
@@ -106,8 +106,8 @@ import javax.security.auth.login.LoginException;
@Inject @Inject
Helper(@GerritServerConfig final Config config, Helper(@GerritServerConfig final Config config,
@Named(LdapModule.GROUPS_BYINCLUDE_CACHE) @Named(LdapModule.PARENT_GROUPS_CACHE)
Cache<String, ImmutableSet<String>> groupsByInclude) { Cache<String, ImmutableSet<String>> parentGroups) {
this.config = config; this.config = config;
this.server = LdapRealm.optional(config, "server"); this.server = LdapRealm.optional(config, "server");
this.username = LdapRealm.optional(config, "username"); this.username = LdapRealm.optional(config, "username");
@@ -132,7 +132,7 @@ import javax.security.auth.login.LoginException;
} else { } else {
connectTimeoutMillis = null; connectTimeoutMillis = null;
} }
this.groupsByInclude = groupsByInclude; this.parentGroups = parentGroups;
this.connectionPoolConfig = getPoolProperties(config); this.connectionPoolConfig = getPoolProperties(config);
} }
@@ -310,8 +310,8 @@ import javax.security.auth.login.LoginException;
private void recursivelyExpandGroups(final Set<String> groupDNs, private void recursivelyExpandGroups(final Set<String> groupDNs,
final LdapSchema schema, final DirContext ctx, final String groupDN) { final LdapSchema schema, final DirContext ctx, final String groupDN) {
if (groupDNs.add(groupDN) && schema.accountMemberField != null) { if (groupDNs.add(groupDN) && schema.accountMemberField != null) {
ImmutableSet<String> cachedGroupDNs = groupsByInclude.getIfPresent(groupDN); ImmutableSet<String> cachedParentsDNs = parentGroups.getIfPresent(groupDN);
if (cachedGroupDNs == null) { if (cachedParentsDNs == null) {
// Recursively identify the groups it is a member of. // Recursively identify the groups it is a member of.
ImmutableSet.Builder<String> dns = ImmutableSet.builder(); ImmutableSet.Builder<String> dns = ImmutableSet.builder();
try { try {
@@ -330,10 +330,10 @@ import javax.security.auth.login.LoginException;
} catch (NamingException e) { } catch (NamingException e) {
LdapRealm.log.warn("Could not find group " + groupDN, e); LdapRealm.log.warn("Could not find group " + groupDN, e);
} }
cachedGroupDNs = dns.build(); cachedParentsDNs = dns.build();
groupsByInclude.put(groupDN, cachedGroupDNs); parentGroups.put(groupDN, cachedParentsDNs);
} }
for (String dn : cachedGroupDNs) { for (String dn : cachedParentsDNs) {
recursivelyExpandGroups(groupDNs, schema, ctx, dn); recursivelyExpandGroups(groupDNs, schema, ctx, dn);
} }
} }

4
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
View File

@@ -33,7 +33,7 @@ public class LdapModule extends CacheModule {
static final String USERNAME_CACHE = "ldap_usernames"; static final String USERNAME_CACHE = "ldap_usernames";
static final String GROUP_CACHE = "ldap_groups"; static final String GROUP_CACHE = "ldap_groups";
static final String GROUP_EXIST_CACHE = "ldap_group_existence"; static final String GROUP_EXIST_CACHE = "ldap_group_existence";
static final String GROUPS_BYINCLUDE_CACHE = "ldap_groups_byinclude"; static final String PARENT_GROUPS_CACHE = "ldap_groups_byinclude";
@Override @Override
@@ -55,7 +55,7 @@ public class LdapModule extends CacheModule {
.expireAfterWrite(1, HOURS) .expireAfterWrite(1, HOURS)
.loader(LdapRealm.ExistenceLoader.class); .loader(LdapRealm.ExistenceLoader.class);
cache(GROUPS_BYINCLUDE_CACHE, cache(PARENT_GROUPS_CACHE,
String.class, String.class,
new TypeLiteral<ImmutableSet<String>>() {}) new TypeLiteral<ImmutableSet<String>>() {})
.expireAfterWrite(1, HOURS); .expireAfterWrite(1, HOURS);

4
gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
View File

@@ -128,9 +128,9 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
db.get().accountGroupByIdAud().insert(newIncludedGroupsAudits); db.get().accountGroupByIdAud().insert(newIncludedGroupsAudits);
db.get().accountGroupById().insert(newIncludedGroups.values()); db.get().accountGroupById().insert(newIncludedGroups.values());
for (AccountGroupById agi : newIncludedGroups.values()) { for (AccountGroupById agi : newIncludedGroups.values()) {
groupIncludeCache.evictMemberIn(agi.getIncludeUUID()); groupIncludeCache.evictParentGroupsOf(agi.getIncludeUUID());
} }
groupIncludeCache.evictMembersOf(group.getGroupUUID()); groupIncludeCache.evictSubgroupsOf(group.getGroupUUID());
} }
return result; return result;

4
gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java
View File

@@ -91,9 +91,9 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
writeAudits(toRemove); writeAudits(toRemove);
db.get().accountGroupById().delete(toRemove); db.get().accountGroupById().delete(toRemove);
for (final AccountGroupById g : toRemove) { for (final AccountGroupById g : toRemove) {
groupIncludeCache.evictMemberIn(g.getIncludeUUID()); groupIncludeCache.evictParentGroupsOf(g.getIncludeUUID());
} }
groupIncludeCache.evictMembersOf(internalGroup.getGroupUUID()); groupIncludeCache.evictSubgroupsOf(internalGroup.getGroupUUID());
} }
return Response.none(); return Response.none();

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/ProjectWatch.java
View File

@@ -166,7 +166,7 @@ public class ProjectWatch {
for (AccountGroupMember m : db.accountGroupMembers().byGroup(ig.getId())) { for (AccountGroupMember m : db.accountGroupMembers().byGroup(ig.getId())) {
matching.accounts.add(m.getAccountId()); matching.accounts.add(m.getAccountId());
} }
for (AccountGroup.UUID m : args.groupIncludes.membersOf(uuid)) { for (AccountGroup.UUID m : args.groupIncludes.subgroupsOf(uuid)) {
if (seen.add(m)) { if (seen.add(m)) {
q.add(m); q.add(m);
} }