Fix merge conflicts in group REST API

Chad's change to include groups by UUID conflicted with Edwin's
change providing REST API endpoints to inspect members of a group.

Change-Id: Ic1083b376a1d7a63bc3199778a258ae1c3b5416d

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.group;
 
+import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.extensions.restapi.RestResource;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -30,12 +31,20 @@ public class GroupResource implements RestResource {
     this.control = control;
   }
 
+  public GroupDescription.Basic getGroup() {
+    return control.getGroup();
+  }
+
   public String getName() {
-    return control.getGroup().getName();
+    return getGroup().getName();
   }
 
   public AccountGroup.UUID getGroupUUID() {
-    return control.getGroup().getGroupUUID();
+    return getGroup().getGroupUUID();
+  }
+
+  public boolean isInternal() {
+    return getGroup() instanceof GroupDescription.Internal;
   }
 
   public GroupControl getControl() {

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

@@ -14,65 +14,59 @@
 
 package com.google.gerrit.server.group;
 
-import com.google.gerrit.common.data.GroupDetail;
+import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ChildCollection;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
-import com.google.gerrit.reviewdb.client.AccountGroupInclude;
+import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.account.GroupControl;
-import com.google.gerrit.server.account.GroupDetailFactory;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
 public class IncludedGroupsCollection implements
     ChildCollection<GroupResource, IncludedGroupResource> {
-
   private final DynamicMap<RestView<IncludedGroupResource>> views;
   private final Provider<ListIncludedGroups> list;
   private final GroupControl.Factory groupControlFactory;
-  private final GroupCache groupCache;
+  private final Provider<ReviewDb> dbProvider;
-  private final GroupDetailFactory.Factory groupDetailFactory;
 
   @Inject
-  IncludedGroupsCollection(final DynamicMap<RestView<IncludedGroupResource>> views,
+  IncludedGroupsCollection(DynamicMap<RestView<IncludedGroupResource>> views,
-      final Provider<ListIncludedGroups> list,
+      Provider<ListIncludedGroups> list,
-      final GroupControl.Factory groupControlFactory,
+      GroupCache groupCache,
-      final GroupCache groupCache,
+      GroupControl.Factory groupControlFactory,
-      final GroupDetailFactory.Factory groupDetailFactory) {
+      Provider<ReviewDb> dbProvider) {
     this.views = views;
     this.list = list;
     this.groupControlFactory = groupControlFactory;
-    this.groupCache = groupCache;
+    this.dbProvider = dbProvider;
-    this.groupDetailFactory = groupDetailFactory;
   }
 
   @Override
-  public RestView<GroupResource> list() throws ResourceNotFoundException,
+  public RestView<GroupResource> list() {
-      AuthException {
     return list.get();
   }
 
   @Override
-  public IncludedGroupResource parse(final GroupResource parent, final String id)
+  public IncludedGroupResource parse(GroupResource parent, String id)
-      throws ResourceNotFoundException, Exception {
+      throws ResourceNotFoundException, OrmException {
-    final GroupResource groupResource =
+    if (!parent.isInternal()) {
-        GroupsCollection.parse(id, groupControlFactory);
+      throw new ResourceNotFoundException(parent.getGroupUUID().get());
-    final AccountGroup group =
+    }
-        groupCache.get(parent.getControl().getGroup().getGroupUUID());
+
-    final GroupDetail groupDetail =
+    GroupDescription.Internal p = (GroupDescription.Internal) parent.getGroup();
-        groupDetailFactory.create(group.getId()).call();
+    GroupResource included = GroupsCollection.parse(id, groupControlFactory);
-    if (groupDetail.includes != null) {
+    AccountGroupIncludeByUuid in = dbProvider.get()
-      for (final AccountGroupInclude groupInclude : groupDetail.includes) {
+        .accountGroupIncludesByUuid().get(
-        final AccountGroup includedGroup =
+          new AccountGroupIncludeByUuid.Key(
-            groupCache.get(groupInclude.getIncludeId());
+              p.getAccountGroup().getId(),
-        if (includedGroup.getGroupUUID().equals(groupResource.getGroupUUID())) {
+              included.getGroupUUID()));
-          return new IncludedGroupResource(groupResource.getControl());
+    if (in != null) {
-        }
+      return new IncludedGroupResource(included.getControl());
-      }
     }
     throw new ResourceNotFoundException(id);
   }

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

@@ -14,59 +14,79 @@
 
 package com.google.gerrit.server.group;
 
+import static com.google.common.base.Strings.nullToEmpty;
 import com.google.common.collect.Lists;
-import com.google.gerrit.common.data.GroupDescriptions;
+import com.google.gerrit.common.data.GroupDescription;
-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.ResourceNotFoundException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupInclude;
+import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
-import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.account.GroupControl;
-import com.google.gerrit.server.account.GroupDetailFactory;
 import com.google.gerrit.server.group.GetGroup.GroupInfo;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 
+import org.slf4j.Logger;
+
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 
 public class ListIncludedGroups implements RestReadView<GroupResource> {
-  private final GroupControl.Factory groupControlFactory;
+  private static final Logger log = org.slf4j.LoggerFactory.getLogger(ListIncludedGroups.class);
-  private final GroupCache groupCache;
+
-  private final GroupDetailFactory.Factory groupDetailFactory;
+  private final GroupControl.Factory controlFactory;
+  private final Provider<ReviewDb> dbProvider;
 
   @Inject
-  ListIncludedGroups(final GroupControl.Factory groupControlFactory,
+  ListIncludedGroups(GroupControl.Factory controlFactory,
-      final GroupCache groupCache,
+      Provider<ReviewDb> dbProvider) {
-      final GroupDetailFactory.Factory groupDetailFactory) {
+    this.controlFactory = controlFactory;
-    this.groupControlFactory = groupControlFactory;
+    this.dbProvider = dbProvider;
-    this.groupCache = groupCache;
-    this.groupDetailFactory = groupDetailFactory;
   }
 
   @Override
-  public List<GroupInfo> apply(final GroupResource resource)
+  public List<GroupInfo> apply(GroupResource rsrc)
-      throws AuthException, BadRequestException, ResourceConflictException,
+      throws ResourceNotFoundException, OrmException {
-      Exception {
+    if (!rsrc.isInternal()) {
-    final List<GroupInfo> includedGroups = Lists.newArrayList();
+      throw new ResourceNotFoundException(rsrc.getGroupUUID().get());
-
-    final GroupControl groupControl =
-        groupControlFactory.validateFor(resource.getGroupUUID());
-    final AccountGroup group =
-        groupCache.get(groupControl.getGroup().getGroupUUID());
-    final GroupDetail groupDetail =
-        groupDetailFactory.create(group.getId()).call();
-
-    if (groupDetail.includes != null) {
-      for (final AccountGroupInclude groupInclude : groupDetail.includes) {
-        final AccountGroup includedGroup =
-            groupCache.get(groupInclude.getIncludeId());
-        includedGroups.add(GetGroup.parse(GroupDescriptions
-            .forAccountGroup(includedGroup)));
-      }
     }
 
-    return includedGroups;
+    boolean ownerOfParent = rsrc.getControl().isOwner();
+    List<GroupInfo> included = Lists.newArrayList();
+    for (AccountGroupIncludeByUuid u : dbProvider.get()
+        .accountGroupIncludesByUuid()
+        .byGroup(groupId(rsrc))) {
+      try {
+        GroupControl i = controlFactory.controlFor(u.getIncludeUUID());
+        if (ownerOfParent || i.isVisible()) {
+          included.add(GetGroup.parse(i.getGroup()));
+        }
+      } catch (NoSuchGroupException notFound) {
+        log.warn(String.format("Group %s no longer available, included into ",
+            u.getIncludeUUID(),
+            rsrc.getGroup().getName()));
+        continue;
+      }
+    }
+    Collections.sort(included, new Comparator<GroupInfo>() {
+      @Override
+      public int compare(GroupInfo a, GroupInfo b) {
+        int cmp = nullToEmpty(a.name).compareTo(nullToEmpty(b.name));
+        if (cmp != 0) {
+          return cmp;
+        }
+        return nullToEmpty(a.id).compareTo(nullToEmpty(b.name));
+      }
+    });
+    return included;
+  }
+
+  private static AccountGroup.Id groupId(GroupResource rsrc) {
+    GroupDescription.Basic d = rsrc.getGroup();
+    return ((GroupDescription.Internal) d).getAccountGroup().getId();
   }
 }