LDAP-cache to minimize nbr of queries when unnesting groups.

A new cache named "ldap_groups_byinclude" is introduced
to help lessen the number of queries needed to resolve
the nested LDAP-groups.

Depending on the LDAP tree structure, the unnesting of
the LDAP-groups will in many cases generate most of the
queries against the LDAP-server. Even though the users
have lots of security groups in common, no effort was
previously made to reuse the looked up LDAP-group
hierarchies.

This change introduce a cache which maps the group DNs
inheritence. The expiration time is set to 1h, which
allows any LDAP changes to be reflected in gerrit within
a reasonable time. Though for most companies the
hierarchical group structure is mostly static.

Change-Id: I913f0e8fffb9116d24ebeb12d004dfa2495a1080

3 files changed, 41 insertions, 16 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 5e2cc1826b..1959995f7b 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -530,6 +530,10 @@ configured on this server.  This cache should be configured with a
 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 2265bc29ec..644f5dff9e 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.server.config.GerritServerConfig;
 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 @@ import javax.net.ssl.SSLSocketFactory;
 @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 @@ import javax.net.ssl.SSLSocketFactory;
   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 @@ import javax.net.ssl.SSLSocketFactory;
     } else {
       readTimeOutMillis = null;
     }
+    this.groupsByInclude = groupsByInclude;
   }
 
   private Properties createContextProperties() {
@@ -207,24 +214,31 @@ import javax.net.ssl.SSLSocketFactory;
   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 29533b9a51..f1b15f9bbc 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 @@ package com.google.gerrit.server.auth.ldap;
 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 @@ public class LdapModule extends CacheModule {
   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 @@ public class LdapModule extends CacheModule {
       .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);