Remove access of groups related db tables from GroupIncludeCacheImpl

Change-Id: I5672d0a82e38e89acd950bf46ab006c5dfb42cbf

gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java

@@ -14,13 +14,15 @@
 
 package com.google.gerrit.server.account;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupById;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.group.Groups;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
@@ -30,9 +32,7 @@ import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.Objects;
-import java.util.List;
-import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -137,25 +137,18 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
   static class SubgroupsLoader
       extends CacheLoader<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> {
     private final SchemaFactory<ReviewDb> schema;
+    private final Groups groups;
 
     @Inject
-    SubgroupsLoader(SchemaFactory<ReviewDb> sf) {
+    SubgroupsLoader(SchemaFactory<ReviewDb> sf, Groups groups) {
       schema = sf;
+      this.groups = groups;
     }
 
     @Override
     public ImmutableList<AccountGroup.UUID> load(AccountGroup.UUID key) throws OrmException {
       try (ReviewDb db = schema.open()) {
-        List<AccountGroup> group = db.accountGroups().byUUID(key).toList();
+        return groups.getIncludes(db, key).collect(toImmutableList());
-        if (group.size() != 1) {
-          return ImmutableList.of();
-        }
-
-        Set<AccountGroup.UUID> ids = new HashSet<>();
-        for (AccountGroupById agi : db.accountGroupById().byGroup(group.get(0).getId())) {
-          ids.add(agi.getIncludeUUID());
-        }
-        return ImmutableList.copyOf(ids);
       }
     }
   }
@@ -163,47 +156,43 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
   static class ParentGroupsLoader
       extends CacheLoader<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> {
     private final SchemaFactory<ReviewDb> schema;
+    private final GroupCache groupCache;
+    private final Groups groups;
 
     @Inject
-    ParentGroupsLoader(SchemaFactory<ReviewDb> sf) {
+    ParentGroupsLoader(SchemaFactory<ReviewDb> sf, GroupCache groupCache, Groups groups) {
       schema = sf;
+      this.groupCache = groupCache;
+      this.groups = groups;
     }
 
     @Override
     public ImmutableList<AccountGroup.UUID> load(AccountGroup.UUID key) throws OrmException {
       try (ReviewDb db = schema.open()) {
-        Set<AccountGroup.Id> ids = new HashSet<>();
+        return groups
-        for (AccountGroupById agi : db.accountGroupById().byIncludeUUID(key)) {
+            .getParentGroups(db, key)
-          ids.add(agi.getGroupId());
+            .map(groupCache::get)
-        }
+            .map(AccountGroup::getGroupUUID)
-
+            .filter(Objects::nonNull)
-        Set<AccountGroup.UUID> groupArray = new HashSet<>();
+            .collect(toImmutableList());
-        for (AccountGroup g : db.accountGroups().get(ids)) {
-          groupArray.add(g.getGroupUUID());
-        }
-        return ImmutableList.copyOf(groupArray);
       }
     }
   }
 
   static class AllExternalLoader extends CacheLoader<String, ImmutableList<AccountGroup.UUID>> {
     private final SchemaFactory<ReviewDb> schema;
+    private final Groups groups;
 
     @Inject
-    AllExternalLoader(SchemaFactory<ReviewDb> sf) {
+    AllExternalLoader(SchemaFactory<ReviewDb> sf, Groups groups) {
       schema = sf;
+      this.groups = groups;
     }
 
     @Override
     public ImmutableList<AccountGroup.UUID> load(String key) throws Exception {
       try (ReviewDb db = schema.open()) {
-        Set<AccountGroup.UUID> ids = new HashSet<>();
+        return groups.getExternalGroups(db).collect(toImmutableList());
-        for (AccountGroupById agi : db.accountGroupById().all()) {
-          if (!AccountGroup.isInternalGroup(agi.getIncludeUUID())) {
-            ids.add(agi.getIncludeUUID());
-          }
-        }
-        return ImmutableList.copyOf(ids);
       }
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/group/Groups.java

@@ -80,10 +80,21 @@ public class Groups {
     return Streams.stream(accountGroupMembers).map(AccountGroupMember::getAccountId);
   }
 
+  public Stream<AccountGroup.UUID> getIncludes(ReviewDb db, AccountGroup.UUID groupUuid)
+      throws OrmException {
+    Optional<AccountGroup> foundGroup = get(db, groupUuid);
+    if (!foundGroup.isPresent()) {
+      return Stream.empty();
+    }
+
+    AccountGroup group = foundGroup.get();
+    return getIncludes(db, group.getId());
+  }
+
   public Stream<AccountGroup.UUID> getIncludes(ReviewDb db, AccountGroup.Id groupId)
       throws OrmException {
     ResultSet<AccountGroupById> accountGroupByIds = db.accountGroupById().byGroup(groupId);
-    return Streams.stream(accountGroupByIds).map(AccountGroupById::getIncludeUUID);
+    return Streams.stream(accountGroupByIds).map(AccountGroupById::getIncludeUUID).distinct();
   }
 
   public Stream<AccountGroup.Id> getGroupsWithMember(ReviewDb db, Account.Id accountId)
@@ -92,4 +103,18 @@ public class Groups {
         db.accountGroupMembers().byAccount(accountId);
     return Streams.stream(accountGroupMembers).map(AccountGroupMember::getAccountGroupId);
   }
+
+  public Stream<AccountGroup.Id> getParentGroups(ReviewDb db, AccountGroup.UUID includedGroupUuid)
+      throws OrmException {
+    ResultSet<AccountGroupById> accountGroupByIds =
+        db.accountGroupById().byIncludeUUID(includedGroupUuid);
+    return Streams.stream(accountGroupByIds).map(AccountGroupById::getGroupId);
+  }
+
+  public Stream<AccountGroup.UUID> getExternalGroups(ReviewDb db) throws OrmException {
+    return Streams.stream(db.accountGroupById().all())
+        .map(AccountGroupById::getIncludeUUID)
+        .distinct()
+        .filter(groupUuid -> !AccountGroup.isInternalGroup(groupUuid));
+  }
 }