1 files changed, 50 insertions, 10 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
index d9b12ac338..2a8e7c92a3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
@@ -21,11 +21,34 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.InternalUser;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
+import com.google.inject.Singleton;
 
 /** Access control management for a group of accounts managed in Gerrit. */
 public class GroupControl {
+
+  @Singleton
+  public static class GenericFactory {
+    private final GroupBackend groupBackend;
+
+    @Inject
+    GenericFactory(final GroupBackend gb) {
+      groupBackend = gb;
+    }
+
+    public GroupControl controlFor(final CurrentUser who,
+        final AccountGroup.UUID groupId)
+        throws NoSuchGroupException {
+      final GroupDescription.Basic group = groupBackend.get(groupId);
+      if (group == null) {
+        throw new NoSuchGroupException(groupId);
+      }
+      return new GroupControl(who, group);
+    }
+  }
+
   public static class Factory {
     private final GroupCache groupCache;
     private final Provider<CurrentUser> user;
@@ -45,7 +68,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)
@@ -54,10 +77,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);
     }
 
@@ -69,6 +96,15 @@ public class GroupControl {
       }
       return c;
     }
+
+    public GroupControl validateFor(final AccountGroup.UUID groupUUID)
+        throws NoSuchGroupException {
+      final GroupControl c = controlFor(groupUUID);
+      if (!c.isVisible()) {
+        throw new NoSuchGroupException(groupUUID);
+      }
+      return c;
+    }
   }
 
   private final CurrentUser user;
@@ -80,8 +116,8 @@ public class GroupControl {
     group =  gd;
   }
 
-  GroupControl(CurrentUser who, AccountGroup ag) {
-    this(who, GroupDescriptions.forAccountGroup(ag));
+  public GroupDescription.Basic getGroup() {
+    return group;
   }
 
   public CurrentUser getCurrentUser() {
@@ -90,7 +126,9 @@ public class GroupControl {
 
   /** Can this user see this group exists? */
   public boolean isVisible() {
-    return group.isVisibleToAll()
+    AccountGroup accountGroup = GroupDescriptions.toAccountGroup(group);
+    return (accountGroup != null && accountGroup.isVisibleToAll())
+      || user instanceof InternalUser
       || user.getEffectiveGroups().contains(group.getGroupUUID())
       || isOwner();
   }
@@ -123,19 +161,21 @@ public class GroupControl {
     return canSeeMembers();
   }
 
-  public boolean canAddGroup(AccountGroup.Id id) {
+  public boolean canAddGroup(AccountGroup.UUID uuid) {
     return isOwner();
   }
 
-  public boolean canRemoveGroup(AccountGroup.Id id) {
+  public boolean canRemoveGroup(AccountGroup.UUID uuid) {
     return isOwner();
   }
 
-  public boolean canSeeGroup(AccountGroup.Id id) {
+  public boolean canSeeGroup(AccountGroup.UUID uuid) {
     return canSeeMembers();
   }
 
   private boolean canSeeMembers() {
-    return group.isVisibleToAll() || isOwner();
+    AccountGroup accountGroup = GroupDescriptions.toAccountGroup(group);
+    return (accountGroup != null && accountGroup.isVisibleToAll())
+        || isOwner();
   }
 }