From 36816b9fc903713cdfc36b66f5aed1c46c4a1def Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 18 Jan 2013 10:43:56 -0800 Subject: [PATCH] Allow group backends to guess on relevant UUIDs Expose all cheaply known group UUIDs from the ProjectCache, enumerating groups used by access controls. This allows a backend that has a large number of groups to filter its getKnownGroups() output to only groups that may be relevant for this Gerrit server. The best use case to consider is an LDAP server at a large organization. A typical user may belong to 50 LDAP groups, but only 3 are relevant to this Gerrit server. Taking the intersection of the two groups limits the output Gerrit displays to users, or uses when considering same group visibility. Change-Id: I0fb5053f24af52014fd860e795d08cd38fc25f1c --- .../com/google/gerrit/server/auth/ldap/Helper.java | 2 +- .../gerrit/server/auth/ldap/LdapGroupBackend.java | 14 +++++++++++++- .../google/gerrit/server/git/ProjectConfig.java | 5 +++++ .../google/gerrit/server/project/ProjectCache.java | 10 ++++++++++ .../gerrit/server/project/ProjectCacheImpl.java | 14 ++++++++++++++ .../gerrit/server/project/RefControlTest.java | 5 +++++ 6 files changed, 48 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java index 23f780be08..0151dde40f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java @@ -241,7 +241,7 @@ import javax.security.auth.login.LoginException; if (actual.isEmpty()) { return Collections.emptySet(); } else { - return Collections.unmodifiableSet(actual); + return ImmutableSet.copyOf(actual); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java index 91445f3297..7b1353b7b5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java @@ -32,6 +32,7 @@ import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupMembership; import com.google.gerrit.server.account.ListGroupMembership; import com.google.gerrit.server.auth.ldap.Helper.LdapSchema; +import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.name.Named; @@ -65,6 +66,7 @@ public class LdapGroupBackend implements GroupBackend { private final Helper helper; private final LoadingCache> membershipCache; private final LoadingCache existsCache; + private final ProjectCache projectCache; private final Provider userProvider; @Inject @@ -72,9 +74,11 @@ public class LdapGroupBackend implements GroupBackend { Helper helper, @Named(GROUP_CACHE) LoadingCache> membershipCache, @Named(GROUP_EXIST_CACHE) LoadingCache existsCache, + ProjectCache projectCache, Provider userProvider) { this.helper = helper; this.membershipCache = membershipCache; + this.projectCache = projectCache; this.existsCache = existsCache; this.userProvider = userProvider; } @@ -174,7 +178,15 @@ public class LdapGroupBackend implements GroupBackend { } try { - return new ListGroupMembership(membershipCache.get(id)); + final Set groups = membershipCache.get(id); + return new ListGroupMembership(groups) { + @Override + public Set getKnownGroups() { + Set g = Sets.newHashSet(groups); + g.retainAll(projectCache.guessRelevantGroupUUIDs()); + return g; + } + }; } catch (ExecutionException e) { log.warn(String.format("Cannot lookup membershipsOf %s in LDAP", id), e); return GroupMembership.EMPTY; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java index 1c1c403f29..1a6a5f263b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java @@ -228,6 +228,11 @@ public class ProjectConfig extends VersionedMetaData { return groupsByUUID.get(uuid); } + /** @return set of all groups used by this configuration. */ + public Set getAllGroupUUIDs() { + return Collections.unmodifiableSet(groupsByUUID.keySet()); + } + /** * @return the project's rules.pl ObjectId, if present in the branch. * Null if it doesn't exist. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java index 7a11131168..697b838bee 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java @@ -14,8 +14,11 @@ package com.google.gerrit.server.project; +import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Project; +import java.util.Set; + /** Cache of project information, including access rights. */ public interface ProjectCache { /** @return the parent state for all projects on this server. */ @@ -41,6 +44,13 @@ public interface ProjectCache { /** @return sorted iteration of projects. */ public abstract Iterable all(); + /** + * @return estimated set of relevant groups extracted from hot project access + * rules. If the cache is cold or too small for the entire project set + * of the server, this set may be incomplete. + */ + public abstract Set guessRelevantGroupUUIDs(); + /** * Filter the set of registered project names by common prefix. * diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java index a7fdb4e8b6..1b28a04722 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.project; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.Sets; +import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.config.AllProjectsName; @@ -36,6 +37,7 @@ import org.slf4j.LoggerFactory; import java.util.Collections; import java.util.Iterator; import java.util.NoSuchElementException; +import java.util.Set; import java.util.SortedSet; import java.util.concurrent.ExecutionException; import java.util.concurrent.locks.Lock; @@ -170,6 +172,18 @@ public class ProjectCacheImpl implements ProjectCache { } } + @Override + public Set guessRelevantGroupUUIDs() { + Set groups = Sets.newHashSet(); + for (Project.NameKey n : all()) { + ProjectState p = byName.getIfPresent(n); + if (p != null) { + groups.addAll(p.getConfig().getAllGroupUUIDs()); + } + } + return groups; + } + @Override public Iterable byName(final String pfx) { final Iterable src; diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index 0deeebdc80..ab5ad59a2c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -306,6 +306,11 @@ public class RefControlTest extends TestCase { @Override public void onCreateProject(Project.NameKey newProjectName) { } + + @Override + public Set guessRelevantGroupUUIDs() { + return Collections.emptySet(); + } }; Injector injector = Guice.createInjector(new FactoryModule() {