Add cache to ldap group provider

plugin/trino-ldap-group-provider/pom.xml

@@ -40,6 +40,16 @@
             <artifactId>log</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>io.airlift</groupId>
+            <artifactId>units</artifactId>
+        </dependency>
+
+        <dependency>
+            <groupId>io.trino</groupId>
+            <artifactId>trino-cache</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>io.trino</groupId>
             <artifactId>trino-plugin-toolkit</artifactId>

plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapFilteringGroupProvider.java

@@ -13,9 +13,12 @@
  */
 package io.trino.plugin.ldapgroup;
 
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.Inject;
 import io.airlift.log.Logger;
+import io.trino.cache.EvictableCacheBuilder;
 import io.trino.plugin.base.ldap.LdapClient;
 import io.trino.plugin.base.ldap.LdapQuery;
 import io.trino.spi.security.GroupProvider;
@@ -24,6 +27,7 @@
 import javax.naming.directory.Attribute;
 import javax.naming.directory.SearchResult;
 
+import java.time.Duration;
 import java.util.Optional;
 import java.util.Set;
 
@@ -42,6 +46,7 @@ public class LdapFilteringGroupProvider
     private final String groupBaseDN;
     private final String groupsNameAttribute;
     private final String combinedGroupSearchFilter;
+    private final LoadingCache<String, Set<String>> groupsCache;
 
     @Inject
     public LdapFilteringGroupProvider(
@@ -61,18 +66,35 @@ public LdapFilteringGroupProvider(
         combinedGroupSearchFilter = filteringConfig.getLdapGroupsSearchFilter()
                 .map(filter -> String.format("(&(%s)(%s={0}))", filter, groupsSearchMemberAttribute))
                 .orElse(String.format("(%s={0})", groupsSearchMemberAttribute));
+
+        groupsCache = EvictableCacheBuilder.newBuilder()
+                .expireAfterWrite(Duration.ofMillis(config.getGroupsCacheTtl().toMillis()))
+                .maximumSize(1000)
+                .shareResultsAndFailuresEvenIfDisabled()
+                .build(CacheLoader.from(this::loadGroups));
     }
 
     /**
      * Perform an LDAP search for groups, fetching only the names, and returning the name of each group.
      * Filters groups by user membership AND filter expression {@link LdapFilteringGroupProviderConfig#getLdapGroupsSearchFilter()}.
      * If {@link LdapGroupProviderConfig#getLdapGroupsNameAttribute()} is missing from group document, fallback on full name.
-     * Swallows LDAP exceptions.
+     * Results are cached per user based on the configured cache TTL.
      *
      * @return Names of groups that the user is a member of
      */
     @Override
     public Set<String> getGroups(String user)
+    {
+        return groupsCache.getUnchecked(user);
+    }
+
+    /**
+     * Loads groups from LDAP for the specified user.
+     * Swallows LDAP exceptions.
+     *
+     * @return Names of groups that the user is a member of
+     */
+    private Set<String> loadGroups(String user)
     {
         Optional<String> userDistinguishedName;
         try {

plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapGroupProviderConfig.java

@@ -16,8 +16,12 @@
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
 import io.airlift.configuration.ConfigSecuritySensitive;
+import io.airlift.units.Duration;
+import io.airlift.units.MinDuration;
 import jakarta.validation.constraints.NotNull;
 
+import java.util.concurrent.TimeUnit;
+
 public class LdapGroupProviderConfig
 {
     private String ldapAdminUser;
@@ -26,6 +30,7 @@ public class LdapGroupProviderConfig
     private String ldapUserSearchFilter = "(uid={0})";
     private String ldapGroupsNameAttribute = "cn";
     private boolean ldapUseGroupFilter;
+    private Duration groupsCacheTtl = new Duration(0, TimeUnit.SECONDS);
 
     @NotNull
     public String getLdapAdminUser()
@@ -110,4 +115,19 @@ public LdapGroupProviderConfig setLdapUseGroupFilter(boolean ldapUseGroupFilter)
         this.ldapUseGroupFilter = ldapUseGroupFilter;
         return this;
     }
+
+    @NotNull
+    @MinDuration("0ms")
+    public Duration getGroupsCacheTtl()
+    {
+        return groupsCacheTtl;
+    }
+
+    @Config("ldap.group-cache-ttl")
+    @ConfigDescription("Duration to cache groups retrieved from LDAP. Set to 0 to disable caching, default value is 0s (disabled)")
+    public LdapGroupProviderConfig setGroupsCacheTtl(Duration groupsCacheTtl)
+    {
+        this.groupsCacheTtl = groupsCacheTtl;
+        return this;
+    }
 }

plugin/trino-ldap-group-provider/src/main/java/io/trino/plugin/ldapgroup/LdapSingleQueryGroupProvider.java

@@ -13,10 +13,13 @@
  */
 package io.trino.plugin.ldapgroup;
 
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Streams;
 import com.google.inject.Inject;
 import io.airlift.log.Logger;
+import io.trino.cache.EvictableCacheBuilder;
 import io.trino.plugin.base.ldap.LdapClient;
 import io.trino.plugin.base.ldap.LdapQuery;
 import io.trino.spi.security.GroupProvider;
@@ -27,6 +30,7 @@
 import javax.naming.ldap.LdapName;
 import javax.naming.ldap.Rdn;
 
+import java.time.Duration;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -46,6 +50,7 @@ public class LdapSingleQueryGroupProvider
     private final String groupsNameAttribute;
     private final String userSearchFilter;
     private final String userMemberOfAttribute;
+    private final LoadingCache<String, Set<String>> groupsCache;
 
     @Inject
     public LdapSingleQueryGroupProvider(LdapClient ldapClient, LdapGroupProviderConfig config, LdapSingleQueryGroupProviderConfig singleQueryGroupProviderConfig)
@@ -57,6 +62,12 @@ public LdapSingleQueryGroupProvider(LdapClient ldapClient, LdapGroupProviderConf
         this.groupsNameAttribute = config.getLdapGroupsNameAttribute();
         this.userSearchFilter = config.getLdapUserSearchFilter();
         this.userMemberOfAttribute = singleQueryGroupProviderConfig.getLdapUserMemberOfAttribute();
+
+        groupsCache = EvictableCacheBuilder.newBuilder()
+                .expireAfterWrite(Duration.ofMillis(config.getGroupsCacheTtl().toMillis()))
+                .maximumSize(1000)
+                .shareResultsAndFailuresEvenIfDisabled()
+                .build(CacheLoader.from(this::loadGroups));
     }
 
     /**
@@ -65,14 +76,26 @@ public LdapSingleQueryGroupProvider(LdapClient ldapClient, LdapGroupProviderConf
      * If multiple users match the search, the first one is used.
      * If the requested group attribute is missing from the LDAP document, it will be ignored
      * (i.e., the function will not error).
-     * Swallows all LDAP exceptions.
+     * Results are cached per user based on the configured cache TTL.
      *
      * @param user Username of user, used with filter expression {@link LdapGroupProviderConfig#getLdapUserSearchFilter()}.
      * @return The relative domain name of all the values of attribute
-     *         {@link LdapSingleQueryGroupProviderConfig#getLdapUserMemberOfAttribute()}.
+     * {@link LdapSingleQueryGroupProviderConfig#getLdapUserMemberOfAttribute()}.
      */
     @Override
     public Set<String> getGroups(String user)
+    {
+        return groupsCache.getUnchecked(user);
+    }
+
+    /**
+     * Loads groups from LDAP for the specified user.
+     * Swallows all LDAP exceptions.
+     *
+     * @return The relative domain name of all the values of attribute
+     *         {@link LdapSingleQueryGroupProviderConfig#getLdapUserMemberOfAttribute()}.
+     */
+    private Set<String> loadGroups(String user)
     {
         try {
             return ldapClient.executeLdapQuery(ldapAdminUser, ldapAdminPassword,

plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderConfig.java

@@ -13,6 +13,7 @@
  */
 package io.trino.plugin.ldapgroup;
 
+import io.airlift.units.Duration;
 import org.junit.jupiter.api.Test;
 
 import java.util.Map;
@@ -32,7 +33,8 @@ public void testDefaults()
                 .setLdapUserBaseDN(null)
                 .setLdapUserSearchFilter("(uid={0})")
                 .setLdapGroupsNameAttribute("cn")
-                .setLdapUseGroupFilter(false));
+                .setLdapUseGroupFilter(false)
+                .setGroupsCacheTtl(Duration.valueOf("0m")));
     }
 
     @Test
@@ -44,15 +46,17 @@ public void testExplicitPropertyMappings()
                 "ldap.user-base-dn", "dc=trino,dc=io",
                 "ldap.user-search-filter", "(accountName={0})",
                 "ldap.group-name-attribute", "groupName",
-                "ldap.use-group-filter", "true");
+                "ldap.use-group-filter", "true",
+                "ldap.group-cache-ttl", "10m");
 
         LdapGroupProviderConfig expected = new LdapGroupProviderConfig()
                 .setLdapAdminUser("cn=admin,dc=trino,dc=io")
                 .setLdapAdminPassword("admin")
                 .setLdapUserBaseDN("dc=trino,dc=io")
                 .setLdapUserSearchFilter("(accountName={0})")
                 .setLdapGroupsNameAttribute("groupName")
