Add a groups map ProjectAccessInfo.

This lets the access UI in PolyGerrit show group names without doing
many roundtrips to discover the names.

Change-Id: Iaeb652776c80401d115038dd4abdb0342bfc47d7

Documentation/rest-api-access.txt

@@ -263,7 +263,41 @@ The entries in the map are sorted by project name.
       ],
       "can_upload": true,
       "can_add": true,
-      "config_visible": true
+      "config_visible": true,
+      "groups": {
+         "53a4f647a89ea57992571187d8025f830625192a": {
+           "url": "#/admin/groups/uuid-53a4f647a89ea57992571187d8025f830625192a",
+           "options": {},
+           "description": "Gerrit Site Administrators",
+           "group_id": 1,
+           "owner": "Administrators",
+           "owner_id": "53a4f647a89ea57992571187d8025f830625192a",
+           "created_on": "2009-06-08 23:31:00.000000000",
+           "name": "Administrators"
+         },
+         "global:Registered-Users": {
+           "options": {},
+           "name": "Registered Users"
+         },
+         "global:Project-Owners": {
+           "options": {},
+           "name": "Project Owners"
+         },
+         "15bfcd8a6de1a69c50b30cedcdcc951c15703152": {
+           "url": "#/admin/groups/uuid-15bfcd8a6de1a69c50b30cedcdcc951c15703152",
+           "options": {},
+           "description": "Users who perform batch actions on Gerrit",
+           "group_id": 2,
+           "owner": "Administrators",
+           "owner_id": "53a4f647a89ea57992571187d8025f830625192a",
+           "created_on": "2009-06-08 23:31:00.000000000",
+           "name": "Non-Interactive Users"
+         },
+         "global:Anonymous-Users": {
+           "options": {},
+           "name": "Anonymous Users"
+         }
+      }
     },
     "MyProject": {
       "revision": "61157ed63e14d261b6dca40650472a9b0bd88474",
@@ -368,6 +402,10 @@ Whether the calling user can add any ref.
 |`config_visible`     |not set if `false`|
 Whether the calling user can see the `refs/meta/config` branch of the
 project.
+|`groups`            |A map of group UUID to
+link:rest-api-groups.html#group-info[GroupInfo] objects, describing
+the group UUIDs used in the `local` map. Groups that are not visible
+are omitted from the `groups` map.
 |==================================
 
 

Documentation/rest-api-projects.txt

@@ -1005,7 +1005,23 @@ As result a link:#project-access-info[ProjectAccessInfo] entity is returned.
     ],
     "can_upload": true,
     "can_add": true,
-    "config_visible": true
+    "config_visible": true,
+    "groups": {
+      "c2ce4749a32ceb82cd6adcce65b8216e12afb41c": {
+        "url": "#/admin/groups/uuid-c2ce4749a32ceb82cd6adcce65b8216e12afb41c",
+        "options": {},
+        "description": "Users who perform batch actions on Gerrit",
+        "group_id": 2,
+        "owner": "Administrators",
+        "owner_id": "d5b7124af4de52924ed397913e2c3b37bf186948",
+        "created_on": "2009-06-08 23:31:00.000000000",
+        "name": "Non-Interactive Users"
+      },
+      "global:Anonymous-Users": {
+        "options": {},
+        "name": "Anonymous Users"
+      }
+    }
   }
 ----
 
@@ -1086,7 +1102,13 @@ As result a link:#project-access-info[ProjectAccessInfo] entity is returned.
     ],
     "can_upload": true,
     "can_add": true,
-    "config_visible": true
+    "config_visible": true,
+    "groups": {
+      "global:Anonymous-Users": {
+        "options": {},
+        "name": "Anonymous Users"
+      }
+    }
   }
 ----
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java

@@ -207,6 +207,51 @@ public class AccessIT extends AbstractDaemonTest {
     gApi.projects().name(newProjectName).access();
   }
 
