Merge "Refactor GroupMembersFactory"

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java

@@ -30,8 +30,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.account.GroupControl;
-import com.google.gerrit.server.account.GroupMembersFactory;
+import com.google.gerrit.server.account.GroupMembers;
-import com.google.gerrit.server.account.GroupMembersFactory.Factory;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.patch.AddReviewer;
 import com.google.gerrit.server.project.NoSuchProjectException;
@@ -60,7 +59,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
   private final ProjectCache projectCache;
   private final AccountCache accountCache;
   private final GroupControl.Factory groupControlFactory;
-  private final Factory groupMembersFactory;
+  private final GroupMembers.Factory groupMembersFactory;
   private final IdentifiedUser.GenericFactory userFactory;
   private final Provider<CurrentUser> currentUser;
   private final SuggestAccountsEnum suggestAccounts;
@@ -73,7 +72,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
       final ProjectControl.Factory projectControlFactory,
       final ProjectCache projectCache, final AccountCache accountCache,
       final GroupControl.Factory groupControlFactory,
-      final GroupMembersFactory.Factory groupMembersFactory,
+      final GroupMembers.Factory groupMembersFactory,
       final IdentifiedUser.GenericFactory userFactory,
       final Provider<CurrentUser> currentUser,
       @GerritServerConfig final Config cfg, final GroupCache groupCache) {
@@ -281,7 +280,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
 
     try {
       final Set<Account> members =
-          groupMembersFactory.create(project, group.getUUID()).call();
+          groupMembersFactory.create().listAccounts(group.getUUID(), project);
 
       if (members.isEmpty()) {
         return false;

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

@@ -26,65 +26,54 @@ import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gwtorm.client.OrmException;
 import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
 
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
-import java.util.concurrent.Callable;
 
-public class GroupMembersFactory implements Callable<Set<Account>> {
+public class GroupMembers {
   public interface Factory {
-    GroupMembersFactory create(Project.NameKey project,
+    GroupMembers create();
-        AccountGroup.UUID groupUUID);
   }
 
-  private GroupCache groupCache;
+  private final GroupCache groupCache;
   private final GroupDetailFactory.Factory groupDetailFactory;
   private final AccountCache accountCache;
   private final ProjectControl.GenericFactory projectControl;
   private final IdentifiedUser currentUser;
 
-  private final Project.NameKey project;
-  private final AccountGroup.UUID groupUUID;
-
-
   @Inject
-  GroupMembersFactory(final GroupCache groupCache,
+  GroupMembers(final GroupCache groupCache,
       final GroupDetailFactory.Factory groupDetailFactory,
       final AccountCache accountCache,
       final ProjectControl.GenericFactory projectControl,
-      final IdentifiedUser currentUser,
+      final IdentifiedUser currentUser) {
-      @Assisted final Project.NameKey project,
-      @Assisted final AccountGroup.UUID groupUUID) {
     this.groupCache = groupCache;
     this.groupDetailFactory = groupDetailFactory;
     this.accountCache = accountCache;
     this.projectControl = projectControl;
     this.currentUser = currentUser;
-
-    this.project = project;
-    this.groupUUID = groupUUID;
   }
 
-  @Override
+  public Set<Account> listAccounts(final AccountGroup.UUID groupUUID,
-  public Set<Account> call() throws NoSuchGroupException,
+      final Project.NameKey project) throws NoSuchGroupException,
       NoSuchProjectException, OrmException {
-    return listAccounts(groupUUID, new HashSet<AccountGroup.UUID>());
+    return listAccounts(groupUUID, project, new HashSet<AccountGroup.UUID>());
   }
 
   private Set<Account> listAccounts(final AccountGroup.UUID groupUUID,
-      final Set<AccountGroup.UUID> seen) throws NoSuchGroupException,
+      final Project.NameKey project, final Set<AccountGroup.UUID> seen)
-      OrmException, NoSuchProjectException {
+      throws NoSuchGroupException, OrmException, NoSuchProjectException {
     if (AccountGroup.PROJECT_OWNERS.equals(groupUUID)) {
-      return getProjectOwners(seen);
+      return getProjectOwners(project, seen);
     } else {
-      return getGroupMembers(groupCache.get(groupUUID), seen);
+      return getGroupMembers(groupCache.get(groupUUID), project, seen);
     }
   }
 
-  private Set<Account> getProjectOwners(final Set<AccountGroup.UUID> seen)
+  private Set<Account> getProjectOwners(final Project.NameKey project,
-      throws NoSuchProjectException, NoSuchGroupException, OrmException {
+      final Set<AccountGroup.UUID> seen) throws NoSuchProjectException,
+      NoSuchGroupException, OrmException {
     seen.add(AccountGroup.PROJECT_OWNERS);
     if (project == null) {
       return Collections.emptySet();
@@ -97,15 +86,15 @@ public class GroupMembersFactory implements Callable<Set<Account>> {
     final HashSet<Account> projectOwners = new HashSet<Account>();
     for (final AccountGroup.UUID ownerGroup : ownerGroups) {
       if (!seen.contains(ownerGroup)) {
-        projectOwners.addAll(listAccounts(ownerGroup, seen));
+        projectOwners.addAll(listAccounts(ownerGroup, project, seen));
       }
     }
     return projectOwners;
   }
 
   private Set<Account> getGroupMembers(final AccountGroup group,
-      final Set<AccountGroup.UUID> seen) throws NoSuchGroupException,
+      final Project.NameKey project, final Set<AccountGroup.UUID> seen)
-      OrmException, NoSuchProjectException {
+      throws NoSuchGroupException, OrmException, NoSuchProjectException {
     seen.add(group.getGroupUUID());
     final GroupDetail groupDetail =
         groupDetailFactory.create(group.getId()).call();
@@ -121,7 +110,7 @@ public class GroupMembersFactory implements Callable<Set<Account>> {
         final AccountGroup includedGroup =
             groupCache.get(groupInclude.getIncludeId());
         if (!seen.contains(includedGroup.getGroupUUID())) {
-          members.addAll(listAccounts(includedGroup.getGroupUUID(), seen));
+          members.addAll(listAccounts(includedGroup.getGroupUUID(), project, seen));
         }
       }
     }

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java

@@ -23,7 +23,7 @@ import com.google.gerrit.server.RequestCleanup;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.account.GroupDetailFactory;
-import com.google.gerrit.server.account.GroupMembersFactory;
+import com.google.gerrit.server.account.GroupMembers;
 import com.google.gerrit.server.account.PerformCreateGroup;
 import com.google.gerrit.server.account.PerformRenameGroup;
 import com.google.gerrit.server.account.VisibleGroups;
@@ -94,6 +94,6 @@ public class GerritRequestModule extends FactoryModule {
     factory(PerformRenameGroup.Factory.class);
     factory(VisibleGroups.Factory.class);
     factory(GroupDetailFactory.Factory.class);
-    factory(GroupMembersFactory.Factory.class);
+    factory(GroupMembers.Factory.class);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/patch/AddReviewer.java

@@ -28,7 +28,7 @@ import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.account.GroupCache;
-import com.google.gerrit.server.account.GroupMembersFactory;
+import com.google.gerrit.server.account.GroupMembers;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.mail.AddReviewerSender;
 import com.google.gerrit.server.project.ChangeControl;
@@ -57,7 +57,7 @@ public class AddReviewer implements Callable<ReviewerResult> {
   private final AddReviewerSender.Factory addReviewerSenderFactory;
   private final AccountResolver accountResolver;
   private final GroupCache groupCache;
-  private final GroupMembersFactory.Factory groupMembersFactory;
+  private final GroupMembers.Factory groupMembersFactory;
   private final ChangeControl.Factory changeControlFactory;
   private final ReviewDb db;
   private final IdentifiedUser currentUser;
@@ -72,7 +72,7 @@ public class AddReviewer implements Callable<ReviewerResult> {
   @Inject
   AddReviewer(final AddReviewerSender.Factory addReviewerSenderFactory,
       final AccountResolver accountResolver, final GroupCache groupCache,
-      final GroupMembersFactory.Factory groupMembersFactory,
+      final GroupMembers.Factory groupMembersFactory,
       final ChangeControl.Factory changeControlFactory, final ReviewDb db,
       final IdentifiedUser.GenericFactory identifiedUserFactory,
       final IdentifiedUser currentUser, final ApprovalTypes approvalTypes,
@@ -122,8 +122,8 @@ public class AddReviewer implements Callable<ReviewerResult> {
         }
 
         final Set<Account> members =
-            groupMembersFactory.create(control.getProject().getNameKey(),
+            groupMembersFactory.create().listAccounts(group.getGroupUUID(),
-                group.getGroupUUID()).call();
+                control.getProject().getNameKey());
         if (members == null || members.size() == 0) {
           result.addError(new ReviewerResult.Error(
               ReviewerResult.Error.Type.GROUP_EMPTY, reviewer));