Remove access of groups related db tables from DeleteIncludedGroups

Change-Id: I8ef63474fc2d04a17703a5e7612cef74e00e84fd

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

@@ -15,50 +15,37 @@
 package com.google.gerrit.server.group;
 
 import com.google.common.collect.ImmutableList;
-import com.google.gerrit.audit.AuditService;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.reviewdb.client.Account;
 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.CurrentUser;
 import com.google.gerrit.server.account.GroupControl;
-import com.google.gerrit.server.account.GroupIncludeCache;
 import com.google.gerrit.server.group.AddIncludedGroups.Input;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.HashSet;
+import java.util.Set;
 
 @Singleton
 public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input> {
   private final GroupsCollection groupsCollection;
-  private final GroupIncludeCache groupIncludeCache;
   private final Provider<ReviewDb> db;
-  private final Provider<CurrentUser> self;
-  private final AuditService auditService;
+  private final Provider<GroupsUpdate> groupsUpdateProvider;
 
   @Inject
   DeleteIncludedGroups(
       GroupsCollection groupsCollection,
-      GroupIncludeCache groupIncludeCache,
       Provider<ReviewDb> db,
-      Provider<CurrentUser> self,
-      AuditService auditService) {
+      @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
     this.groupsCollection = groupsCollection;
-    this.groupIncludeCache = groupIncludeCache;
     this.db = db;
-    this.self = self;
-    this.auditService = auditService;
+    this.groupsUpdateProvider = groupsUpdateProvider;
   }
 
   @Override
@@ -76,43 +63,19 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
           String.format("Cannot delete groups from group %s", internalGroup.getName()));
     }
 
-    final Map<AccountGroup.UUID, AccountGroupById> includedGroups =
-        getIncludedGroups(internalGroup.getId());
-    final List<AccountGroupById> toRemove = new ArrayList<>();
+    Set<AccountGroup.UUID> internalGroupsToRemove = new HashSet<>();
     for (String includedGroup : input.groups) {
       GroupDescription.Basic d = groupsCollection.parse(includedGroup);
-      AccountGroupById g = includedGroups.remove(d.getGroupUUID());
-      if (g != null) {
-        toRemove.add(g);
-      }
+      internalGroupsToRemove.add(d.getGroupUUID());
     }
 
-    if (!toRemove.isEmpty()) {
-      writeAudits(toRemove);
-      db.get().accountGroupById().delete(toRemove);
-      for (AccountGroupById g : toRemove) {
-        groupIncludeCache.evictParentGroupsOf(g.getIncludeUUID());
-      }
-      groupIncludeCache.evictSubgroupsOf(internalGroup.getGroupUUID());
-    }
+    groupsUpdateProvider
+        .get()
+        .deleteIncludedGroups(db.get(), internalGroup.getGroupUUID(), internalGroupsToRemove);
 
     return Response.none();
   }
 
-  private Map<AccountGroup.UUID, AccountGroupById> getIncludedGroups(AccountGroup.Id groupId)
-      throws OrmException {
-    final Map<AccountGroup.UUID, AccountGroupById> groups = new HashMap<>();
-    for (AccountGroupById g : db.get().accountGroupById().byGroup(groupId)) {
-      groups.put(g.getIncludeUUID(), g);
-    }
-    return groups;
-  }
-
-  private void writeAudits(List<AccountGroupById> toRemoved) {
-    final Account.Id me = self.get().getAccountId();
-    auditService.dispatchDeleteGroupsFromGroup(me, toRemoved);
-  }
-
   @Singleton
   static class DeleteIncludedGroup
       implements RestModifyView<IncludedGroupResource, DeleteIncludedGroup.Input> {

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

@@ -322,4 +322,39 @@ public class GroupsUpdate {
     }
     groupIncludeCache.evictSubgroupsOf(parentGroupUuid);
   }
+
+  public void deleteIncludedGroups(
+      ReviewDb db, AccountGroup.UUID parentGroupUuid, Set<AccountGroup.UUID> includedGroupUuids)
+      throws OrmException {
+    Optional<AccountGroup> foundParentGroup = groups.get(db, parentGroupUuid);
+    if (!foundParentGroup.isPresent()) {
+      // TODO(aliceks): Throw an exception?
+      return;
+    }
+
+    AccountGroup parentGroup = foundParentGroup.get();
+    AccountGroup.Id parentGroupId = parentGroup.getId();
+    Set<AccountGroupById> includedGroupsToRemove = new HashSet<>();
+    for (AccountGroup.UUID includedGroupUuid : includedGroupUuids) {
+      boolean isIncluded = groups.isIncluded(db, parentGroupId, includedGroupUuid);
+      if (isIncluded) {
+        AccountGroupById.Key key = new AccountGroupById.Key(parentGroupId, includedGroupUuid);
+        includedGroupsToRemove.add(new AccountGroupById(key));
+      }
+    }
+
+    if (includedGroupsToRemove.isEmpty()) {
+      return;
+    }
+
+    if (currentUser != null) {
+      auditService.dispatchDeleteGroupsFromGroup(
+          currentUser.getAccountId(), includedGroupsToRemove);
+    }
+    db.accountGroupById().delete(includedGroupsToRemove);
+    for (AccountGroupById groupToRemove : includedGroupsToRemove) {
+      groupIncludeCache.evictParentGroupsOf(groupToRemove.getIncludeUUID());
+    }
+    groupIncludeCache.evictSubgroupsOf(parentGroupUuid);
+  }
 }