+  @Test
+  public void permissionsGroupMap() throws Exception {
+    // Add initial permission set
+    ProjectAccessInput accessInput = newProjectAccessInput();
+    AccessSectionInfo accessSection = newAccessSectionInfo();
+
+    PermissionInfo push = newPermissionInfo();
+    PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+    push.rules.put(SystemGroupBackend.PROJECT_OWNERS.get(), pri);
+    accessSection.permissions.put(Permission.PUSH, push);
+
+    PermissionInfo read = newPermissionInfo();
+    pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+    read.rules.put(SystemGroupBackend.ANONYMOUS_USERS.get(), pri);
+    accessSection.permissions.put(Permission.READ, read);
+
+    accessInput.add.put(REFS_ALL, accessSection);
+    ProjectAccessInfo result = pApi.access(accessInput);
+    assertThat(result.groups.keySet())
+        .containsExactly(
+            SystemGroupBackend.PROJECT_OWNERS.get(), SystemGroupBackend.ANONYMOUS_USERS.get());
+
+    // Check the name, which is what the UI cares about; exhaustive
+    // coverage of GroupInfo should be in groups REST API tests.
+    assertThat(result.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).name)
+        .isEqualTo("Project Owners");
+    // Strip the ID, since it is in the key.
+    assertThat(result.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).id).isNull();
+
+    // Get call returns groups too.
+    ProjectAccessInfo loggedInResult = pApi.access();
+    assertThat(loggedInResult.groups.keySet())
+        .containsExactly(
+            SystemGroupBackend.PROJECT_OWNERS.get(), SystemGroupBackend.ANONYMOUS_USERS.get());
+    assertThat(loggedInResult.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).name)
+        .isEqualTo("Project Owners");
+    assertThat(loggedInResult.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).id).isNull();
+
+    // PROJECT_OWNERS is invisible to anonymous user, so we strip it.
+    setApiUserAnonymous();
+    ProjectAccessInfo anonResult = pApi.access();
+    assertThat(anonResult.groups.keySet())
+        .containsExactly(SystemGroupBackend.ANONYMOUS_USERS.get());
+  }
+
   @Test
   public void updateParentAsUser() throws Exception {
     // Create child

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.extensions.api.access;
 
+import com.google.gerrit.extensions.common.GroupInfo;
 import com.google.gerrit.extensions.common.ProjectInfo;
 import java.util.Map;
 import java.util.Set;
@@ -27,4 +28,5 @@ public class ProjectAccessInfo {
   public Boolean canUpload;
   public Boolean canAdd;
   public Boolean configVisible;
+  public Map<String, GroupInfo> groups;
 }

gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java

@@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.GetAccess;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -50,7 +51,7 @@ public class ListAccess implements RestReadView<TopLevelResource> {
   @Override
   public Map<String, ProjectAccessInfo> apply(TopLevelResource resource)
       throws ResourceNotFoundException, ResourceConflictException, IOException,
-          PermissionBackendException {
+          PermissionBackendException, OrmException {
     Map<String, ProjectAccessInfo> access = new TreeMap<>();
     for (String p : projects) {
       access.put(p, getAccess.apply(new Project.NameKey(p)));

gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java

@@ -18,6 +18,7 @@ import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE
 import static com.google.gerrit.server.permissions.ProjectPermission.CREATE_REF;
 import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE;
 import static com.google.gerrit.server.permissions.RefPermission.READ;
+import static java.util.stream.Collectors.toMap;
 
 import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.Iterables;
@@ -30,6 +31,7 @@ import com.google.gerrit.extensions.api.access.AccessSectionInfo;
 import com.google.gerrit.extensions.api.access.PermissionInfo;
 import com.google.gerrit.extensions.api.access.PermissionRuleInfo;
 import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
+import com.google.gerrit.extensions.common.GroupInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -43,9 +45,11 @@ import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.group.GroupJson;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -55,9 +59,15 @@ import java.util.HashSet;
 import java.util.Map;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Singleton
 public class GetAccess implements RestReadView<ProjectResource> {
+  private static final Logger LOG = LoggerFactory.getLogger(GetAccess.class);
+
+  /** Marker value used in {@code Map<?, GroupInfo>} for groups not visible to current user. */
+  private static final GroupInfo INVISIBLE_SENTINEL = new GroupInfo();
 
   public static final ImmutableBiMap<PermissionRule.Action, PermissionRuleInfo.Action> ACTION_TYPE =
       ImmutableBiMap.of(
@@ -81,6 +91,7 @@ public class GetAccess implements RestReadView<ProjectResource> {
   private final MetaDataUpdate.Server metaDataUpdateFactory;
   private final ProjectControl.GenericFactory projectControlFactory;
   private final GroupBackend groupBackend;
+  private final GroupJson groupJson;
 
   @Inject
   public GetAccess(
@@ -92,7 +103,8 @@ public class GetAccess implements RestReadView<ProjectResource> {
       MetaDataUpdate.Server metaDataUpdateFactory,
       ProjectJson projectJson,
       ProjectControl.GenericFactory projectControlFactory,
-      GroupBackend groupBackend) {
+      GroupBackend groupBackend,
+      GroupJson groupJson) {
     this.user = self;
     this.permissionBackend = permissionBackend;
     this.groupControlFactory = groupControlFactory;
@@ -102,11 +114,12 @@ public class GetAccess implements RestReadView<ProjectResource> {
     this.projectControlFactory = projectControlFactory;
     this.metaDataUpdateFactory = metaDataUpdateFactory;
     this.groupBackend = groupBackend;
+    this.groupJson = groupJson;
   }
 
   public ProjectAccessInfo apply(Project.NameKey nameKey)
       throws ResourceNotFoundException, ResourceConflictException, IOException,
-          PermissionBackendException {
+          PermissionBackendException, OrmException {
     try {
       return apply(new ProjectResource(projectControlFactory.controlFor(nameKey, user.get())));
     } catch (NoSuchProjectException e) {
@@ -117,7 +130,7 @@ public class GetAccess implements RestReadView<ProjectResource> {
   @Override
   public ProjectAccessInfo apply(ProjectResource rsrc)
       throws ResourceNotFoundException, ResourceConflictException, IOException,
-          PermissionBackendException {
+          PermissionBackendException, OrmException {
     // Load the current configuration from the repository, ensuring it's the most
     // recent version available. If it differs from what was in the project
     // state, force a cache flush now.
@@ -151,27 +164,27 @@ public class GetAccess implements RestReadView<ProjectResource> {
 
     info.local = new HashMap<>();
     info.ownerOf = new HashSet<>();
-    Map<AccountGroup.UUID, Boolean> visibleGroups = new HashMap<>();
+    Map<AccountGroup.UUID, GroupInfo> visibleGroups = new HashMap<>();
     boolean checkReadConfig = check(perm, RefNames.REFS_CONFIG, READ);
 
     for (AccessSection section : config.getAccessSections()) {
       String name = section.getName();
       if (AccessSection.GLOBAL_CAPABILITIES.equals(name)) {
         if (pc.isOwner()) {
-          info.local.put(name, createAccessSection(section));
+          info.local.put(name, createAccessSection(visibleGroups, section));
           info.ownerOf.add(name);
 
         } else if (checkReadConfig) {
-          info.local.put(section.getName(), createAccessSection(section));
+          info.local.put(section.getName(), createAccessSection(visibleGroups, section));
         }
 
       } else if (RefConfigSection.isValid(name)) {
         if (pc.controlForRef(name).isOwner()) {
-          info.local.put(name, createAccessSection(section));
+          info.local.put(name, createAccessSection(visibleGroups, section));
           info.ownerOf.add(name);
 
         } else if (checkReadConfig) {
-          info.local.put(name, createAccessSection(section));
+          info.local.put(name, createAccessSection(visibleGroups, section));
 
         } else if (check(perm, name, READ)) {
           // Filter the section to only add rules describing groups that
@@ -184,26 +197,18 @@ public class GetAccess implements RestReadView<ProjectResource> {
             Permission dstPerm = null;
 
             for (PermissionRule srcRule : srcPerm.getRules()) {
-              AccountGroup.UUID group = srcRule.getGroup().getUUID();
+              AccountGroup.UUID groupId = srcRule.getGroup().getUUID();
-              if (group == null) {
+              if (groupId == null) {
                 continue;
               }
 
-              Boolean canSeeGroup = visibleGroups.get(group);
+              GroupInfo group = loadGroup(visibleGroups, groupId);
-              if (canSeeGroup == null) {
-                try {
-                  canSeeGroup = groupControlFactory.controlFor(group).isVisible();
-                } catch (NoSuchGroupException e) {
-                  canSeeGroup = Boolean.FALSE;
-                }
-                visibleGroups.put(group, canSeeGroup);
-              }
 
-              if (canSeeGroup) {
+              if (group != INVISIBLE_SENTINEL) {
                 if (dstPerm == null) {
                   if (dst == null) {
                     dst = new AccessSection(name);
-                    info.local.put(name, createAccessSection(dst));
+                    info.local.put(name, createAccessSection(visibleGroups, dst));
                   }
                   dstPerm = dst.getPermission(srcPerm.getName(), true);
                 }
@@ -244,9 +249,37 @@ public class GetAccess implements RestReadView<ProjectResource> {
     info.canAdd = toBoolean(perm.testOrFalse(CREATE_REF));
     info.configVisible = checkReadConfig || pc.isOwner();
 
+    info.groups =
+        visibleGroups
+            .entrySet()
+            .stream()
+            .filter(e -> e.getValue() != INVISIBLE_SENTINEL)
+            .collect(toMap(e -> e.getKey().get(), e -> e.getValue()));
+
     return info;
   }
 
+  private GroupInfo loadGroup(Map<AccountGroup.UUID, GroupInfo> visibleGroups, AccountGroup.UUID id)
+      throws OrmException {
+    GroupInfo group = visibleGroups.get(id);
+    if (group == null) {
+      try {
+        GroupControl control = groupControlFactory.controlFor(id);
+        group = INVISIBLE_SENTINEL;
+        if (control.isVisible()) {
+          group = groupJson.format(control.getGroup());
+          group.id = null;
+        }
+      } catch (NoSuchGroupException e) {
+        LOG.warn("NoSuchGroupException; ignoring group " + id, e);
+        group = INVISIBLE_SENTINEL;
+      }
+      visibleGroups.put(id, group);
+    }
+
+    return group;
+  }
+
   private static boolean check(PermissionBackend.ForProject ctx, String ref, RefPermission perm)
       throws PermissionBackendException {
     try {
@@ -257,7 +290,8 @@ public class GetAccess implements RestReadView<ProjectResource> {
     }
   }
 
-  private AccessSectionInfo createAccessSection(AccessSection section) {
+  private AccessSectionInfo createAccessSection(
+      Map<AccountGroup.UUID, GroupInfo> groups, AccessSection section) throws OrmException {
     AccessSectionInfo accessSectionInfo = new AccessSectionInfo();
     accessSectionInfo.permissions = new HashMap<>();
     for (Permission p : section.getPermissions()) {
@@ -273,6 +307,7 @@ public class GetAccess implements RestReadView<ProjectResource> {
         AccountGroup.UUID group = r.getGroup().getUUID();
         if (group != null) {
           pInfo.rules.put(group.get(), info);
+          loadGroup(groups, group);
         }
       }
       accessSectionInfo.permissions.put(p.getName(), pInfo);

gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java

@@ -44,6 +44,7 @@ import com.google.gerrit.server.group.GroupsCollection;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -91,7 +92,8 @@ public class SetAccess implements RestModifyView<ProjectResource, ProjectAccessI
   @Override
   public ProjectAccessInfo apply(ProjectResource rsrc, ProjectAccessInput input)
       throws ResourceNotFoundException, ResourceConflictException, IOException, AuthException,
-          BadRequestException, UnprocessableEntityException, PermissionBackendException {
+          BadRequestException, UnprocessableEntityException, OrmException,
+          PermissionBackendException {
     List<AccessSection> removals = getAccessSections(input.remove);
     List<AccessSection> additions = getAccessSections(input.add);
     MetaDataUpdate.User metaDataUpdateUser = metaDataUpdateFactory.get();