-                .setLdapUseGroupFilter(true);
+                .setLdapUseGroupFilter(true)
+                .setGroupsCacheTtl(Duration.valueOf("10m"));
 
         assertFullMapping(properties, expected);
     }

plugin/trino-ldap-group-provider/src/test/java/io/trino/plugin/ldapgroup/TestLdapGroupProviderIntegration.java

@@ -36,6 +36,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
 import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT;
@@ -267,6 +268,60 @@ private void assertGetGroupsConcurrently(ConfigBuilder configBuilder)
         latch.await();
     }
 
+    @Test
+    public void testGroupsCacheWithTtl()
+    {
+        for (ConfigBuilder configBuilder : CONFIG_BUILDERS) {
+            // Test with cache enabled (10 minute TTL)
+            Map<String, String> config = configBuilder.apply(new HashMap<>(baseConfig));
+            config.put("ldap.group-cache-ttl", "10m");
+            GroupProvider groupsProvider = factory.create(config);
+
+            // First call - should query LDAP
+            long start = System.nanoTime();
+            Set<String> groups1 = groupsProvider.getGroups("alicea");
+            long firstCallTime = System.nanoTime() - start;
+            assertThat(groups1).containsAll(ImmutableSet.of("clients", "qualityAssurance", "developers"));
+
+            // Second call - should be cached (much faster)
+            start = System.nanoTime();
+            Set<String> groups2 = groupsProvider.getGroups("alicea");
+            long cachedCallTime = System.nanoTime() - start;
+            assertThat(groups2).isEqualTo(groups1);
+
+            // Cached call should be significantly faster (at least 10x faster as a rough heuristic)
+            // Note: This is a heuristic test - actual performance may vary
+            assertThat(cachedCallTime).isLessThan(firstCallTime / 10);
+        }
+    }
+
+    @Test
+    public void testGroupsCacheExpiration()
+            throws InterruptedException
+    {
+        for (ConfigBuilder configBuilder : CONFIG_BUILDERS) {
+            // Test with very short cache TTL (100ms)
+            Map<String, String> config = configBuilder.apply(new HashMap<>(baseConfig));
+            config.put("ldap.group-cache-ttl", "100ms");
+            GroupProvider groupsProvider = factory.create(config);
+
+            // First call
+            Set<String> groups1 = groupsProvider.getGroups("alicea");
+            assertThat(groups1).containsAll(ImmutableSet.of("clients", "qualityAssurance", "developers"));
+
+            // Immediate second call should be cached
+            Set<String> groups2 = groupsProvider.getGroups("alicea");
+            assertThat(groups2).isEqualTo(groups1);
+
+            // Wait for cache to expire
+            MILLISECONDS.sleep(150);
+
+            // Call after expiration should query LDAP again (result should still be correct)
+            Set<String> groups3 = groupsProvider.getGroups("alicea");
+            assertThat(groups3).containsAll(ImmutableSet.of("clients", "qualityAssurance", "developers"));
+        }
+    }
+
     @FunctionalInterface
     public interface ConfigBuilder
     {