opendev/gerrit
Code Issues Proposed changes

Merge "Improve method and variable names related to group inclusion caches"

Browse Source
This commit is contained in:
David Pursehouse
2014-11-27 05:07:51 +00:00
committed by Gerrit Code Review
parent 5df54ae192 6a2a970b5e
commit 5250b8d352
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());
@@ -152,7 +152,7 @@ public class PerformCreateGroup {
auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), includeList); auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), includeList);
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

@@ -96,7 +96,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;
@@ -110,8 +110,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");
@@ -128,7 +128,7 @@ import javax.security.auth.login.LoginException;
} else { } else {
readTimeOutMillis = null; readTimeOutMillis = null;
} }
this.groupsByInclude = groupsByInclude; this.parentGroups = parentGroups;
this.connectionPoolConfig = getPoolProperties(config); this.connectionPoolConfig = getPoolProperties(config);
} }
@@ -303,8 +303,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 {
@@ -323,10 +323,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

@@ -126,9 +126,9 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
auditService.dispatchAddGroupsToGroup(me, newIncludedGroups.values()); auditService.dispatchAddGroupsToGroup(me, newIncludedGroups.values());
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);
} }