Merge "LDAP-cache to minimize nbr of queries when unnesting groups." into stable-2.5
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 5e2cc18..1959995 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -530,6 +530,10 @@
low maxAge setting, to ensure LDAP modifications are picked up in
a timely fashion.
+cache `"ldap_groups_byinclude"`::
++
+Caches the hierarchical structure of LDAP groups.
+
cache `"ldap_usernames"`::
+
Caches a mapping of LDAP username to Gerrit account identity. The
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 2265bc2..644f5df 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
@@ -14,6 +14,8 @@
package com.google.gerrit.server.auth.ldap;
+import com.google.common.cache.Cache;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.account.AccountException;
@@ -22,6 +24,7 @@
import com.google.gerrit.util.ssl.BlindSSLSocketFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.google.inject.name.Named;
import org.eclipse.jgit.lib.Config;
@@ -48,6 +51,7 @@
@Singleton class Helper {
static final String LDAP_UUID = "ldap:";
+ private final Cache<String, ImmutableSet<String>> groupsByInclude;
private final Config config;
private final String server;
private final String username;
@@ -58,7 +62,9 @@
private final String readTimeOutMillis;
@Inject
- Helper(@GerritServerConfig final Config config) {
+ Helper(@GerritServerConfig final Config config,
+ @Named(LdapModule.GROUPS_BYINCLUDE_CACHE)
+ Cache<String, ImmutableSet<String>> groupsByInclude) {
this.config = config;
this.server = LdapRealm.required(config, "server");
this.username = LdapRealm.optional(config, "username");
@@ -73,6 +79,7 @@
} else {
readTimeOutMillis = null;
}
+ this.groupsByInclude = groupsByInclude;
}
private Properties createContextProperties() {
@@ -207,24 +214,31 @@
private void recursivelyExpandGroups(final Set<String> groupDNs,
final LdapSchema schema, final DirContext ctx, final String groupDN) {
if (groupDNs.add(groupDN) && schema.accountMemberField != null) {
- // Recursively identify the groups it is a member of.
- //
- try {
- final Name compositeGroupName = new CompositeName().add(groupDN);
- final Attribute in =
- ctx.getAttributes(compositeGroupName).get(schema.accountMemberField);
- if (in != null) {
- final NamingEnumeration<?> groups = in.getAll();
- try {
- while (groups.hasMore()) {
- final String nextDN = (String) groups.next();
- recursivelyExpandGroups(groupDNs, schema, ctx, nextDN);
+ ImmutableSet<String> cachedGroupDNs = groupsByInclude.getIfPresent(groupDN);
+ if (cachedGroupDNs == null) {
+ // Recursively identify the groups it is a member of.
+ ImmutableSet.Builder<String> dns = ImmutableSet.builder();
+ try {
+ final Name compositeGroupName = new CompositeName().add(groupDN);
+ final Attribute in =
+ ctx.getAttributes(compositeGroupName).get(schema.accountMemberField);
+ if (in != null) {
+ final NamingEnumeration<?> groups = in.getAll();
+ try {
+ while (groups.hasMore()) {
+ dns.add((String) groups.next());
+ }
+ } catch (PartialResultException e) {
}
- } catch (PartialResultException e) {
}
+ } catch (NamingException e) {
+ LdapRealm.log.warn("Could not find group " + groupDN, e);
}
- } catch (NamingException e) {
- LdapRealm.log.warn("Could not find group " + groupDN, e);
+ cachedGroupDNs = dns.build();
+ groupsByInclude.put(groupDN, cachedGroupDNs);
+ }
+ for (String dn : cachedGroupDNs) {
+ recursivelyExpandGroups(groupDNs, schema, ctx, dn);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
index 29533b9..f1b15f9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
@@ -17,6 +17,7 @@
import static java.util.concurrent.TimeUnit.HOURS;
import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -32,6 +33,7 @@
static final String USERNAME_CACHE = "ldap_usernames";
static final String GROUP_CACHE = "ldap_groups";
static final String GROUP_EXIST_CACHE = "ldap_group_existence";
+ static final String GROUPS_BYINCLUDE_CACHE = "ldap_groups_byinclude";
@Override
@@ -53,6 +55,11 @@
.expireAfterWrite(1, HOURS)
.loader(LdapRealm.ExistenceLoader.class);
+ cache(GROUPS_BYINCLUDE_CACHE,
+ String.class,
+ new TypeLiteral<ImmutableSet<String>>() {})
+ .expireAfterWrite(1, HOURS);
+
bind(Realm.class).to(LdapRealm.class).in(Scopes.SINGLETON);
bind(Helper.class);
