Merge "Support managing visible external groups within internal groups"

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

@@ -67,7 +67,7 @@ public class GroupControl {
       if (group == null) {
         throw new NoSuchGroupException(groupId);
       }
-      return new GroupControl(user.get(), group);
+      return controlFor(GroupDescriptions.forAccountGroup(group));
     }
 
     public GroupControl controlFor(final AccountGroup.UUID groupId)
@@ -76,10 +76,14 @@ public class GroupControl {
       if (group == null) {
         throw new NoSuchGroupException(groupId);
       }
-      return new GroupControl(user.get(), group);
+      return controlFor(group);
     }
 
-    public GroupControl controlFor(final AccountGroup group) {
+    public GroupControl controlFor(AccountGroup group) {
+      return controlFor(GroupDescriptions.forAccountGroup(group));
+    }
+
+    public GroupControl controlFor(GroupDescription.Basic group) {
       return new GroupControl(user.get(), group);
     }
 
@@ -111,10 +115,6 @@ public class GroupControl {
     group =  gd;
   }
 
-  GroupControl(CurrentUser who, AccountGroup ag) {
-    this(who, GroupDescriptions.forAccountGroup(ag));
-  }
-
   public GroupDescription.Basic getGroup() {
     return group;
   }

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

@@ -18,6 +18,7 @@ import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -94,28 +95,31 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
     Account.Id me = ((IdentifiedUser) control.getCurrentUser()).getAccountId();
 
     for (String includedGroup : input.groups) {
+      GroupDescription.Basic d;
       try {
-        GroupResource includedGroupResource = groupsCollection.get().parse(includedGroup);
+        d = groupsCollection.get().parse(includedGroup);
-        if (!control.canAddGroup(includedGroupResource.getGroupUUID())) {
+      } catch (ResourceNotFoundException e) {
-          throw new AuthException(String.format("Cannot add group: %s",
+        badRequest.addError(new NoSuchGroupException(includedGroup));
-              includedGroupResource.getName()));
+        continue;
       }
 
-        if (!newIncludedGroups.containsKey(includedGroupResource.getGroupUUID())) {
+      if (!control.canAddGroup(d.getGroupUUID())) {
+        throw new AuthException(String.format("Cannot add group: %s",
+            d.getName()));
+      }
+
+      if (!newIncludedGroups.containsKey(d.getGroupUUID())) {
         AccountGroupIncludeByUuid.Key agiKey =
             new AccountGroupIncludeByUuid.Key(group.getId(),
-                  includedGroupResource.getGroupUUID());
+                d.getGroupUUID());
         AccountGroupIncludeByUuid agi = db.accountGroupIncludesByUuid().get(agiKey);
         if (agi == null) {
           agi = new AccountGroupIncludeByUuid(agiKey);
-            newIncludedGroups.put(includedGroupResource.getGroupUUID(), agi);
+          newIncludedGroups.put(d.getGroupUUID(), agi);
           newIncludedGroupsAudits.add(new AccountGroupIncludeByUuidAudit(agi, me));
         }
       }
-        result.add(new GroupInfo(includedGroupResource.getGroup()));
+      result.add(new GroupInfo(d));
-      } catch (ResourceNotFoundException e) {
-        badRequest.addError(new NoSuchGroupException(includedGroup));
-      }
     }
 
     badRequest.failOnError();

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.group;
 import com.google.common.base.Objects;
 import com.google.common.base.Strings;
 import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.data.GroupDescriptions;
 import com.google.gerrit.common.errors.NameAlreadyUsedException;
 import com.google.gerrit.common.errors.PermissionDeniedException;
@@ -107,8 +108,8 @@ class CreateGroup implements RestModifyView<TopLevelResource, Input> {
   private AccountGroup.Id owner(Input input) throws BadRequestException {
     if (input.ownerId != null) {
       try {
-        GroupResource rsrc = groups.parse(Url.decode(input.ownerId));
+        GroupDescription.Basic d = groups.parse(Url.decode(input.ownerId));
-        AccountGroup owner = GroupDescriptions.toAccountGroup(rsrc.getGroup());
+        AccountGroup owner = GroupDescriptions.toAccountGroup(d);
         if (owner == null) {
           throw new BadRequestException("ownerId must be internal group");
         }

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.group;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -74,24 +75,24 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
     final BadRequestHandler badRequest = new BadRequestHandler("removing included groups");
 
     for (final String includedGroup : input.groups) {
+      GroupDescription.Basic d;
       try {
-        final GroupResource includedGroupResource = groupsCollection.get().parse(includedGroup);
+        d = groupsCollection.get().parse(includedGroup);
-
+      } catch (ResourceNotFoundException e) {
-        if (!control.canRemoveGroup(includedGroupResource.getGroupUUID())) {
+        badRequest.addError(new NoSuchGroupException(includedGroup));
-          throw new AuthException(String.format("Cannot delete group: %s",
+        continue;
-              includedGroupResource.getName()));
       }
 
-        final AccountGroupIncludeByUuid g =
+      if (!control.canRemoveGroup(d.getGroupUUID())) {
-            includedGroups.remove(includedGroupResource.getGroupUUID());
+        throw new AuthException(String.format("Cannot delete group: %s",
+            d.getName()));
+      }
+
+      AccountGroupIncludeByUuid g = includedGroups.remove(d.getGroupUUID());
       if (g != null) {
         toRemove.add(g);
       }
-      } catch (ResourceNotFoundException e) {
-        badRequest.addError(new NoSuchGroupException(includedGroup));
     }
-    }
-
     badRequest.failOnError();
 
     if (!toRemove.isEmpty()) {

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.group;
 
+import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.registration.DynamicMap;
@@ -81,23 +82,28 @@ public class GroupsCollection implements
     } else if(!(user instanceof IdentifiedUser)) {
       throw new ResourceNotFoundException(id);
     }
-    return parse(id.get());
+
+    GroupControl ctl = groupControlFactory.controlFor(parse(id.get()));
+    if (!ctl.isVisible()) {
+      throw new ResourceNotFoundException(id);
+    }
+    return new GroupResource(ctl);
   }
 
-  GroupResource parse(String id) throws ResourceNotFoundException {
+  GroupDescription.Basic parse(String id) throws ResourceNotFoundException {
-    try {
     AccountGroup.UUID uuid = new AccountGroup.UUID(id);
     if (groupBackend.handles(uuid)) {
-        return check(id, groupControlFactory.controlFor(uuid));
+      GroupDescription.Basic d = groupBackend.get(uuid);
+      if (d != null) {
+        return d;
       }
-    } catch (NoSuchGroupException noSuchGroup) {
     }
 
     // Might be a legacy AccountGroup.Id.
     if (id.matches("^[1-9][0-9]*$")) {
       try {
         AccountGroup.Id legacyId = AccountGroup.Id.parse(id);
-        return check(id, groupControlFactory.controlFor(legacyId));
+        return groupControlFactory.controlFor(legacyId).getGroup();
       } catch (IllegalArgumentException invalidId) {
       } catch (NoSuchGroupException e) {
       }
@@ -106,23 +112,15 @@ public class GroupsCollection implements
     // Might be a group name, be nice and accept unique names.
     GroupReference ref = GroupBackends.findExactSuggestion(groupBackend, id);
     if (ref != null) {
-      try {
+      GroupDescription.Basic d = groupBackend.get(ref.getUUID());
-        return check(id, groupControlFactory.controlFor(ref.getUUID()));
+      if (d != null) {
-      } catch (NoSuchGroupException e) {
+        return d;
       }
     }
 
     throw new ResourceNotFoundException(id);
   }
 
-  private static GroupResource check(String id, GroupControl ctl)
-      throws ResourceNotFoundException {
-    if (ctl.isVisible()) {
-      return new GroupResource(ctl);
-    }
-    throw new ResourceNotFoundException(id);
-  }
-
   @SuppressWarnings("unchecked")
   @Override
   public CreateGroup create(TopLevelResource root, IdString name) {

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

@@ -63,9 +63,7 @@ public class IncludedGroupsCollection implements
       throw new ResourceNotFoundException(id);
     }
 
-    GroupDescription.Basic member =
+    GroupDescription.Basic member = groupsCollection.get().parse(id.get());
-        groupsCollection.get().parse(id.get()).getGroup();
-
     if (isMember(parent, member)
         && resource.getControl().canSeeGroup(member.getGroupUUID())) {
       return new IncludedGroupResource(resource, member);