Simplify code related to GroupDetailFactory

Change-Id: I05c9ad209897bdbd48f116243ddd66a5c6e13eb8

gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java

@@ -19,16 +19,19 @@ import com.google.gerrit.reviewdb.client.AccountGroupMember;
 import java.util.List;
 
 public class GroupDetail {
-  public List<AccountGroupMember> members;
-  public List<AccountGroupById> includes;
+  private List<AccountGroupMember> members;
+  private List<AccountGroupById> includes;
 
-  public GroupDetail() {}
-
-  public void setMembers(List<AccountGroupMember> m) {
-    members = m;
+  public GroupDetail(List<AccountGroupMember> members, List<AccountGroupById> includes) {
+    this.members = members;
+    this.includes = includes;
   }
 
-  public void setIncludes(List<AccountGroupById> i) {
-    includes = i;
+  public List<AccountGroupMember> getMembers() {
+    return members;
+  }
+
+  public List<AccountGroupById> getIncludes() {
+    return includes;
   }
 }

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.account;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.data.GroupDetail;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -50,10 +51,9 @@ public class GroupDetailFactory implements Callable<GroupDetail> {
   @Override
   public GroupDetail call() throws OrmException, NoSuchGroupException {
     control = groupControl.validateFor(groupId);
-    GroupDetail detail = new GroupDetail();
-    detail.setMembers(loadMembers());
-    detail.setIncludes(loadIncludes());
-    return detail;
+    List<AccountGroupMember> members = loadMembers();
+    List<AccountGroupById> includes = loadIncludes();
+    return new GroupDetail(members, includes);
   }
 
   private List<AccountGroupMember> loadMembers() throws OrmException {
@@ -67,14 +67,14 @@ public class GroupDetailFactory implements Callable<GroupDetail> {
   }
 
   private List<AccountGroupById> loadIncludes() throws OrmException {
-    List<AccountGroupById> groups = new ArrayList<>();
-
-    for (AccountGroupById m : db.accountGroupById().byGroup(groupId)) {
-      if (control.canSeeGroup()) {
-        groups.add(m);
-      }
+    if (!control.canSeeGroup()) {
+      return ImmutableList.of();
     }
 
+    List<AccountGroupById> groups = new ArrayList<>();
+    for (AccountGroupById m : db.accountGroupById().byGroup(groupId)) {
+      groups.add(m);
+    }
     return groups;
   }
 }

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

@@ -104,17 +104,13 @@ public class GroupMembers {
     final GroupDetail groupDetail = groupDetailFactory.create(group.getId()).call();
 
     final Set<Account> members = new HashSet<>();
-    if (groupDetail.members != null) {
-      for (AccountGroupMember member : groupDetail.members) {
-        members.add(accountCache.get(member.getAccountId()).getAccount());
-      }
+    for (AccountGroupMember member : groupDetail.getMembers()) {
+      members.add(accountCache.get(member.getAccountId()).getAccount());
     }
-    if (groupDetail.includes != null) {
-      for (AccountGroupById groupInclude : groupDetail.includes) {
-        final AccountGroup includedGroup = groupCache.get(groupInclude.getIncludeUUID());
-        if (includedGroup != null && !seen.contains(includedGroup.getGroupUUID())) {
-          members.addAll(listAccounts(includedGroup.getGroupUUID(), project, seen));
-        }
+    for (AccountGroupById groupInclude : groupDetail.getIncludes()) {
+      final AccountGroup includedGroup = groupCache.get(groupInclude.getIncludeUUID());
+      if (includedGroup != null && !seen.contains(includedGroup.getGroupUUID())) {
+        members.addAll(listAccounts(includedGroup.getGroupUUID(), project, seen));
       }
     }
     return members;

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

@@ -102,20 +102,16 @@ public class ListMembers implements RestReadView<GroupResource> {
       return Collections.emptyMap();
     }
 
-    if (groupDetail.members != null) {
-      for (AccountGroupMember m : groupDetail.members) {
-        if (!members.containsKey(m.getAccountId())) {
-          members.put(m.getAccountId(), accountLoader.get(m.getAccountId()));
-        }
+    for (AccountGroupMember m : groupDetail.getMembers()) {
+      if (!members.containsKey(m.getAccountId())) {
+        members.put(m.getAccountId(), accountLoader.get(m.getAccountId()));
       }
     }
 
     if (recursive) {
-      if (groupDetail.includes != null) {
-        for (AccountGroupById includedGroup : groupDetail.includes) {
-          if (!seenGroups.contains(includedGroup.getIncludeUUID())) {
-            members.putAll(getMembers(includedGroup.getIncludeUUID(), seenGroups));
-          }
+      for (AccountGroupById includedGroup : groupDetail.getIncludes()) {
+        if (!seenGroups.contains(includedGroup.getIncludeUUID())) {
+          members.putAll(getMembers(includedGroup.getIncludeUUID(), seenGroups));
         }
       }
     }