Add direct members and directly included groups to GroupInfo

When listing groups it is now possible to request that the direct group
members and the directly included groups are included into the returned
GroupInfos. This is done by setting the options 'o=MEMBERS' and
'o=INCLUDES'.

In addition there is a new REST endpoint '/groups/<group-id>/detail' by
which a group with its direct members and directly included groups can
be retrieved.

This new endpoint is used to retrieve the group for populating the
AccountGroupMembersScreen. By this 2 requests are saved which would
otherwise be needed to fetch the direct group members and the directly
included groups.

Change-Id: Ib94e5c01cc0880aa508f703f4156ac7fe627d742
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>

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

@@ -16,12 +16,14 @@ package com.google.gerrit.server.account;
 
 import com.google.common.collect.Lists;
 import com.google.gerrit.common.errors.NoSuchGroupException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.group.GroupJson;
 import com.google.gerrit.server.group.GroupJson.GroupInfo;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
 import java.util.List;
@@ -37,7 +39,8 @@ public class GetGroups implements RestReadView<AccountResource> {
   }
 
   @Override
-  public List<GroupInfo> apply(AccountResource resource) {
+  public List<GroupInfo> apply(AccountResource resource)
+      throws ResourceNotFoundException, OrmException {
     IdentifiedUser user = resource.getUser();
     Account.Id userId = user.getAccountId();
     List<GroupInfo> groups = Lists.newArrayList();

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

@@ -82,8 +82,8 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
 
   @Override
   public List<GroupInfo> apply(GroupResource resource, Input input)
-      throws MethodNotAllowedException, AuthException, BadRequestException,
-      OrmException {
+      throws ResourceNotFoundException, MethodNotAllowedException,
+      AuthException, BadRequestException, OrmException {
     AccountGroup group = resource.toAccountGroup();
     if (group == null) {
       throw new MethodNotAllowedException();
@@ -153,8 +153,8 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
 
     @Override
     public GroupInfo apply(GroupResource resource, Input input)
-        throws MethodNotAllowedException, AuthException, BadRequestException,
-        OrmException {
+        throws ResourceNotFoundException, MethodNotAllowedException,
+        AuthException, BadRequestException, OrmException {
       AddIncludedGroups.Input in = new AddIncludedGroups.Input();
       in.groups = ImmutableList.of(id);
       List<GroupInfo> list = put.get().apply(resource, in);
@@ -177,7 +177,9 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
     }
 
     @Override
-    public Object apply(IncludedGroupResource resource, PutIncludedGroup.Input input) {
+    public Object apply(IncludedGroupResource resource,
+        PutIncludedGroup.Input input) throws ResourceNotFoundException,
+        OrmException {
       // Do nothing, the group is already included.
       return get.get().apply(resource);
     }

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

@@ -78,8 +78,8 @@ class CreateGroup implements RestModifyView<TopLevelResource, Input> {
 
   @Override
   public GroupInfo apply(TopLevelResource resource, Input input)
-      throws AuthException, BadRequestException, OrmException,
-      NameAlreadyUsedException {
+      throws ResourceNotFoundException, AuthException, BadRequestException,
+      OrmException, NameAlreadyUsedException {
     if (input == null) {
       input = new Input();
     }

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

@@ -0,0 +1,38 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.group;
+
+import com.google.gerrit.common.groups.ListGroupsOption;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.group.GroupJson.GroupInfo;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+
+public class GetDetail implements RestReadView<GroupResource> {
+  private final GroupJson json;
+
+  @Inject
+  GetDetail(GroupJson json) {
+    this.json = json.addOption(ListGroupsOption.MEMBERS)
+        .addOption(ListGroupsOption.INCLUDES);
+  }
+
+  @Override
+  public GroupInfo apply(GroupResource rsrc) throws ResourceNotFoundException,
+      OrmException {
+    return json.format(rsrc);
+  }
+}

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

@@ -14,8 +14,10 @@
 
 package com.google.gerrit.server.group;
 
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.group.GroupJson.GroupInfo;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
 class GetGroup implements RestReadView<GroupResource> {
@@ -27,7 +29,8 @@ class GetGroup implements RestReadView<GroupResource> {
   }
 
   @Override
-  public GroupInfo apply(GroupResource resource) {
+  public GroupInfo apply(GroupResource resource)
+      throws ResourceNotFoundException, OrmException {
     return json.format(resource.getGroup());
   }
 }

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

@@ -14,8 +14,10 @@
 
 package com.google.gerrit.server.group;
 
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.group.GroupJson.GroupInfo;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
 public class GetIncludedGroup implements RestReadView<IncludedGroupResource>  {
@@ -27,7 +29,8 @@ public class GetIncludedGroup implements RestReadView<IncludedGroupResource>  {
   }
 
   @Override
-  public GroupInfo apply(IncludedGroupResource rsrc) {
+  public GroupInfo apply(IncludedGroupResource rsrc)
+      throws ResourceNotFoundException, OrmException {
     return json.format(rsrc.getMemberDescription());
   }
 }

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

@@ -20,6 +20,7 @@ import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.group.GroupJson.GroupInfo;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
 public class GetOwner implements RestReadView<GroupResource> {
@@ -34,7 +35,8 @@ public class GetOwner implements RestReadView<GroupResource> {
   }
 
   @Override
-  public GroupInfo apply(GroupResource resource) throws ResourceNotFoundException {
+  public GroupInfo apply(GroupResource resource)
+      throws ResourceNotFoundException, OrmException {
     AccountGroup group = resource.toAccountGroup();
     if (group == null) {
       throw new ResourceNotFoundException();

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

@@ -14,23 +14,75 @@
 
 package com.google.gerrit.server.group;
 
+import static com.google.gerrit.common.groups.ListGroupsOption.INCLUDES;
+import static com.google.gerrit.common.groups.ListGroupsOption.MEMBERS;
+
 import com.google.common.base.Strings;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.data.GroupDescriptions;
+import com.google.gerrit.common.groups.ListGroupsOption;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.account.AccountInfo;
 import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.GroupControl;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.List;
 
 public class GroupJson {
   private final GroupCache groupCache;
+  private final GroupControl.Factory groupControlFactory;
+  private final Provider<ListMembers> listMembers;
+  private final Provider<ListIncludedGroups> listIncludes;
+  private EnumSet<ListGroupsOption> options;
 
   @Inject
-  GroupJson(GroupCache groupCache) {
+  GroupJson(GroupCache groupCache, GroupControl.Factory groupControlFactory,
+      Provider<ListMembers> listMembers,
+      Provider<ListIncludedGroups> listIncludes) {
     this.groupCache = groupCache;
+    this.groupControlFactory = groupControlFactory;
+    this.listMembers = listMembers;
+    this.listIncludes = listIncludes;
+
+    options = EnumSet.noneOf(ListGroupsOption.class);
   }
 
-  public GroupInfo format(GroupDescription.Basic group) {
+  public GroupJson addOption(ListGroupsOption o) {
+    options.add(o);
+    return this;
+  }
+
+  public GroupJson addOptions(Collection<ListGroupsOption> o) {
+    options.addAll(o);
+    return this;
+  }
+
+  public GroupInfo format(GroupResource rsrc) throws ResourceNotFoundException,
+      OrmException {
+    GroupInfo info = init(rsrc.getGroup());
+    initMembersAndIncludes(rsrc, info);
+    return info;
+  }
+
+  public GroupInfo format(GroupDescription.Basic group)
+      throws ResourceNotFoundException, OrmException {
+    GroupInfo info = init(group);
+    if (options.contains(MEMBERS) || options.contains(INCLUDES)) {
+      GroupResource rsrc =
+          new GroupResource(groupControlFactory.controlFor(group));
+      initMembersAndIncludes(rsrc, info);
+    }
+    return info;
+  }
+
+  private GroupInfo init(GroupDescription.Basic group) {
     GroupInfo info = new GroupInfo();
     info.id = Url.encode(group.getGroupUUID().get());
     info.name = Strings.emptyToNull(group.getName());
@@ -53,6 +105,18 @@ public class GroupJson {
     return info;
   }
 
+  private GroupInfo initMembersAndIncludes(GroupResource rsrc, GroupInfo info)
+      throws ResourceNotFoundException, OrmException {
+    if (options.contains(MEMBERS)) {
+      info.members = listMembers.get().apply(rsrc);
+    }
+
+    if (options.contains(INCLUDES)) {
+      info.includes = listIncludes.get().apply(rsrc);
+    }
+    return info;
+  }
+
   public static class GroupInfo {
     final String kind = "gerritcodereview#group";
     public String id;
@@ -65,5 +129,9 @@ public class GroupJson {
     public Integer groupId;
     public String owner;
     public String ownerId;
+
+    // These fields are only supplied for internal groups, but only if requested
+    public List<AccountInfo> members;
+    public List<GroupInfo> includes;
   }
 }

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

@@ -22,9 +22,11 @@ import com.google.common.collect.Sets;
 import com.google.gerrit.common.data.GroupDescriptions;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.errors.NoSuchGroupException;
+import com.google.gerrit.common.groups.ListGroupsOption;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.extensions.restapi.Url;
@@ -40,6 +42,7 @@ import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.group.GroupJson.GroupInfo;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gson.reflect.TypeToken;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -47,6 +50,7 @@ import org.kohsuke.args4j.Option;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -63,6 +67,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
   private final IdentifiedUser.GenericFactory userFactory;
   private final Provider<GetGroups> accountGetGroups;
   private final GroupJson json;
+  private EnumSet<ListGroupsOption> options;
 
   @Option(name = "--project", aliases = {"-p"},
       usage = "projects for which the groups should be listed")
@@ -92,6 +97,16 @@ public class ListGroups implements RestReadView<TopLevelResource> {
   @Option(name = "-m", metaVar = "MATCH", usage = "match group substring")
   private String matchSubstring;
 
+  @Option(name = "-o", multiValued = true, usage = "Output options per group")
+  public void addOption(ListGroupsOption o) {
+    options.add(o);
+  }
+
+  @Option(name = "-O", usage = "Output option flags, in hex")
+  void setOptionFlagsHex(String hex) {
+    options.addAll(ListGroupsOption.fromBits(Integer.parseInt(hex, 16)));
+  }
+
   @Inject
   protected ListGroups(final GroupCache groupCache,
       final GroupControl.Factory groupControlFactory,
@@ -106,6 +121,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
     this.userFactory = userFactory;
     this.accountGetGroups = accountGetGroups;
     this.json = json;
+    this.options = EnumSet.noneOf(ListGroupsOption.class);
   }
 
   public Account.Id getUser() {
@@ -130,7 +146,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
         new TypeToken<Map<String, GroupInfo>>() {}.getType());
   }
 
-  public List<GroupInfo> get() throws NoSuchGroupException {
+  public List<GroupInfo> get() throws ResourceNotFoundException, OrmException {
     List<GroupInfo> groupInfos;
     if (user != null) {
       if (owned) {
@@ -151,7 +167,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
             for (final GroupReference groupRef : groupsRefs) {
               final AccountGroup group = groupCache.get(groupRef.getUUID());
               if (group == null) {
-                throw new NoSuchGroupException(groupRef.getUUID());
+                throw new ResourceNotFoundException(groupRef.getUUID().get());
               }
               groups.put(group.getGroupUUID(), group);
             }
@@ -162,21 +178,23 @@ public class ListGroups implements RestReadView<TopLevelResource> {
         }
         groupInfos = Lists.newArrayListWithCapacity(groupList.size());
         for (AccountGroup group : groupList) {
-          groupInfos.add(json.format(GroupDescriptions.forAccountGroup(group)));
+          groupInfos.add(json.addOptions(options).format(
+              GroupDescriptions.forAccountGroup(group)));
         }
       }
     }
     return groupInfos;
   }
 
-  private List<GroupInfo> getGroupsOwnedBy(IdentifiedUser user) {
+  private List<GroupInfo> getGroupsOwnedBy(IdentifiedUser user)
+      throws ResourceNotFoundException, OrmException {
     List<GroupInfo> groups = Lists.newArrayList();
     for (AccountGroup g : filterGroups(groupCache.all())) {
       GroupControl ctl = groupControlFactory.controlFor(g);
       try {
         if (genericGroupControlFactory.controlFor(user, g.getGroupUUID())
             .isOwner()) {
-          groups.add(json.format(ctl.getGroup()));
+          groups.add(json.addOptions(options).format(ctl.getGroup()));
         }
       } catch (NoSuchGroupException e) {
         continue;

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

@@ -20,9 +20,6 @@ import com.google.common.collect.Maps;
 import com.google.common.collect.Ordering;
 import com.google.gerrit.common.data.GroupDetail;
 import com.google.gerrit.common.errors.NoSuchGroupException;
-import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Account;
@@ -62,24 +59,27 @@ public class ListMembers implements RestReadView<GroupResource> {
 
   @Override
   public List<AccountInfo> apply(final GroupResource resource)
-      throws AuthException, BadRequestException, ResourceConflictException,
-      ResourceNotFoundException, Exception {
+      throws ResourceNotFoundException, OrmException {
     if (resource.toAccountGroup() == null) {
       throw new ResourceNotFoundException(resource.getGroupUUID().get());
     }
-    final Map<Account.Id, AccountInfo> members =
-        getMembers(resource.getGroupUUID(), new HashSet<AccountGroup.UUID>());
-    final List<AccountInfo> memberInfos = Lists.newArrayList(members.values());
-    Collections.sort(memberInfos, new Comparator<AccountInfo>() {
-      @Override
-      public int compare(AccountInfo a, AccountInfo b) {
-        return ComparisonChain.start()
-            .compare(a.name, b.name, Ordering.natural().nullsFirst())
-            .compare(a.email, b.email, Ordering.natural().nullsFirst())
-            .compare(a._account_id, b._account_id, Ordering.natural().nullsFirst()).result();
-      }
-    });
-    return memberInfos;
+    try {
+      final Map<Account.Id, AccountInfo> members =
+          getMembers(resource.getGroupUUID(), new HashSet<AccountGroup.UUID>());
+      final List<AccountInfo> memberInfos = Lists.newArrayList(members.values());
+      Collections.sort(memberInfos, new Comparator<AccountInfo>() {
+        @Override
+        public int compare(AccountInfo a, AccountInfo b) {
+          return ComparisonChain.start()
+              .compare(a.name, b.name, Ordering.natural().nullsFirst())
+              .compare(a.email, b.email, Ordering.natural().nullsFirst())
+              .compare(a._account_id, b._account_id, Ordering.natural().nullsFirst()).result();
+        }
+      });
+      return memberInfos;
+    } catch (NoSuchGroupException e) {
+      throw new ResourceNotFoundException(resource.getGroupUUID().get());
+    }
   }
 
   private Map<Account.Id, AccountInfo> getMembers(

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

@@ -37,6 +37,7 @@ public class Module extends RestApiModule {
 
     get(GROUP_KIND).to(GetGroup.class);
     put(GROUP_KIND).to(PutGroup.class);
+    get(GROUP_KIND, "detail").to(GetDetail.class);
     post(GROUP_KIND, "members").to(AddMembers.class);
     post(GROUP_KIND, "members.add").to(AddMembers.class);
     post(GROUP_KIND, "members.delete").to(DeleteMembers.class);